feat(gateway): add rejected-record listing and node removal recovery APIs - #1046
Merged
Conversation
…APIs Three operator recovery additions in the same family as Admin.RemoveCvm: - Admin.ListRejectedInstances reports the instance records this node currently refuses to import, with the reason and whether the instance still holds local data-plane state. Rejections were previously logged only on transitions, so finding what to remove meant grepping old logs or restarting the gateway to re-log them. - Admin.RemoveNode tombstones a decommissioned gateway node's replicated records (info, status, sync address) and drops it from the sync peer set immediately. The __peer_addr tombstone doubles as the cluster-wide removal signal: every gateway watches the prefix and prunes its own peer set when the deletion replicates, so no restart is needed. An address that was never written does not count as removed, because bootstrap can add a peer before its address record has synced in. - Admin.DeleteZtDomain now works on a corrupt config record. It used to gate deletion on get_zt_domain_config, which cannot tell missing from unreadable, so a corrupt record was permanently stuck — the same trap RemoveCvm was added to fix for instance records. Both removal RPCs are idempotent and report record_existed so a mistyped ID is visible instead of silently succeeding.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds operator-focused recovery APIs to dstack-gateway’s Admin RPC surface, extending the existing “explicit recovery instead of raw KV deletion” approach. It improves observability into rejected instance records, enables safe removal of decommissioned gateway nodes, and makes ZT-domain deletion resilient to corrupt config records.
Changes:
- Add
Admin.ListRejectedInstancesto report currently refused instance records, including reason, rejection class, and whether the instance is still active in the local data plane. - Add
Admin.RemoveNodeplus KV/watch plumbing so node removals replicate via a__peer_addrtombstone and peers prune without restart. - Allow
Admin.DeleteZtDomainto delete a domain even when itscert/{domain}/configrecord exists but is unreadable.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| dstack/gateway/src/main_service/tests.rs | Adds tests covering rejected-instance listing visibility, node removal idempotency/self-protection, peer pruning on replicated tombstones, and deletion of corrupt ZT-domain configs. |
| dstack/gateway/src/main_service.rs | Implements Proxy::rejected_instances() and Proxy::remove_node(), and adds a WaveKV watch to prune peers when __peer_addr deletions replicate. |
| dstack/gateway/src/kv/mod.rs | Adds node-removal tombstoning (sync_remove_node), peer removal/pruning helpers, a __peer_addr prefix watch, and a “config exists even if corrupt” check for ZT domains. |
| dstack/gateway/src/admin_service.rs | Exposes the new Admin RPC methods, maps internal rejection types to RPC payloads, and updates ZT-domain deletion gating to use existence checks instead of decode. |
| dstack/gateway/rpc/proto/gateway_rpc.proto | Extends the Admin service with ListRejectedInstances and RemoveNode plus new request/response message types. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- RemoveNode now reports record_existed when any of the node's persistent records (info, status, or sync address) was live, so a node registered via SetNodeUrl but never booted is still visible to the operator. The three deletes also run under one write handle. - Loading instances no longer logs each undecodable record at error! level. That log line fired on every reload and every rejected-record listing, bypassing the transition-only reporting the reload path already has. The decode error now travels in LoadedInstances instead, which also gives ListRejectedInstances the actual error to show rather than a generic "record does not decode".
…stone RemoveNode wrote the __peer_addr tombstone first and asked the peer set second. The tombstone wakes the peer-address watcher, so on a multi-threaded runtime prune_removed_peers() could drop the peer before the RPC path's own remove_peer() ran, and removed_from_peer_set would report false for a peer that was present when the request began. Drop the peer before publishing the tombstone, so the reported membership no longer depends on scheduling. Add a regression test that races remove_node against a live watcher on a multi-threaded runtime.
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
Three operator recovery additions in the same family as
Admin.RemoveCvm(#1044).Behavior
Admin.ListRejectedInstancesReports the instance records this node currently refuses to import, with the rejection reason (
unusable/lost_conflict) and whether the instance still holds local data-plane state (so the operator knows a removal would also drop live routing).Rejections were previously logged only on transitions (by design, see #1035), so finding what to remove meant grepping old logs or restarting the gateway to re-log them. The listing is recomputed from the store on every call, so it is current even right after a restart.
Admin.RemoveNodeRemoves a decommissioned gateway node:
node/info/,node/status/, and__peer_addr/;The
__peer_addrtombstone doubles as the cluster-wide removal signal: every gateway watches the prefix and prunes its own peer set when the deletion replicates, so no restart is needed anywhere. An address that was never written does not count as removed, because bootstrap can add a peer before its address record has synced in.A node removed by mistake rejoins when it restarts (startup re-registers its records), or via
SetNodeUrlfrom any live gateway. A node cannot remove itself.Admin.DeleteZtDomainon corrupt recordsDeletion used to be gated on
get_zt_domain_config, which cannot tell a missing record from an unreadable one, so a corruptcert/{domain}/configwas permanently stuck ("ZT-Domain config not found") — the same trapRemoveCvmwas added to fix for instance records. Deletion now checks for the record itself and works regardless of decodability.Both removal RPCs are idempotent and report
record_existed, so a mistyped ID is visible to the operator instead of silently succeeding.Verification
cargo fmt --all --checkcargo test -p dstack-gateway --all-features(156 tests, 5 new)cargo clippy -p dstack-gateway --all-features -- -D warnings -D clippy::expect_used -D clippy::unwrap_used --allow unused_variables