Fail fast on 401, and reload the service account token - #9
Closed
maboelnour wants to merge 1 commit into
Closed
Conversation
maboelnour
marked this pull request as draft
July 27, 2026 19:10
Owner
|
There are 2 architecturally different approaches in the PR and they can't happen at the same time.
Either pick 1 or 2 but not both! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What's wrong
After a cluster upgrade our scaltainer pod quietly stopped scaling anything, logging this on every tick:
The deployment exists. The API server returned 401 — it couldn't authenticate us at all. But
get_servicewraps every exception into "Could not find resource…", so a rejected credential reads like a missing object.And it never stops.
NetworkErroris aRuntimeError, anditerate_servicesrescuesRuntimeErrorso one bad service doesn't stop the rest. Sensible for a missing deployment, wrong for a dead credential: every service fails on every tick, forever, and the process still exits 0. The pod looks healthy from the outside.Why the credential dies
We read the token once at startup and hand Kubeclient the string:
Since Kubernetes 1.21, in-cluster tokens are short-lived and kubelet rotates the file in place. We keep presenting our boot-time copy while a valid token sits on disk next to us. Clusters have been papering over this with
--service-account-extend-token-expiration; when that stopped applying for us, the latent bug became an outage.The fix
Hand over the path instead of the contents, so Kubeclient can re-read it:
This needs both a code change and a version bump, because neither works alone:
bearer_token:bearer_token_file:4.10.0 added a
get_headersthat re-reads the file per request (#566, closing #561); it doesn't exist in 4.9.3. So the gemspec goes to~> 4.10— not just the lockfile.~> 4.9would still permit 4.9.3, wherebearer_token_fileis accepted silently and read only once, quietly reintroducing this exact bug with no error to hint at it. Lockfile pins 4.13.0.Also: stop swallowing 401
Even with the above, a rejected credential shouldn't look like a missing resource and shouldn't loop forever. Adds
Scaltainer::AuthenticationError, converts a 401 into it, and exits 77 so the orchestrator restarts us.The non-obvious bit: it's a
StandardError, not anApplicationErrorlike everything else here. That's deliberate — if it inherited fromApplicationErrorit would be aRuntimeError,iterate_serviceswould swallow it, and nothing would change.runner.rbalso needs two explicit re-raises, since the barerescue => einget_serviceandscale_outwould otherwise relabel it right back.Only 401. 403 and 404 keep skipping and continuing as before — a restart can't fix those, so crash-looping on them would hide the cause and block the other services. A 401 is always fixed by a restart. Swarm is untouched.
Exit code 77 is just to keep this distinguishable in
kubectl describe. Happy to make it plain1.Tests
New specs cover the token path being passed rather than its contents, the exception hierarchy (that it isn't a
RuntimeError), 401 converting while 403/404 pass through, and all three call sites propagating it while still swallowing ordinary errors.Notes
FROM ruby:2.7, so this is fine, but worth knowing the floor moved.Gemfile.lockwas edited by hand;httpstays at 4.4.1, which still satisfies the relaxed>= 3.0, < 6.0. Worth abundle installto confirm the resolution.