diff --git a/spinifex/daemon/daemon_system_instance.go b/spinifex/daemon/daemon_system_instance.go index 84b3863b4..cb5afd126 100644 --- a/spinifex/daemon/daemon_system_instance.go +++ b/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 diff --git a/spinifex/daemon/daemon_system_instance_test.go b/spinifex/daemon/daemon_system_instance_test.go index 70c15b32f..62140fa48 100644 --- a/spinifex/daemon/daemon_system_instance_test.go +++ b/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 // ---------------------------------------------------------------------------