Skip to content

[nexus] consolidate the VMM unregistration saga helper - #11698

Open
hawkw wants to merge 2 commits into
eliza/stop-creating-nicelyfrom
eliza/move-the-unregister-stuff-into-saga-helpers
Open

hawkw wants to merge 2 commits into
eliza/stop-creating-nicelyfrom
eliza/move-the-unregister-stuff-into-saga-helpers

Conversation

@hawkw

@hawkw hawkw commented Nov 4, 2026 •

Copy link
Copy Markdown
Member

#11696 added enough additional code to the sis_ensure_unregistered unwinding action in the instance_start saga that it felt like the easiest way to propagate the same changes to the instance_migrate saga's unwinding actions was to move all of it into the shared helper. This PR does that. I figured it was a bit nicer to do this in a follow up branch to try and keep the already-giant diff in #11696 a little more manageable.

@hawkw
hawkw added this pull request to stack #11699 November 4, 2026 01:46
@hawkw
hawkw force-pushed the eliza/move-the-unregister-stuff-into-saga-helpers branch 2 times, most recently from 6721869 to 02f9896 Compare November 4, 2026 19:29
@hawkw
hawkw marked this pull request as ready for review November 4, 2026 19:36

@karencfv karencfv left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice refactor!

"sled_id" => sled_id.to_string(),
));

// `ensure_vmm_unregistered` will not retry errors checking if the sled is

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// `ensure_vmm_unregistered` will not retry errors checking if the sled is
// `ensure_vmm_unregistered` will not retry error checking if the sled is

// about it will ever see any evidence of its existence anyway!
// We can now safely proceed to telling the sled-agent to disown
// it.
Err(Error::ObjectNotFound { .. }) => {}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't hurt to add Error::Gone and Error::NotFound for future-proofing

// If the sled no longer exists in the database at all, that's...quite
// surprising! But, we can hopefully assume that the sled, and any VMMs
// running on it, no longer exist.
Err(Error::ObjectNotFound {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

@hawkw
hawkw force-pushed the eliza/move-the-unregister-stuff-into-saga-helpers branch from 02f9896 to a15da41 Compare November 6, 2026 23:52

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants