Detect configuration template generation errors.

[bwplotka]

Sorry for starting 🔥 again.. (:

I find this quite confusing that if I make a mistake like this, so use a field that is not defined before, there is 0 build error, and nothing changes in generated files.

  store+: {
      image: obs.config.thanosImage,
      version: obs.config.thanosVersion,
      args+: [
       "try and error",
      ],
      objectStorageConfig: obs.config.objectStorageConfig,
      replicas: '${{THANOS_STORE_REPLICAS}}',
      resources: {
        requests: {
          cpu: '${THANOS_STORE_CPU_REQUEST}',
          memory: '${THANOS_STORE_MEMORY_REQUEST}',
        },
        limits: {
          cpu: '${THANOS_STORE_CPU_LIMIT}',
          memory: '${THANOS_STORE_MEMORY_LIMIT}',
        },
      },

I know that this is silly mistake of mine, I probably modify some config struct not the container itself, but as a newbie, I have 0 idea where is the field that I want and if the field is called args, arg or arguments without looking manually into some deep (3rd level, because configuration -> kube-thanos -> ksonnet ) repo on exact commit.

AC:

• Changing undefined fields should cause generation error.

I believe it's because of our language choice. I would add this issue as another example of jsonnet not being suitable for this kind of production configuration.

Curious, would CUE @metalmatze help with that?

[brancz]

Changing undefined fields should cause generation error.

It should cause a validation error, which it does at CI time, but we should have validations as part of the tooling earlier I agree.

[metalmatze]

That's kind of the reasoning behind ksonnet, I guess. We get kind of "type" safety by using their generated functions over just using plain objects. It's very cumbersome to right on the other hand with a lot of trade off.
The best solution right now would be to basically always dry-run templates against an Apiserver and see what happens.

As for CUE: Yes, that would be something that would have been caught by the closed structs:
https://github.com/cuelang/cue/blob/master/doc/ref/spec.md#closed-structs

[krasi-georgiev]

I am facing similiar issues, need to dig too deep into configs to find what I am looking for. As @brancz mentioned this is the best solution so far, but I would also want to keep the conversation open for trying out alternatives.

[brancz]

kubernetes/kubernetes#64830 basically alludes to the point that this is not a language problem, but no matter what generates the manifest, we always have to dry run them against a cluster which would catch the same and more problems.

[bwplotka]

I agree that we have to run dry runs. But we can catch 1000x more things in generation time if we would use type safe language and that is a language choice problem.

[brancz]

Not disagreeing in a general case, but if the build and validation happen within the same process, which we're perfectly capable of doing, then what's the difference? :)

I'm not arguing against trying cue at all, but I also think we haven't improved our existing tooling sufficiently.

[esnible]

If you want I can try to add the dry runs to detect configuration generation errors.

Before we can add dry runs we need to change generation so that the dry runs pass.

As an experiment, I did kubectl apply --dry-run=client -f configuration/examples/<env>/manifests and repeated with --dry-run=server.

For me,

• the dry run of examples/base always succeeds.
• the dry run of examples/dev succeeds on client but fails on server. This is because my server lacks a dex namespace. Creating one fixes it.
• the dry run of examples/local fails on both, because I lack kind "OpenTelemetryCollector" in version "opentelemetry.io/v1alpha1".

Thoughts: Perhaps Observatorium should generate a namespace for dex if we generate a deployment for Dex. Perhaps the Makefile should install at least the CR https://github.com/open-telemetry/opentelemetry-operator/blob/main/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml somehow when tracing is enabled.