upgrade sdk-go - #503
Conversation
WalkthroughBumps multiple dependencies in Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![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
🧹 Nitpick comments (1)
go.mod (1)
58-60: Note: Using pseudo-versions for Open Cluster Management packages.The Open Cluster Management packages are using pseudo-versions (commit-based versions) rather than tagged releases. This is appropriate for incorporating the upstream fix mentioned in the PR description.
However, consider creating a follow-up task to upgrade to official tagged releases once they become available, as pseudo-versions can complicate dependency management and reproducibility.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` around lines 58 - 60, The go.mod currently pins Open Cluster Management modules using pseudo-versions (open-cluster-management.io/api, open-cluster-management.io/ocm, open-cluster-management.io/sdk-go); create a follow-up task in the repo tracker to replace those pseudo-versions with official tagged releases once available, and include steps in that task to update the go.mod entries to the released versions, run `go get`/`go mod tidy` to refresh go.sum, and run CI/tests (unit/integration) to validate compatibility after the upgrade.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@go.mod`:
- Line 195: go.mod currently pins sigs.k8s.io/controller-runtime v0.23.3 while
other k8s.io modules are at v0.35.2, which is outside the officially tested
compatibility matrix; either downgrade the k8s.io/* modules (e.g.,
k8s.io/apimachinery, k8s.io/client-go, k8s.io/api) to v0.35.0 to match
sigs.k8s.io/controller-runtime v0.23.3, or upgrade
sigs.k8s.io/controller-runtime to a release that explicitly supports k8s.io
v0.35.2; update the matching module lines in go.mod and run go mod tidy to
ensure the dependency graph is consistent.
---
Nitpick comments:
In `@go.mod`:
- Around line 58-60: The go.mod currently pins Open Cluster Management modules
using pseudo-versions (open-cluster-management.io/api,
open-cluster-management.io/ocm, open-cluster-management.io/sdk-go); create a
follow-up task in the repo tracker to replace those pseudo-versions with
official tagged releases once available, and include steps in that task to
update the go.mod entries to the released versions, run `go get`/`go mod tidy`
to refresh go.sum, and run CI/tests (unit/integration) to validate compatibility
after the upgrade.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 08055c4f-80fd-46f4-954a-c554e69f7985
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (1)
go.mod
| sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.31.2 // indirect | ||
| sigs.k8s.io/controller-runtime v0.22.4 // indirect | ||
| sigs.k8s.io/json v0.0.0-20241014173422-cfa47c3a1cc8 // indirect | ||
| sigs.k8s.io/controller-runtime v0.23.3 // indirect |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
controller-runtime v0.23.3 Kubernetes compatibility matrix
💡 Result:
controller-runtime v0.23.3 compatibility (tested/supported)
| controller-runtime | k8s.io/* + client-go it vendors | Corresponding Kubernetes release line | Minimum Go |
|---|---|---|---|
| v0.23.3 | v0.35.0 | Kubernetes v1.35.x (client-go v0.35.x matches Kubernetes v1.35.x) |
Go 1.25 |
Notes:
- controller-runtime minor versions are tested against a specific client-go minor (others might work but are “by chance” and not supported/tested). [1]
- v0.23.3’s
go.modpinsk8s.io/api,k8s.io/apimachinery,k8s.io/client-go, etc. to v0.35.0 and setsgo 1.25.0. [2] - client-go documents that for Kubernetes releases (>=1.17),
v0.x.ytags are created to match each Kubernetesv1.x.yrelease (e.g.,v0.17.0== Kubernetesv1.17.0). [3]
Sources: [1] [2] [3]
🏁 Script executed:
grep -E "k8s.io/(api|apimachinery|client-go)" go.mod | head -20Repository: openshift-online/maestro
Length of output: 297
Update controller-runtime to match officially tested Kubernetes API version.
controller-runtime v0.23.3 is officially tested and pinned to k8s.io v0.35.0, but the repository uses k8s.io v0.35.2. While v0.35.2 (a patch release) might work in practice, this combination is not officially supported or tested. Align the k8s.io packages to v0.35.0 to match the controller-runtime v0.23.3 compatibility matrix, or upgrade controller-runtime to a version that officially supports v0.35.2.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@go.mod` at line 195, go.mod currently pins sigs.k8s.io/controller-runtime
v0.23.3 while other k8s.io modules are at v0.35.2, which is outside the
officially tested compatibility matrix; either downgrade the k8s.io/* modules
(e.g., k8s.io/apimachinery, k8s.io/client-go, k8s.io/api) to v0.35.0 to match
sigs.k8s.io/controller-runtime v0.23.3, or upgrade
sigs.k8s.io/controller-runtime to a release that explicitly supports k8s.io
v0.35.2; update the matching module lines in go.mod and run go mod tidy to
ensure the dependency graph is consistent.
Signed-off-by: Wei Liu <liuweixa@redhat.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
go.mod (1)
199-199: Pseudo-version for structured-merge-diff - consider tracking for future update.The
structured-merge-diff/v6dependency uses a pseudo-version pointing to a specific commit rather than an official release. This is likely pulled transitively by another dependency. Consider tracking upstream for when an official v6.3.2+ release becomes available to improve reproducibility.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` at line 199, The go.mod currently pins sigs.k8s.io/structured-merge-diff/v6 to a pseudo-version (d9cc6641c482) which is transient; track the upstream project and, when an official v6.3.2+ tag is published, replace the pseudo-version with the released semantic version (or add a go.mod replace directive pointing to the released module) to improve reproducibility and avoid pinning to a commit for sigs.k8s.io/structured-merge-diff/v6.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@go.mod`:
- Line 199: The go.mod currently pins sigs.k8s.io/structured-merge-diff/v6 to a
pseudo-version (d9cc6641c482) which is transient; track the upstream project
and, when an official v6.3.2+ tag is published, replace the pseudo-version with
the released semantic version (or add a go.mod replace directive pointing to the
released module) to improve reproducibility and avoid pinning to a commit for
sigs.k8s.io/structured-merge-diff/v6.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2f17a476-f151-4aea-b640-45abcdc757ae
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (2)
go.modtest/integration/resource_test.go
|
/cc @clyang82 |
2 participants
open-cluster-management-io/sdk-go#209 this pr to fix the flaky e2e test