mirror of
https://github.com/mulgadc/spinifex.git
synced 2026-10-15 17:11:30 +00:00
fix: re-assert boot-volume in-use state on daemon-restart reconnect (#441)
reconnectInstance re-asserted the instance running state but never the boot volume's in-use state, so after a daemon restart describe-instances showed the VM running while describe-volumes showed its root volume available — a split-brain that makes start-instances attempt to double-launch a live QEMU. Extract the launch path's boot-volume promotion into markBootVolumesInUse and call it from reconnectInstance so launch and recovery keep volume state consistent. Add a reconnect unit test asserting the boot volume is marked in-use and non-boot volumes are left to their own attach flow.
This commit is contained in:
+21
-11
@@ -160,17 +160,7 @@ func (m *Manager) launch(instance *VM) error {
|
||||
}
|
||||
|
||||
// Mark boot volumes as "in-use" now that instance is confirmed running.
|
||||
if m.deps.VolumeStateUpdater != nil {
|
||||
instance.EBSRequests.Mu.Lock()
|
||||
for _, ebsReq := range instance.EBSRequests.Requests {
|
||||
if ebsReq.Boot {
|
||||
if err := m.deps.VolumeStateUpdater.UpdateVolumeState(ebsReq.Name, "in-use", instance.ID, ""); err != nil {
|
||||
slog.Error("Failed to update volume state to in-use", "volumeId", ebsReq.Name, "err", err)
|
||||
}
|
||||
}
|
||||
}
|
||||
instance.EBSRequests.Mu.Unlock()
|
||||
}
|
||||
m.markBootVolumesInUse(instance)
|
||||
|
||||
if m.deps.Hooks.OnInstanceUp != nil {
|
||||
// Launch path: per-instance subscribe failures are logged and the
|
||||
@@ -186,6 +176,26 @@ func (m *Manager) launch(instance *VM) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// markBootVolumesInUse re-asserts "in-use" status for an instance's boot
|
||||
// volumes once it is confirmed running. Used by the launch path and the
|
||||
// daemon-restart reconnect path so both keep volume state consistent with a
|
||||
// running instance. Errors are logged, not fatal.
|
||||
func (m *Manager) markBootVolumesInUse(instance *VM) {
|
||||
if m.deps.VolumeStateUpdater == nil {
|
||||
return
|
||||
}
|
||||
instance.EBSRequests.Mu.Lock()
|
||||
defer instance.EBSRequests.Mu.Unlock()
|
||||
for _, ebsReq := range instance.EBSRequests.Requests {
|
||||
if !ebsReq.Boot {
|
||||
continue
|
||||
}
|
||||
if err := m.deps.VolumeStateUpdater.UpdateVolumeState(ebsReq.Name, "in-use", instance.ID, ""); err != nil {
|
||||
slog.Error("Failed to update volume state to in-use", "volumeId", ebsReq.Name, "err", err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const (
|
||||
nbdReadyTimeout = 5 * time.Second
|
||||
qemuStartupTimeout = 5 * time.Second
|
||||
|
||||
@@ -318,6 +318,10 @@ func (m *Manager) reconnectInstance(instance *VM) error {
|
||||
|
||||
instance.Status = StateRunning
|
||||
|
||||
// Re-assert boot-volume in-use state; a daemon restart otherwise leaves the
|
||||
// root volume "available" while the instance runs (split-brain, siv-464).
|
||||
m.markBootVolumesInUse(instance)
|
||||
|
||||
if err := m.writeRunningState(); err != nil {
|
||||
return fmt.Errorf("failed to persist reconnected instance state: %w", err)
|
||||
}
|
||||
|
||||
@@ -885,6 +885,35 @@ func TestReconnectInstance(t *testing.T) {
|
||||
assert.NotNil(t, saved["i-no-hook"], "the reconnected instance must appear in the persisted snapshot")
|
||||
})
|
||||
|
||||
t.Run("reconnect re-asserts boot-volume in-use, skips non-boot", func(t *testing.T) {
|
||||
stateUpdater := &fakeVolumeStateUpdater{}
|
||||
m := NewManager()
|
||||
m.SetDeps(Deps{
|
||||
NodeID: "test-node",
|
||||
StateStore: newFakeStateStore(),
|
||||
VolumeStateUpdater: stateUpdater,
|
||||
})
|
||||
|
||||
orig := attachQMPForReconnect
|
||||
t.Cleanup(func() { attachQMPForReconnect = orig })
|
||||
attachQMPForReconnect = func(_ *Manager, v *VM) error { fakeAttachQMP(t, v); return nil }
|
||||
|
||||
instance := &VM{ID: "i-reconnect-vol", Status: StatePending}
|
||||
instance.EBSRequests.Requests = []types.EBSRequest{
|
||||
{Name: "vol-root", Boot: true},
|
||||
{Name: "vol-data", Boot: false},
|
||||
}
|
||||
m.Insert(instance)
|
||||
|
||||
require.NoError(t, m.reconnectInstance(instance))
|
||||
|
||||
calls := stateUpdater.snapshot()
|
||||
require.Len(t, calls, 1,
|
||||
"reconnect must re-assert exactly the boot volume — a daemon restart otherwise leaves the root volume 'available' while the VM runs (siv-464)")
|
||||
assert.Equal(t, volumeStateUpdate{VolumeID: "vol-root", State: "in-use", InstanceID: "i-reconnect-vol"}, calls[0],
|
||||
"the boot volume must be marked in-use under the running instance; non-boot volumes are owned by their own attach flow")
|
||||
})
|
||||
|
||||
t.Run("AttachQMP succeeds, OnInstanceUp errors: QMP closed and nil'd", func(t *testing.T) {
|
||||
m := NewManager()
|
||||
hookErr := errors.New("nats subscribe failed")
|
||||
|
||||
Reference in New Issue
Block a user