Rabbitmq vhost and user support#661
Rabbitmq vhost and user support#661openshift-merge-bot[bot] merged 3 commits intoopenstack-k8s-operators:mainfrom
Conversation
e50135d to
be135db
Compare
ad702cf to
60495bd
Compare
60495bd to
e758a1e
Compare
|
Merge Failed. This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset. |
e758a1e to
4fa3f92
Compare
2733681 to
a64b80e
Compare
|
This change depends on a change that failed to merge. Change openstack-k8s-operators/infra-operator#523 is needed. |
a64b80e to
5a447c5
Compare
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/4abb83ae60334914a602b1f65b84bf5b ✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 06m 34s |
5a447c5 to
74553cf
Compare
74553cf to
1f3ba45
Compare
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/33f4651ad8ab4dd893b171346d711ae5 ❌ openstack-k8s-operators-content-provider FAILURE in 18m 30s |
|
recheck |
| @@ -0,0 +1,75 @@ | |||
| /* | |||
| Copyright 2025. | |||
There was a problem hiding this comment.
Uhh, might as well remove line 2, or put a company name or something.
| description: Vhost - RabbitMQ vhost name | ||
| type: string | ||
| required: | ||
| - cluster |
There was a problem hiding this comment.
The schema checker is flagging this:
ERROR: "NoNewRequiredFields": crd/ironicinspectors.ironic.openstack.org version/v1beta1 field/^.spec.messagingBus.cluster is new and may not be required
ERROR: "NoNewRequiredFields": crd/ironicinspectors.ironic.openstack.org version/v1beta1 field/^.spec.notificationsBus.cluster is new and may not be required
There was a problem hiding this comment.
That's expected, we are swapping a required field - rabbitMqClusterName - with its replacement, same with NotificationsBusInstance. This will require overriding the schema checker (not just for ironic but all operators).
| "k8s.io/apimachinery/pkg/types" | ||
| "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" | ||
| ) | ||
|
|
There was a problem hiding this comment.
Can you please describe what the intended create behaviour is when they use a deprecated field? Will the webhook populate the new notificationBus.cluster or will it fail with an error? Whichever it is, it would be ideal to have a test for that here.
There was a problem hiding this comment.
It might just need an extra assert below that demonstrates notificationBus.cluster equals "rabbitmq"
There was a problem hiding this comment.
Existing ironic crs will have their rabbitMqClusterName transparently translated into messagingBus.Cluster via openstack-operator (see depends-on pr openstack-k8s-operators/openstack-operator#1779), further updates will raise an error and ask to use the new interface.
|
golangci-lint-full is failing with a few nits. I have restored the checklist in the PR description and I suspect that running |
steveb
left a comment
There was a problem hiding this comment.
See inline and my other comment
7901596 to
88bdbd5
Compare
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/7fe405b399a44c49aff0c569a620b8c2 ❌ openstack-k8s-operators-content-provider FAILURE in 12m 33s |
Thanks Steve, I should have addressed the lint errors. |
|
recheck |
Add support for a dedicated rabbitmq cluster for notifications.
Add new messagingBus and notificationsBus interfaces to hold cluster,
user and vhost names for optional usage.
The controller adds these values to the TransportURL create request when present.
Additionally, we migrate RabbitMQ cluster name to RabbitMq config struct
using DefaultRabbitMqConfig from infra-operator to automatically
populate the new Cluster field from legacy RabbitMqClusterName.
Example usage:
spec:
messagingBus:
cluster: rpc-rabbitmq
user: rpc-user
vhost: rpc-vhost
notificationsBus:
cluster: notifications-rabbitmq
user: notifications-user
vhost: notifications-vhost
Jira: https://issues.redhat.com/browse/OSPRH-23819
88bdbd5 to
0116b23
Compare
|
Merge Failed. This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset. |
|
recheck |
|
This change depends on a change that failed to merge. Change openstack-k8s-operators/openstack-operator#1797 is needed. |
|
recheck |
|
/retest |
|
/test ironic-operator-build-deploy-kuttl |
0116b23 to
c68484b
Compare
|
This change depends on a change that failed to merge. Change openstack-k8s-operators/openstack-operator#1797 is needed. |
|
recheck |
|
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: lmiccini 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 |
|
/override ci/prow/precommit-check |
|
@stuggi: Overrode contexts on behalf of stuggi: ci/prow/precommit-check DetailsIn response to this:
Instructions 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. |
6dce1a5
into
openstack-k8s-operators:main
Add support for a dedicated rabbitmq cluster for notifications.
Add new messagingBus and notificationsBus interfaces to hold cluster, user and vhost names for optional usage.
The controller adds these values to the TransportURL create request when present.
Additionally, we migrate RabbitMQ cluster name to RabbitMq config struct using DefaultRabbitMqConfig from infra-operator to automatically populate the new Cluster field from legacy RabbitMqClusterName.
Example usage:
Jira: https://issues.redhat.com/browse/OSPRH-23819
Depends-on: openstack-k8s-operators/openstack-operator#1797
Checklist before requesting a review
pre-commit run --all