Expose volume move preflight and dispatch on the API - #662
Open
traviswu-bigstack wants to merge 6 commits into
Open
traviswu-bigstack wants to merge 6 commits into
traviswu-bigstack wants to merge 6 commits into
Conversation
traviswu-bigstack
requested review from
a team and
raven-pan
as code owners
September 14, 2026 14:12
CMP must not SSH into a node to ask whether a volume may move to another
storage tier or to dispatch the move. Adds GET
.../volumes/{volumeId}/move-preflight and POST .../volumes/{volumeId}/move
under the datacenter-scoped Volumes routes, backed by hex_sdk's
cinder_move_preflight. The full blocker list, not just the first reason, is
returned so a refused move tells the caller everything wrong at once.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Travis Wu <travis.wu@bigstack.co>
Task 4 review Critical: moveVolume ran the preflight but never dispatched anything, so a passing request returned 202 and nothing happened. Now calls the new hex_sdk cinder_move_volume, which runs the preflight itself and dispatches the retype on pass. Refusals map exactly as the preflight does (with the full blocker list); a preflight pass whose dispatch then fails (E_DISPATCH_FAILED) maps to 500, not a refusal; a successful dispatch stays 202 Accepted. The GET .../move-preflight route is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Travis Wu <travis.wu@bigstack.co>
Fix round 2 part B: a success payload missing "code" fell through MoveDispatchStatus's default to 409, so a real success would have read as a false Conflict until the shell emitter added "code":"OK" (cubecos 68edef97). TestMoveDispatchStatus fed the mapper a literal "OK" string and never exercised JSON, so it could not have caught this. Adds golden-sample tests that unmarshal the shell's actual stdout through the same rawMoveDispatch -> MoveDispatch path RunMoveVolume uses: a successful dispatch, an E_DISPATCH_FAILED, a refusal asserting all blockers and every snake_case preflight field survive the unmarshal, and an explicit case pinning that a payload with no "code" key must never be reported as success. No mapper or handler logic changed; the tests confirm it was already correct. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Travis Wu <travis.wu@bigstack.co>
… sample RunMovePreflight and RunMoveVolume used exec.Command with no deadline. The preflight can legitimately take a minute or more -- a `ceph fsid` per tier at 20s, a `cubectl node exec` at 30s, plus several openstack calls -- and an HTTP handler with no timeout ties up a request indefinitely. Both now use exec.CommandContext with a five minute budget, the way the repo's other slow shell-outs do, and a timeout maps to 504 rather than 500: the retype may well still be running on the cluster, so it is not an internal fault. The dispatch-failure golden sample carried "cinder retype command failed" while the shell's ERROR_CINDER_MOVE_DISPATCH_FAILED is "retype command failed", and the case asserted only the status, so the sample pinned nothing. The reason is corrected and now asserted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013cE5vKGkQQn3nAnn9TLKAU Signed-off-by: Travis Wu <travis.wu@bigstack.co>
traviswu-bigstack
force-pushed
the
travis.wu/storage-tier-change
branch
from
September 14, 2026 14:14
fafb76d to
979c415
Compare
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013cE5vKGkQQn3nAnn9TLKAU Signed-off-by: Travis Wu <travis.wu@bigstack.co>
cinder_move_preflight now separates advisory notes from refusals: the ceph fsid/pool check no longer blocks a move, because retype cannot reach the rbd driver's same-cluster shortcut that cubecos#1490 is about, and every CubeCOS install ships the same hardcoded fsid and pool -- so blocking on it refused real cluster-to-cluster moves. Adds Warnings to both the preflight and the dispatch types, in the same position in the public and raw structs so the direct struct conversion still holds, and documents the array in the OpenAPI spec as required and present on success, so a caller reading only "ok" is unaffected. Bumps the openapi submodule. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013cE5vKGkQQn3nAnn9TLKAU Signed-off-by: Travis Wu <travis.wu@bigstack.co>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
feature
Which issue(s) this PR fixes?
None directly. Part of the storage-tier-change work; paired with
travis.wu/storage-tier-changein cubecos (bigstack-oss/cubecos#1492) and cubecmp.What this PR does?
Exposes the volume move on the API, so a client can ask "can this volume move to another storage tier, and why not" and dispatch the move without SSH-ing into a node.
Two datacenter-scoped routes, matching the convention every other route in
docs.yamlfollows:GET /api/v1/datacenters/{dataCenter}/volumes/{volumeId}/move-preflight?destType=<tier>POST /api/v1/datacenters/{dataCenter}/volumes/{volumeId}/movewith{"destType":"<tier>"}Both shell out to the
hex_sdkentry points added in the paired cubecos PR. Status mapping:OK→ 200/202,E_NO_SUCH_TYPE/E_SAME_TYPE→ 400, other refusals → 409,E_DISPATCH_FAILED→ 500, timeout → 504. A refusal carries the fullblockersarray, not just the first reason — that is the point of the preflight.Two details worth a reviewer's eye:
hex_sdkemitssrc_type/dst_type/size_gb/attached_to; the API returnssrcType/dstType/sizeGb/attachedTo. An internalrawMovePreflightunmarshals the former and converts. An earlier revision used camelCase tags for parsing and silently zeroed all four fields — golden-sample tests now pin the seam so that cannot regress.Test results (optional)
1). make sure the api docs have been updated
api/cube-cos-openapibumped (branchtravis.wu/storage-tier-change, pushed) andapi/docs.jsonregenerated at build time — it is gitignored and correctly not committed.Known gap: the 504 added here is not yet documented in
docs.yaml. It should be added before merge.2). make sure the api works properly
go test ./internal/cubecos/ -vandgo build ./...both clean. These endpoints have never run against a realhex_sdk— the golden samples pin hand-written bytes that match the shell emitter, not captured stdout. The acceptance script in the cubecos PR is what would exercise the real path.🤖 Generated with Claude Code