Use shared singleton vhost/user CRs with per-consumer finalizers#577
Merged
openshift-merge-bot[bot] merged 1 commit intoMay 13, 2026
Conversation
lmiccini
force-pushed
the
shared-singleton-vhost-user
branch
2 times, most recently
from
431fc58 to
eebffe6
Compare
lmiccini
changed the title
|
Build failed (check pipeline). Post ❌ openstack-k8s-operators-content-provider FAILURE in 11m 24s |
Contributor
Author
|
recheck |
Contributor
Author
|
/test infra-operator-build-deploy-kuttl |
lmiccini
force-pushed
the
shared-singleton-vhost-user
branch
from
eebffe6 to
813b0a8
Compare
Multiple TransportURL CRs pointing to the same RabbitMQ vhost/user (e.g., cinder and heat both using vhost "openstack") would each create separate RabbitMQVhost/RabbitMQUser CRs. Deleting any one of them destroyed the RabbitMQ resource for all other consumers. This replaces per-TransportURL CRs with shared singletons named deterministically via CanonicalVhostName/CanonicalUserName. Each consumer adds a per-TransportURL finalizer (turl.openstack.org/t-<name>) so the RabbitMQ resource is only deleted when all consumers are gone. The RabbitMQ cluster CR is set as owner for automatic GC on cluster deletion. Shared CR lifecycle: - Orphaned user CRs are labeled (rabbitmq.openstack.org/orphaned) instead of deleted, keeping them reclaimable by new consumers. Auto-deletion only proceeds after admin removes the cleanup-blocked finalizer. - Vhost CRs show "Waiting for external finalizers" status when deletion is blocked by user finalizer cleanup. - User controller cascades vhost CR deletion after its own cleanup. Migration from legacy per-TransportURL CRs: - Legacy CRs (with TransportURL owner refs) are cleaned up after canonical CRs are created. - Credentials are pre-copied to the canonical secret to avoid password regeneration that would break existing connections. - Webhook skips legacy CRs and deleting CRs in uniqueness checks. Webhook hardening: - Reject permission changes on shared users with active consumers. - Enforce RabbitmqClusterName immutability on user, vhost, and policy. - Validate RabbitMQPolicy Pattern as valid regex at admission. - Remove duplicate +kubebuilder:webhook markers from apis/ package. - Propagate admission context and add timeouts for k8s API calls. API client hardening: - Add context.Context to all RabbitMQ API client methods. - Bound error response reads to 1MB and drain bodies on success. - Track actual vhost/policy name in RabbitMQPolicyStatus so reconcileDelete targets the correct resource. Controller fixes: - Move ObservedGeneration to end of reconcileNormal (user, vhost, policy) so it is only set after all work succeeds. - Propagate PatchInstance errors via named return instead of swallowing them. - Guard per-consumer finalizer names against the Kubernetes 63-char limit with SHA-256 hash truncation. - Add periodic 5-minute requeue for long TransportURL names where watch-based reverse mapping fails. Backup/restore: - Add backup.openstack.org labels to auto-generated credential secrets so Velero includes them. - Set restore-order to 10 so the secret is restored before the RabbitMQUser CR (order 40). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
lmiccini
force-pushed
the
shared-singleton-vhost-user
branch
from
813b0a8 to
fcc8556
Compare
Contributor
Author
|
retest |
Contributor
Author
|
/test images |
stuggi
reviewed
May 13, 2026
| TransportURLFinalizerPrefix = "turl.openstack.org/t-" | ||
|
|
||
| // maxFinalizerNameSegment is the Kubernetes limit for the name segment after "/" | ||
| maxFinalizerNameSegment = 63 |
Contributor
There was a problem hiding this comment.
nit, could use validation.DNS1123LabelMaxLength like in https://github.com/openstack-k8s-operators/lib-common/blob/main/modules/common/webhook/rfc.go#L38C16-L38C48
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lmiccini, stuggi The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-merge-bot
Bot
merged commit ae35cd9
into
openstack-k8s-operators:main
7 checks passed
lmiccini
added a commit
to lmiccini/infra-operator
that referenced
this pull request
May 22, 2026
The RabbitMQUser webhook (added in PR openstack-k8s-operators#577) validates that the referenced RabbitMQVhost exists before allowing creation. The kuttl test created the vhost and user in the same step, causing a race where the user webhook could fire before the vhost was committed. Split the single create step into two: create and assert the vhost first, then create the dependent resources (user, policy, transporturl). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
lmiccini
added a commit
to lmiccini/infra-operator
that referenced
this pull request
May 22, 2026
The RabbitMQUser webhook (added in PR openstack-k8s-operators#577) validates that the referenced RabbitMQVhost exists before allowing creation. The kuttl test created the vhost and user in the same step, causing a race where the user webhook could fire before the vhost was committed. Split the single create step into two: create and assert the vhost first, then create the dependent resources (user, policy, transporturl). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
lmiccini
added a commit
to lmiccini/infra-operator
that referenced
this pull request
May 22, 2026
The RabbitMQUser webhook (added in PR openstack-k8s-operators#577) validates that the referenced RabbitMQVhost exists before allowing creation. The kuttl test created the vhost and user in the same step, causing a race where the user webhook could fire before the vhost was committed. Split the single create step into two: create and assert the vhost first, then create the dependent resources (user, policy, transporturl). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.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
2 participants
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.
Multiple TransportURL CRs pointing to the same RabbitMQ vhost/user
(e.g., cinder and heat both using vhost "openstack") would each
create separate RabbitMQVhost/RabbitMQUser CRs. Deleting any one
of them destroyed the RabbitMQ resource for all other consumers.
This replaces per-TransportURL CRs with shared singletons named
deterministically via CanonicalVhostName/CanonicalUserName. Each
consumer adds a per-TransportURL finalizer (turl.openstack.org/t-)
so the RabbitMQ resource is only deleted when all consumers are gone.
The RabbitMQ cluster CR is set as owner for automatic GC on cluster
deletion.
Shared CR lifecycle:
instead of deleted, keeping them reclaimable by new consumers.
Auto-deletion only proceeds after admin removes the
cleanup-blocked finalizer.
deletion is blocked by user finalizer cleanup.
Migration from legacy per-TransportURL CRs:
canonical CRs are created.
password regeneration that would break existing connections.
Webhook hardening:
API client hardening:
reconcileDelete targets the correct resource.
Controller fixes:
policy) so it is only set after all work succeeds.
swallowing them.
63-char limit with SHA-256 hash truncation.
where watch-based reverse mapping fails.
Backup/restore:
secrets so Velero includes them.
RabbitMQUser CR (order 40).
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com