-
Notifications
You must be signed in to change notification settings - Fork 1.4k
CORS-4262: update openshift/api & capi v1.11 #10060
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
base: main
Are you sure you want to change the base?
Conversation
|
@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 |
6741582 to
587e814
Compare
|
/assign @vr4manta |
|
kubernetes/apimachinery@c58e197 has broken A LOT of unit tests |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
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 |
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
6257c12 to
809dfa0
Compare
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.
809dfa0 to
51d83c3
Compare
|
/hold cancel Rebased. Linter should be fixed. Good to go! |
|
@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. |
Vendors capi v1.11
Not all providers have upgraded to v1.11 so this uses WIP PRs to unstick us