CORS-4262: update openshift/api & capi v1.11 #10060
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@patrickdillon: This pull request references CORS-4262 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.21.0" version, but no target version was set. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
oh no, the units tests! /this-is-fine |
|
/hold |
openshift-ci
bot
added
the
do-not-merge/hold
patrickdillon
force-pushed
the
capi-one-eleven
branch
from
6741582 to
587e814
Compare
|
/assign @vr4manta |
|
kubernetes/apimachinery@c58e197 has broken A LOT of unit tests |
patrickdillon
force-pushed
the
capi-one-eleven
branch
from
173dd7d to
2d6abf3
Compare
|
I went down a rabbit hole of trying to bump capi to v1beta2 as v1beta1 is flagged by the linter. It's a pretty big effort so I will nolint it and create a jira. Also still need to fix a few unit tests. |
|
/test e2e-ibmcloud-ovn e2e-nutanix-ovn |
patrickdillon
force-pushed
the
capi-one-eleven
branch
3 times, most recently
from
a6a1014 to
6257c12
Compare
CAPI divided the API into subpackages. For example: sigs.k8s.io/cluster-api/exp/ipam/api/v1beta1 -> sigs.k8s.io/cluster-api/api/ipam/v1beta1 sigs.k8s.io/cluster-api/api/v1beta1 -> sigs.k8s.io/cluster-api/api/core/v1beta1 See: kubernetes-sigs/cluster-api#12262 This updates the import paths accordingly.
Featuregate was removed from openshift/api in openshift/api#2494
capi v1.11 removed support for v1beta1 clusters in the GenerateSecret function. This institutes a simple workaround of creating a private copy of the function until we are ready to upgrade all of our usage to v1beta2.
Changes in the apimachinery and client-go packages broke our unit
tests after upgrade. client-go stopped serializing the empty
preferences: {} field for the kubeconfig. apimachinery, with
kubernetes/apimachinery@c58e197
caused much more extensive breakage by changing the format with
which errors are returned, and our unit tests hard code the
expected error messages.
For the most part, I used claude to fix these issues, and here
is the summary it created:
Changes Made
Root Cause: The k8s apimachinery package (errors.go:93-96) now uses json.Marshal()
to format BadValue in error messages instead of Go's native format. Error Format Changes:
Arrays: []string{"a", "b"} → ["a","b"] (JSON format, no spaces)
Nil values: []string(nil) → null
Structs: aws.Subnet{ID:"x", Roles:...} → {"id":"x"} (JSON with lowercase keys)
Custom types: gcp.OSImage{Name:"x", Project:"y"} → {"name":"x","project":"y"}
RunMain is deprecated. Putting a nolint on it and created https://issues.redhat.com/browse/OCPBUGS-64696 to ask the agent team to fix eventually.
go generate ./pkg/types/installconfig.go
patrickdillon
force-pushed
the
capi-one-eleven
branch
2 times, most recently
from
809dfa0 to
51d83c3
Compare
|
/hold cancel Rebased. Linter should be fixed. Good to go! |
|
@sadasu: This pull request references CORS-4262 which is a valid jira issue. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sadasu 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 |
openshift-ci
bot
added
the
approved
|
/retest |
tthvo
left a comment
tthvo
left a comment
There was a problem hiding this comment.
/lgtm
Hope we won't run into this again... 😅
This bumps openshift/api to latest and bumps cluster-api to v1.11. CAPI v1.11 contains breaking changes and not all providers have completed migrating to v1.11 yet. Unfortunately this breaks us and makes it so we are unable to update openshift/api (due to conflicts with the latest apimachinery package which is incompatible with older versions of CAPI). Therefore, I am vendoring in the work-in-progress changes in CAPA, CAPZ, & CAPX. The installer only uses this code to generate the manifests (the controllers are vendored separately), so the risk here is relatively low. This is meant as a temporary measure to unblock us from making progress. Once these providers have completed the merge to v1.11, we should be fine to remove these replaces and vendor as usual.
go mod vendor
Upgrading from capi v1beta1 -> v1beta2 will take a not insignificant amount of work. I have captured that work in https://issues.redhat.com/browse/CORS-3563 and set nolint to disable the linters from failing on this package.
patrickdillon
force-pushed
the
capi-one-eleven
branch
from
51d83c3 to
1071658
Compare
openshift-ci
bot
added
lgtm
patrickdillon
force-pushed
the
capi-one-eleven
branch
from
89499fb to
1071658
Compare
|
/retest |
|
/verified by multiple e2e tests |
openshift-ci-robot
added
the
verified
|
@patrickdillon: This PR has been marked as verified by In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
/skip |
|
@patrickdillon: 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-sigs/prow repository. I understand the commands that are listed here. |
5 participants
Vendors capi v1.11
Not all providers have upgraded to v1.11 so this uses the following unmerged PRs to unstick us:
As much as vendoring these PRs smells, the alternative of being stuck, unable to vendor is worse. These packages are only used in the installer to generate manifests--the controllers themselves are untouched (note that this PR does not bump the controllers for the providers that have already migrated to v1.11, which we should do, but can do in a separate PR as this one has grown very large). So the risk seems low, with the major downside being that we are stuck to update that provider until these PRs get merge. We would update these packages back to the provider as quickly as possible after the merge.