Rabbitmq vhost and user support#824
Rabbitmq vhost and user support#824openshift-merge-bot[bot] merged 5 commits intoopenstack-k8s-operators:mainfrom
Conversation
|
/test telemetry-operator-build-deploy-kuttl |
f3d8a14 to
b7891d7
Compare
|
/test telemetry-operator-build-deploy-kuttl |
b7891d7 to
c508071
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. |
c508071 to
ea915a4
Compare
5c776f5 to
adbdc4b
Compare
| if spec.RabbitMqClusterName == "" { | ||
| spec.RabbitMqClusterName = "rabbitmq" | ||
| } | ||
| rabbitmqv1.DefaultRabbitMqConfig(&spec.MessagingBus, spec.RabbitMqClusterName) |
There was a problem hiding this comment.
Would Ceilometer would need to listen to the Notification rabbitmq? Ceilometer is the consumer of the notifications produced by other services (or itself). If Ceilometer is not subscribed to notifications, no one will consume that, IMHO.
|
Why do we have to have both Notification and Messages RabbitMQs configuration in every service when our services only ever use one RabbitMQ? |
vyzigold
left a comment
There was a problem hiding this comment.
I have a question. I see in the config templates, we have a if / else construct, meaning each service can use only one of the busses at the time. Wouldn't it make sense to have something similar in the controller code? To call transportURLCreateOrUpdate conditionally only for one of the busses, so that there aren't unneeded transporturl CRs.
Another thing we'll need to consider and maybe document related to what Juan noted. If each service can now choose from 2 busses and ceilometer will receive notifications only on one of the bus as well, there won't be any notification metrics available from services using the other bus.
|
Actually, I did a deep dive on Ceilometer, Aodh and CloudKitty code regarding messaging and I have found that both Ceilometer and Aodh only use Notifications, they never send or receive any kind of rpc messages. As highlighted previously, Ceilometer is the only consumer of Notifications in that bus, so it is critical that it is connected to that specific RabbitMQ instance. Aodh only makes use of Notification bus also, while CloudKitty uses the tcp Messaging Bus and never sends Notifications. |
cd36d6f to
fe7d760
Compare
Hey Juan, thanks for having a look. I took at stab at trying to standardize every operator to use the same two interfaces - messagingBus and notificationsBus - for rpc and notifications. Using one or the other (or both) depends on how each service works. |
9012c1a to
94d477f
Compare
|
This change depends on a change that failed to merge. Change openstack-k8s-operators/openstack-operator#1779 is needed. |
94d477f to
8bd837c
Compare
|
This change depends on a change that failed to merge. Change openstack-k8s-operators/openstack-operator#1779 is needed. |
855f3cf to
ce08c2f
Compare
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/3ca9f8046bc04f348344d708fb8473f1 ❌ openstack-k8s-operators-content-provider FAILURE in 15m 07s |
|
recheck |
|
/test telemetry-operator-build-deploy-kuttl |
|
/test telemetry-operator-build-deploy |
|
It looks very good now, I made a comment for a field that has been forgotten. Otherwise it lgtm. The precommit-check complains about adding a new mandatory field: this would break backwards compatibility so it needs to be optional. The other linting errors should be very easy to fix. |
ce08c2f to
8d290e5
Compare
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/42e6964c396748af9baea5b0f6285ef5 ❌ openstack-k8s-operators-content-provider FAILURE in 12m 05s |
Thanks, I should have addressed both the lint and leftover field. I think we should override the crd check (not just for telemetry but for most of the operators), or at least this is our current plan. |
|
recheck |
|
This change depends on a change that failed to merge. Change openstack-k8s-operators/openstack-operator#1779 is needed. |
|
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-23909
8d290e5 to
873d6fa
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 |
|
[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. |
fe8dc27
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:
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-23909
Depends-on: openstack-k8s-operators/openstack-operator#1797