Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow configuring the mongodb connection using environment variables … #279

Conversation

BobClaerhout
Copy link
Contributor

…in a configmap

Signed-off-by: Bob Claerhout claerhout.bob@gmail.com

@BobClaerhout BobClaerhout force-pushed the configureTheMongoDbConnectionWithAConfigMap branch 4 times, most recently from 64750cb to 31e7b27 Compare August 6, 2021 13:40
@calohmn
Copy link
Contributor

calohmn commented Aug 9, 2021

So, if I see it correctly, in order to use this, one would for example create a configMap myMongoDBConfig and put
HONO_MONGODB_HOST: myHost
in it.
For the deployment, one would then use

$ helm install --set deviceRegistryExample.mongoDBBasedDeviceRegistry.mongodb.configMap=myMongoDBConfig --set deviceRegistryExample.type=mongodb eclipse-hono eclipse-iot/hono

A disadvantage of this approach would be that the overall configuration when looking at the deployed resources would be more complicated. The eclipse-hono-service-device-registry-conf secret and the application.yml in it wouldn't contain the complete picture, but the mongo DB parts would be overwritten by the env entries in the eclipse-hono-service-device-registry Deployment spec.
But I see that offering a possibility to override certain parts of the config like this (or setting env variables in general) could be a useful feature.
I would suggest making this approach more generic then, applying it to other components as well:

Instead of using .Values.deviceRegistryExample.mongoDBBasedDeviceRegistry.mongodb,
the entry to set could be
.Values.deviceRegistryExample.mongoDBBasedDeviceRegistry.envConfigMap
(and also .Values.adapters.http.envConfigMap for example).

This could then be included like this:

{{- include "hono.component.envConfigMap" $args | indent 6 }}

(The envConfigMap value would be taken from the "componentConfig" entry of the $args parameter).

And coming back to the disadvantage above, in this particular case of overriding the MongoDB config, one could also set --set deviceRegistryExample.mongoDBBasedDeviceRegistry.mongodb.host=set_via_env to make the eclipse-hono-service-device-registry-conf secret application.yml more understandable and prevent errors caused by the potential mix of the otherwise used mongoDB default config properties with the properties overridden via env variables (if for example forgetting to override some value).

@BobClaerhout BobClaerhout force-pushed the configureTheMongoDbConnectionWithAConfigMap branch 6 times, most recently from 9a0201a to 535baa6 Compare August 9, 2021 14:03
@sophokles73
Copy link
Member

I am not sure if I understand the purpose of supporting this. @BobClaerhout can you maybe explain which advantage you see in configuring the Mongo DB connection in a ConfigMap as compared to a dedicated yaml file that you specify when deploying the chart via helm?

@BobClaerhout
Copy link
Contributor Author

We are currently using ArgoCD to deploy our applications to different clusters. Currently, our configuration is scattered all over the place because every helm deploy (we have about 20) has it's own configuration. For us, it would be helpful to have the ability to group all configuration in one seperate helm chart. This seperate helm chart, create the configuration (configmaps and secrets) for all other deploys resulting in using the default deploy as close as possible.

@sophokles73
Copy link
Member

Currently, our configuration is scattered all over the place because every helm deploy (we have about 20) has it's own configuration.

So, my understanding is that you want to deploy Hono to 20 different kubernetes clusters using configuration that is (potentially) specific to the particular target environment, right?

For us, it would be helpful to have the ability to group all configuration in one seperate helm chart.

Not sure if I can follow. What do you mean with all configuration? The 20 specific sets of configuration properties for the different target environments? And by one separate helm chart you are referring to a helm chart that you have created yourself?

@BobClaerhout
Copy link
Contributor Author

BobClaerhout commented Aug 9, 2021

So, my understanding is that you want to deploy Hono to 20 different kubernetes clusters using configuration that is (potentially) specific to the particular target environment, right?

No, we have around 20 helm charts deployed to 1 cluster. These 20 helm deploys all have there separate configurations. And it is the configuration of those 20 helm charts we would like to group in one addition helm deploy with the configuration for all of them.

Not sure if I can follow. What do you mean with all configuration? The 20 specific sets of configuration properties for the different target environments? And by one separate helm chart you are referring to a helm chart that you have created yourself?

Yes, a new helm chart we've created ourselves. Since most of the environments will use similar setups (but different passwords, endpoints, ...) we should be able to only configure one helm chart (using a values file per environment) compared to having a values files for each helm deploy within an environment (which would result in a factor 20 more values files)

@BobClaerhout BobClaerhout force-pushed the configureTheMongoDbConnectionWithAConfigMap branch 2 times, most recently from d4b4df4 to 3834180 Compare August 9, 2021 15:12
@calohmn
Copy link
Contributor

calohmn commented Aug 16, 2021

Yes, a new helm chart we've created ourselves. Since most of the environments will use similar setups (but different passwords, endpoints, ...) we should be able to only configure one helm chart (using a values file per environment) compared to having a values files for each helm deploy within an environment (which would result in a factor 20 more values files)

With the changes in this PR, there would only be the possibility to override/set config options that can be set via env variables in the main container of the Hono components. So, when there are differences in other part of the configuration of your environments (requiring values.yaml changes), this approach wouldn't work.

Is it, that the other helm charts you are deploying use a similar mechanism of being configured via values taken from configmaps?
In general, in the scenario you describe, I would have thought of defining an umbrella chart containing the 20 helm charts as subcharts, and then configuring the environment specific settings in the values.yaml of that chart.
I haven't used ArgoCD but from what I read, there seem to be possibilites to also apply templating to the umbrella values.yaml, using variables in values. Integration with 'helmfile' seems to be an option there (argoproj/argo-cd#2143).

@@ -284,6 +284,9 @@ jaegerBackendExample:
# secretName: "other-stuff"
# mountPath: "/etc/other"

# The configMap to get additional environment variabes from for this deploy.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: variabes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch!

@BobClaerhout BobClaerhout force-pushed the configureTheMongoDbConnectionWithAConfigMap branch from 3834180 to ab8506e Compare August 17, 2021 07:20
…in a configmap

Signed-off-by: Bob Claerhout <claerhout.bob@gmail.com>
@BobClaerhout BobClaerhout force-pushed the configureTheMongoDbConnectionWithAConfigMap branch from ab8506e to 72f846a Compare August 17, 2021 07:20
@BobClaerhout
Copy link
Contributor Author

With the changes in this PR, there would only be the possibility to override/set config options that can be set via env variables in the main container of the Hono components. So, when there are differences in other part of the configuration of your environments (requiring values.yaml changes), this approach wouldn't work.

Yes, indeed. This requires all configuration properties you'd like to change to be configurable via env variables. I only see this as something additional (which allows me to do what I want currently).

Is it, that the other helm charts you are deploying use a similar mechanism of being configured via values taken from configmaps?
In general, in the scenario you describe, I would have thought of defining an umbrella chart containing the 20 helm charts as subcharts, and then configuring the environment specific settings in the values.yaml of that chart.
I haven't used ArgoCD but from what I read, there seem to be possibilites to also apply templating to the umbrella values.yaml, using variables in values. Integration with 'helmfile' seems to be an option there (argoproj/argo-cd#2143).

Yes, you're right. And in fact we have something similar right now which pushes the configuration down from an umbrella chart. However, we have to do an helm upgrade in order to change configuration which is not what we want when using GitOps. Next to that, the proposed solution in the argoCd issue also requires some changes because we need an init container for each container having that configuration (If I read it correctly). I think this PR is a small addition to what's already available and would make our live a little bit easier :). Do you have any objections against this? If so, what are your objections?

@calohmn
Copy link
Contributor

calohmn commented Aug 17, 2021

I think this PR is a small addition to what's already available and would make our live a little bit easier :). Do you have any objections against this? If so, what are your objections?

No objections from my side, I also think it can be useful.

Copy link
Contributor

@calohmn calohmn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@calohmn calohmn merged commit 2fcb53a into eclipse:master Aug 17, 2021
@sophokles73 sophokles73 added enhancement New feature or request Hono labels Apr 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Hono
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants