Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create TransportURL from the watcher controller #16

Merged
merged 3 commits into from
Dec 12, 2024

Conversation

amoralej
Copy link
Contributor

@amoralej amoralej commented Nov 28, 2024

This patch is adding the rabbitmq TransportURL from the watcher controller.

it is also creating the functional envtest and kuttl tests.

Related: OSPRH-11422

This Patch is creating the required RabbitMQ infra via TransportURL CR.

- It adds RabbitMqClusterName param to the spec and
  WatcherRabbitMQTransportURLReadyCondition to the conditions list.
- It adds logic to the watcher controller to create a TransportURL
  object from rabbitmq in infra-operator.
- Adds functional envtest tests for the added feature.
For consistency with WatcherAPI ones.
@amoralej amoralej changed the title [WIP] Rabbitmq Create TransportURL from the watcher controller Dec 11, 2024
@amoralej amoralej requested review from cescgina and abays December 11, 2024 14:26
Copy link
Contributor

@cescgina cescgina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link

@abays abays left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@raukadah
Copy link
Contributor

/lgtm

@amoralej One question Do we want to integrate rabbitMqClusterName: rabbitmq in https://github.com/openstack-k8s-operators/watcher-operator/blob/main/config/samples/watcher_v1beta1_watcher.yaml ?

@amoralej
Copy link
Contributor Author

@amoralej One question Do we want to integrate rabbitMqClusterName: rabbitmq in https://github.com/openstack-k8s-operators/watcher-operator/blob/main/config/samples/watcher_v1beta1_watcher.yaml ?

It has a meaningful default value which matches the default openstackcontrolplane installation, so I'd say we don't need to add it to the sample. Although I could if there is any reason to do it.

@raukadah
Copy link
Contributor

@amoralej One question Do we want to integrate rabbitMqClusterName: rabbitmq in https://github.com/openstack-k8s-operators/watcher-operator/blob/main/config/samples/watcher_v1beta1_watcher.yaml ?

It has a meaningful default value which matches the default openstackcontrolplane installation, so I'd say we don't need to add it to the sample. Although I could if there is any reason to do it.

Yes, correct, the default value of rabbitMqClusterName matches with the openstackcontrolplane rabbitmq cluster name field, We donot need to add it there. Thank you for answering it.

@cescgina
Copy link
Contributor

/approve there are several lgtm to already

@cescgina
Copy link
Contributor

/approve

Copy link

openshift-ci bot commented Dec 12, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cescgina

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

The pull request process is described here

Needs approval from an approver in each of these files:

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

@openshift-merge-bot openshift-merge-bot bot merged commit 261e29f into openstack-k8s-operators:main Dec 12, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants