CNTRLPLANE-3599: Remove controller-runtime v0.19.7 pin for k8s 1.36 bump - #8683
Conversation
Remove the replace directive that pinned controller-runtime to v0.19.7. The pin was added when webhook.Validator deprecation in v0.20 broke vendored dependencies, but all CAPI providers and HyperShift code have since migrated to CustomValidator/CustomDefaulter. Bump cluster-node-tuning-operator to release-4.20 branch which uses the new webhook.CustomValidator interface compatible with v0.22. This also pulls in k8s.io/kubelet v0.33.3 (from v0.32.2). Signed-off-by: Bryan Cox <brcox@redhat.com> Commit-Message-Assisted-by: Claude (via Claude Code)
Revendor after removing the controller-runtime v0.19.7 replace directive. Updates: - sigs.k8s.io/controller-runtime v0.19.7 -> v0.22.4 - cluster-node-tuning-operator to release-4.20 branch - k8s.io/kubelet v0.32.2 -> v0.33.3 - golang.org/x/exp/maps added as new NTO dependency Regenerated test fixtures with updated resourceVersion values. Signed-off-by: Bryan Cox <brcox@redhat.com> Commit-Message-Assisted-by: Claude (via Claude Code)
- Remove mgr argument from controller.NewUnmanaged() call in proxy controller (v0.22 dropped the manager.Manager parameter) - Add APIVersion and FieldsType to ManagedFieldsEntry in apply_test (v0.22 fake client validates these fields) - Remove TypeMeta from expected objects in annotation enforcer test (v0.22 fake client clears TypeMeta on Get, matching real API server) Signed-off-by: Bryan Cox <brcox@redhat.com> Commit-Message-Assisted-by: Claude (via Claude Code)
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@bryan-cox: This pull request explicitly references no 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. |
|
Skipping CI for Draft Pull Request. |
openshift-ci
Bot
added
do-not-merge/work-in-progress
📝 WalkthroughWalkthroughThis PR updates Go module requirements (bumping OpenShift modules and k8s.io/kubelet; removing a controller-runtime replace), changes AzurePrivateLinkService controller conflict handling to requeue after one second (and updates tests), normalizes nil vs empty slices in several infra controller tests before YAML diffs, adjusts a managed-fields test fixture, and switches a proxy controller instantiation to use controller.NewUnmanaged. Sequence Diagram(s)sequenceDiagram
participant AzurePLSController
participant KubernetesAPI
participant ReconcilerResult
AzurePLSController->>KubernetesAPI: patch finalizer (add/remove)
KubernetesAPI-->>AzurePLSController: conflict error
AzurePLSController->>ReconcilerResult: return RequeueAfter = 1s
Suggested reviewers
🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bryan-cox The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
approved
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
bryan-cox
changed the title
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8683 +/- ##
=======================================
Coverage 41.43% 41.43%
=======================================
Files 756 756
Lines 93647 93647
=======================================
Hits 38802 38802
Misses 52124 52124
Partials 2721 2721
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
openshift-ci
Bot
added
the
area/platform/azure
|
@bryan-cox: This pull request references OCPSTRAT-3036 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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@control-plane-operator/controllers/azureprivatelinkservice/controller.go`:
- Line 272: Add a short code comment above each conflict-handling return that
currently uses ctrl.Result{RequeueAfter: time.Second} (the branches checking
apierrors.IsConflict(err) during finalizer updates in azureprivatelinkservice
controller.go) that explains why a fixed 1s delay is required instead of
ctrl.Result{Requeue: true} (e.g., to allow in-flight finalizer updates or
related cache propagation to settle and avoid hot-looping), and state the
intended behavior and any assumptions (retry interval tolerances, expected
conflict frequency); also update the PR/commit message to include the same
rationale so reviewers can see the justification.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: e662d8f4-ffaa-4b7b-adf8-f656b39b73be
📒 Files selected for processing (3)
control-plane-operator/controllers/azureprivatelinkservice/controller.gocontrol-plane-operator/controllers/azureprivatelinkservice/controller_test.gocontrol-plane-operator/controllers/hostedcontrolplane/infra/infra_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- control-plane-operator/controllers/hostedcontrolplane/infra/infra_test.go
- Normalize nil Items slices to empty slices in infra tests (v0.22 fake client returns nil for empty list results) - Replace deprecated Requeue with RequeueAfter in Azure PLS controller conflict handling (Requeue field removed in controller-runtime v0.22) Signed-off-by: Bryan Cox <brcox@redhat.com> Commit-Message-Assisted-by: Claude (via Claude Code)
openshift-ci
Bot
added
the
area/platform/gcp
bryan-cox
changed the title
|
@bryan-cox: This pull request references CNTRLPLANE-3598 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. |
bryan-cox
changed the title
|
@bryan-cox: This pull request references CNTRLPLANE-3599 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. |
|
/pipeline required |
|
Scheduling tests matching the |
|
/verified by e2e |
openshift-ci-robot
added
the
verified
|
@bryan-cox: This PR has been marked as verified by 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. |
AI Test Failure AnalysisJob: Generated by hypershift-analyze-e2e-failure post-step using Claude claude-opus-4-6 |
| if err := r.Update(ctx, azPLS); err != nil { | ||
| if apierrors.IsConflict(err) { | ||
| return ctrl.Result{Requeue: true}, nil | ||
| return ctrl.Result{RequeueAfter: time.Second}, nil |
There was a problem hiding this comment.
Requeue is deprecated in controller-runtime v0.22.4 — the // Deprecated: Use RequeueAfter instead. annotation at vendor/sigs.k8s.io/controller-runtime/pkg/reconcile/reconcile.go:41 triggers staticcheck SA1019, which make lint enforces.
Upstream source: https://github.com/kubernetes-sigs/controller-runtime/blob/v0.22.4/pkg/reconcile/reconcile.go#L31-L42
AI-assisted response via Claude Code
| name: capi-provider | ||
| namespace: hcp-namespace | ||
| resourceVersion: "1" | ||
| resourceVersion: "2" |
There was a problem hiding this comment.
These are auto-generated golden test fixtures managed by support/testutil/testutil.go:27 (CompareWithFixture), regenerated via UPDATE=true go test.
The test at hypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.go:5289 reconciles each component twice (for range 2). The fake client sets rv="1" on Create (vendor/.../fake/client.go:421) and increments on Update (vendor/.../fake/client.go:600). With controller-runtime v0.22.4, the second reconcile pass now triggers an Update, bumping rv from "1" to "2". All 4 component fixtures changed identically.
AI-assisted response via Claude Code
Test Resultse2e-aws
e2e-aks
|
|
Now I have all the evidence needed to produce the final report. Test Failure Analysis CompleteJob Information
Test Failure AnalysisErrorSummaryThis is a pre-existing flaky test unrelated to the PR changes. The Root CauseThe failure is a timing/resource-pressure flake in Why the test failed:
Why this is NOT caused by the PR:
Recommendations
Evidence
|
|
/retest |
|
/lgtm |
|
Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage. |
|
/pipeline required |
|
Scheduling tests matching the |
AI Test Failure AnalysisJob: Generated by hypershift-analyze-e2e-failure post-step using Claude claude-opus-4-6 |
AI Test Failure AnalysisJob: Generated by hypershift-analyze-e2e-failure post-step using Claude claude-opus-4-6 |
|
@bryan-cox: all tests passed! 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. |
5 participants
What this PR does / why we need it:
Removes the
replace sigs.k8s.io/controller-runtime => sigs.k8s.io/controller-runtime v0.19.7directive from go.mod that was blocking the k8s 1.36 dependency bump. The pin was originally added becausewebhook.Validatordeprecation in v0.20 broke vendored dependencies, but all CAPI providers and HyperShift code have since migrated toCustomValidator/CustomDefaulter.Changes:
webhook.CustomValidatorinterface)controller.NewUnmanaged()call in proxy controller (v0.22 dropped themanager.Managerparameter)APIVersionandFieldsTypeWhich issue(s) this PR fixes:
Unblocks the k8s 1.36 dependency rebase for HyperShift.
Special notes for your reviewer:
The original concern noted in the replace directive comment — "webhook.Validator deprecation in v0.20 breaks everything, conversion is nontrivial" — turned out to be already resolved. Only 2 compile errors and 4 test adaptation issues remained, all straightforward fixes.
Checklist:
Summary by CodeRabbit
Tests
Bug Fixes
Chores