fix: restart RDS after failed storage grow (#732)

spinifex/handlers/rds/modify_test.go

@@ -605,6 +605,11 @@ func TestModifyDBInstance_FailsTheInstanceAndKeepsThePendingValues(t *testing.T)
 	require.NotNil(t, rec.PendingModifiedValues)
 	assert.Equal(t, int64(50), aws.Int64Value(rec.PendingModifiedValues.AllocatedStorage))
 	assert.Equal(t, int64(20), rec.AllocatedStorage, "the record reports the size the volume actually has")
+	assert.Equal(t, []string{"stop:" + testInstance, "start:" + testInstance}, h.cmdr.calls)
+
+	messages := h.eventMessages(t)
+	assert.Contains(t, messages, "DB instance restarted after its storage grow failed; storage is unchanged.")
+	assert.Contains(t, messages, "the DB instance could not be modified: grow the data volume vol-rdsdata01 to 50 GiB: the volume store rejected the resize")
 }
 
 func TestModifyDBInstance_RequiresAnIdentifier(t *testing.T) {

spinifex/handlers/rds/storage.go

@@ -66,7 +66,7 @@ func (s *Service) growInstanceStorage(ctx context.Context, accountID string, rec
 		return err
 	}
 	if err := s.growDataVolume(ctx, rec.DataVolumeID, targetGiB); err != nil {
-		return err
+		return s.restartAfterStorageGrowFailure(ctx, accountID, rec, err)
 	}
 	if err := context.Cause(ctx); err != nil {
 		return err
@@ -77,6 +77,30 @@ func (s *Service) growInstanceStorage(ctx context.Context, accountID string, rec
 	return nil
 }
 
+// Restarts with a bounded context that survives request cancellation. A lost
+// modify lease transfers recovery to its new holder, so the stale holder stops.
+func (s *Service) restartAfterStorageGrowFailure(
+	ctx context.Context,
+	accountID string,
+	rec *DBInstanceRecord,
+	growErr error,
+) error {
+	if errors.Is(context.Cause(ctx), errModifyLeaseLost) {
+		return growErr
+	}
+
+	restartCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), rollbackTimeout)
+	defer cancel()
+
+	if err := s.startInstanceVM(restartCtx, rec.InstanceID); err != nil {
+		return errors.Join(growErr, fmt.Errorf("restart the DB VM after the storage grow failed: %w", err))
+	}
+	s.RecordEvent(restartCtx, accountID, EventSourceTypeDBInstance, rec.DBInstanceIdentifier,
+		"DB instance restarted after its storage grow failed; storage is unchanged.",
+		EventCategoryRecovery, EventCategoryAvailability)
+	return growErr
+}
+
 // Grows the data volume itself. Idempotent: a resumed grow re-reads the volume
 // and returns without a second modify once it is already at the target, which
 // ModifyVolume would otherwise reject as a shrink.

spinifex/handlers/rds/storage_test.go

@@ -175,15 +175,47 @@ func TestGrowInstanceStorage_ProceedsWhenTheEngineWillNotStop(t *testing.T) {
 	assert.Equal(t, int64(50), h.storage.sizes[testDataVolume])
 }
 
-// A grow that fails leaves the VM down rather than restarting it onto a volume
-// whose size nobody knows: the reconciler resumes from the recorded request.
-func TestGrowInstanceStorage_DoesNotRestartTheVMWhenTheGrowFails(t *testing.T) {
+// A rejected grow must not turn a failed modification into an outage. The VM
+// restarts on its unchanged volume before the original error is returned.
+func TestGrowInstanceStorage_RestartsTheVMWhenTheGrowFails(t *testing.T) {
 	h := newModifyHarness(t)
 	h.storage.modifyErr = errors.New("the volume store is unavailable")
 	rec := modifiableRecord()
 
 	err := h.svc.growInstanceStorage(t.Context(), testAccountID, &rec, 50)
 	require.Error(t, err)
+	assert.Contains(t, err.Error(), "the volume store is unavailable")
+	assert.Equal(t, []string{"stop:" + testInstance, "start:" + testInstance}, h.cmdr.calls)
+}
+
+// Both failures matter when the grow and its recovery fail: the first explains
+// the unchanged storage and the second explains the continuing outage.
+func TestGrowInstanceStorage_ReportsTheGrowAndRestartFailures(t *testing.T) {
+	h := newModifyHarness(t)
+	h.storage.modifyErr = errors.New("the volume store is unavailable")
+	h.storage.onModify = func() { h.cmdr.err = errors.New("the node did not answer") }
+	rec := modifiableRecord()
+
+	err := h.svc.growInstanceStorage(t.Context(), testAccountID, &rec, 50)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "the volume store is unavailable")
+	assert.Contains(t, err.Error(), "restart the DB VM after the storage grow failed")
+	assert.Contains(t, err.Error(), "the node did not answer")
+	assert.Equal(t, []string{"stop:" + testInstance, "start:" + testInstance}, h.cmdr.calls)
+}
+
+// Recovery belongs to the new lease holder after a takeover. The stale holder
+// must not restart the VM while its replacement may be retrying the grow.
+func TestGrowInstanceStorage_DoesNotRestartAfterLosingTheModifyLease(t *testing.T) {
+	h := newModifyHarness(t)
+	ctx, cancel := context.WithCancelCause(t.Context())
+	h.storage.modifyErr = errors.New("the volume store is unavailable")
+	h.storage.onModify = func() { cancel(errModifyLeaseLost) }
+	rec := modifiableRecord()
+
+	err := h.svc.growInstanceStorage(ctx, testAccountID, &rec, 50)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "the volume store is unavailable")
 	assert.Equal(t, []string{"stop:" + testInstance}, h.cmdr.calls)
 }