sled agent: remove unused migration-related types #2755
Merged
Conversation
This file contains 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
Tidy up the migration-related types in sled agent's Nexus API. This is just a
bit of pre-work to reduce the size of subsequent commits that change this API.
In `sled-agent/src/params.rs`:
- Rename migration-related types so that they indicate whether they're used with
migration sources or migration targets (without having to look at their
fields' names).
- Remove types and variants that were only used for migration sources.
- Remove `InstanceRuntimeStateRequested` and replace it with
`InstanceStateRequested`.
- Remove the `Migrating` variant from `InstanceStateRequested`.
Sled agent does need an API that allows Nexus to set an instance's migration ID,
but this doesn't (won't) actually change any Propolis state or user-visible
instance state, so subsequent changes will implement this in a separate API.
Clean up the call sites that are affected by this change. For now, just add
`todo!()`s and comment out the Nexus call sites that use migration types that
were removed here. These will be replaced in subsequent commits that create
sled agent's new migration APIs.
luqmana
approved these changes
Apr 4, 2023
| @@ -1297,7 +1292,7 @@ async fn sic_instance_ensure( | |||
| &opctx, | |||
| &authz_instance, | |||
| &db_instance, | |||
| runtime_params, | |||
| InstanceStateRequested::Running, | |||
There was a problem hiding this comment.
Love to see the simplification!
|
Thanks for reviewing, @luqmana! @smklein @jordanhendricks - since this PR is one of the smaller, less impactful ones in this sequence, I'm going to go ahead and merge it to unblock its considerably larger successor. The bits of code that this PR removed will come back in a different form in future PRs, so there should still be plenty of room for us to discuss the final shape of the sled agent/Nexus API. |
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.
Tidy up the migration-related types in sled agent's Nexus API. This is just a bit of pre-work to reduce the size of subsequent commits that change this API.
In
sled-agent/src/params.rs:InstanceRuntimeStateRequestedand replace it withInstanceStateRequested.Migratingvariant fromInstanceStateRequested.Sled agent does need an API that allows Nexus to set an instance's migration ID, but this doesn't (won't) actually change any Propolis state or user-visible instance state, so subsequent changes will implement this in a separate API.
Clean up the call sites that are affected by this change. For now, just add
todo!()s and comment out the Nexus call sites that use migration types that were removed here. These will be replaced in subsequent commits that create sled agent's new migration APIs.