Common Notifications bus interface for services

[bogdando]

Extends openstack-k8s-operators/nova-operator#948

Implemented via #1424

Add a top level field in OpenStackControlPlaneSpec field that holds a
a RabbitMQ cluster CR name reference to be used for publishing and consuming
of notifications in the managed OpenStack instance.

Allow overriding it in OpenStack services templates.

Note about a special handling expected for an empty value by the services
that will be supporting this interface. It should provide backwards
compatibility during oscp and services CRDs upgrades.

There is no an empty value handling top scope (cannot disable notifications top-scope as a cluster-wide), however.
It may only take a default value of a 'rabbitmq'. Use the service templates to override it for an empty value, if needed.

The proposed CRD design is not expandable into supporting additional
messaging backends (provided via direct transport_url secret refs, or
the like). For that, the structure should be used instead of a simple string value.

Assume openstack-operator controller should implement a basic
inheritance mechanism from the top level field to the service template
based on the following rules:

if OSCP.Spec.<service_name>.Template.NotificationsBusInstance == nil
	OSCP.Spec.<service_name>.Template.NotificationsBusInstance = OSCP.Spec.NotificationsBus.RabbitMqClusterName

We leave further handling of empty values for the services specific
implementation.

Assume a similar pattern to be applied later for
KeystoneAPI.Spec.RabbitMqClusterName and other services that need to
consume or publish notifications to keep things consistent across the
board.

Jira: OSPRH-15389

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: bogdando
Once this PR has been reviewed and has the lgtm label, please assign abays for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[dprince]

From the nova-operator PR I didn't see anything using a mechanism like TransportURL. Is the plan here to query for RabbitMQ instances directly in the service operators? The TransportURL mechanism certainly has some pros and cons but one thing in particular is it allows service operators to request and wait for a transportURL before a rabbit MQ is running. Where do we plan on having checks in this implementation to wait for the notification Bus rabbitMQ instance to be running?

[bogdando]

From the nova-operator PR I didn't see anything using a mechanism like TransportURL. Is the plan here to query for RabbitMQ instances directly in the service operators? The TransportURL mechanism certainly has some pros and cons but one thing in particular is it allows service operators to request and wait for a transportURL before a rabbit MQ is running. Where do we plan on having checks in this implementation to wait for the notification Bus rabbitMQ instance to be running?

Transport URL will be prepared and submitted from top scope Nova to its sub services in the follow up implementation patches, like I drafted it here openstack-k8s-operators/nova-operator#952 and @auniyal61 started implementing tests for it as well openstack-k8s-operators/nova-operator#958

This unit of work focses on CRD design only, this is why it could be not an obvious task to figure the planned impl from commit messages and docs strings. I hope that answers your question @dprince

[softwarefactory-project-zuul]

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/e2922439ae414415bfc2cea184c64f9e

✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 58m 21s
✔️ podified-multinode-edpm-deployment-crc SUCCESS in 1h 11m 46s
✔️ cifmw-crc-podified-edpm-baremetal SUCCESS in 1h 31m 46s
❌ adoption-standalone-to-crc-ceph-provider RETRY_LIMIT in 12m 11s
✔️ openstack-operator-tempest-multinode SUCCESS in 1h 41m 30s
❌ openstack-operator-kuttl FAILURE in 26m 24s (non-voting)

[gibizer]

After the today's discussion I think we should park this for now. When other service operators are ready to follow this pattern, or has comments on it, then we can re-open this discussion. The compute DFG will move forward with the nova-operator CRD change openstack-k8s-operators/nova-operator#948 which will introduce only a single NovaSpecCore field appearing in the nova template part of the OpenStackControlPlane CR. That is enough for us for the high prio use case for now.

[softwarefactory-project-zuul]

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/f49b225457ff4c218e26f03bd62ba52c

✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 54m 00s
✔️ podified-multinode-edpm-deployment-crc SUCCESS in 1h 11m 55s
✔️ cifmw-crc-podified-edpm-baremetal SUCCESS in 1h 26m 29s
❌ adoption-standalone-to-crc-ceph-provider RETRY_LIMIT Host unreachable in 2h 37m 59s
✔️ openstack-operator-tempest-multinode SUCCESS in 1h 46m 20s
❌ openstack-operator-kuttl FAILURE in 28m 26s (non-voting)

no longer bound onto Nova scope

[bogdando]

Folks, this one no longer blocks Nova, but I believe we should resume work on this. The functional tests and code already works, we simply need to decide if we want this sooner than later (i.e. keep it parked for ever), and create required changes for the service operators as a dependency work (https://issues.redhat.com/browse/OSPRH-16969)
@stuggi @dprince

[stuggi]

Folks, this one no longer blocks Nova, but I believe we should resume work on this. The functional tests and code already works, we simply need to decide if we want this sooner than later (i.e. keep it parked for ever), and create required changes for the service operators as a dependency work (https://issues.redhat.com/browse/OSPRH-16969) @stuggi @dprince

Wasn't the plan to not have this for FR3 and instead for FR4 or so? If so, shouldn't we wait until we branch for it?

[bogdando]

@stuggi I see fr3 branch was created recently, so could we merge?

[stuggi]

@bogdando should we land this CRD only change or wouldn't it be better to have at least nova as an example using it?
glance and cinder now also has the notificationsBus implemented

[bogdando]

@bogdando should we land this CRD only change or wouldn't it be better to have at least nova as an example using it? glance and cinder now also has the notificationsBus implemented

that's a good question, we might want to add Nova and all services that already support that interface into the list of "supported services". @fmount WDYT?

Initially, I planned to decouple CRD design and impl #1424 @stuggi - so we should adjust for Nova glance cinder etc there (see also https://issues.redhat.com/browse/OSPRH-16969)

[fmount]

@bogdando should we land this CRD only change or wouldn't it be better to have at least nova as an example using it? glance and cinder now also has the notificationsBus implemented

that's a good question, we might want to add Nova and all services that already support that interface into the list of "supported services". @fmount WDYT?

Initially, I planned to decouple CRD design and impl #1424 @stuggi - so we should adjust for Nova glance cinder etc there (see also https://issues.redhat.com/browse/OSPRH-16969)

Hi @bogdando,
I think you can add nova to this patch but I agree we can include glance/cinder/manila to the list. I see this change is only about defining the CRD, which is fine, and once it lands we can add storage services as well.

[openshift-ci]

@bogdando: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/openstack-operator-build-deploy-kuttl	60e0ea3	link	true	/test openstack-operator-build-deploy-kuttl
ci/prow/openstack-operator-build-deploy-kuttl-4-18	60e0ea3	link	true	/test openstack-operator-build-deploy-kuttl-4-18

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

[bogdando]

@auniyal61 as you are working on Neutron CRD integration already, and other services are catching up with notifications interface, do you mind to kindly take over this one and update it for extended list of services? It is currently only scoped for Nova, but this is no longer accurate. There is also an implementation for this one here #1424

[fmount]

@gibizer @bogdando because I'm pushing the storage compoenents, I think this patch is obsoleted by: #1591

[gibizer]

@gibizer @bogdando because I'm pushing the storage compoenents, I think this patch is obsoleted by: #1591

I'm not working on it at the moment, I think neither @bogdando. So it is OK to be superseeded

[bogdando]

@gibizer @bogdando because I'm pushing the storage compoenents, I think this patch is obsoleted by: #1591

I'm not working on it at the moment, I think neither @bogdando. So it is OK to be superseeded

just please take the linked implementation patch made for Nova for further consideration

[auniyal61]

@auniyal61 as you are working on Neutron CRD integration already, and other services are catching up with notifications interface, do you mind to kindly take over this one and update it for extended list of services? It is currently only scoped for Nova, but this is no longer accurate. There is also an implementation for this one here #1424

@bogdando sorry I did not see this earlier and could not respond.
I am seein this today only as it was mentioned in other PR.