Bump versions of infrastructure providers (CAPI and CAPO)#616
Bump versions of infrastructure providers (CAPI and CAPO)#616
Conversation
3e9c97c to
7abf6b7
Compare
sd109
left a comment
There was a problem hiding this comment.
Nice work! Just a few of minor suggestions from me.
60192c4 to
67e4d26
Compare
e0af6ca to
7d9a7ee
Compare
charts/openstack-cluster/templates/control-plane/kubeadm-control-plane.yaml
Show resolved
Hide resolved
| infrastructureRef: | ||
| kind: OpenStackMachineTemplate | ||
| apiGroup: infrastructure.cluster.x-k8s.io | ||
| name: {{ include "openstack-cluster.controlplane.mt.name" . }} |
There was a problem hiding this comment.
Did we have to drop namespace here? Do we need namespace in the outer KubeadmControlPlane CRD?
There was a problem hiding this comment.
We do have to drop namespace yes - I'll go check on whether it can live on the outer KubeadmControlPlane CRD.
There was a problem hiding this comment.
KubeadmControlPlane expects to find its infrastructureRef resources in the same namespace as itself, so I think this is fine - its just a reference to the OpenStackMachineTemplates that the chart creates, which will end up in the same namespace as the KubeadmControlPlane.
| {{- $nodeGroup := deepCopy $.Values.nodeGroupDefaults | mustMerge $nodeGroupOverrides }} | ||
| --- | ||
| apiVersion: bootstrap.cluster.x-k8s.io/v1beta1 | ||
| apiVersion: bootstrap.cluster.x-k8s.io/v1beta2 |
There was a problem hiding this comment.
Was v1beta1 removed? I think we probabably need some bit notes in the release notes about what capi-helm-chart versions depend on, like what is the minimum version, so that the upgrades are smooth. I am hoping we can bump all the capi capo operators, nothing breaks using the older version of the helm chart, then we move to this version of the helm chart, and we are good, but I am assuming we were too late for that? (Sorry being slow getting my head around the upgrade path here).
There was a problem hiding this comment.
v1beta1 wasn't/isn't removed in this version, so clusters defined with that version can still be worked on by the updated CAPI - we test this by deploying a cluster at the latest tag in this repo, then upgrading to the tip of the feature branch (here: https://github.com/azimuth-cloud/capi-helm-charts/actions/runs/18321667523/job/52195259366?pr=616).
There was a problem hiding this comment.
Again, I think this is covered by the chart-upgrade test, but I'll test with other bits of the Azimuth stack to make sure this all works as expected.
dependencies.json
Outdated
| "cluster-api": "v1.11.2", | ||
| "cluster-api-janitor-openstack": "0.9.1", | ||
| "cluster-api-provider-openstack": "v0.11.3", | ||
| "cluster-api-provider-openstack": "v0.13.0", |
There was a problem hiding this comment.
Do we need to move to v0.12.x first?
There was a problem hiding this comment.
I don't think so, or at least there is no requirement that I can find for that. The biggest issue would be deprecation of the v1alpha6 and v1alpha7 APIs, which we have already moved away from in #423 (https://github.com/kubernetes-sigs/cluster-api-provider-openstack/releases/tag/v0.12.0).
There was a problem hiding this comment.
FWIW CAPI explicitly supports N+3 upgrades so I'd guess that CAPO follows something similar (or at least doesn't strictly require N+1).
There was a problem hiding this comment.
I think that the chart-upgrade CI test tests this path, dependencies are installed from the latest tag (so 0.11.3 in this case), and then they are bumped to the versions in this PR.
As the test passes, I think the skip-version upgrade is okay.
JohnGarbutt
left a comment
There was a problem hiding this comment.
OK, awesome work getting this green.
In my head, its not clear on the upgrade path here for Azimuth and Mangum, in terms of bumping all the operators and the helm chart version. I am guessing when we install the new operators, we must stop using the old helm chart version, and start using this version? And the operators will do the version upgrade on the CRDs that are already in the cluster "for us"?
|
Greate work, hope this PR will be merged soon. |
|
FYI: kubernetes-sigs/cluster-api-provider-openstack#2883 Not sure what the implications are for this PR, but worth being aware of. There's some additional discussion about the issue in CAPI and CAPO Slack channels. |
|
CAPO release v0.13.2 now reverts to v1beta1 API version apparently: https://github.com/kubernetes-sigs/cluster-api-provider-openstack/releases/tag/v0.13.2 |
8290fa1 to
e8f42f0
Compare
9ba3fe6 to
5dc7abb
Compare
8c7cba4 to
49d354b
Compare
142d8b8 to
b19f736
Compare
5ac9af9 to
93ec7e3
Compare
ef48395 to
450a253
Compare
|
Test that this change works with a values file from the current release: https://github.com/azimuth-cloud/capi-helm-charts/actions/runs/22406545200/job/64867882070 |
No description provided.