-
Notifications
You must be signed in to change notification settings - Fork 79
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
Conversation
[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 |
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 |
There was a problem hiding this comment.
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:
-
OpenShift won't use Kubernetes v0.29.0 dependencies until 4.16:
https://github.com/openshift/api/blob/release-4.16/go.mod#L8 -
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 -
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
New PR is here with v2.6.0 version of the structs and bundle is here: #679 |
Build failed (check pipeline). Post https://review.rdoproject.org/zuul/buildset/502b712df68c4776a7e5eca4a315df45 ❌ openstack-k8s-operators-content-provider FAILURE in 5m 53s |
@dprince: The following tests failed, say
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. |
No description provided.