fix(vpn): retry cert rotation when job fails instead of dropping it#392
Open
Ady0333 wants to merge 1 commit into
Open
fix(vpn): retry cert rotation when job fails instead of dropping it#392Ady0333 wants to merge 1 commit into
Ady0333 wants to merge 1 commit into
Conversation
A failed or suspended cert-rotation Job caused the reconciler to return
ctrl.Result{} with no error, so the work queue forgot the VpnKeyRotation
permanently and certs expired with no recovery. Requeue after 30s and
reset jobCreationInProgress so the job is recreated.
Signed-off-by: Ady0333 <adityashinde1525@gmail.com>
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.
Description
When a VPN certificate-rotation Job enters a Failed or Suspended state,
reconcileVpnKeyRotationConfig returned ctrl.Result{} with a nil error. Back in
ReconcileVpnKeyRotation, this produced return ctrl.Result{}, nil controller-runtime treats that as
success and removes the VpnKeyRotation object from the work queue permanently.
Because the VpnKeyRotation controller registers only For(&VpnKeyRotation{}) (no
Owns(&batchv1.Job{})), Job status changes generate no reconcile events. The object would only ever
be reconciled again on a spec change or a controller restart. Meanwhile the existing certificates
march toward CertificateExpiryTime with no replacement once they expire, the slice's gateway-pair
TLS handshakes fail and all cross-cluster traffic on that slice blackouts, with no self-healing.
The jobCreationInProgress flag was also never reset on the failure path, so even a forced reconcile
would skip triggerJobsForCertCreation and never recreate the failed Job.
This is realistic: Kubernetes Jobs fail routinely (image pull errors, transient API failures,
ResourceQuota rejection, node pressure, backoffLimit exhaustion). It is a silent failure no error, no requeue, no stuck-loop to alert on.
Fix: In both JobStatusError || JobStatusSuspended branches of reconcileVpnKeyRotationConfig, reset
jobCreationInProgress and return ctrl.Result{RequeueAfter: 30 * time.Second} (matching the cadence
already used for JobStatusRunning/JobNotCreated). The reconciler now retries rotation and recreates
the failed Job, self-healing once the underlying cause clears.
How Has This Been Tested?
Added Test_VpnCertRotationJobFailure_NoRequeue in service/vpn_cert_rotation_job_failure_bug_test.go. The test was first written to confirm the bug (asserting RequeueAfter == 0 and
jobCreationInProgress stuck true), then updated to verify the fix.
Checklist:
Does this PR introduce a breaking change for other components like worker-operator?
No. The change only affects the failure-handling path of the VpnKeyRotation reconciler successful
rotation behavior is unchanged. No API, CRD, or worker-facing contract changes.