-
Notifications
You must be signed in to change notification settings - Fork 66
TQ: Add External API for aborting a membership change #9741
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
1cc9c8f
e537ea0
e8e2ff1
301cdd3
e7142b9
d1ea568
cc07147
a741173
c37e8dc
2fb50c6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -79,6 +79,7 @@ api_versions!([ | |
| // | date-based version should be at the top of the list. | ||
| // v | ||
| // (next_yyyymmddnn, IDENT), | ||
| (2026020200, TRUST_QUORUM_ABORT_CONFIG), | ||
| (2026013100, READ_ONLY_DISKS_NULLABLE), | ||
| (2026013001, READ_ONLY_DISKS), | ||
| (2026013000, INSTANCES_EXTERNAL_SUBNETS), | ||
|
|
@@ -4819,6 +4820,24 @@ pub trait NexusExternalApi { | |
| req: TypedBody<params::RackMembershipAddSledsRequest>, | ||
| ) -> Result<HttpResponseOk<RackMembershipStatus>, HttpError>; | ||
|
|
||
| /// Abort the latest rack membership change | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added a comment, which is bound to be frustrating to someone due to lack of detail, which really can't be provided without mentioning trust quorum.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like it
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Of course you do. You wrote it! |
||
| /// | ||
| /// This operation is synchronous. Upon returning from the API call, a | ||
| /// success response indicates that the prior membership change was aborted. | ||
| /// An error response indicates that there is no active membership change | ||
| /// in progress (previous changes have completed) or that the current | ||
| /// membership change could not be aborted. | ||
| #[endpoint { | ||
| method = POST, | ||
| path = "/v1/system/hardware/racks/{rack_id}/membership/abort", | ||
| tags = ["experimental"], | ||
| versions = VERSION_TRUST_QUORUM_ABORT_CONFIG.. | ||
| }] | ||
| async fn rack_membership_abort( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this take a specific
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can only abort the latest membership, as only one trust quorum reconfiguration is allowed at a time. I could see how that could be worrisome in the case of dueling administrators. However, it can only occur during the trust quorum prepare phase, which should be very short (I need to activate the bg task immediately rather than waiting on timeout in a future PR), so it's hard to do the wrong thing on human timescales. Additionally, even if it was done by accident, no harm, no foul. An admin can just reissue the last command to add the sleds again and it will work. |
||
| rqctx: RequestContext<Self::Context>, | ||
| path_params: Path<params::RackPath>, | ||
| ) -> Result<HttpResponseOk<RackMembershipStatus>, HttpError>; | ||
|
|
||
| /// Retrieve the rack cluster membership status | ||
| /// | ||
| /// Returns the status for the most recent change, or a specific version if | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is what we discussed and I think this makes sense adjacent to
add