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

fix: Add retry loop for secret client if initial token is invalid #60

Merged
merged 1 commit into from
Mar 26, 2020
Merged

Conversation

bnevis-i
Copy link
Collaborator

@bnevis-i bnevis-i commented Mar 20, 2020

Address a race condition where the container may be started before
security-secrets-setup has created the secrets token for the service.

Signed-off-by: Bryon Nevis [email protected]

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

When user performs

docker-compose -f docker-compose-blah.yml up -d

A number of core services such as support-logging, core-metadata, core-data, and others will fail to start with a message:

level=ERROR ts=2020-03-20T16:33:16.413174639Z app=edgex-core-command source=secret.go:67 msg="unable to create SecretClient: HTTP response with status code 403, message: failed to lookup token"

Issue Number:

What is the new behavior?

Expect to see messages similar to the following which shows that retries are happening. You will either get secrets-token.json doesn't exist, OR will fail to lookup token, depending on whether or not /tmp/edgex/secrets is empty.

level=INFO ts=2020-03-20T19:02:58.651792922Z app=edgex-core-data source=secret.go:60 msg="Creating SecretClient"
level=INFO ts=2020-03-20T19:02:58.65186062Z app=edgex-core-data source=secret.go:71 msg="Attempting to create secret client (attmpt 1 of 11)"
level=ERROR ts=2020-03-20T19:31:52.47329444Z app=edgex-core-data source=secret.go:75 msg="unable to parse secret store configuration: open /tmp/edgex/secrets/edgex-core-data/secrets-token.json: no such file or directory"
level=ERROR ts=2020-03-20T19:02:58.774292175Z app=edgex-core-data source=secret.go:86 msg="Retryable failure (after 1s delay) while creating SecretClient: HTTP response with status code 503, message: failed to lookup token"
level=INFO ts=2020-03-20T19:02:59.774681651Z app=edgex-core-data source=secret.go:71 msg="Attempting to create secret client (attmpt 2 of 11)"
level=ERROR ts=2020-03-20T19:31:52.47329444Z app=edgex-core-data source=secret.go:75 msg="unable to parse secret store configuration: open /tmp/edgex/secrets/edgex-core-data/secrets-token.json: no such file or directory"
level=ERROR ts=2020-03-20T19:02:59.809673211Z app=edgex-core-data source=secret.go:86 msg="Retryable failure (after 1s delay) while creating SecretClient: HTTP response with status code 403, message: failed to lookup token"
level=INFO ts=2020-03-20T19:03:00.810353941Z app=edgex-core-data source=secret.go:71 msg="Attempting to create secret client (attmpt 3 of 11)"
level=ERROR ts=2020-03-20T19:31:52.47329444Z app=edgex-core-data source=secret.go:75 msg="unable to parse secret store configuration: open /tmp/edgex/secrets/edgex-core-data/secrets-token.json: no such file or directory"
level=ERROR ts=2020-03-20T19:03:00.846627763Z app=edgex-core-data source=secret.go:86 msg="Retryable failure (after 1s delay) while creating SecretClient: HTTP response with status code 403, message: failed to lookup token"
level=INFO ts=2020-03-20T19:03:01.846973046Z app=edgex-core-data source=secret.go:71 msg="Attempting to create secret client (attmpt 4 of 11)"
level=INFO ts=2020-03-20T19:03:01.881978472Z app=edgex-core-data source=secret.go:82 msg="Created SecretClient"

Does this PR introduce a breaking change?

  • Yes
  • No

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

In order to test, modify go.mod in edgex-go

module github.com/edgexfoundry/edgex-go

replace github.com/edgexfoundry/go-mod-bootstrap => github.com/bnevis-i/go-mod-bootstrap v0.0.0-20200320192440-3829504ae15b

require (

and rebuild docker containers. Update developer scripts to refer to newly built containers. Bring up compose stack as above.

Other information

@tingyuz
Copy link
Contributor

tingyuz commented Mar 24, 2020

With the recent change of #58 the proxy-setup has same issue as described here. This will be a pre-required PR for proxy-setup to move forward.

@bnevis-i
Copy link
Collaborator Author

@AnthonyMBonafide are you open to help review this?

@AnthonyMBonafide
Copy link
Contributor

Yes, l will take a look

bootstrap/handlers/secret/secret.go Show resolved Hide resolved
bootstrap/handlers/secret/secret.go Outdated Show resolved Hide resolved
bootstrap/handlers/secret/secret.go Outdated Show resolved Hide resolved
@codecov-io
Copy link

codecov-io commented Mar 26, 2020

Codecov Report

Merging #60 into master will increase coverage by 0.41%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #60      +/-   ##
==========================================
+ Coverage   55.76%   56.18%   +0.41%     
==========================================
  Files           8        9       +1     
  Lines         208      493     +285     
==========================================
+ Hits          116      277     +161     
- Misses         91      206     +115     
- Partials        1       10       +9     
Impacted Files Coverage Δ
bootstrap/config/environment.go 91.20% <0.00%> (-8.80%) ⬇️
bootstrap/flags/flags.go 58.82% <0.00%> (-1.56%) ⬇️
bootstrap/config/provider.go 85.00% <0.00%> (-0.72%) ⬇️
bootstrap/config/file.go 0.00% <0.00%> (ø)
bootstrap/config/config.go 0.00% <0.00%> (ø)
bootstrap/registration/registry.go 50.84% <0.00%> (ø)

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 f5ed8c2...b77ab47. Read the comment docs.

@bnevis-i
Copy link
Collaborator Author

Complete rewrite to use the startupTimer instead. Please re-review.

Address a race condition where the container may be started before
security-secrets-setup has created the secrets token for the service.

Signed-off-by: Bryon Nevis <[email protected]>
Copy link
Contributor

@AnthonyMBonafide AnthonyMBonafide left a comment

Choose a reason for hiding this comment

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

LGTM! I verified these changes with unit tests.

@AnthonyMBonafide AnthonyMBonafide merged commit ecac4d1 into edgexfoundry:master Mar 26, 2020
@bnevis-i bnevis-i deleted the issue-230 branch April 24, 2020 00:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants