fix(ec2): fail RunInstances when public-IP auto-assign fails (mulga-siv-275) (#345)

* fix(ec2): fail RunInstances when public-IP auto-assign fails

PrepareRunInstances swallowed an AllocateIP failure for a
MapPublicIpOnLaunch subnet (logged WARN, fell through), so the instance
booted running with no public IP. With no qemu-hostfwd fallback such an
instance is unreachable. Mirror the adjacent AddNAT-failure path: detach
and delete the auto-created ENI, deallocate capacity, drop the instance,
and surface InsufficientAddressCapacity.

* test(daemon): deterministically pick a fitting 2-vCPU type in capacity filter test

CapacityFilterRespectsAllocation selected the first 2-vCPU type by Go
map-iteration order, intermittently picking a type near the
schedulable-memory boundary the allocator then rejected (~3/5 failures in
isolation on small hosts). Select the smallest-memory 2-vCPU type from the
capacity-aware set so it always fits.

spinifex/daemon/daemon_test.go

@@ -539,16 +539,19 @@ func TestHandleEC2DescribeInstanceTypes(t *testing.T) {
 		capMsgData, err := json.Marshal(capInput)
 		require.NoError(t, err)
 
-		// Pick a 2-vCPU type from the capacity-aware set so we know the
-		// host has room for it — the raw instanceTypes map contains
-		// entries (e.g. r5.large at 16 GiB) that exceed schedulable
-		// memory on small test hosts.
+		// Pick the smallest-memory 2-vCPU type from the capacity-aware set
+		// so it is guaranteed to fit the host. Selecting the first match by
+		// map-iteration order is non-deterministic and intermittently picks a
+		// type near the schedulable-memory boundary that the allocator then
+		// rejects, flaking the test on small hosts.
 		var instanceType2CPU *ec2.InstanceTypeInfo
-		for _, it := range daemon.resourceMgr.GetAvailableInstanceTypeInfos(false) {
-			if it.VCpuInfo != nil && it.VCpuInfo.DefaultVCpus != nil && *it.VCpuInfo.DefaultVCpus == 2 &&
-				it.MemoryInfo != nil && it.MemoryInfo.SizeInMiB != nil {
+		for _, it := range daemon.resourceMgr.GetAvailableInstanceTypeInfos(true) {
+			if it.VCpuInfo == nil || it.VCpuInfo.DefaultVCpus == nil || *it.VCpuInfo.DefaultVCpus != 2 ||
+				it.MemoryInfo == nil || it.MemoryInfo.SizeInMiB == nil {
+				continue
+			}
+			if instanceType2CPU == nil || *it.MemoryInfo.SizeInMiB < *instanceType2CPU.MemoryInfo.SizeInMiB {
 				instanceType2CPU = it
-				break
 			}
 		}
 		require.NotNil(t, instanceType2CPU, "Should find a 2-vCPU type that fits the host")

spinifex/handlers/ec2/instance/service_impl.go

@@ -813,7 +813,27 @@ func (s *InstanceServiceImpl) PrepareRunInstances(input *ec2.RunInstancesInput,
 					az := s.config.AZ
 					publicIP, poolName, allocErr := s.ipAllocator.AllocateIP(region, az, handlers_ec2_vpc.PurposeENIPublic, "", *eni.NetworkInterfaceId, instance.ID)
 					if allocErr != nil {
-						slog.Warn("PrepareRunInstances: failed to allocate public IP", "instanceId", instance.ID, "err", allocErr)
+						// No qemu-hostfwd fallback exists: an instance on a
+						// MapPublicIpOnLaunch subnet with no public IP is
+						// unreachable, so fail the launch instead of booting a
+						// running-but-unreachable instance. The ENI is attached
+						// with a primary IP — detach before delete (in-use ENIs
+						// reject deletion).
+						slog.Error("PrepareRunInstances: public IP allocation failed — aborting launch",
+							"instanceId", instance.ID, "eniId", *eni.NetworkInterfaceId, "err", allocErr)
+						if detErr := s.eniCreator.DetachENI(accountID, *eni.NetworkInterfaceId); detErr != nil {
+							slog.Warn("PrepareRunInstances: failed to detach ENI after public-IP allocation failure",
+								"eniId", *eni.NetworkInterfaceId, "err", detErr)
+						}
+						if _, delErr := s.eniDeleter.DeleteNetworkInterface(&ec2.DeleteNetworkInterfaceInput{
+							NetworkInterfaceId: eni.NetworkInterfaceId,
+						}, accountID); delErr != nil {
+							slog.Warn("PrepareRunInstances: failed to delete ENI after public-IP allocation failure",
+								"eniId", *eni.NetworkInterfaceId, "err", delErr)
+						}
+						lastRunErr = errors.New(awserrors.ErrorInsufficientAddressCapacity)
+						s.resourceMgr.Deallocate(instanceType)
+						continue
 					} else {
 						if updateErr := s.eniCreator.UpdateENIPublicIP(accountID, *eni.NetworkInterfaceId, publicIP, poolName); updateErr != nil {
 							slog.Warn("PrepareRunInstances: failed to update ENI with public IP", "eniId", *eni.NetworkInterfaceId, "err", updateErr)

spinifex/handlers/ec2/instance/service_impl_test.go

@@ -2824,6 +2824,51 @@ func TestPrepareRunInstances_NATFailureRollsBackPublicIP(t *testing.T) {
 	}
 }
 
+// TestPrepareRunInstances_PublicIPAllocFailureAbortsLaunch regresses the
+// silent failure where a failed public-IP allocation (pool exhausted) was
+// logged at WARN and ignored, so the instance booted running with no public
+// IP. There is no qemu-hostfwd fallback, so such an instance is unreachable.
+// The fix must fail the launch: detach + delete the auto-created ENI,
+// deallocate capacity, drop the instance, and surface InsufficientAddressCapacity.
+func TestPrepareRunInstances_PublicIPAllocFailureAbortsLaunch(t *testing.T) {
+	eni := &fakeENICreator{
+		subnet: &SubnetInfo{SubnetID: "subnet-1", VpcID: "vpc-1", MapPublicIpOnLaunch: true},
+		createOut: &ec2.CreateNetworkInterfaceOutput{
+			NetworkInterface: &ec2.NetworkInterface{
+				NetworkInterfaceId: aws.String("eni-pubip-fail"),
+				MacAddress:         aws.String("aa:bb:cc:dd:ee:02"),
+				PrivateIpAddress:   aws.String("10.0.0.50"),
+				VpcId:              aws.String("vpc-1"),
+			},
+		},
+	}
+	ipam := &fakeIPAllocator{err: errors.New("InsufficientAddressCapacity: pool wan exhausted")}
+	deleter := &fakeENIDeleter{}
+	svc, prov := prepareSvcWithENI(t, eni, ipam)
+	svc.eniDeleter = deleter
+
+	_, instances, _, err := svc.PrepareRunInstances(&ec2.RunInstancesInput{
+		InstanceType: aws.String("t3.micro"),
+		ImageId:      aws.String("ami-1"),
+		SubnetId:     aws.String("subnet-1"),
+		MinCount:     aws.Int64(1),
+		MaxCount:     aws.Int64(1),
+	}, "acc")
+
+	require.Error(t, err)
+	assert.Equal(t, awserrors.ErrorInsufficientAddressCapacity, err.Error(),
+		"pool exhaustion must surface as InsufficientAddressCapacity, not a silent IP-less boot")
+	assert.Empty(t, instances, "instance with no public IP must not be returned")
+
+	// The ENI is attached with a primary IP; rollback must detach before
+	// delete (in-use ENIs reject deletion) so no eniKV record leaks.
+	assert.Equal(t, 1, eni.detachCalls, "rollback must detach the ENI before delete")
+	assert.Equal(t, []string{"eni-pubip-fail"}, deleter.calls, "rollback must delete the auto-created ENI")
+	assert.Equal(t, 0, eni.updateCalls, "no public IP was allocated, so the ENI must never be updated with one")
+
+	require.Len(t, prov.deallocated, 1, "public-IP allocation failure must trigger Deallocate")
+}
+
 // natWirePayload mirrors utils.natEvent for test-side decoding.
 type natWirePayload struct {
 	VpcId      string `json:"vpc_id"`