Summary
HandleConfigUpdates only writes the owned v1.ConfigMap to match spec.packages[*].configMap when completedNodes == nodeCount on the SAME reconcile that sees a CM diff. If that condition only becomes true after the lifecycle reaches a steady state, and no further events fire, the operator goes idle with the K8s ConfigMap permanently out of sync with spec.
The trigger pattern requires three things to line up — each individually plausible — but when all three combine, the resulting flake is non-deterministic but reproducible:
- A previous spec generation left a stale entry in
Status.ConfigUpdates[<pkg>] (never cleared because no HandleConfigUpdates call in the next generation reached line 1250).
- The new spec's
configInterrupts contains a glob that happens to match the stale entry's key.
- The lifecycle re-runs for the stale entry (interrupt → post-interrupt → complete) without ever satisfying the
completedNodes == nodeCount gate for the new CM diff, then the operator goes silent.
Reproducer
k8s-tests/chainsaw/skyhook/config-skyhook step glob update is the canonical reproducer. It flakes in CI — sometimes the post-GC reconcile catches the gate open and the CM gets written; sometimes it doesn't.
The relevant sequence (test step ordering):
no-interrupt update: dexter's ui.properties key changes. HandleConfigUpdates writes Status.ConfigUpdates[dexter] = [\"ui.properties\"]. Lifecycle reaches config/complete with no interrupt fired (no configInterrupt matched ui.properties under the old spec). Entry is never cleared because no further CM diff resets the gate.
glob update: applies a new spec where dexter.configInterrupts now contains *.properties (in addition to game.properties), and dexter.configMap.game.properties becomes \"changed via glob\".
Evidence from a failing run
Captured at assert-timeout (270 s after the spec apply):
```yaml
spec:
packages:
dexter:
configInterrupts:
'*.properties': { services: [rsyslog], type: service }
game.properties: { services: [rsyslog], type: service }
configMap:
game.properties: |
changed via glob # ← new spec applied
ui.properties: |
changed
interrupt: { type: reboot }
status:
observedGeneration: 6
status: complete
configUpdates:
dexter:
- ui.properties # ← stale from the previous step
nodeState:
kind-worker:
dexter|1.2.3:
stage: post-interrupt
state: complete # ← lifecycle 'completed' for the wrong reason
```
The K8s ConfigMap config-skyhook-dexter-1.2.3 still has data.game.properties: \"changed\" (the previous step's value). 4 minutes 30 seconds of zero operator log lines between the last reconcile (19:26:21) and the assert timeout (19:30:51) confirms the controller went idle without ever satisfying the CM-write gate.
Root cause
HandleConfigUpdates (operator/internal/controller/skyhook_controller.go:1185-1281):
```go
if !reflect.DeepEqual(oldConfigMap.Data, newConfigMap.Data) {
for _, node := range skyhook.GetNodes() {
...
if !exists && node.IsPackageComplete(_package) {
completedNodes++
}
...
}
if completedNodes == nodeCount || erroringNode {
...
err := r.Update(ctx, newConfigMap)
}
}
```
The CM write is conditional on every selected node passing \!PodExists && IsPackageComplete in the same reconcile that observes the diff. There are two compounding issues:
Status.ConfigUpdates is not cleaned up on spec changes. It's only cleared by RemoveConfigUpdates (skyhook.go:128), called in three places: package removed from spec (skyhook_controller.go:1038), version change (:1073), or successful HandleConfigUpdates (:1250). A spec edit that changes a configMap value but happens to leave the package lifecycle in a complete state doesn't refresh Status.ConfigUpdates.
- A stale
Status.ConfigUpdates entry can drive a spurious interrupt cycle when the next spec's configInterrupts contains a pattern (literal or glob) that matches it. NodeState.HasInterrupt (skyhook_types.go:544-554) preferentially uses the matching-interrupt path when Status.ConfigUpdates[pkg] is non-empty:
```go
if len(config[_package.Name]) > 0 {
hasInterrupt = len(interrupt[_package.Name]) > 0
} else {
hasInterrupt = _package.HasInterrupt()
}
```
NextStage then routes Config → Interrupt for the stale entry's match.
- No safety-net requeue. Once the operator reaches
status: complete, it relies on event sources (Pod/Node/Skyhook watches) to trigger further reconciles. None of those normally fire just because the owned CM diverges from spec, so a missed window is permanent.
Why it flakes
For the CM to be written, HandleConfigUpdates needs a single reconcile in which:
- dexter is at
post-interrupt/complete in node state
- the post-interrupt pod has been deleted and the deletion has propagated to the informer cache (
PodExists=false)
HasInterrupt(dexter) is still computable as true so IsPackageComplete returns true at StagePostInterrupt
pod_controller.go:86-95 writes node state first, then deletes the pod. The intermediate reconcile (triggered by the Node-annotation write) sees dexter=complete but PodExists=true → gate closed. The follow-up reconcile (triggered by Pod deletion) might be the one that opens the gate — but only if the informer cache has converged by then, and only if it isn't coalesced with a stale snapshot. Concurrent 409 contention on node-status patches (visible in the failing run's logs as 4-level-deep errorCauses nesting) widens the miss window.
That's the entire story: the operator gets exactly one good window per spec change. Miss it, the CM is stale forever.
Suggested fixes (pick one)
Option A — unconditional CM sync. Decouple the r.Update(newConfigMap) write at line 1271 from the completedNodes == nodeCount gate. The K8s ConfigMap is owner-referenced by the Skyhook and is the operator's source of truth for what containers will mount on the next stage run; it should always match the spec. Keep the node-state reset (Upsert(StageConfig, InProgress)) gated by completedNodes — that's the lifecycle-progression decision — but write the CM eagerly.
Option B — safety-net requeue. When HandleConfigUpdates observes a CM diff but cannot satisfy the gate, return (false, nil) with the reconciler setting ctrl.Result{RequeueAfter: 30 * time.Second}. Bounded, cheap, and self-healing — eventually the gate opens or the operator notices the persistent divergence.
I'd lean toward A — it's the cleaner invariant ("owned resources match spec") and it eliminates a class of subtle timing dependencies, not just this one. Option B is the less invasive change but leaves the race in place; it just guarantees we eventually win it.
A third option worth weighing: also clear Status.ConfigUpdates[pkg] whenever a generation change is observed for that package's spec, so the stale-entry-drives-spurious-cycle path goes away independently of the CM-sync fix.
Impact
chainsaw/config-skyhook flakes in CI on the glob update step
- Production impact: any operator user who applies a spec that updates a package's ConfigMap content while a stale
Status.ConfigUpdates entry exists and the new spec's configInterrupts glob matches that stale entry can have their ConfigMap permanently out of sync until the next spec edit that retriggers HandleConfigUpdates to completion. The user-visible Skyhook status will report complete, masking the divergence.
Workaround for CI
Skip the glob update step in config-skyhook/chainsaw-test.yaml with a # TODO(<this-issue>) comment until the fix lands. Tracked in PR #242 (the StageInterrupt trap fix unmasked this — config-skyhook was previously failing at an earlier step).
Summary
HandleConfigUpdatesonly writes the ownedv1.ConfigMapto matchspec.packages[*].configMapwhencompletedNodes == nodeCounton the SAME reconcile that sees a CM diff. If that condition only becomes true after the lifecycle reaches a steady state, and no further events fire, the operator goes idle with the K8s ConfigMap permanently out of sync with spec.The trigger pattern requires three things to line up — each individually plausible — but when all three combine, the resulting flake is non-deterministic but reproducible:
Status.ConfigUpdates[<pkg>](never cleared because noHandleConfigUpdatescall in the next generation reached line1250).configInterruptscontains a glob that happens to match the stale entry's key.completedNodes == nodeCountgate for the new CM diff, then the operator goes silent.Reproducer
k8s-tests/chainsaw/skyhook/config-skyhookstepglob updateis the canonical reproducer. It flakes in CI — sometimes the post-GC reconcile catches the gate open and the CM gets written; sometimes it doesn't.The relevant sequence (test step ordering):
no-interrupt update: dexter'sui.propertieskey changes.HandleConfigUpdateswritesStatus.ConfigUpdates[dexter] = [\"ui.properties\"]. Lifecycle reachesconfig/completewith no interrupt fired (no configInterrupt matchedui.propertiesunder the old spec). Entry is never cleared because no further CM diff resets the gate.glob update: applies a new spec wheredexter.configInterruptsnow contains*.properties(in addition togame.properties), anddexter.configMap.game.propertiesbecomes\"changed via glob\".Evidence from a failing run
Captured at assert-timeout (270 s after the spec apply):
```yaml
spec:
packages:
dexter:
configInterrupts:
'*.properties': { services: [rsyslog], type: service }
game.properties: { services: [rsyslog], type: service }
configMap:
game.properties: |
changed via glob # ← new spec applied
ui.properties: |
changed
interrupt: { type: reboot }
status:
observedGeneration: 6
status: complete
configUpdates:
dexter:
- ui.properties # ← stale from the previous step
nodeState:
kind-worker:
dexter|1.2.3:
stage: post-interrupt
state: complete # ← lifecycle 'completed' for the wrong reason
```
The K8s
ConfigMap config-skyhook-dexter-1.2.3still hasdata.game.properties: \"changed\"(the previous step's value). 4 minutes 30 seconds of zero operator log lines between the last reconcile (19:26:21) and the assert timeout (19:30:51) confirms the controller went idle without ever satisfying the CM-write gate.Root cause
HandleConfigUpdates(operator/internal/controller/skyhook_controller.go:1185-1281):```go
if !reflect.DeepEqual(oldConfigMap.Data, newConfigMap.Data) {
for _, node := range skyhook.GetNodes() {
...
if !exists && node.IsPackageComplete(_package) {
completedNodes++
}
...
}
if completedNodes == nodeCount || erroringNode {
...
err := r.Update(ctx, newConfigMap)
}
}
```
The CM write is conditional on every selected node passing
\!PodExists && IsPackageCompletein the same reconcile that observes the diff. There are two compounding issues:Status.ConfigUpdatesis not cleaned up on spec changes. It's only cleared byRemoveConfigUpdates(skyhook.go:128), called in three places: package removed from spec (skyhook_controller.go:1038), version change (:1073), or successfulHandleConfigUpdates(:1250). A spec edit that changes a configMap value but happens to leave the package lifecycle in acompletestate doesn't refreshStatus.ConfigUpdates.Status.ConfigUpdatesentry can drive a spurious interrupt cycle when the next spec'sconfigInterruptscontains a pattern (literal or glob) that matches it.NodeState.HasInterrupt(skyhook_types.go:544-554) preferentially uses the matching-interrupt path whenStatus.ConfigUpdates[pkg]is non-empty:```go
if len(config[_package.Name]) > 0 {
hasInterrupt = len(interrupt[_package.Name]) > 0
} else {
hasInterrupt = _package.HasInterrupt()
}
```
NextStagethen routes Config → Interrupt for the stale entry's match.status: complete, it relies on event sources (Pod/Node/Skyhook watches) to trigger further reconciles. None of those normally fire just because the owned CM diverges from spec, so a missed window is permanent.Why it flakes
For the CM to be written,
HandleConfigUpdatesneeds a single reconcile in which:post-interrupt/completein node statePodExists=false)HasInterrupt(dexter)is still computable astruesoIsPackageCompletereturnstrueatStagePostInterruptpod_controller.go:86-95writes node state first, then deletes the pod. The intermediate reconcile (triggered by the Node-annotation write) seesdexter=completebutPodExists=true→ gate closed. The follow-up reconcile (triggered by Pod deletion) might be the one that opens the gate — but only if the informer cache has converged by then, and only if it isn't coalesced with a stale snapshot. Concurrent 409 contention on node-status patches (visible in the failing run's logs as 4-level-deeperrorCausesnesting) widens the miss window.That's the entire story: the operator gets exactly one good window per spec change. Miss it, the CM is stale forever.
Suggested fixes (pick one)
Option A — unconditional CM sync. Decouple the
r.Update(newConfigMap)write at line1271from thecompletedNodes == nodeCountgate. The K8s ConfigMap is owner-referenced by the Skyhook and is the operator's source of truth for what containers will mount on the next stage run; it should always match the spec. Keep the node-state reset (Upsert(StageConfig, InProgress)) gated by completedNodes — that's the lifecycle-progression decision — but write the CM eagerly.Option B — safety-net requeue. When
HandleConfigUpdatesobserves a CM diff but cannot satisfy the gate, return(false, nil)with the reconciler settingctrl.Result{RequeueAfter: 30 * time.Second}. Bounded, cheap, and self-healing — eventually the gate opens or the operator notices the persistent divergence.I'd lean toward A — it's the cleaner invariant ("owned resources match spec") and it eliminates a class of subtle timing dependencies, not just this one. Option B is the less invasive change but leaves the race in place; it just guarantees we eventually win it.
A third option worth weighing: also clear
Status.ConfigUpdates[pkg]whenever a generation change is observed for that package's spec, so the stale-entry-drives-spurious-cycle path goes away independently of the CM-sync fix.Impact
chainsaw/config-skyhookflakes in CI on theglob updatestepStatus.ConfigUpdatesentry exists and the new spec'sconfigInterruptsglob matches that stale entry can have their ConfigMap permanently out of sync until the next spec edit that retriggersHandleConfigUpdatesto completion. The user-visible Skyhook status will reportcomplete, masking the divergence.Workaround for CI
Skip the
glob updatestep inconfig-skyhook/chainsaw-test.yamlwith a# TODO(<this-issue>)comment until the fix lands. Tracked in PR #242 (the StageInterrupt trap fix unmasked this — config-skyhook was previously failing at an earlier step).