Hi HyperShift maintainers,
I think there may be a small race/idempotency issue in the adaptDeployment path (control-plane-operator/controllers/hostedcontrolplane/v2/kas/deployment.go:87).
When this path runs, it calls updateBootstrapInitContainer based on cached Secret state from before the update. It then writes the updated Secret state.
If the next reconcile starts before the controller-runtime cache has observed that state change, it can read the previous cached object where the old Secret contents are still visible. In that case, the same path can call updateBootstrapInitContainer again.
A possible interleaving is:
first reconcile:
cached Secret state from before the update
adaptDeployment calls updateBootstrapInitContainer
writes the updated Secret state
second reconcile before the cache observes that change:
the old Secret contents are still visible
adaptDeployment takes the same branch again
updateBootstrapInitContainer is called a second time
an already-created/already-updated result may surface as an error or stale rewrite
For create/update paths, the repeated call should usually be harmless only if the already-created/already-updated case is treated as success, or if the path re-reads current state before calling updateBootstrapInitContainer again.
A minimal fix may be to handle the operation-specific already-exists/already-updated case around updateBootstrapInitContainer, or to do a live read before repeating the create/update.
Please let me know if updateBootstrapInitContainer already has the needed idempotence guarantee, or if there is another guard I am missing in this path.
Hi HyperShift maintainers,
I think there may be a small race/idempotency issue in the
adaptDeploymentpath (control-plane-operator/controllers/hostedcontrolplane/v2/kas/deployment.go:87).When this path runs, it calls
updateBootstrapInitContainerbased on cached Secret state from before the update. It then writes the updated Secret state.If the next reconcile starts before the controller-runtime cache has observed that state change, it can read the previous cached object where the old Secret contents are still visible. In that case, the same path can call
updateBootstrapInitContaineragain.A possible interleaving is:
For create/update paths, the repeated call should usually be harmless only if the already-created/already-updated case is treated as success, or if the path re-reads current state before calling
updateBootstrapInitContaineragain.A minimal fix may be to handle the operation-specific already-exists/already-updated case around
updateBootstrapInitContainer, or to do a live read before repeating the create/update.Please let me know if
updateBootstrapInitContaineralready has the needed idempotence guarantee, or if there is another guard I am missing in this path.