feat(gateway): add an admin API to remove a CVM - #1044
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a narrowly scoped administrative RPC to remove a single CVM instance from replicated WaveKV state and the local gateway data plane, providing an operator recovery path for bad/unreadable instance records.
Changes:
- Add
Admin.RemoveCvmRPC and request message to the gateway proto. - Implement
AdminRpcHandler::remove_cvm, calling into the gatewayProxyto delete the instance from WaveKV and local state. - Add a regression test covering removal when the stored instance record is unreadable.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
dstack/gateway/src/main_service.rs |
Adds Proxy::remove_cvm to sync a tombstone to WaveKV and drop the instance from local ProxyState (with WG reconfigure when needed). |
dstack/gateway/src/admin_service.rs |
Wires new remove_cvm admin RPC handler and emits an operator audit log entry. |
dstack/gateway/rpc/proto/gateway_rpc.proto |
Adds RemoveCvm RPC and RemoveCvmRequest message to the Admin service. |
dstack/gateway/src/main_service/tests.rs |
Adds a test proving an operator can remove a CVM even if its KV record is unreadable, and that the operation is idempotent. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…come Address review findings on the RemoveCvm admin RPC: - Reconfigure WireGuard unconditionally. The tombstone write and the in-memory removal are not repeated on a retry, so gating reconfigure on them left a failed reconfigure with no retry path and the removed CVM's WireGuard peer stuck on the interface. - Return record_existed/removed_locally to the operator. A mistyped instance_id still writes a tombstone, so it previously reported an indistinguishable success; now both fields come back false. - Move RemoveCvmRequest out of the DNS credential section in the proto. - Start the admin removal log message with lowercase per code style.
The handler only rejected leading/trailing whitespace, so an ID with internal whitespace, control characters (log injection via the warn! audit line), or unbounded length was accepted and written into a KV tombstone key. Reuse import::validate_id — every identifier a legitimate gateway writes satisfies it, so this rejects only typos.
kvinwang
force-pushed
the
fix/gateway-admin-remove-cvm
branch
from
August 12, 2026 09:29
1c7f861 to
4ec0563
Compare
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
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.
Summary
Add an explicit, narrowly scoped Admin RPC for removing one CVM from replicated state and the local data plane.
This is stacked on #1035. That PR deliberately preserves instance records that an older gateway cannot decode or validate. This PR provides the corresponding manual operator recovery path without exposing arbitrary raw-KV deletion.
Behavior
Admin.RemoveCvmtakes aninstance_idand:inst/<instance_id>to persistent WaveKV;ProxyStatewhen present;The operation is idempotent and still succeeds when the instance record is unreadable or the CVM is already absent locally. The response reports
record_existedandremoved_locally, so a mistypedinstance_id(which still writes a tombstone) is visible to the operator instead of silently reporting success. The administrative action is logged atwarn!level.Verification
cargo fmt --all --checkcargo test -p dstack-gateway --all-features(140 tests)cargo clippy -p dstack-gateway --all-features -- -D warnings -D clippy::expect_used -D clippy::unwrap_used --allow unused_variables