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

Bump to rabbitmq 2.7.0 structs, also new bundle #676

Closed
wants to merge 2 commits into from

Conversation

dprince
Copy link
Contributor

@dprince dprince commented Feb 17, 2024

No description provided.

@openshift-ci openshift-ci bot requested review from abays and viroel February 17, 2024 13:05
Copy link
Contributor

openshift-ci bot commented Feb 17, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dprince

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

github.com/rabbitmq/cluster-operator/v2 v2.5.0
k8s.io/apimachinery v0.28.6
github.com/rabbitmq/cluster-operator/v2 v2.7.0
k8s.io/apimachinery v0.29.0
Copy link
Contributor

Choose a reason for hiding this comment

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

We specifically tried to avoid this bump in the controller-runtime work and instead tried to match the same version used by our target release of controller-runtime:
https://github.com/kubernetes-sigs/controller-runtime/blob/release-0.16/go.mod#L24

Is this bump necessitated by the RabbitMQ upgrade to get IPv6 working? Or could we use a slightly less recent version and still get IPv6 fixes?

The things to note are:

  1. OpenShift won't use Kubernetes v0.29.0 dependencies until 4.16:
    https://github.com/openshift/api/blob/release-4.16/go.mod#L8

  2. controller-runtime won't use it until v0.17.0
    2a. We can't do controller-runtime v0.17.0 until UBI9 ships with Go 1.21 and we can have a FIPS compatible build of the go-toolset

  3. If we're going to do this, we should probably ensure Envtest tests the equivalent version of Kubernetes. We we should also bump ENVTEST to Kubernetes 1.29 as part of this:
    https://github.com/openstack-k8s-operators/openstack-operator/blob/main/Makefile#L56-L57

The only downside I could see to this was that v0.29.x will bring features that we can't use until we're using OCP 4.16. I couldn't find anything that we're currently using that has been deprecated between 0.28 and 0.29:
https://github.com/kubernetes/kubernetes/blob/master/CHANGELOG/CHANGELOG-1.29.md#deprecation
https://github.com/kubernetes/kubernetes/blob/master/CHANGELOG/CHANGELOG-1.29.md#api-change-1

Copy link
Contributor Author

@dprince dprince Feb 18, 2024

Choose a reason for hiding this comment

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

I think backing the rabbitmq-operator off to the v2.6.0 tag will still get us the IPv6 feature, and it looks like the go.mod entries for that tag of the rabbitmq/cluster-operator are at v0.28.4 for that tag.

Only issue is if there are other rabbitmq/cluster-operator features we'd have to branch from v2.6.0 to maintain the "strict" standard. We'd be stuck on that version for quite a while I feel (which isn't great).

Alternatively, I could add a replace() section to the go.mod to pin it to v0.28 versions of k8s.io dependencies. It appears that initial CI is passing and I doubt rabbitmq/cluster-operator is using any of the shiny new k8s.io features yet. Probably just keeping pace with the latest version. We could just say if it functionally works it is fine.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I doubt it would cause any issues either. But it was something that folks had concerns about while we were bumping controller-runtime.

I personally would be happy to go with this version. But maybe we should socialise it and make sure we're not overlooking any potential issues?

@dprince
Copy link
Contributor Author

dprince commented Feb 19, 2024

New PR is here with v2.6.0 version of the structs and bundle is here: #679

Copy link

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://review.rdoproject.org/zuul/buildset/502b712df68c4776a7e5eca4a315df45

openstack-k8s-operators-content-provider FAILURE in 5m 53s
⚠️ podified-multinode-edpm-deployment-crc SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider
⚠️ cifmw-crc-podified-edpm-baremetal SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider
⚠️ cifmw-data-plane-adoption-osp-17-to-extracted-crc SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider
⚠️ openstack-operator-tempest-multinode SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider

Copy link
Contributor

openshift-ci bot commented Feb 26, 2024

@dprince: 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/functional 9291719 link true /test functional
ci/prow/precommit-check 9291719 link true /test precommit-check
ci/prow/openstack-operator-build-deploy-kuttl 9291719 link true /test openstack-operator-build-deploy-kuttl

Full PR test history. Your PR dashboard.

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

@dprince dprince closed this Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants