Adding a config.toml value to the helm chart

[arschles]

Fixes gomods/athens-charts#22

What is the problem I am trying to address?

There's currently no way to add a complete config.toml file to an Athens pod(s) running in Kubernetes

How is the fix applied?

I added a configToml section to the values.yaml file in the Helm chart, where you can set enabled: true and then set a data field to a complete config.toml if you want. The chart will then create a new ConfigMap and mount it to /config/config.toml inside the new pod(s) that get created when you helm install

Mention the issue number it fixes or add the details of the changes if it doesn't have a specific issue.

Fixes gomods/athens-charts#22

cc/ @hypnoglow - let me know if this is what you had in mind. Also, I didn't test this out yet, so let me know if it looks right to you!

[hypnoglow]

One thing is missing is that we need chart version bump in Chart.yaml. Otherwise LGTM, thanks!

[hypnoglow]

Do you prefer enabled-switch approach?

Why not to just always use this config, without any conditions? I guess the default values should be fine, so we can leave it empty as you did. If not, we can provide some useful settings, maybe from https://github.com/gomods/athens/blob/master/config.dev.toml but still always mouning the config to the container.

[arschles]

@hypnoglow I set enabled: false to keep the Kubernetes deployment the same, by default. So, unless someone wants to customize the config file, there won't be any new config map the next time they deploy.

Regarding useful settings, the Athens image already has that config.dev.toml file baked into it. Is that what you were thinking?

[hypnoglow]

I mean we may omit enabled switch completely.

The reason is that I suppose config.dev.toml serves only for documentation purposes, and it contains only values that are Athens defaults. Bundling this file into the image actually is the same as passing empty config.

If I'm wrong and config.dev.toml contains some values that are not Athens default, we can simply put them here in the chart default values. This way we also don't need enabled switch.

For example:

configToml: |
  SomeNonDefaultValue = "foo"

So if users want to customize config, they will rewrite configToml with their value, otherwise they have the same defaults. enabled switch is not needed in both cases.

What do you think?

[arschles]

@hypnoglow ah, I think I partially see now what you mean. Regarding the defaults, I would say that there are several "layers" of defaults.

There are hard-coded ones like here the embedded config.dev.toml one, and then the ones in the helm values.yaml file. To be honest, I doubt the defaults in code match, the toml file and the helm values.yaml file all match up. However, there are reasons why we do have all three layers. I'm happy to explain them if you'd like or that would help 😄

Regarding the overrides, are you saying that the configToml: key could have just part of a config toml file, and that we would use it to override the one that is embedded in the Athens docker image?

[hypnoglow]

Regarding the defaults, I would say that there are several "layers" of defaults.
However, there are reasons why we do have all three layers.

Ah, it's a bit clearer now, thanks for explanations. However, don't those multiple layers add unnecessary complexity to the overall Athens configuration? BTW I guess it is going a bit offtopic. If you believe we should stick to those "layers" for now and keep Helm chart in accordance, we should go with it then.

Regarding the overrides, are you saying that the configToml: key could have just part of a config toml file, and that we would use it to override the one that is embedded in the Athens docker image?

Yes, exactly. My point was that we can use default config options directly in helm values that will always override embedded config.

[arschles]

However, don't those multiple layers add unnecessary complexity to the overall Athens configuration?

@hypnoglow I personally think they do, but there is a need for folks to customize configuration at each of the levels, and we have the defaults in place to show where to customize configuration.

For example, the Helm values.yaml is there show some examples of how folks can customize their Kubernetes deployment. Those fields get translated to environment values passed to Athens, however, so the configuration mechanism doesn't change from other deployments of Athens, for what that's worth 😄 .

If you believe we should stick to those "layers" for now and keep Helm chart in accordance, we should go with it then.

I do think we need to stick with the layers, and I'd like to see them documented (I just created an issue for that, and I'm planning to do it today).

I agree that ideally the helm values.yaml file should be up to date with the config.dev.toml and the hard-coded defaults, and we can try to do that, but inevitably they will become out of sync. I am going to try to write documentation clear enough that folks can both understand where the layers are, how to read each one (config.toml, values.yaml, etc...), and that they may diverge.

My point was that we can use default config options directly in helm values that will always override embedded config.

The merging option is interesting, but wouldn't someone set environment variables or provide a values.yaml file if they want to override just part of the config.toml file? This PR is intended to allow people to override everything at once.

Let me know what you think, and thank you for the discussion!

[hypnoglow]

@arschles Thank you for your efforts on improving documentation.
I believe that we can try to reduce layers complexity, somewhat like this:

1. Standalone app can run with defaults (should be documented) with the possible custom config.toml or environment variables provided by user.
2. Docker should run with defaults or probably with minimal default config.toml with only docker-specific options tuned, if it is even required. Custom configuration should be provided as config.toml-volume or environment variables by user.
3. Helm Chart and other Kubernetes deployment builds on top of the Docker image, but with default configuration tuned with k8s-specific values (only if required), present in values.yaml or similar and allowing to be customized by user.

This way users will always know that they get the same configuration, whether they running Athens standalone, in Docker or in Kubernetes.

The merging option is interesting, but wouldn't someone set environment variables or provide a values.yaml file if they want to override just part of the config.toml file? This PR is intended to allow people to override everything at once.

I didn't mean merging. To recap what I suggested:

1. If we use enabled-switch approach, then with enabled: false helm chart runs with Docker-builtin config.toml and custom configuration can be passed only with environment variables. With enabled: true we completely override Docker-builtin config.toml file with contents in data key. In this approach, configToml.data contains no default configuration, and users setting enabled: true will not know that this will override default Docker-builtin config completely and consequences.

2. If we don't use enabled switch, then configToml contents ALWAYS override Docker-builtin config.toml, so users don't even have to know about that config file built in the docker image. This way we can embed default configuration for Helm deployment directly into values.yaml, and it always get injected. If user needs to change it, they can add/remove configuration options in their custom values.yaml-file.

[arschles]

@hypnoglow you're welcome

Standalone app can run with defaults (should be documented) with the possible custom config.toml or environment variables provided by user.

This is working now. You can use the -config_file flag (we will need to document that)

Docker should run with defaults or probably with minimal default config.toml with only docker-specific options tuned, if it is even required. Custom configuration should be provided as config.toml-volume or environment variables by user.

We would need to them remove the current config.toml file from the docker image, which will cause backward compatibility issues. And that is the root problem we will need to solve.

Would it be helpful for Athens to have an environment variable like CONFIG_FILE:

• Set to /path/to/file.toml to tell Athens to read from a specific config file. you can already pass it the -config_file flag, but that is more difficult when running in a docker container
• Set to off to tell Athens to completely ignore the config file and go with hard-coded defaults and environment variables

Helm Chart and other Kubernetes deployment builds on top of the Docker image, but with default configuration tuned with k8s-specific values (only if required), present in values.yaml or similar and allowing to be customized by user.

We still will need to keep the default config.toml in the docker image, but we can also make it easier here to mount your own config.toml file or turn it off

What do you think?

[arschles]

(P.S. I'll respond much more quickly this time. I have far less travel this year 😄)

[arschles]

Closing because you can already achieve this with configEnvVars. I'll come back to this if that approach ends up being too clumsy or difficult