Repository navigation
chore(deps): bump github.com/container-storage-interface/spec from 1.8.0 to 1.13.0 - #6229
Conversation
dependabot
Bot
added
dependencies
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
|
Hi @dependabot[bot]. Thanks for your PR. I'm waiting for a fluid-cloudnative member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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/test-infra repository. |
There was a problem hiding this comment.
🟡 Changes recommended
The dependency upgrade breaks CSI driver compilation because the csi-common default servers lack required embeddings and RPCs, so those implementations and the vendor tree must be updated.
1 open issue
What changed in this PR
Updates the Container Storage Interface (CSI) dependency from v1.8.0 to v1.13.0 for Fluid's CSI integration.
Changes:
- Updates the dependency version, checksums, and vendor metadata.
- Adds generated gRPC bindings with expanded health and snapshot APIs.
- Removes the legacy protobuf wrappers package from vendored dependencies.
| File | Description |
|---|---|
vendor/modules.txt |
Records CSI v1.13.0 and its Go requirement; removes the wrappers entry. |
vendor/github.com/golang/protobuf/ptypes/wrappers/wrappers.pb.go |
Removes the legacy wrapper aliases. |
vendor/github.com/container-storage-interface/spec/lib/go/csi/csi_grpc.pb.go |
Adds generated CSI client and server bindings. |
go.sum |
Updates CSI module checksums. |
go.mod |
Bumps the CSI dependency to v1.13.0. |
Review effort: Balanced
| require ( | ||
| github.com/agiledragon/gomonkey/v2 v2.13.0 | ||
| github.com/container-storage-interface/spec v1.8.0 | ||
| github.com/container-storage-interface/spec v1.13.0 |
cheyang
left a comment
cheyang
left a comment
There was a problem hiding this comment.
This bump cannot merge as-is: the module does not compile. Both reviewers independently reproduced the break — since CSI spec v1.10.0 the generated gRPC server interfaces require embedding csi.Unimplemented*Server, and fluid's CSI plugin is built on the vendored kubernetes-csi/drivers/pkg/csi-common v1.0.2 (repo archived since 2021, frozen at the 2019 state), whose Default*Server types predate that pattern. go build ./... fails at the head with three errors in csi-common/utils.go while the base builds clean, and that one incompatibility is the demonstrated cause of every red job on this PR (build, lint, staticcheck, unittest, all kind-e2e/backward-compat entries). The blast radius is contained: with a 3-line embedding probe (reverted — go mod vendor would wipe it) the whole module builds and all pkg/csi tests pass, so the rest of the tree is source-compatible with 1.13.0 and nothing behavioral changes. The real gate is a decision, not a patch: nothing in fluid uses the spec 1.9–1.13 additions, and kubernetes-csi/drivers will never cut a release with the embeds, so the choices are fork+replace it, inline/fork the ~200 lines of csi-common helpers fluid uses, or decline the bump (note: re-pinning to 1.9.x is not an escape — v1.9.0 also fails to compile, controllerServer is missing ControllerModifyVolume; the compile-safe boundary is staying at ≤ 1.8.0). Copilot's advisory review independently flagged the same mechanism. The only disagreement between the reviewers was the version boundary in one remediation option — settled in favor of ≤ 1.8.0 with a reproduced downgrade experiment. Repro contract tests: TestCSISpecCompatHarness on branch verify/csi-spec-bump-claude and the four contracts on verify/csi-spec-1-13-bump-codex.
Next round
[MUST] F1: make the module compile at the head — embed csi.Unimplemented*Server via a fork/replace or by inlining the csi-common helpers (a raw vendor edit is not shippable), or decline the bump; re-pinning to 1.9.x is not a fix.
[SHOULD] Q1: maintainer decision on CSI spec currency direction (fork+replace / inline csi-common / @dependabot ignore), decided once rather than per-bump.
[SHOULD] F3: keep a compile-time csi.*Server conformance test if the bump is redone, so future spec bumps fail fast.
[Filed] none
Non-blocking observations (2) — not part of this round's ask; fix now, follow up, or file an issue as you see fit
- go.mod:61 [minor] — Remediation direction for CSI spec currency (merges Claude Q1 and Codex F2)
Q1 (merged Q1/F2; the F2 sub-claim 'pin < v1.10.0 still compiles' was rebutted — the safe boundary is <= v1.8.0): what does fluid gain from csi spec >= 1.9? Nothing references the additions, and landing the bump requires replacing the archived csi-common dependency first. The one-time decision: (a) close the bump and have dependabot ignore the dependency, (b) stay at/pin <= v1.8.0 (equivalent to declining; a 1.9.x pin would fail CI exactly like the current head), or (c) inline/fork the csi-common helpers with Unimplemented* embeds and then take the bump — the only durable path, and a small refactoring PR of its own rather than a chore bump. Worth deciding once rather than per-dependabot-PR. - pkg/csi/plugins:1 [nit] — If the bump is redone, keep a compile-time CSI interface-conformance test (Codex F3, confirmed by Claude)
F3 (nit, non-blocking): if maintainers replace csi-common and redo this bump, keep a small compile-time conformance test (var _ csi.IdentityServer/ControllerServer/NodeServer assertions, as in the review harnesses) so a future spec bump fails fast with a clear message instead of surfacing only via CI build failures.
Verified review delta
- F1: open (initial-pr)
- Q1: open (initial-pr)
- F3: open (initial-pr)
Code: needs-work. CI: failure; attribution: pr-regression. Required CI checks still apply.
| require ( | ||
| github.com/agiledragon/gomonkey/v2 v2.13.0 | ||
| github.com/container-storage-interface/spec v1.8.0 | ||
| github.com/container-storage-interface/spec v1.13.0 |
There was a problem hiding this comment.
F1 (consensus, confirmed by both reviewers and Copilot): bumping the CSI spec to 1.13.0 breaks the build. Since spec v1.10.0 the generated csi_grpc.pb.go (protoc-gen-go-grpc v1.3.0) requires every server implementation to embed the UnimplementedServer structs — mustEmbedUnimplemented{Identity,Controller,Node}Server() is now an interface method — and adds new RPCs (ControllerModifyVolume, GetSnapshot, ControllerListVolumeHealth, ControllerGetVolumeHealth, NodeGetVolumeHealth, NodeGetStorageHealth). The vendored kubernetes-csi/drivers csi-common v1.0.2 DefaultServer types — and fluid's controllerServer/nodeServer in pkg/csi/plugins which embed them — predate that pattern (the repo is archived and frozen at the 2019 state), so vendor/.../csi-common/utils.go:76/84/92 fail to compile and with them pkg/csi/plugins, pkg/csi and cmd/csi. This is also the proven cause of every red CI job on this PR (build, lint, staticcheck, unittest, all e2e). Verified differentially: base compiles, head does not, and a 3-line embed probe (unshippable as-is — go mod vendor would wipe it) makes everything green, so the break is exactly the interface incompatibility. Fix requires fluid-side work: (a) replace-directive fork of kubernetes-csi/drivers with csi-common regenerated, (b) drop/inline the ~200 lines of csi-common helpers fluid actually uses (the durable path — also removes an archived dependency), or (c) decline the bump. Note a silent re-pin to 1.9.x is not a fix: no spec version >= 1.9.0 compiles against csi-common v1.0.0/1.0.2 without changes (v1.9.0 fails with 'controllerServer missing ControllerModifyVolume').
Bumps [github.com/container-storage-interface/spec](https://github.com/container-storage-interface/spec) from 1.8.0 to 1.13.0. - [Release notes](https://github.com/container-storage-interface/spec/releases) - [Commits](container-storage-interface/spec@v1.8.0...v1.13.0) --- updated-dependencies: - dependency-name: github.com/container-storage-interface/spec dependency-version: 1.13.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
7b5d039 to
93c51d5
Compare
|
…se to 93c51d5, F1 still broken Head force-updated by dependabot rebase onto new master (91a58c1, which took the Go 1.26.10 bump). Delta is pure rebase churn: vendored CSI spec files byte-identical, kubernetes-csi/drivers still v1.0.2. Re-verified: new base builds clean (spec v1.8.0), re-verify.sh reports F1 STILL-BROKEN at 93c51d5, CI fully red with the identical csi-common interface errors under go1.26.10. Evidence in results/r2-*. Advance .last-reviewed to 93c51d5. Production code untouched. Co-Authored-By: Claude Code <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
The CSI dependency upgrade breaks compilation because the existing identity, controller, and node servers, including legacy csicommon implementations, do not satisfy the newly required interfaces.
2 open issues
Review effort: Balanced
| require ( | ||
| github.com/agiledragon/gomonkey/v2 v2.14.4 | ||
| github.com/container-storage-interface/spec v1.8.0 | ||
| github.com/container-storage-interface/spec v1.13.0 |
Bumps github.com/container-storage-interface/spec from 1.8.0 to 1.13.0.
Release notes
Sourced from github.com/container-storage-interface/spec's releases.
... (truncated)
Commits
b516e92Merge pull request #607 from container-storage-interface/dependabot/go_module...3c04186Bump golang.org/x/net from 0.48.0 to 0.55.0ad56747Merge pull request #606 from mdzraf/SnapTopologyPOC5334f34Introducing Snapshot Topologycd9e7adMerge pull request #604 from gnufied/implement-volume-health8e06851Add documentation about reserved fields6f80119Update wording for DATA_LOSS and capabilities72e2e56Update the CSI spec to address the review comments0c33790Add volume health proposale74e0e8Fix RESOURCE_EXHAUSTED error code from 13 to 8 (#605)