RHOAIENG-29976 - update ntb-controllers dependencies #667
Conversation
WalkthroughThis update focuses on version bumps for build tools, dependencies, and Go versions in the Makefiles and go.mod files of the notebook-controller and odh-notebook-controller components. Additionally, a minor refactor changes a struct field and method signature in the odh-notebook-controller webhook, updating the admission decoder from pointer to value type. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant NotebookWebhook
participant admission.Decoder
Client->>NotebookWebhook: Initialize
Note right of NotebookWebhook: Decoder field is now value type
Client->>NotebookWebhook: InjectDecoder(decoder)
NotebookWebhook->>admission.Decoder: Assign decoder value
Client->>NotebookWebhook: Use webhook as usual
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested labels
Suggested reviewers
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 📜 Recent review detailsConfiguration used: .coderabbit.yaml ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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). (2)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
components/odh-notebook-controller/controllers/notebook_webhook.go (1)
469-478: Redundant re-decoding adds unnecessary workInside
maybeRestartRunningNotebookyou call bothw.Decoder.Decode(req, updatedNotebook) w.Decoder.DecodeRaw(req.OldObject, oldNotebook)The first decode duplicates work already done in
Handleand allocates.
Consider passing the already-decodednotebook/req.Object.Rawto the helper to avoid extra deserialization.components/notebook-controller/Makefile (1)
106-108: Minor nit: keep comment width consistentThe description line is now > 80 chars. Wrap or split for readability.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
components/notebook-controller/go.sumis excluded by!**/*.sumcomponents/odh-notebook-controller/go.sumis excluded by!**/*.sum
📒 Files selected for processing (5)
components/notebook-controller/Makefile(2 hunks)components/notebook-controller/go.mod(1 hunks)components/odh-notebook-controller/Makefile(2 hunks)components/odh-notebook-controller/controllers/notebook_webhook.go(2 hunks)components/odh-notebook-controller/go.mod(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-06-29T12:32:38.270Z
Learning: The `ci/prow/odh-notebook-controller-unit` test in the opendatahub-io/kubeflow repository is a known flaky test with existing Jira tickets RHOAIENG-15909 and RHOAIENG-15907 tracking the issue. Test failures from this CI job are often not related to code changes and typically pass on rerun.
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-07-17T17:36:30.551Z
Learning: Step 6 of the DEPENDENCIES.md file in opendatahub-io/kubeflow repository requires updating the Go version and UBI version in the external openshift/release repository's CI operator configuration file at ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-master.yaml, specifically updating the build_root.image_stream_tag.tag to use the correct Go version and base_images.ubi_minimal.tag to match the UBI version changes.
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-07-14T11:32:05.952Z
Learning: PR #649 in opendatahub-io/kubeflow partially addressed context.Background() replacement but left 4 instances in Eventually blocks within OAuth-related tests in notebook_controller_test.go that still need to be replaced with the global ctx variable.
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-07-17T17:37:03.652Z
Learning: Step 6 in the DEPENDENCIES.md file for opendatahub-io/kubeflow refers to updating Go version in the openshift/release repository configuration files at https://github.com/openshift/release/blob/master/ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-main.yaml and https://github.com/openshift/release/blob/master/ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-v1.10-branch.yaml, not the local CI/CD files in the kubeflow repository. The files need updates to the build_root.image_stream_tag.tag from golang-1.23 to golang-1.24 and base_images.ubi_minimal.tag from "8" to "9".
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-07-17T17:37:03.652Z
Learning: Step 6 in the DEPENDENCIES.md file for opendatahub-io/kubeflow refers to updating Go version in the openshift/release repository configuration files at https://github.com/openshift/release/blob/master/ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-main.yaml and https://github.com/openshift/release/blob/master/ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-v1.10-branch.yaml, not the local CI/CD files in the kubeflow repository.
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#605
File: components/odh-notebook-controller/controllers/notebook_controller_test.go:1126-1131
Timestamp: 2025-07-02T04:00:16.948Z
Learning: In the opendatahub-io/kubeflow repository's notebook controller tests, OAuth finalizer tests should be streamlined into single test blocks rather than multiple "It" blocks, as checking finalizer removal races with object deletion and is typically unobservable in Kubernetes.
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-07-15T20:03:27.428Z
Learning: CodeRabbit has specific technical commands that must be written exactly as specified to trigger proper functionality. The `@coderabbitai full review` command is different from a general request to "perform full review" - it triggers a re-run of the regular review process that sends line comments, while a general request might be interpreted as a request for comprehensive analysis.
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#623
File: components/odh-notebook-controller/controllers/notebook_runtime_test.go:179-179
Timestamp: 2025-06-25T06:54:57.600Z
Learning: In the file `components/odh-notebook-controller/controllers/notebook_runtime_test.go`, there is a suggestion to repurpose `testCase.ConfigMap` as `expectedConfigMap` in the test structure to improve clarity by separating input from expected results. This improvement is deferred to be addressed as part of GitHub issue #634.
components/notebook-controller/Makefile (4)
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-07-17T17:36:30.551Z
Learning: Step 6 of the DEPENDENCIES.md file in opendatahub-io/kubeflow repository requires updating the Go version and UBI version in the external openshift/release repository's CI operator configuration file at ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-master.yaml, specifically updating the build_root.image_stream_tag.tag to use the correct Go version and base_images.ubi_minimal.tag to match the UBI version changes.
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-07-17T17:37:03.652Z
Learning: Step 6 in the DEPENDENCIES.md file for opendatahub-io/kubeflow refers to updating Go version in the openshift/release repository configuration files at https://github.com/openshift/release/blob/master/ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-main.yaml and https://github.com/openshift/release/blob/master/ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-v1.10-branch.yaml, not the local CI/CD files in the kubeflow repository. The files need updates to the build_root.image_stream_tag.tag from golang-1.23 to golang-1.24 and base_images.ubi_minimal.tag from "8" to "9".
Learnt from: jiridanek
PR: #623
File: components/odh-notebook-controller/controllers/notebook_runtime_test.go:179-179
Timestamp: 2025-06-25T06:54:57.600Z
Learning: In the file components/odh-notebook-controller/controllers/notebook_runtime_test.go, there is a suggestion to repurpose testCase.ConfigMap as expectedConfigMap in the test structure to improve clarity by separating input from expected results. This improvement is deferred to be addressed as part of GitHub issue #634.
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-07-17T17:37:03.652Z
Learning: Step 6 in the DEPENDENCIES.md file for opendatahub-io/kubeflow refers to updating Go version in the openshift/release repository configuration files at https://github.com/openshift/release/blob/master/ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-main.yaml and https://github.com/openshift/release/blob/master/ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-v1.10-branch.yaml, not the local CI/CD files in the kubeflow repository.
components/odh-notebook-controller/controllers/notebook_webhook.go (1)
Learnt from: jiridanek
PR: #623
File: components/odh-notebook-controller/controllers/notebook_runtime_test.go:179-179
Timestamp: 2025-06-25T06:54:57.600Z
Learning: In the file components/odh-notebook-controller/controllers/notebook_runtime_test.go, there is a suggestion to repurpose testCase.ConfigMap as expectedConfigMap in the test structure to improve clarity by separating input from expected results. This improvement is deferred to be addressed as part of GitHub issue #634.
components/odh-notebook-controller/Makefile (5)
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-07-17T17:36:30.551Z
Learning: Step 6 of the DEPENDENCIES.md file in opendatahub-io/kubeflow repository requires updating the Go version and UBI version in the external openshift/release repository's CI operator configuration file at ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-master.yaml, specifically updating the build_root.image_stream_tag.tag to use the correct Go version and base_images.ubi_minimal.tag to match the UBI version changes.
Learnt from: jiridanek
PR: #623
File: components/odh-notebook-controller/controllers/notebook_runtime_test.go:179-179
Timestamp: 2025-06-25T06:54:57.600Z
Learning: In the file components/odh-notebook-controller/controllers/notebook_runtime_test.go, there is a suggestion to repurpose testCase.ConfigMap as expectedConfigMap in the test structure to improve clarity by separating input from expected results. This improvement is deferred to be addressed as part of GitHub issue #634.
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-07-17T17:37:03.652Z
Learning: Step 6 in the DEPENDENCIES.md file for opendatahub-io/kubeflow refers to updating Go version in the openshift/release repository configuration files at https://github.com/openshift/release/blob/master/ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-main.yaml and https://github.com/openshift/release/blob/master/ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-v1.10-branch.yaml, not the local CI/CD files in the kubeflow repository. The files need updates to the build_root.image_stream_tag.tag from golang-1.23 to golang-1.24 and base_images.ubi_minimal.tag from "8" to "9".
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-07-17T17:37:03.652Z
Learning: Step 6 in the DEPENDENCIES.md file for opendatahub-io/kubeflow refers to updating Go version in the openshift/release repository configuration files at https://github.com/openshift/release/blob/master/ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-main.yaml and https://github.com/openshift/release/blob/master/ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-v1.10-branch.yaml, not the local CI/CD files in the kubeflow repository.
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-06-29T12:32:38.270Z
Learning: The ci/prow/odh-notebook-controller-unit test in the opendatahub-io/kubeflow repository is a known flaky test with existing Jira tickets RHOAIENG-15909 and RHOAIENG-15907 tracking the issue. Test failures from this CI job are often not related to code changes and typically pass on rerun.
components/odh-notebook-controller/go.mod (8)
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-07-17T17:37:03.652Z
Learning: Step 6 in the DEPENDENCIES.md file for opendatahub-io/kubeflow refers to updating Go version in the openshift/release repository configuration files at https://github.com/openshift/release/blob/master/ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-main.yaml and https://github.com/openshift/release/blob/master/ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-v1.10-branch.yaml, not the local CI/CD files in the kubeflow repository. The files need updates to the build_root.image_stream_tag.tag from golang-1.23 to golang-1.24 and base_images.ubi_minimal.tag from "8" to "9".
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-07-17T17:36:30.551Z
Learning: Step 6 of the DEPENDENCIES.md file in opendatahub-io/kubeflow repository requires updating the Go version and UBI version in the external openshift/release repository's CI operator configuration file at ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-master.yaml, specifically updating the build_root.image_stream_tag.tag to use the correct Go version and base_images.ubi_minimal.tag to match the UBI version changes.
Learnt from: jiridanek
PR: #623
File: components/odh-notebook-controller/controllers/notebook_runtime_test.go:179-179
Timestamp: 2025-06-25T06:54:57.600Z
Learning: In the file components/odh-notebook-controller/controllers/notebook_runtime_test.go, there is a suggestion to repurpose testCase.ConfigMap as expectedConfigMap in the test structure to improve clarity by separating input from expected results. This improvement is deferred to be addressed as part of GitHub issue #634.
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-07-17T17:37:03.652Z
Learning: Step 6 in the DEPENDENCIES.md file for opendatahub-io/kubeflow refers to updating Go version in the openshift/release repository configuration files at https://github.com/openshift/release/blob/master/ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-main.yaml and https://github.com/openshift/release/blob/master/ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-v1.10-branch.yaml, not the local CI/CD files in the kubeflow repository.
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-07-14T11:32:05.952Z
Learning: PR #649 in opendatahub-io/kubeflow partially addressed context.Background() replacement but left 4 instances in Eventually blocks within OAuth-related tests in notebook_controller_test.go that still need to be replaced with the global ctx variable.
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-06-29T12:32:38.270Z
Learning: The ci/prow/odh-notebook-controller-unit test in the opendatahub-io/kubeflow repository is a known flaky test with existing Jira tickets RHOAIENG-15909 and RHOAIENG-15907 tracking the issue. Test failures from this CI job are often not related to code changes and typically pass on rerun.
Learnt from: jiridanek
PR: #605
File: components/odh-notebook-controller/controllers/notebook_controller_test.go:1126-1131
Timestamp: 2025-07-02T04:00:16.948Z
Learning: In the opendatahub-io/kubeflow repository's notebook controller tests, OAuth finalizer tests should be streamlined into single test blocks rather than multiple "It" blocks, as checking finalizer removal races with object deletion and is typically unobservable in Kubernetes.
Learnt from: jiridanek
PR: #605
File: components/odh-notebook-controller/controllers/notebook_controller_test.go:1032-1032
Timestamp: 2025-07-01T15:23:02.926Z
Learning: In the opendatahub-io/kubeflow repository's notebook controller tests, there is a global context variable ctx defined in suite_test.go that is properly initialized with cancellation support in BeforeSuite. Tests should use this global context instead of creating new context.Background() instances for consistency with the test suite architecture.
components/notebook-controller/go.mod (4)
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-07-17T17:37:03.652Z
Learning: Step 6 in the DEPENDENCIES.md file for opendatahub-io/kubeflow refers to updating Go version in the openshift/release repository configuration files at https://github.com/openshift/release/blob/master/ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-main.yaml and https://github.com/openshift/release/blob/master/ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-v1.10-branch.yaml, not the local CI/CD files in the kubeflow repository. The files need updates to the build_root.image_stream_tag.tag from golang-1.23 to golang-1.24 and base_images.ubi_minimal.tag from "8" to "9".
Learnt from: jiridanek
PR: #623
File: components/odh-notebook-controller/controllers/notebook_runtime_test.go:179-179
Timestamp: 2025-06-25T06:54:57.600Z
Learning: In the file components/odh-notebook-controller/controllers/notebook_runtime_test.go, there is a suggestion to repurpose testCase.ConfigMap as expectedConfigMap in the test structure to improve clarity by separating input from expected results. This improvement is deferred to be addressed as part of GitHub issue #634.
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-07-17T17:36:30.551Z
Learning: Step 6 of the DEPENDENCIES.md file in opendatahub-io/kubeflow repository requires updating the Go version and UBI version in the external openshift/release repository's CI operator configuration file at ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-master.yaml, specifically updating the build_root.image_stream_tag.tag to use the correct Go version and base_images.ubi_minimal.tag to match the UBI version changes.
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-07-17T17:37:03.652Z
Learning: Step 6 in the DEPENDENCIES.md file for opendatahub-io/kubeflow refers to updating Go version in the openshift/release repository configuration files at https://github.com/openshift/release/blob/master/ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-main.yaml and https://github.com/openshift/release/blob/master/ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-v1.10-branch.yaml, not the local CI/CD files in the kubeflow repository.
🔇 Additional comments (4)
components/odh-notebook-controller/go.mod (1)
13-18: Version-skew between controller-runtime (v0.21.0) and k8s libs (v0.33.0)controller-runtime v0.21.x is built against Kubernetes v0.33.x? No – v0.21 tracks k8s 1.31 / v0.31.
Mixing v0.21 withk8s.io/* v0.33.0is outside the compatibility matrix and usually breaks at compile or runtime (API additions missing).Either bump controller-runtime ≥ v0.23 (tracks 0.33) or pin the k8s libs back to 0.31.
components/notebook-controller/go.mod (1)
11-15: controller-runtime / k8s version skew identical to odh-notebook-controllerSee previous comment – adjust versions to stay on a supported pair.
components/odh-notebook-controller/Makefile (2)
15-16: ENVTEST_K8S_VERSION (1.27) lags behind client libraries (v0.33 / k8s 1.33)Running envtest against an older control-plane while compiling against newer client-go
may hide regressions. Consider upgradingENVTEST_K8S_VERSIONto 1.33 to match libs.
235-248: Tool versions bumped – 👍Moving to controller-gen v0.18 and latest setup-envtest is needed for k8s 1.33 support.
components/odh-notebook-controller/controllers/notebook_webhook.go
Outdated
Show resolved
Hide resolved
| go 1.24 | ||
|
|
||
| toolchain go1.24.5 | ||
| go 1.24.5 |
There was a problem hiding this comment.
go 1.24.5 is an invalid module directive – use major.minor only
go.mod only accepts the major.minor form (e.g. go 1.24).
go 1.24.5 will cause go: invalid value go 1.24.5 at go mod tidy / build time.
-go 1.24.5
+go 1.24📝 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.
| go 1.24.5 | |
| --- a/components/odh-notebook-controller/go.mod | |
| +++ b/components/odh-notebook-controller/go.mod | |
| @@ -1,3 +1,3 @@ | |
| -module components/odh-notebook-controller | |
| +module components/odh-notebook-controller | |
| -go 1.24.5 | |
| +go 1.24 |
🤖 Prompt for AI Agents
In components/odh-notebook-controller/go.mod at line 3, the Go version directive
is incorrectly set to "go 1.24.5", which is invalid. Change this to use only the
major.minor format by updating the line to "go 1.24" to comply with Go module
requirements and avoid build errors.
Since we moved to golang 1.24, we can now use the latest setup-envtest https://github.com/kubernetes-sigs/controller-runtime/blob/948d554cd9c7692f21139384afaafaea8524626f/tools/setup-envtest/README.md
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
components/notebook-controller/go.mod (1)
75-75: Re-evaluate fixedgrpcreplacePinning
google.golang.org/grpcto v1.56.3 can silently block security patches (1.60.x is current). If this replace is only here to satisfy transitive version mismatches, consider:- replace google.golang.org/grpc => google.golang.org/grpc v1.56.3 + // keep grpc aligned with k8s / controller-runtime requirements + replace google.golang.org/grpc => google.golang.org/grpc v1.60.0or drop the replace entirely and let
go mod tidyresolve the highest compatible version.Confirm that no dependency (e.g.
client_golang v1.22.0) now requires ≥ v1.57 before merging.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
components/notebook-controller/go.sumis excluded by!**/*.sumcomponents/odh-notebook-controller/go.sumis excluded by!**/*.sum
📒 Files selected for processing (5)
components/notebook-controller/Makefile(2 hunks)components/notebook-controller/go.mod(1 hunks)components/odh-notebook-controller/Makefile(2 hunks)components/odh-notebook-controller/controllers/notebook_webhook.go(1 hunks)components/odh-notebook-controller/go.mod(1 hunks)
🧰 Additional context used
🧠 Learnings (16)
📓 Common learnings
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-06-29T12:32:38.270Z
Learning: The `ci/prow/odh-notebook-controller-unit` test in the opendatahub-io/kubeflow repository is a known flaky test with existing Jira tickets RHOAIENG-15909 and RHOAIENG-15907 tracking the issue. Test failures from this CI job are often not related to code changes and typically pass on rerun.
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-07-17T17:36:30.551Z
Learning: Step 6 of the DEPENDENCIES.md file in opendatahub-io/kubeflow repository requires updating the Go version and UBI version in the external openshift/release repository's CI operator configuration file at ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-master.yaml, specifically updating the build_root.image_stream_tag.tag to use the correct Go version and base_images.ubi_minimal.tag to match the UBI version changes.
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-07-14T11:32:05.952Z
Learning: PR #649 in opendatahub-io/kubeflow partially addressed context.Background() replacement but left 4 instances in Eventually blocks within OAuth-related tests in notebook_controller_test.go that still need to be replaced with the global ctx variable.
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-07-17T17:37:03.652Z
Learning: Step 6 in the DEPENDENCIES.md file for opendatahub-io/kubeflow refers to updating Go version in the openshift/release repository configuration files at https://github.com/openshift/release/blob/master/ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-main.yaml and https://github.com/openshift/release/blob/master/ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-v1.10-branch.yaml, not the local CI/CD files in the kubeflow repository. The files need updates to the build_root.image_stream_tag.tag from golang-1.23 to golang-1.24 and base_images.ubi_minimal.tag from "8" to "9".
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-07-17T17:37:03.652Z
Learning: Step 6 in the DEPENDENCIES.md file for opendatahub-io/kubeflow refers to updating Go version in the openshift/release repository configuration files at https://github.com/openshift/release/blob/master/ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-main.yaml and https://github.com/openshift/release/blob/master/ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-v1.10-branch.yaml, not the local CI/CD files in the kubeflow repository.
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#605
File: components/odh-notebook-controller/controllers/notebook_controller_test.go:1126-1131
Timestamp: 2025-07-02T04:00:16.948Z
Learning: In the opendatahub-io/kubeflow repository's notebook controller tests, OAuth finalizer tests should be streamlined into single test blocks rather than multiple "It" blocks, as checking finalizer removal races with object deletion and is typically unobservable in Kubernetes.
Learnt from: jstourac
PR: opendatahub-io/kubeflow#667
File: components/odh-notebook-controller/controllers/notebook_webhook.go:60-60
Timestamp: 2025-07-24T14:44:14.742Z
Learning: In controller-runtime v0.21.0, the DecoderInjector interface was completely removed along with the automatic injection mechanism. PR #2736 changed admission.Decoder from a struct to an interface and eliminated the need for InjectDecoder methods. Webhooks must now create their own decoder using admission.NewDecoder(scheme) and store it as admission.Decoder (interface type) rather than *admission.Decoder (pointer to struct). The InjectDecoder method should be removed entirely from webhook implementations.
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#623
File: components/odh-notebook-controller/controllers/notebook_runtime_test.go:179-179
Timestamp: 2025-06-25T06:54:57.600Z
Learning: In the file `components/odh-notebook-controller/controllers/notebook_runtime_test.go`, there is a suggestion to repurpose `testCase.ConfigMap` as `expectedConfigMap` in the test structure to improve clarity by separating input from expected results. This improvement is deferred to be addressed as part of GitHub issue #634.
Learnt from: jstourac
PR: opendatahub-io/kubeflow#667
File: components/notebook-controller/go.mod:3-3
Timestamp: 2025-07-31T14:56:11.653Z
Learning: In the opendatahub-io/kubeflow repository, the build machines have Go 1.24.3 available as the latest image, which should be considered when setting Go version requirements in go.mod files.
📚 Learning: in controller-runtime v0.21.0, the decoderinjector interface was completely removed along with the a...
Learnt from: jstourac
PR: opendatahub-io/kubeflow#667
File: components/odh-notebook-controller/controllers/notebook_webhook.go:60-60
Timestamp: 2025-07-24T14:44:14.742Z
Learning: In controller-runtime v0.21.0, the DecoderInjector interface was completely removed along with the automatic injection mechanism. PR #2736 changed admission.Decoder from a struct to an interface and eliminated the need for InjectDecoder methods. Webhooks must now create their own decoder using admission.NewDecoder(scheme) and store it as admission.Decoder (interface type) rather than *admission.Decoder (pointer to struct). The InjectDecoder method should be removed entirely from webhook implementations.
Applied to files:
components/odh-notebook-controller/controllers/notebook_webhook.go
📚 Learning: in the file `components/odh-notebook-controller/controllers/notebook_runtime_test.go`, there is a su...
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#623
File: components/odh-notebook-controller/controllers/notebook_runtime_test.go:179-179
Timestamp: 2025-06-25T06:54:57.600Z
Learning: In the file `components/odh-notebook-controller/controllers/notebook_runtime_test.go`, there is a suggestion to repurpose `testCase.ConfigMap` as `expectedConfigMap` in the test structure to improve clarity by separating input from expected results. This improvement is deferred to be addressed as part of GitHub issue #634.
Applied to files:
components/odh-notebook-controller/controllers/notebook_webhook.gocomponents/odh-notebook-controller/Makefilecomponents/odh-notebook-controller/go.modcomponents/notebook-controller/Makefilecomponents/notebook-controller/go.mod
📚 Learning: step 6 of the dependencies.md file in opendatahub-io/kubeflow repository requires updating the go ve...
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-07-17T17:36:30.551Z
Learning: Step 6 of the DEPENDENCIES.md file in opendatahub-io/kubeflow repository requires updating the Go version and UBI version in the external openshift/release repository's CI operator configuration file at ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-master.yaml, specifically updating the build_root.image_stream_tag.tag to use the correct Go version and base_images.ubi_minimal.tag to match the UBI version changes.
Applied to files:
components/odh-notebook-controller/controllers/notebook_webhook.gocomponents/odh-notebook-controller/Makefilecomponents/odh-notebook-controller/go.modcomponents/notebook-controller/Makefilecomponents/notebook-controller/go.mod
📚 Learning: step 6 in the dependencies.md file for opendatahub-io/kubeflow refers to updating go version in the ...
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-07-17T17:37:03.652Z
Learning: Step 6 in the DEPENDENCIES.md file for opendatahub-io/kubeflow refers to updating Go version in the openshift/release repository configuration files at https://github.com/openshift/release/blob/master/ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-main.yaml and https://github.com/openshift/release/blob/master/ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-v1.10-branch.yaml, not the local CI/CD files in the kubeflow repository. The files need updates to the build_root.image_stream_tag.tag from golang-1.23 to golang-1.24 and base_images.ubi_minimal.tag from "8" to "9".
Applied to files:
components/odh-notebook-controller/controllers/notebook_webhook.gocomponents/odh-notebook-controller/Makefilecomponents/odh-notebook-controller/go.modcomponents/notebook-controller/Makefilecomponents/notebook-controller/go.mod
📚 Learning: step 6 in the dependencies.md file for opendatahub-io/kubeflow refers to updating go version in the ...
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-07-17T17:37:03.652Z
Learning: Step 6 in the DEPENDENCIES.md file for opendatahub-io/kubeflow refers to updating Go version in the openshift/release repository configuration files at https://github.com/openshift/release/blob/master/ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-main.yaml and https://github.com/openshift/release/blob/master/ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-v1.10-branch.yaml, not the local CI/CD files in the kubeflow repository.
Applied to files:
components/odh-notebook-controller/controllers/notebook_webhook.gocomponents/odh-notebook-controller/Makefilecomponents/odh-notebook-controller/go.modcomponents/notebook-controller/Makefilecomponents/notebook-controller/go.mod
📚 Learning: pr #649 in opendatahub-io/kubeflow partially addressed context.background() replacement but left 4 i...
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-07-14T11:32:05.952Z
Learning: PR #649 in opendatahub-io/kubeflow partially addressed context.Background() replacement but left 4 instances in Eventually blocks within OAuth-related tests in notebook_controller_test.go that still need to be replaced with the global ctx variable.
Applied to files:
components/odh-notebook-controller/controllers/notebook_webhook.gocomponents/odh-notebook-controller/go.modcomponents/notebook-controller/go.mod
📚 Learning: coderabbit has specific technical commands that must be written exactly as specified to trigger prop...
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-07-15T20:03:27.428Z
Learning: CodeRabbit has specific technical commands that must be written exactly as specified to trigger proper functionality. The `@coderabbitai full review` command is different from a general request to "perform full review" - it triggers a re-run of the regular review process that sends line comments, while a general request might be interpreted as a request for comprehensive analysis.
Applied to files:
components/odh-notebook-controller/controllers/notebook_webhook.gocomponents/notebook-controller/go.mod
📚 Learning: the `ci/prow/odh-notebook-controller-unit` test in the opendatahub-io/kubeflow repository is a known...
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-06-29T12:32:38.270Z
Learning: The `ci/prow/odh-notebook-controller-unit` test in the opendatahub-io/kubeflow repository is a known flaky test with existing Jira tickets RHOAIENG-15909 and RHOAIENG-15907 tracking the issue. Test failures from this CI job are often not related to code changes and typically pass on rerun.
Applied to files:
components/odh-notebook-controller/Makefilecomponents/odh-notebook-controller/go.modcomponents/notebook-controller/go.mod
📚 Learning: the `go` directive in go.mod files accepts patch versions (e.g., `go 1.24.3`) as confirmed by offici...
Learnt from: jstourac
PR: opendatahub-io/kubeflow#667
File: components/notebook-controller/go.mod:0-0
Timestamp: 2025-07-31T15:13:14.686Z
Learning: The `go` directive in go.mod files accepts patch versions (e.g., `go 1.24.3`) as confirmed by official Go documentation. The syntax `go <version>` supports valid Go versions including major.minor.patch format, not just major.minor as previously understood.
Applied to files:
components/odh-notebook-controller/go.modcomponents/notebook-controller/go.mod
📚 Learning: in the opendatahub-io/kubeflow repository, the build machines have go 1.24.3 available as the latest...
Learnt from: jstourac
PR: opendatahub-io/kubeflow#667
File: components/notebook-controller/go.mod:3-3
Timestamp: 2025-07-31T14:56:11.653Z
Learning: In the opendatahub-io/kubeflow repository, the build machines have Go 1.24.3 available as the latest image, which should be considered when setting Go version requirements in go.mod files.
Applied to files:
components/odh-notebook-controller/go.modcomponents/notebook-controller/go.mod
📚 Learning: in the opendatahub-io/kubeflow repository's notebook controller tests, oauth finalizer tests should ...
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#605
File: components/odh-notebook-controller/controllers/notebook_controller_test.go:1126-1131
Timestamp: 2025-07-02T04:00:16.948Z
Learning: In the opendatahub-io/kubeflow repository's notebook controller tests, OAuth finalizer tests should be streamlined into single test blocks rather than multiple "It" blocks, as checking finalizer removal races with object deletion and is typically unobservable in Kubernetes.
Applied to files:
components/odh-notebook-controller/go.modcomponents/notebook-controller/go.mod
📚 Learning: go loop variable capture behavior changed in go 1.22. before go 1.22, loop variables are captured by...
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#623
File: components/odh-notebook-controller/controllers/notebook_runtime_test.go:134-138
Timestamp: 2025-06-18T17:11:27.225Z
Learning: Go loop variable capture behavior changed in Go 1.22. Before Go 1.22, loop variables are captured by reference in closures, requiring the `variable := variable` pattern to avoid issues. Starting with Go 1.22, loop variables are automatically captured per iteration, making this pattern unnecessary.
Applied to files:
components/notebook-controller/go.mod
📚 Learning: in go 1.22+, loop variables are automatically captured per iteration, making the `variable := variab...
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#623
File: components/odh-notebook-controller/controllers/notebook_runtime_test.go:134-138
Timestamp: 2025-06-18T17:11:27.225Z
Learning: In Go 1.22+, loop variables are automatically captured per iteration, making the `variable := variable` pattern unnecessary to avoid closure capture issues. Projects using Go 1.22+ don't need this pattern in their range loops with closures.
Applied to files:
components/notebook-controller/go.mod
📚 Learning: when analyzing ci logs for warnings and errors, it's important to distinguish between infrastructure...
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-07-16T07:12:32.244Z
Learning: When analyzing CI logs for warnings and errors, it's important to distinguish between infrastructure warnings (like GitHub Actions setup, git hints, package manager warnings) and application-specific issues. The "ERROR: Reconciler error" messages and "reflector.go:462 watch ended with context canceled" messages in Kubernetes tests are typically expected test scenarios during cleanup, not actual failures.
Applied to files:
components/notebook-controller/go.mod
📚 Learning: when analyzing ci logs for warnings and errors, it's important to distinguish between infrastructure...
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-07-16T07:12:32.244Z
Learning: When analyzing CI logs for warnings and errors, it's important to distinguish between infrastructure warnings (like GitHub Actions Node.js deprecations) and application-specific issues. The "ERROR: logging before flag.Parse" messages in Go tests are typically harmless initialization messages, not actual errors.
Applied to files:
components/notebook-controller/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). (3)
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (11)
components/odh-notebook-controller/controllers/notebook_webhook.go (1)
60-60: Correct decoder field type for controller-runtime v0.21.0.This change from
*admission.Decodertoadmission.Decoderis required for the controller-runtime v0.21.0 upgrade. The DecoderInjector interface was completely removed and admission.Decoder changed from a struct to an interface, making this field type change necessary.components/notebook-controller/Makefile (2)
146-146: Tool version upgrades look good.The controller-gen upgrade to v0.18.0 is consistent with the dependency updates across the project.
154-158: ENVTEST_VERSION parameterization is a good improvement.Adding the parameterized ENVTEST_VERSION variable makes version management cleaner and more maintainable.
components/odh-notebook-controller/Makefile (3)
15-17: Well-documented Kubernetes version update.The update to Kubernetes 1.28 is properly justified with clear comments explaining the reasoning (targeting OCP 4.15 as the oldest supported version).
237-237: Controller-gen upgrade is consistent.The upgrade to v0.18.0 matches the version used in the notebook-controller, maintaining consistency across components.
245-249: ENVTEST improvements with helpful documentation.The parameterized ENVTEST_VERSION and the documentation link make this more maintainable and developer-friendly.
components/odh-notebook-controller/go.mod (3)
3-3: Go version directive is correct.Using
go 1.24.3is valid syntax in go.mod files. The Go documentation confirms that patch versions are accepted in the go directive, not just major.minor format.
8-21: Comprehensive dependency updates look good.The updates to core dependencies (Kubernetes libraries, controller-runtime, logging libraries) are consistent with the overall dependency refresh in this PR.
25-31: Test dependency updates are appropriate.The test dependencies (Gomega, OpenTelemetry) have been updated to current versions.
components/notebook-controller/go.mod (2)
3-5: Confirm build-pipeline support forgo 1.24.3+toolchain go1.24.5The patch-level
godirective is valid, but note that:
- Builds performed with exactly Go 1.24.2 (or older 1.24.x) will now fail outright.
- The
toolchaindirective triggers an automatic download of Go 1.24.5 when the local toolchain is older – this can break in air-gapped CI images or restricted networks.Double-check that:
• All CI images already contain ≥ 1.24.3, or allow remote downloads.
• Any downstream vendors (OpenShift CI, prow jobs, etc.) are not pinned to 1.24.2.If either of those is questionable, reverting to
go 1.24(major.minor) while keeping thetoolchainline gives you safety without losing local reproducibility.
13-16: Large Kubernetes/API bump – ensure controller-runtime matrix remains alignedMoving from k8s v0.29 → v0.33 together with controller-runtime v0.21 is coherent (0.21 tracks k8s 0.33), but please verify at compile-time/runtime that:
• Envtest images/tooling were bumped to the matching
setup-envtestrelease.
• Any CRDapiextensions.k8s.io/v1schemas generated by older controller-gen still validate against 1.33*.
• No deprecated APIs (e.g.batch/v1beta1) linger in the codebase – they were dropped in 1.32+.A quick
make testmay pass, yet integration clusters on an older minor will not.
|
/retest |
@harshad16 WDYT about this one? Shall we update this grpc also? The replace was introduced 2 years ago via #218. |
@coderabbitai please check whether there are some breaking changes or risks for this grpc upgrade between current v1.56.3 and v1.60.0 you propose. Also, check against the latest release grpc and explain why you didn't propose it to upgrade. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@jstourac I'll analyze the gRPC upgrade path for you, checking for breaking changes and explaining my version recommendation. 🌐 Web query: 💡 Result: Between versions v1.56.3 and v1.65.0 of grpc-go, several breaking changes and important updates were introduced. Here's a summary of the key changes: Version v1.56.3 (Released July 2024):
Version v1.65.0 (Released July 2024):
For a comprehensive list of changes, including minor updates and fixes, you can refer to the grpc-go releases page. 🌐 Web query: 💡 Result: The latest stable release of the Go gRPC library (grpc-go) as of August 2025 is v1.74.2, published on July 22, 2025, according to the official GitHub releases page[1][7]. There are no official releases of grpc-go with version numbers v1.66, v1.67, or v1.68; the version numbers have incremented beyond those, with v1.73.0 and v1.74.x being the most recent in 2025[1][7]. The most notable features added in recent versions include updates to load balancing policies, support for new xDS features, improvements to health checking, and raising the minimum supported Go version to 1.23[1]. For official and up-to-date release information, the canonical source remains the grpc-go GitHub Releases page[1][7]. Citations:
@jstourac Based on my research, here's a comprehensive analysis of the gRPC upgrade considerations: Current State vs. Latest Versions
Breaking Changes Between v1.56.3 and v1.60.0Major Breaking Changes:
Security & Behavior Changes:
Why I Suggested v1.60.0 Instead of Latest v1.74.2The reason I recommended v1.60.0 rather than the latest v1.74.2 is ecosystem compatibility:
RecommendationsOption 1 (Conservative): Stick with v1.60.0 for now
Option 2 (More current): Consider v1.65.0 or v1.67.0
Option 3 (Latest): Upgrade to v1.74.2
Suggested Testing Approach# Test the upgrade incrementally
go mod edit -replace=google.golang.org/grpc=google.golang.org/[email protected]
go mod tidy
make test
# If successful, consider testing v1.65.0 or laterThe conservative v1.60.0 upgrade I suggested balances security improvements with compatibility risk, which is why I didn't jump directly to the latest version. 🧠 Learnings used |
|
/retest |
|
Thanks for the PR work well,
The grpc bit was last updated due to CVE /lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: harshad16 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 |
https://issues.redhat.com/browse/RHOAIENG-29976
This updates dependencies for both odh-notebook-controller and notebook-controller. Apart from that, the envtest is also updated to a recent verson.
Description
closes: #661
closes: #660
How Has This Been Tested?
make test
Merge criteria:
Summary by CodeRabbit