Skip to content

Commit

Permalink
go back to using instance state when observing migrations
Browse files Browse the repository at this point in the history
The instance runtime state machine in sled agent requires that an
inbound migration appear to be completed only once (so that there's only
one "move the destination Propolis ID to the current Propolis ID and
clear the destination ID" operation per successful migration). This
operation (clearing the instance's active migration ID) is itself what
prevents future Propolis state updates from being interpreted as a
completed migration (once the migration ID is cleared from sled agent's
runtime state, it will no longer match the old ID that Propolis is
reporting).

In the brave new world where sled agent doesn't track instance runtime
state at all, substantially all of this code will be removed: there will
be no instance runtime state to transition to anything; sled agent will
just pass migration statuses straight through to Nexus.
  • Loading branch information
gjcolombo committed Jun 18, 2024
1 parent c12cea5 commit ecc9c33
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 10 deletions.
19 changes: 11 additions & 8 deletions sled-agent/src/common/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,15 +114,22 @@ impl ObservedPropolisState {
/// runtime state and an instance state monitor response received from
/// Propolis.
pub fn new(
migration: Option<&MigrationRuntimeState>,
instance_runtime: &InstanceRuntimeState,
propolis_state: &InstanceStateMonitorResponse,
) -> Self {
// If there's no migration currently registered with this sled, report
// the current state and that no migration is currently in progress,
// even if Propolis has some migration data to share. (This case arises
// when Propolis returns state from a previous migration that sled agent
// has already retired.)
let Some(migration) = migration else {
//
// N.B. This needs to be read from the instance runtime state and not
// the migration runtime state to ensure that, once a migration in
// completes, the "completed" observation is reported to
// `InstanceStates::apply_propolis_observation` exactly once.
// Otherwise that routine will try to apply the "inbound migration
// complete" instance state transition twice.
let Some(migration_id) = instance_runtime.migration_id else {
return Self {
vmm_state: PropolisInstanceState(propolis_state.state),
migration_status: ObservedMigrationStatus::NoMigration,
Expand All @@ -136,12 +143,8 @@ impl ObservedPropolisState {
&propolis_state.migration.migration_in,
&propolis_state.migration.migration_out,
) {
(Some(inbound), _) if inbound.id == migration.migration_id => {
inbound
}
(_, Some(outbound)) if outbound.id == migration.migration_id => {
outbound
}
(Some(inbound), _) if inbound.id == migration_id => inbound,
(_, Some(outbound)) if outbound.id == migration_id => outbound,
_ => {
// Sled agent believes this instance should be migrating, but
// Propolis isn't reporting a matching migration yet, so assume
Expand Down
2 changes: 1 addition & 1 deletion sled-agent/src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ impl InstanceRunner {
match request {
Some(Update { state, tx }) => {
let observed = ObservedPropolisState::new(
self.state.migration(),
self.state.instance(),
&state,
);
let reaction = self.observe_state(&observed).await;
Expand Down
2 changes: 1 addition & 1 deletion sled-agent/src/sim/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ impl SimInstanceInner {
}

self.state.apply_propolis_observation(&ObservedPropolisState::new(
self.state.migration(),
self.state.instance(),
&self.last_response,
))
} else {
Expand Down

0 comments on commit ecc9c33

Please sign in to comment.