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

Be more judicious about cleaning up service account tokens #10041

Merged
merged 2 commits into from Jun 19, 2015

Conversation

liggitt
Copy link
Member

@liggitt liggitt commented Jun 18, 2015

As we moved some of our controllers to use service account credentials to run, we ran into a bug (openshift/origin#3298) where the token controller was "cleaning up" tokens it didn't find a service account for. As a result, those controllers' reflectors API calls started failing.

Somehow, the service account whose addition to the serviceAccounts store triggered token creation is missing from the same store when the secret addition is observed. @deads2k, @smarterclayton and I are still trying to figure out how that is possible, but in the meantime, this PR:

  • adds better logging around failed token auth to make it easier to debug situations like this
  • verifies a service account is actually missing if there is a cache miss before cleaning up a token for it

@liggitt
Copy link
Member Author

liggitt commented Jun 18, 2015

@lavalamp PTAL

@k8s-bot
Copy link

k8s-bot commented Jun 18, 2015

GCE e2e build/test passed for commit 0e9a4d52ec271090e5dadb0c73ac862d4ce91f31.

@k8s-bot
Copy link

k8s-bot commented Jun 18, 2015

GCE e2e build/test passed for commit 1d3dca67962272b82610571a9619d97daf343ed4.

@@ -24,6 +24,8 @@ import (
"io/ioutil"
"strings"

"github.com/golang/glog"

Copy link
Member

Choose a reason for hiding this comment

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

nit: this and jwt-go should be in the same block, and they should be after the kubernetes imports.

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

@lavalamp
Copy link
Member

just a few pedantic comments...

@lavalamp
Copy link
Member

LGTM

@lavalamp lavalamp added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 19, 2015
@k8s-bot
Copy link

k8s-bot commented Jun 19, 2015

GCE e2e build/test passed for commit 1ee5578.

@satnam6502
Copy link
Contributor

Merging since this fixes a bug.

satnam6502 added a commit that referenced this pull request Jun 19, 2015
Be more judicious about cleaning up service account tokens
@satnam6502 satnam6502 merged commit 7e2e78b into kubernetes:master Jun 19, 2015
@liggitt liggitt deleted the token_delete_live_check branch June 26, 2015 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants