CNTRLPLANE-3600: Bump k8s to v0.36.1, controller-runtime to v0.24.1, CAPI to v1.12.8#8695
CNTRLPLANE-3600: Bump k8s to v0.36.1, controller-runtime to v0.24.1, CAPI to v1.12.8#8695bryan-cox wants to merge 5 commits into
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
Skipping CI for Draft Pull Request. |
|
@bryan-cox: This pull request references CNTRLPLANE-3600 which is a valid jira issue. DetailsIn 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. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR updates Go toolchain targets and refreshes dependency versions in Sequence Diagram(s)sequenceDiagram
participant Manager as ctrl.NewWebhookManagedBy
participant Setup as SetupWebhookWithManager
participant Conversion as conversion.NewWebhookHandler
participant Validator as hostedClusterValidator/nodePoolValidator
Manager->>Setup: register typed webhooks
Setup->>Conversion: create handler with NewRegistry()
Setup->>Validator: attach typed Default/Validate methods
Validator->>Validator: process *hyperv1.HostedCluster and *hyperv1.NodePool
🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8695 +/- ##
=======================================
Coverage 43.37% 43.38%
=======================================
Files 771 771
Lines 95718 95683 -35
=======================================
- Hits 41520 41512 -8
+ Misses 51313 51294 -19
+ Partials 2885 2877 -8
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
4aff56b to
909422a
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: bryan-cox The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.golangci.yml (1)
89-92: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBroaden regex may mask unrelated
Applydeprecations.The pattern
SA1019: (.*)\.Apply is deprecatedmatches any receiver's.Applydeprecation, not justclient.Apply. This risks silently suppressing unrelated future SA1019 warnings for otherApply-named symbols in the ecosystem, beyond the stated controller-runtime migration scope in the comment.♻️ Proposed tighter regex
- - linters: - - staticcheck - text: 'SA1019: (.*)\.Apply is deprecated' + - linters: + - staticcheck + text: 'SA1019: (.*)client\.Apply is deprecated'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.golangci.yml around lines 89 - 92, Tighten the staticcheck suppression in the .golangci.yml exclude rule so it only matches the controller-runtime client Apply deprecation, not any `.Apply` symbol. Update the regex near the existing `client.Apply` comment to anchor on the specific receiver or package name used by the deprecated API, and keep the scope limited to the `staticcheck` SA1019 entry so unrelated future Apply deprecations are not hidden.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.golangci.yml:
- Around line 89-92: Tighten the staticcheck suppression in the .golangci.yml
exclude rule so it only matches the controller-runtime client Apply deprecation,
not any `.Apply` symbol. Update the regex near the existing `client.Apply`
comment to anchor on the specific receiver or package name used by the
deprecated API, and keep the scope limited to the `staticcheck` SA1019 entry so
unrelated future Apply deprecations are not hidden.
|
/retest |
….12.8 Bump core dependencies: - k8s.io/* v0.36.1 - sigs.k8s.io/controller-runtime v0.24.1 - sigs.k8s.io/cluster-api v1.12.8 - sigs.k8s.io/cluster-api-provider-azure v1.23.2 - sigs.k8s.io/cluster-api-provider-ibmcloud v0.13.1 - sigs.k8s.io/cluster-api-provider-gcp v1.12.0 - github.com/openshift/api latest - github.com/openshift/library-go latest - github.com/openshift/client-go latest - Go 1.26.0 Signed-off-by: Bryan Cox <brcox@redhat.com> Commit-Message-Assisted-by: Claude (via Claude Code)
Regenerate all vendored dependencies, CRD manifests, deepcopy functions, and clients for k8s v0.36.1, controller-runtime v0.24.1, and CAPI v1.12.8. Regenerate kube-scheduler test fixtures for new placementGenerate and placementScore extension points in k8s 1.36. Signed-off-by: Bryan Cox <brcox@redhat.com> Commit-Message-Assisted-by: Claude (via Claude Code)
Stub out SetupWebhookWithManager in vendored CAPA and NTO webhook files. These use the old ctrl.NewWebhookManagedBy(mgr).For(r) pattern removed in controller-runtime v0.24. HyperShift never calls these functions so stubbing is safe. Add HasSyncedChecker() to library-go fakeSharedIndexInformer to satisfy the k8s 1.36 SharedIndexInformer interface. Patch from openshift/library-go#2171. Drop this commit once upstream dependencies release controller-runtime v0.24 compatible versions. Signed-off-by: Bryan Cox <brcox@redhat.com> Commit-Message-Assisted-by: Claude (via Claude Code)
…24 generic API
Update webhook registrations to use the typed generic builder
introduced in controller-runtime v0.23:
- NewWebhookManagedBy(mgr).For(&obj{}) → NewWebhookManagedBy(mgr, &obj{})
- Validator/Defaulter methods now use concrete types instead of
runtime.Object (e.g. *hyperv1.HostedCluster, *hyperv1.NodePool)
- Add conversion.NewRegistry() parameter to NewWebhookHandler
- Remove unused apierrors and runtime imports
Signed-off-by: Bryan Cox <brcox@redhat.com>
Commit-Message-Assisted-by: Claude (via Claude Code)
….36 compat - Bump cluster-node-tuning-operator to main branch (508d51a6f2bd) for controller-runtime v0.24 compatible webhook signatures - Bump cluster-api-provider-aws to v2.11.1 for controller-runtime v0.24 compat - Add replace directive for library-go pointing to jubittajohn/library-go rebase-1.36 branch (openshift/library-go#2171) for HasSyncedChecker interface - Add replace directive for kube-openapi to redirect NTO's fake v0.35.1 tag - Add lint exclusions for new controller-runtime v0.24 deprecation warnings (GetEventRecorderFor, client.Apply, ServiceInstanceID) - migration is a separate effort - Regenerate CRDs and vendor Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/test e2e-aws |
|
@bryan-cox: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
|
I have all the evidence. The failure is crystal clear. Let me now produce the final report. Test Failure Analysis CompleteJob Information
Test Failure AnalysisErrorSummaryAll three image builds ( Root CauseThe PR's commit However, the CI build environment has not been updated to match:
Since Go's toolchain management is set to The net result is that all three builds fail identically at the very first Recommendations
Evidence
|
What this PR does / why we need it:
Bumps core Kubernetes and controller-runtime dependencies to support k8s 1.36:
k8s.io/*v0.36.1sigs.k8s.io/controller-runtimev0.24.1sigs.k8s.io/cluster-apiv1.12.8sigs.k8s.io/cluster-api-provider-azurev1.23.2sigs.k8s.io/cluster-api-provider-ibmcloudv0.13.1sigs.k8s.io/cluster-api-provider-gcpv1.12.0github.com/openshift/apilatestgithub.com/openshift/library-golatestgithub.com/openshift/client-golatestBreaking changes addressed
NewWebhookManagedBy(mgr).For(&obj{})removed; migrated toNewWebhookManagedBy(mgr, &obj{})with typedValidator/DefaulterinterfacesRegistryparameterplacementGenerateandplacementScoreadded (test fixtures regenerated)Temporary vendor patches (commit 3/4)
Vendored CAPA (v2.10.0) and NTO webhook files are stubbed because they use the old
ctrl.NewWebhookManagedBy(mgr).For(r)pattern incompatible with controller-runtime v0.24. HyperShift never calls these functions. Library-gofakeSharedIndexInformeris patched to implementHasSyncedChecker()(from openshift/library-go#2171). This commit is intended to be dropped on rebase once upstream releases compatible versions.Status
make buildpassesgo build ./...passesgo vet ./...passesmake testpassesmake lint— blocked on WIP: OCPSTRAT-3036: Rebase 1.36.2 library-go#2171 merging (golangci-lint reads from module cache, not vendor)make verify— same blockerWhich issue(s) this PR fixes:
Fixes https://issues.redhat.com/browse/CNTRLPLANE-3600
Special notes for your reviewer:
Commit 3 (
chore(api): temporary vendor patches for controller-runtime v0.24) is intentionally separate so it drops cleanly on rebase once upstream dependencies release controller-runtime v0.24 compatible versions.CAPA stays at v2.10.0 — no CAPI v1.12-compatible CAPA release exists yet. A follow-up PR can bump CAPI to v1.13.x when ready.
Checklist:
Summary by CodeRabbit