Add transport URL secret rotation with consumer finalizer - #982
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: lmiccini The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Build failed (check pipeline). Post ✔️ telemetry-openstack-meta-content-provider-master SUCCESS in 3h 01m 44s |
|
/test telemetry-operator-build-deploy |
|
recheck |
| if instance.Status.NotificationsURLSecret != nil { | ||
| rotationPending = *instance.Status.NotificationsURLSecret != "" && | ||
| *instance.Status.NotificationsURLSecret != currentNotifSecret |
There was a problem hiding this comment.
The instance.Status.NotificationsURLSecret variable is unconditionally overwritten before rotationPending is calculated and it never changes so that means that here, the "*instance.Status.NotificationsURLSecret != currentNotifSecret" condition is always false, and thus this controller never activates the 60-second grace period.
This also happens in ceilometer_controller, but it is correctly handled in the cloudkitty_controller.
This might do the trick:
| if instance.Status.NotificationsURLSecret != nil { | |
| rotationPending = *instance.Status.NotificationsURLSecret != "" && | |
| *instance.Status.NotificationsURLSecret != currentNotifSecret | |
| rotationPending := oldNotifSecret != "" && oldNotifSecret != currentNotifSecret |
| rotationPending := false | ||
| if instance.Status.NotificationsURLSecret != nil { | ||
| rotationPending = *instance.Status.NotificationsURLSecret != "" && | ||
| *instance.Status.NotificationsURLSecret != currentNotifSecret | ||
| } |
There was a problem hiding this comment.
Same as in autoscaling, this could be:
| rotationPending := false | |
| if instance.Status.NotificationsURLSecret != nil { | |
| rotationPending = *instance.Status.NotificationsURLSecret != "" && | |
| *instance.Status.NotificationsURLSecret != currentNotifSecret | |
| } | |
| rotationPending := oldNotifSecret != "" && oldNotifSecret != currentNotifSecret |
33b3a65 to
b1ff781
Compare
|
Build failed (check pipeline). Post ✔️ telemetry-openstack-meta-content-provider-master SUCCESS in 3h 53m 34s |
0f28b65 to
507992b
Compare
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
When infra-operator rotates a RabbitMQ transport URL (creating a new secret and user), consumer operators must hold a consumer finalizer on the old secret until all their pods have rolled out with the new credentials. Without this, infra-operator cleans up the old RabbitMQ user while pods are still connected with old credentials, causing message bus outages. Design: 1. Add a consumer finalizer to the current transport URL secret early in reconcile. Set instance.Status.TransportURLSecret for first-time setup only (empty or unchanged); during rotation the status is updated solely by FinalizeSecretRotation at the end of reconcile. 2. Pass transportURL.Status.SecretName directly to sub-CR creation functions and config generation as a parameter — never read from instance.Status.TransportURLSecret for sub-CR specs. 3. Each child (sub-CR) controller records an AppliedInputSecretHash in its status, set only after statefulset.IsReadyForInput / deployment.IsReadyForInput confirms — via an uncached API read — that the workload is fully rolled out with the expected CONFIG_HASH. 4. The parent mirrors a child's Ready condition only when its Generation == ObservedGeneration and AppliedInputSecretHash matches the current input hash; otherwise it sets the condition to Unknown. 5. Guard: FinalizeSecretRotation removes the consumer finalizer from the old secret only when every child reports the expected hash and is ready. The same pattern applies to notification transport URL secrets and application credential secrets where applicable. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@lmiccini: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
Build failed (check pipeline). Post ✔️ telemetry-openstack-meta-content-provider-master SUCCESS in 3h 08m 39s |
2 participants
What this does
When infra-operator rotates a RabbitMQ transport URL (creating a new secret and user), consumer operators must hold a consumer finalizer on the old secret until all their pods have rolled out with the new credentials. Without this, infra-operator cleans up the old RabbitMQ user while pods are still connected with the old credentials, causing message-bus outages.
Approach (updated)
Add a consumer finalizer to the current transport URL secret early in reconcile.
Status.TransportURLSecretis set for first-time setup only (empty or unchanged); during rotation the status is updated solely byFinalizeSecretRotationat the end of reconcile.Pass
transportURL.Status.SecretNamedirectly to sub-CR creation and config generation as a parameter — never readStatus.TransportURLSecretfor sub-CR specs.Each child controller records an
AppliedInputSecretHashin its status, set only afterstatefulset.IsReadyForInput/deployment.IsReadyForInputconfirms — via an uncached API read — that the workload is fully rolled out with the expectedCONFIG_HASH.The parent mirrors a child's Ready condition only when its
Generation == ObservedGenerationandAppliedInputSecretHashmatches the current input hash; otherwise it sets the condition to Unknown.Guard:
FinalizeSecretRotationremoves the consumer finalizer from the old secret only when every child reports the expected hash and is ready. The guard is computed fromConditions.AllSubConditionIsTrue()(notIsReady()), becauseConditions.Init()resets theReadycondition to Unknown on every reconcile.The same pattern applies to notification transport URL secrets and application-credential secrets where applicable.
Dependency
Depends on the lib-common
IsReadyForInput/FinalizeSecretRotationhelpers (currently pinned via areplaceto the fork commit while the lib-common PR is in review).