SANDBOX-1465: update kube & openshift dependencies to 4.20 #718
Conversation
WalkthroughThis PR clears ignored vulnerabilities, upgrades the Go toolchain and Docker toolchain image to Go 1.24, bumps many Go module and Kubernetes/OpenShift dependency versions, updates controller-gen Kubebuilder annotation versions in CRD metadata, and switches reconciler requeue usage from Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (2)📚 Learning: 2025-09-02T10:28:33.280ZApplied to files:
📚 Learning: 2026-01-02T20:13:02.289ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
go.mod
Outdated
| toolchain go1.23.12 | ||
| toolchain go1.24.11 | ||
|
|
||
| replace ( |
rsoaresd
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (17)
.govulncheck.yaml(1 hunks)README.adoc(1 hunks)config/crd/bases/toolchain.dev.openshift.com_idlers.yaml(1 hunks)config/crd/bases/toolchain.dev.openshift.com_memberoperatorconfigs.yaml(1 hunks)config/crd/bases/toolchain.dev.openshift.com_memberstatuses.yaml(1 hunks)config/crd/bases/toolchain.dev.openshift.com_nstemplatesets.yaml(1 hunks)config/crd/bases/toolchain.dev.openshift.com_spacebindingrequests.yaml(1 hunks)config/crd/bases/toolchain.dev.openshift.com_spacerequests.yaml(1 hunks)config/crd/bases/toolchain.dev.openshift.com_toolchainclusters.yaml(1 hunks)config/crd/bases/toolchain.dev.openshift.com_useraccounts.yaml(1 hunks)config/crd/bases/toolchain.dev.openshift.com_workspaces.yaml(1 hunks)controllers/idler/idler_controller.go(1 hunks)controllers/idler/idler_controller_test.go(0 hunks)controllers/memberoperatorconfig/memberoperatorconfig_controller.go(1 hunks)controllers/useraccount/useraccount_controller.go(2 hunks)go.mod(5 hunks)openshift-ci/Dockerfile.tools(1 hunks)
💤 Files with no reviewable changes (1)
- controllers/idler/idler_controller_test.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: GolangCI Lint
- GitHub Check: Test with Coverage
- GitHub Check: Build & push operator bundles & dashboard image for e2e tests
🔇 Additional comments (15)
config/crd/bases/toolchain.dev.openshift.com_memberstatuses.yaml (1)
6-6: LGTM!Controller-gen version annotation update from v0.17.3 to v0.18.0 is consistent with the controller-tools upgrade.
config/crd/bases/toolchain.dev.openshift.com_toolchainclusters.yaml (1)
6-6: LGTM!Controller-gen version annotation update is consistent with the controller-tools upgrade.
config/crd/bases/toolchain.dev.openshift.com_memberoperatorconfigs.yaml (1)
6-6: LGTM!Controller-gen version annotation update is consistent with the controller-tools upgrade.
config/crd/bases/toolchain.dev.openshift.com_idlers.yaml (1)
6-6: LGTM!Controller-gen version annotation update is consistent with the controller-tools upgrade.
config/crd/bases/toolchain.dev.openshift.com_spacebindingrequests.yaml (1)
6-6: LGTM!Controller-gen version annotation update is consistent with the controller-tools upgrade.
config/crd/bases/toolchain.dev.openshift.com_nstemplatesets.yaml (1)
6-6: LGTM! CRD regeneration verified.All CRD files have been consistently regenerated with controller-gen v0.18.0. The version annotation update in the metadata is correct across all 9 CRDs in config/crd/bases/, confirming proper regeneration as part of the controller-tools upgrade.
config/crd/bases/toolchain.dev.openshift.com_useraccounts.yaml (1)
6-6: LGTM!The Kubebuilder controller-gen version annotation update aligns with the controller-tools upgrade in go.mod (v0.17.3 → v0.18.0).
README.adoc (1)
13-13: LGTM!The Go version requirement update is consistent with the go.mod and Dockerfile.tools updates in this PR.
config/crd/bases/toolchain.dev.openshift.com_spacerequests.yaml (1)
6-6: LGTM!Kubebuilder annotation update is consistent with the controller-tools upgrade across all CRD files.
controllers/useraccount/useraccount_controller.go (2)
8-9: LGTM!Minor import formatting change, likely from auto-formatting.
68-68: LGTM!The comment update accurately reflects the requeue semantics. The change aligns with the PR description noting that
result.Requeueis deprecated.config/crd/bases/toolchain.dev.openshift.com_workspaces.yaml (1)
6-6: LGTM!Kubebuilder annotation update is consistent with the controller-tools upgrade.
go.mod (1)
128-130: LGTM!Go toolchain version updates are consistent with the README and Dockerfile changes.
openshift-ci/Dockerfile.tools (1)
12-13: SHA256 hash for Go 1.24.11 is correct. The provided hash matches the official Go distribution and maintains supply chain security..govulncheck.yaml (1)
1-1: This file was newly created in the initial commit with an empty vulnerabilities list. There were no previously ignored vulnerabilities, so no verification is needed.Likely an incorrect or invalid review comment.
| // Note: | ||
| // The Controller will requeue the Request to be processed again if the returned error is non-nil or | ||
| // Result.Requeue is true, otherwise upon completion it will remove the work from the queue. | ||
| // Result.Requeue > 0 is true, otherwise upon completion it will remove the work from the queue. |
There was a problem hiding this comment.
Incorrect comment syntax.
The comment states "Result.Requeue > 0 is true", but Result.Requeue is a boolean field. The comparison operator > cannot be applied to a boolean type.
Apply this fix to correct the comment:
-// Result.Requeue > 0 is true, otherwise upon completion it will remove the work from the queue.
+// Result.Requeue is true or Result.RequeueAfter > 0, otherwise upon completion it will remove the work from the queue.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Result.Requeue > 0 is true, otherwise upon completion it will remove the work from the queue. | |
| // Result.Requeue is true or Result.RequeueAfter > 0, otherwise upon completion it will remove the work from the queue. |
🤖 Prompt for AI Agents
controllers/memberoperatorconfig/memberoperatorconfig_controller.go around line
48: the inline comment incorrectly suggests using "Result.Requeue > 0" which
implies numeric comparison on a boolean; change the comment to state that
Result.Requeue is a boolean (e.g. "Result.Requeue is true" or "Result.Requeue ==
true") and/or rephrase to "when Result.Requeue is true the item will be
requeued" so it no longer uses the '>' operator.
make/run-cicd-script.mk
Outdated
| @@ -1,4 +1,4 @@ | |||
| OWNER_AND_BRANCH_LOCATION=codeready-toolchain/toolchain-cicd/master | |||
| OWNER_AND_BRANCH_LOCATION=rsoaresd/toolchain-cicd/update_4_20_cicd | |||
|
/retest flaky test |
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your changes status has failed because you have indirect coverage changes. Learn more about Unexpected Coverage Changes and reasons for indirect coverage changes. Additional details and impacted files@@ Coverage Diff @@
## master #718 +/- ##
===========================================
- Coverage 82.53% 63.29% -19.24%
===========================================
Files 48 48
Lines 3596 3594 -2
===========================================
- Hits 2968 2275 -693
- Misses 477 1171 +694
+ Partials 151 148 -3
🚀 New features to boost your workflow:
|
| // Note: | ||
| // The Controller will requeue the Request to be processed again if the returned error is non-nil or | ||
| // Result.Requeue is true, otherwise upon completion it will remove the work from the queue. | ||
| // Result.RequeueAfter > 0 is true, otherwise upon completion it will remove the work from the queue. |
There was a problem hiding this comment.
btw, idler controller still uses the Result.Requeue - shouldn't it be removed?
There was a problem hiding this comment.
Yes, thanks!! It was a leftover
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
make/run-cicd-script.mk (1)
11-11: Update comment to reflect current branch.The comment references "master" but line 1 currently points to the
update_4_20_cicdbranch. While this entire change is temporary, the comment should be accurate or generic (e.g., "using version from configured branch").
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
controllers/idler/idler_controller.go(1 hunks)controllers/memberoperatorconfig/memberoperatorconfig_controller.go(1 hunks)controllers/useraccount/useraccount_controller.go(2 hunks)make/run-cicd-script.mk(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- controllers/memberoperatorconfig/memberoperatorconfig_controller.go
- controllers/idler/idler_controller.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build & push operator bundles & dashboard image for e2e tests
🔇 Additional comments (1)
controllers/useraccount/useraccount_controller.go (1)
68-68: LGTM! Correctly migrated from deprecatedResult.RequeuetoRequeueAfter.The changes properly address the deprecation of
Result.Requeueby usingRequeueAfter: time.Second, which maintains the postponed reconciliation behavior while aligning with controller-runtime v0.21.0 best practices. The documentation comment on line 68 has also been updated to reflect this change.Also applies to: 140-142
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alexeykazakov, MatousJobanek, rsoaresd 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (1)
go.mod
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-02T10:28:33.280Z
Learnt from: fbm3307
Repo: codeready-toolchain/member-operator PR: 692
File: controllers/nstemplateset/nstemplatetier.go:137-141
Timestamp: 2025-09-02T10:28:33.280Z
Learning: In the member-operator codebase, MEMBER_OPERATOR_NAMESPACE is only used in OpenShift templates (with ${MEMBER_OPERATOR_NAMESPACE} syntax), not in go templates (which would use {{.MEMBER_OPERATOR_NAMESPACE}} syntax). Go templates in TierTemplateRevision resources use other parameters like SPACE_NAME, NAMESPACE, CONFIG_VALUE, etc., but not MEMBER_OPERATOR_NAMESPACE.
Applied to files:
go.mod
📚 Learning: 2026-01-02T20:13:02.289Z
Learnt from: MikelAlejoBR
Repo: codeready-toolchain/member-operator PR: 720
File: build/Dockerfile.debug:24-25
Timestamp: 2026-01-02T20:13:02.289Z
Learning: For debug builds in the member-operator repository, using `latest` for Delve installation is acceptable. The team prefers to keep Delve up-to-date in debug images and will only pin a specific version if compatibility issues with the Golang version arise.
Applied to files:
go.mod
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build & push operator bundles & dashboard image for e2e tests
🔇 Additional comments (4)
go.mod (4)
15-22: Kubernetes and OpenShift dependency consistency looks good.All
k8s.io/*dependencies are consistently pinned to v0.33.4, andsigs.k8s.io/controller-runtimeis at v0.21.0 with matching versions for related tooling (controller-toolsv0.18.0,kustomize/v5v5.6.0). The comment on line 8-9 clarifies thatopenshift/apiis sourced from the release-4.20 branch, which aligns with the PR objectives.
4-5: Confirm parallel upstream repository updates referenced in PR objectives.The codeready-toolchain internal modules (api, toolchain-common) and openshift/api are pinned to recent pseudo-versions with no go.sum conflicts. Verify that the related pull requests listed in the PR objectives document corresponding updates to these upstream repositories at the versions specified (api: 20260108101803, toolchain-common: 20260108104612, openshift/api: 20251202204302).
128-130: Verify result.Requeue usage with controller-runtime v0.21.0 upgrade.controller-runtime v0.21.0 deprecated the
Result.Requeuefield. If the codebase uses this field, it must be migrated toRequeueAfteror return a non-nil error for immediate requeue. Go 1.24 maintains backward compatibility with nearly all existing code, so the version bump itself is not a blocking concern.
20-20: No action required — the codebase does not use the deprecatedresult.Requeue = truepattern. All usages already employresult.RequeueAfter, which is compatible with controller-runtime v0.21.0.
|
3 participants
Description
Update dependencies
NOTE THAT
result.Requeueis deprecatedRelated PRs
codeready-toolchain/api#495
codeready-toolchain/toolchain-common#503
codeready-toolchain/host-operator#1226
codeready-toolchain/toolchain-e2e#1239
codeready-toolchain/registration-service#565
codeready-toolchain/toolchain-cicd#165
kubesaw/ksctl#137
wa#311
https://github.com/codeready-toolchain/sandboxctl/pull/59
https://github.com/codeready-toolchain/sandbox-sre/pull/2815
https://github.com/codeready-toolchain/mcp-server-devsandbox/pull/49
Issue ticket number and link
SANDBOX-1465
Summary by CodeRabbit
Chores
Documentation
Configuration
Behavior
✏️ Tip: You can customize this high-level summary in your review settings.