[BUG] Deployment left permanently paused if resume's Get call fails — stale timer entry disables the missing-timer recovery

[charu1912]

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 (

Reloader/internal/pkg/handler/pause_deployment.go

Lines 179 to 218 in 007a95b

	func ResumeDeployment(deployment *app.Deployment, namespace string, clients kube.Clients) {
	deploymentName := deployment.Name
	currentDeployment, err := clients.KubernetesClient.AppsV1().Deployments(namespace).Get(context.TODO(), deploymentName, metav1.GetOptions{})
	if err != nil {
	logrus.Errorf("Failed to get deployment '%s' in namespace '%s': %v", deploymentName, namespace, err)
	return
	}
	if !IsPausedByReloader(currentDeployment) {
	logrus.Infof("Deployment '%s' in namespace '%s' not paused by Reloader. Skipping resume", deploymentName, namespace)
	return
	}
	deploymentFuncs := GetDeploymentRollingUpgradeFuncs()
	resumePatch, err := CreateResumePatch()
	if err != nil {
	logrus.Errorf("Failed to create resume patch for deployment '%s': %v", deploymentName, err)
	return
	}
	// Remove the timer
	timerKey := getTimerKey(namespace, deploymentName)
	if timer, exists := activeTimers[timerKey]; exists {
	timer.Stop()
	delete(activeTimers, timerKey)
	logrus.Debugf("Removed pause timer for deployment '%s' in namespace '%s'", deploymentName, namespace)
	}
	err = deploymentFuncs.PatchFunc(clients, namespace, currentDeployment, patchtypes.StrategicMergePatchType, resumePatch)
	if err != nil {
	logrus.Errorf("Failed to resume deployment '%s' in namespace '%s': %v", deploymentName, namespace, err)
	return
	}
	logrus.Infof("Successfully resumed deployment '%s' in namespace '%s'", deploymentName, namespace)
	}

)), 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 (

Reloader/internal/pkg/handler/pause_deployment.go

Lines 104 to 121 in 007a95b

	if !IsPausedByReloader(deployment) {
	logrus.Infof("Deployment '%s' in namespace '%s' already paused", deploymentName, namespace)
	return deployment, nil
	}
	// Deployment has already been paused by reloader, check for timer
	logrus.Debugf("Deployment '%s' in namespace '%s' is already paused by reloader", deploymentName, namespace)
	timerKey := getTimerKey(namespace, deploymentName)
	_, timerExists := activeTimers[timerKey]
	if !timerExists {
	logrus.Warnf("Timer does not exist for already paused deployment '%s' in namespace '%s', creating new one",
	deploymentName, namespace)
	HandleMissingTimer(deployment, pauseDuration, clients, namespace)
	}
	return deployment, nil
	}

)) 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

1. Add deployment.reloader.stakater.com/pause-period: 10s to a Deployment.
2. Trigger a watched ConfigMap/Secret change so Reloader pauses it and schedules the resume timer.
3. 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).
4. 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}}'