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

[Common Config Bootstrapper] Apply env overrides to values only when loaded from file #4448

Closed
lenny-goodell opened this issue Mar 15, 2023 · 3 comments · Fixed by #4449
Closed
Labels
enhancement New feature or request
Milestone

Comments

@lenny-goodell
Copy link
Member

lenny-goodell commented Mar 15, 2023

As decide in 3/15/2023 Architects meeting, Config Provider will be the System of Record for services' values.
This means that env overrides are only applied when configuration is loaded from file, including the common config file.
They are not applied to values loaded from Config Provider.
The overridden values are pushed into the Config Provider the first time the service starts or when the -o flag is used.

@lenny-goodell lenny-goodell added the enhancement New feature or request label Mar 15, 2023
@lenny-goodell lenny-goodell added this to the Minnesota milestone Mar 15, 2023
@lenny-goodell lenny-goodell changed the title [Common Config Bootstrapper] Apply overrides to values once loaded from file [Common Config Bootstrapper] Apply env overrides to values once loaded from file Mar 15, 2023
@lenny-goodell lenny-goodell changed the title [Common Config Bootstrapper] Apply env overrides to values once loaded from file [Common Config Bootstrapper] Apply env overrides to values only when loaded from file Mar 15, 2023
@lenny-goodell lenny-goodell added hold Intended for PRs we want to flag for ongoing review and removed hold Intended for PRs we want to flag for ongoing review labels Mar 15, 2023
lenny-goodell pushed a commit to lenny-goodell/edgex-go that referenced this issue Mar 15, 2023
lenny-goodell pushed a commit that referenced this issue Mar 16, 2023
* feat: Add env override capability for common configuration

closes #4448

Signed-off-by: Leonard Goodell <[email protected]>
@farshidtz
Copy link
Member

I believe the agreement was to remove the EdgeX v2 confusion that was being caused by pushing environment overrides to the config provider during startup. To my understanding, it was decided to apply environment variable overrides only when loading config from a file, that is when config provider is not used, and ignore them otherwise. This was documented in this issue originally as:

As decide in 3/15/2023 Architects meeting, Consul will be the System of Record for services' values.

This means that override are only applied when configuration is loaded from file, including the common config file.

However, the issue description and the implementation is now different and the EdgeX v2 issue persists. The user sets something during startup and it sticks. When unset, the override doesn't go away.

@cloudxxx8
Copy link
Member

@farshidtz if we don't push the environment overrides to the Config Provider during the first time startup, the Config Provider would be seeded with the default config files only. For example, the other service client host will be localhost instead of core-metadata, which is the correct host name in the docker network environment. We leverage the environment variable overrides to adjust the config in the docker-compose files. It won't work in docker environment if we don't push the initial values.

@farshidtz
Copy link
Member

The Common Config ADR is being modified to formalize the decisions: edgexfoundry/edgex-docs#1025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants