feat: Add Self seeding, env var overrides, cmd-line options per ADR 0005-Service-Self-Config.md #59

lenny-goodell · 2020-03-20T19:05:06Z

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://wiki.edgexfoundry.org/display/FA/Contributor%27s+Guide
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

What is the current behavior?

Services rely on config-seed to push configuration into Configuration Provider and to apply environment overrides
Environment overrides don't get applied when local configuration is used
Changes in Configuration Provider trump environment overrides

Issue Number: #14

What is the new behavior?

All services will self-seed configuration into Configuration Provider
All services will apply environment overrides when:
- Using local configuration
- When pushing configuration into Configuration Provider
- When pulling configuration from Configuration Provider
Environment overrides trump changes in Configuration Provider
-o/--overwrite option added to force push configuration into Configuration Provider
-c was added for short version of --confdir for consistency
-r/-registry changed to -r/-registry=<url> for backward compatibility with Device Services
edgex_registry environment variable added back to avoid breaking changes.
Changed edgex_configuration_provider to EDGEX_CONFIGURATION_PROVIDER since it is new for Geneva.

Does this PR introduce a breaking change?

Yes
No

Per ADR 0005-Service-Self-Config.md, backward compatibility has been maintained.

Are there any specific instructions or things that should be known prior to reviewing?

This draft PR (HERE) for edgex-go was used to test these changes with the following test case:

Run core-data with following command-line and environment variable combinations

Precondition for all these testcases:

Database and Consul are running
- start them from docker compose file using "docker-compose -f <file> up redis consul"
- Some test case require clean start of Consul
  - docker-compose -f <file> down
  - docker rm $(docker ps -a)
  - docker volume prune
  - docker-compose -f <file> up redis consul

TestCase: No command line options & no environment variable
Additional Preconditions: None
Command: ./core-data
Result:
Logs contains

Loaded configuration from ./res/configuration.toml"
Using local configuration from file (0 environment overrides applied)
Web server starting (localhost:48080)

Logs don't contain

Anything about environment overrides
Anything about use of Configuration provider
Anything about use of Registry Provider

TestCase: -confdir & -file & -profile & no environment variable
Additional Preconditions: None
Command: ./core-data -confdir=./confg -file=config.toml -profile=junk
Result:

Fails to start
Logs contains:

could not load configuration file (./confg/junk/config.toml): open ./confg/junk/config.toml: The system cannot find the path specified.

TestCase: EDGEX_CONF_DIR & EDGEX_CONFIG_FILE & EDGEX_PROFILE environment variables
Additional Preconditions: None
Command: EDGEX_CONF_DIR=./files EDGEX_CONFIG_FILE=myconfig.toml EDGEX_PROFILE=dummy ./core-data
Result:

Fails to start

Logs contains

Environment override of '-c/-confdir' by environment variable: EDGEX_CONFIG_FILE=./files
Environment override of '-p/-profile' by environment variable: EDGEX_PROFILE=dummy
Environment override of '-f/-file' by environment variable: EDGEX_CONFIG_FILE=myconfig.toml
could not load configuration file (./files/dummy/myconfig.toml): open ./files/dummy/myconfig.toml: The system cannot find the path specified.

TestCase: * Backward compatibility for old edgex_profile environment variable*
Additional Preconditions: None
Command: edgex_profile=dummy ./core-data
Result:

Fails to start

Logs contains

Environment override of '-p/-profile' by environment variable: EDGEX_PROFILE=dummy		could not load configuration file (./res/dummy/configuration.toml): open ./res/dummy/configuration.toml: The system cannot find the pat
h specified.

TestCase: -r & Service_Port=18080 override
Additional Precondition: Consul restarted fresh (no data), change local config to have Registry.Host = 127.0.0.1
Command: Service_Port=18080 ./core-data -r
Result:
Logs contains

Environment override of 'Service.Port' by environment variable: Service_Port=18080
Using Configuration provider (consul) from: http://127.0.0.1:8500"
Configuration has been pushed to into Configuration Provider (1 environment overrides applied)
Using Registry (consul) from http://127.0.0.1:8500"
Web server starting (localhost:18080)

Core data in Consul's list of registered services with port 18080 (consul will show health error due to docker can't hit system's localhost)

TestCase: -cp & Service_Port=58080 override
Additional Precondition: Consul restarted fresh (no data)
Command: Service_Port=58080 ./core-data -cp
Result:
Logs contains

Environment override of 'Service.Port' by environment variable: Service_Port=58080
Using Configuration provider (consul) from: http://localhost:8500"
Configuration has been pushed to into Configuration Provider (1 environment overrides applied)
Web server starting (localhost:58080)

Logs don't contain

Anything about use of Registry Provider

Core data NOT in Consul's list of registered services

TestCase: -cp & Service_Port=58080 override (consul has port as 9999)
Additional Precondition: core-data config is in Consul and Service.Port changed to 9999 in consul
Command: Service_Port=58080 ./core-data -cp
Result:
Logs contains

Environment override of 'Service.Port' by environment variable: Service_Port=58080
Using Configuration provider (consul) from: http://localhost:8500"
Configuration has been pulled from Configuration provider (1 environment overrides applied)
Web server starting (localhost:58080) **** Still set to 58080 ****

Logs don't contain

Anything about use of Registry Provider

Core data NOT in Consul's list of registered services

TestCase: overwrite flag
Additional Precondition: core-data config is in Consul and Writable/PersistData = true in Consul
Command: Writable_PersistData=false ./core-data -cp -o
Result:
Logs contains

Environment variable override of Writable.PersistData by: Writable_PersistData=false
Using Configuration provider (consul) from: http://localhost:8500"
Configuration has been pushed to into Configuration Provider (1 environment overrides applied)
Web server starting (localhost:48080)

Logs don't contain

Anything about use of Registry Provider

Core data NOT in Consul's list of registered services
Writable/PersistData = false in Consul

TestCase: -cp=<url> & no environment variable override
Additional Precondition: core-data config is in Consul and Service.Port changed to 9999 in consul
Command: ./core-data -cp=consul.http://127.0.0.1:8500
Result:
Logs contains

Using Configuration provider (consul) from: http://127.0.0.1:8500"
Configuration has been pulled from Configuration provider (0 environment overrides applied)
Web server starting (localhost:9999) <===  changed to 9999

Logs don't contain

Anything about environment overrides
Anything about use of Registry Provider

Core data NOT in Consul's list of registered services

TestCase: Backwards compatibility for Device Services
Additional Precondition: Consul restarted fresh (no data)
Command: ./core-data -r=consul://localhost:8500
Result:
Logs contains

Config Provider URL created from -r/-registry=<url> flag
Using Configuration provider (consul) from: http://localhost:8500
Configuration has been pushed to into Configuration Provider (0 environment overrides applied)
Using Registry (consul) from http://localhost:8500
Web server starting (localhost:48080)

Logs don't contain

Anything about environment overrides

Core data in Consul's list of registered services with port 48080(consul will show health error due to docker can't hit system's localhost)

TestCase: EDGEX_CONFIGURATION_PROVIDER environment variable
Additional Precondition: Consul has core-data config
Command: EDGEX_CONFIGURATION_PROVIDER=consul.http://127.0.0.1:8500 ./core-data
Result:
Logs contains

Environment override of 'Configuration Provider Information' by environment variable: EDGEX_CONFIGURATION_PROVIDER=consul.http://127.0.0.1:8500		
Using Configuration provider (consul) from: http://127.0.0.1:8500
Configuration has been pulled from Configuration provider (0 environment overrides applied)
Web server starting (localhost:48080)

Logs don't contain

Anything about use of Registry Provider

Core data NOT in Consul's list of registered services

TestCase: Backwards compatibility for edgex_registry environment variable
Additional Precondition: Consul has core-data config, but not service registration.
Command: Service_Port=18080 edgex_registry=consul://127.0.0.1:8500 ./core-data
Result:
Logs contains

Environment variable override of 'Service.Port' by: Service_Port=18080
Environment override of 'Configuration Provider Information' by environment variable: edgex_registry=consul://127.0.0.1:8500
Using Configuration provider (consul) from: http://127.0.0.1:8500"
Configuration has been pulled from Configuration provider (1 environment overrides applied)
Environment override of 'Registry Provider Information' by environment variable: edgex_registry=consul://127.0.0.1:8500
Using Registry (consul) from http://127.0.0.1:8500"
Web server starting (localhost:18080)

Core data in Consul's list of registered services with port 18080 (consul will show health error due to docker can't hit system's localhost)

TestCase: Backwards compatibility for startup_duration & startup_interval environment variables
Additional Precondition: None.
Command: startup_interval=5 startup_duration=10 ./core-data
Result:
Logs contains

Environment override of 'Startup Duration' by environment variable: startup_duration=10
Environment override of 'Startup Interval' by environment variable: startup_interval=5
Using local configuration from file (0 environment overrides applied)
Web server starting (localhost:48080)

TestCase: New EDGEX_STARTUP_INTERVAL & EDGEX_STARTUP_DURATION environment variables
Additional Precondition: None.
Command: EDGEX_STARTUP_INTERVAL=6 EDGEX_STARTUP_DURATION=15 ./core-data
Result:
Logs contains

Environment override of 'Startup Duration' by environment variable: EDGEX_STARTUP_DURATION=15
Environment override of 'Startup Interval' by environment variable: EDGEX_STARTUP_INTERVAL=6
Using local configuration from file (0 environment overrides applied)
Web server starting (localhost:48080)

Other information

jpwhitemn

looking good Lenny. Thanks for adding all the TODOs for 2.0 work.

go.mod

bootstrap/bootstrap.go

codecov-io · 2020-03-20T23:43:17Z

Codecov Report

Merging #59 into master will increase coverage by 6.80%.
The diff coverage is 47.91%.

@@            Coverage Diff             @@
##           master      #59      +/-   ##
==========================================
+ Coverage   47.17%   53.97%   +6.80%     
==========================================
  Files           9        8       -1     
  Lines         602      465     -137     
==========================================
- Hits          284      251      -33     
+ Misses        309      206     -103     
+ Partials        9        8       -1

Impacted Files	Coverage Δ
bootstrap/config/config.go	`0.00% <0.00%> (ø)`
bootstrap/flags/flags.go	`61.42% <63.63%> (+2.60%)`	⬆️
bootstrap/registration/registry.go	`50.84% <67.85%> (ø)`
bootstrap/config/environment.go	`90.90% <90.35%> (-0.76%)`	⬇️
bootstrap/config/provider.go	`85.71% <100.00%> (+0.71%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ecac4d1...32b5fa0. Read the comment docs.

SteveOss

Looks good to me

jim-wang-intel

LGTM

bootstrap/bootstrap.go

bootstrap/config/config.go

bootstrap/bootstrap.go

bootstrap/config/environment.go

lenny-goodell · 2020-03-26T23:30:04Z

@tsconn23 , @michaelestrin , @iain-anderson , I have pushed the requested changes under a new commit. I am in process of running through all my tests cases (above) using my edgex-go PR.

…005-Service-Self-Config.md closes #14 Signed-off-by: lenny <[email protected]>

Signed-off-by: lenny <[email protected]>

lenny-goodell requested review from michaelestrin, AnthonyMBonafide, iain-anderson, jim-wang-intel, jpwhitemn, rsdmike, SteveOss, tonyespy and tsconn23 March 20, 2020 19:05

jpwhitemn reviewed Mar 20, 2020

View reviewed changes

go.mod Show resolved Hide resolved

jim-wang-intel reviewed Mar 20, 2020

View reviewed changes

bootstrap/bootstrap.go Outdated Show resolved Hide resolved

lenny-goodell force-pushed the SelfSeed2 branch from 56382cd to 51c5424 Compare March 20, 2020 23:36

lenny-goodell force-pushed the SelfSeed2 branch from 6cd33d4 to 2c61f4c Compare March 21, 2020 00:17

SteveOss reviewed Mar 23, 2020

View reviewed changes

lenny-goodell force-pushed the SelfSeed2 branch from 2c61f4c to a041e08 Compare March 23, 2020 16:37

jim-wang-intel approved these changes Mar 24, 2020

View reviewed changes

bootstrap/bootstrap.go Outdated Show resolved Hide resolved

tsconn23 suggested changes Mar 24, 2020

View reviewed changes

bootstrap/config/config.go Outdated Show resolved Hide resolved

tsconn23 reviewed Mar 24, 2020

View reviewed changes

bootstrap/bootstrap.go Outdated Show resolved Hide resolved

michaelestrin reviewed Mar 25, 2020

View reviewed changes

bootstrap/bootstrap.go Outdated Show resolved Hide resolved

iain-anderson reviewed Mar 25, 2020

View reviewed changes

bootstrap/config/environment.go Show resolved Hide resolved

lenny-goodell requested review from michaelestrin, tsconn23 and iain-anderson March 26, 2020 23:30

lenny-goodell force-pushed the SelfSeed2 branch from e858aaa to 47efb14 Compare March 27, 2020 00:10

feat: Add Self seeding, env var overrides, cmd-line options per ADR 0…

97911dc

…005-Service-Self-Config.md closes #14 Signed-off-by: lenny <[email protected]>

lenny-goodell force-pushed the SelfSeed2 branch 3 times, most recently from 4a7b71e to ef763dd Compare March 27, 2020 00:54

refact(config): refactored config processing per PR comments

32b5fa0

Signed-off-by: lenny <[email protected]>

lenny-goodell force-pushed the SelfSeed2 branch from ef763dd to 32b5fa0 Compare March 27, 2020 21:21

tsconn23 approved these changes Mar 27, 2020

View reviewed changes

tsconn23 merged commit e56334c into edgexfoundry:master Mar 30, 2020

lenny-goodell deleted the SelfSeed2 branch March 30, 2020 15:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add Self seeding, env var overrides, cmd-line options per ADR 0005-Service-Self-Config.md #59

feat: Add Self seeding, env var overrides, cmd-line options per ADR 0005-Service-Self-Config.md #59

lenny-goodell commented Mar 20, 2020 •

edited

Loading

jpwhitemn left a comment

codecov-io commented Mar 20, 2020 •

edited

Loading

SteveOss left a comment

jim-wang-intel left a comment

lenny-goodell commented Mar 26, 2020

feat: Add Self seeding, env var overrides, cmd-line options per ADR 0005-Service-Self-Config.md #59

feat: Add Self seeding, env var overrides, cmd-line options per ADR 0005-Service-Self-Config.md #59

Conversation

lenny-goodell commented Mar 20, 2020 • edited Loading

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Are there any specific instructions or things that should be known prior to reviewing?

Other information

jpwhitemn left a comment

Choose a reason for hiding this comment

codecov-io commented Mar 20, 2020 • edited Loading

Codecov Report

SteveOss left a comment

Choose a reason for hiding this comment

jim-wang-intel left a comment

Choose a reason for hiding this comment

lenny-goodell commented Mar 26, 2020

lenny-goodell commented Mar 20, 2020 •

edited

Loading

codecov-io commented Mar 20, 2020 •

edited

Loading