[BUG] Deployment left permanently paused if resume's Get call fails — stale timer entry disables the missing-timer recovery
Describe the bug
When using the deployment.reloader.stakater.com/pause-period annotation, a Deployment can be left with spec.paused: true forever if the Kubernetes API is transiently unavailable at the exact moment Reloader's resume timer fires — even though Reloader has a "missing timer" recovery path (HandleMissingTimer) that's supposed to catch exactly this case.
Root cause: in ResumeDeployment (pause_deployment.go:179-218 (https://github.com/stakater/Reloader/blob/007a95b91487c6291095d89efed1e9efead85457/internal/pkg/handler/pause_deployment.go#L179-L218)), the timer bookkeeping entry (activeTimers[timerKey]) is only deleted after a successful Get of the Deployment:
currentDeployment, err := clients.KubernetesClient.AppsV1().Deployments(namespace).Get(...) if err != nil { logrus.Errorf("Failed to get deployment '%s' ...", ...) return // activeTimers[timerKey] is never cleaned up } ... // Remove the timer if timer, exists := activeTimers[timerKey]; exists { timer.Stop() delete(activeTimers, timerKey) }
If that Get call errors, the function returns before reaching the delete(activeTimers, timerKey) line, leaving a stale, already-fired timer entry in the map.
On the next watched change for that Deployment, PauseDeployment (pause_deployment.go:104-121 (https://github.com/stakater/Reloader/blob/007a95b91487c6291095d89efed1e9efead85457/internal/pkg/handler/pause_deployment.go#L104-L121)) only calls HandleMissingTimer (the recovery path) if activeTimers[timerKey] is missing:
timerKey := getTimerKey(namespace, deploymentName) _, timerExists := activeTimers[timerKey]
if !timerExists { HandleMissingTimer(deployment, pauseDuration, clients, namespace) }
Because the stale entry from the failed resume is still present, timerExists is true, so HandleMissingTimer is never invoked — the Deployment stays paused indefinitely, with no further retry ever attempted, even though later changes to it are still detected.
To Reproduce
- Add deployment.reloader.stakater.com/pause-period: 10s to a Deployment.
- Trigger a watched ConfigMap/Secret change so Reloader pauses it and schedules the resume timer.
- Make the apiserver return an error for the Get deployments.apps call in the ~10s window when the resume timer fires (e.g. via a proxy/chaos tool, or naturally under heavy control-plane load).
- Observe: the Deployment remains spec.paused: true forever; subsequent changes to its watched ConfigMaps/Secrets are logged as detected but never trigger a resume.
Expected behavior
Even if a resume attempt fails due to a transient API error, Reloader should eventually recover and un-pause the Deployment — either by retrying the resume, or by letting the existing HandleMissingTimer recovery path do its job on the next change event (which it currently can't, because of the stale activeTimers entry).
Screenshots If applicable, add screenshots to help explain your problem.
Environment
- Operator Version: v1.4.17
- Kubernetes/OpenShift Version: N/A — not version-specific; triggered by any transient apiserver unavailability/timeout during the resume window
Additional context
Hit in multiple clusters during a platform scale-up under heavy control-plane load: a Deployment was paused for a routine Secret/ConfigMap reload, the resume attempt collided with a brief apiserver outage, and the Deployment was left paused indefinitely, causing a downstream health check to time out and fail.
Suggested fix: clean up activeTimers[timerKey] unconditionally on entry to ResumeDeployment (e.g. via defer), not only after the Get/Patch succeed, so a failed resume attempt doesn't permanently disable HandleMissingTimer's recovery on the next change event. Retry/backoff on the Get/Patch calls themselves would also help, but fixing the cleanup ordering alone restores the self-healing behavior the code already appears to intend.
Workaround: kubectl patch deployment -n -p '{"spec":{"paused":false}}'
Source: stakater/Reloader