fix(daemon): fail the system-instance launch when the VM never reaches running (#574)

LaunchRunInstances is a best-effort batch launcher that reports failure only
through VM state: volume preparation, GPU claim and vmMgr.Run errors each call
MarkFailed and continue. launchAMISystemInstance ignored that and always
returned a SystemInstanceOutput, so an EKS control-plane server, ELB or Bedrock
serving VM whose launch failed handed its caller an instance ID for a VM that
was already being torn down.

Check the instance reached running state before returning. Cleanup stays with
the MarkFailed chain LaunchRunInstances has already run.

spinifex/daemon/daemon_system_instance.go

@@ -418,6 +418,11 @@ func (d *Daemon) LaunchSystemInstance(input *handlers_elbv2.SystemInstanceInput)
 // right NIC for the Ec2 datasource on its own. The instance is owned by
 // input.AccountID (the account its pre-created ENI lives in) and tagged
 // input.ManagedBy so customer listings hide it.
+//
+// Sharing the Prepare/Launch pair with RunInstances means a GPU instance type
+// gets the same admission, GPU claim and teardown-time release as a customer
+// VM: PrepareRunInstances gates on a free GPU slot, LaunchRunInstances claims
+// the device, and vm.Manager's cleanup chain releases it.
 func (d *Daemon) launchAMISystemInstance(input *sysinstance.SystemInstanceInput) (*sysinstance.SystemInstanceOutput, error) {
 	if d.instanceService == nil {
 		return nil, errors.New("sysinstance: instance service not initialized")
@@ -499,6 +504,10 @@ func (d *Daemon) launchAMISystemInstance(input *sysinstance.SystemInstanceInput)
 
 	d.instanceService.LaunchRunInstances(context.Background(), instances, runInput, instanceType)
 
+	if err := d.verifySystemInstanceLaunched(inst); err != nil {
+		return nil, err
+	}
+
 	privateIP := input.ENIIP
 	if privateIP == "" && inst.Instance != nil {
 		privateIP = aws.StringValue(inst.Instance.PrivateIpAddress)
@@ -517,6 +526,21 @@ func (d *Daemon) launchAMISystemInstance(input *sysinstance.SystemInstanceInput)
 	}, nil
 }
 
+// verifySystemInstanceLaunched checks that inst actually reached running state
+// after LaunchRunInstances. That call is a best-effort batch launcher: volume
+// preparation, GPU claim and vmMgr.Run failures each mark the instance failed
+// and return nothing, so the outcome is only visible in the VM's state. Without
+// this check a system-instance caller (EKS control plane, ELB, Bedrock serving
+// VM) receives an instance ID for a VM already being torn down and waits on an
+// endpoint that will never answer. Cleanup is left to the MarkFailed chain
+// LaunchRunInstances already ran.
+func (d *Daemon) verifySystemInstanceLaunched(inst *vm.VM) error {
+	if status := d.vmMgr.Status(inst); status != vm.StateRunning {
+		return fmt.Errorf("sysinstance: instance %s did not reach running state (status %q)", inst.ID, status)
+	}
+	return nil
+}
+
 // attachSystemMgmtNIC allocates a management-bridge IP + MAC for a system VM and
 // creates the host-side tap on br-mgmt. The mgmt NIC is mandatory for system
 // instances that boot from an AMI: their VPC ENI has no route off the tenant

spinifex/daemon/daemon_system_instance_test.go

@@ -1,6 +1,7 @@
 package daemon
 
 import (
+	"context"
 	"encoding/json"
 	"fmt"
 	"os"
@@ -84,6 +85,50 @@ func TestWaitForSystemInstance_Timeout(t *testing.T) {
 	}
 }
 
+// ---------------------------------------------------------------------------
+// verifySystemInstanceLaunched
+// ---------------------------------------------------------------------------
+
+func TestVerifySystemInstanceLaunched_Running(t *testing.T) {
+	inst := &vm.VM{ID: "i-up", Status: vm.StateRunning}
+	d := newDaemonWithVMs(inst)
+
+	require.NoError(t, d.verifySystemInstanceLaunched(inst))
+}
+
+// LaunchRunInstances swallows launch errors and leaves the instance in a
+// cleanup state, so every non-running state must surface as a launch failure —
+// a GPU claim failure marks the VM shutting-down via MarkFailed.
+func TestVerifySystemInstanceLaunched_NonRunningStatesFail(t *testing.T) {
+	for _, status := range []vm.InstanceState{
+		vm.StateShuttingDown,
+		vm.StateError,
+		vm.StateTerminated,
+		vm.StatePending,
+		vm.StateProvisioning,
+	} {
+		t.Run(string(status), func(t *testing.T) {
+			inst := &vm.VM{ID: "i-down", Status: status}
+			d := newDaemonWithVMs(inst)
+
+			err := d.verifySystemInstanceLaunched(inst)
+			require.Error(t, err)
+			assert.Contains(t, err.Error(), "i-down")
+			assert.Contains(t, err.Error(), string(status))
+		})
+	}
+}
+
+// MarkFailed transitions synchronously before running its cleanup goroutine, so
+// the check sees the failure without racing the teardown.
+func TestVerifySystemInstanceLaunched_AfterMarkFailedIsAnError(t *testing.T) {
+	inst := &vm.VM{ID: "i-gpu-claim-failed", Status: vm.StateProvisioning}
+	d := newDaemonWithVMs(inst)
+	d.vmMgr.MarkFailed(context.Background(), inst, "gpu_claim_failed")
+
+	require.Error(t, d.verifySystemInstanceLaunched(inst))
+}
+
 // ---------------------------------------------------------------------------
 // buildNetcfgBlob
 // ---------------------------------------------------------------------------