feat(vm): ENI hot-plug Sprint 3c — real OVS tap wiring (#296)

* test: cover Sprint 3b ENI hot-plug surface

Sprint 3b's gateway + daemon + KV-helper additions landed without unit
tests. dev's diff-coverage gate flipped to 28.5% as a result. This adds
the three test files needed to bring the diff coverage of the ENI
hot-plug surface back above the 60% threshold.

- handlers/ec2/vpc/eni_hotplug_helpers_test.go: GetENIRecord, UpdateENI,
  FindENIByAttachment — happy paths, not-found, account scoping,
  round-trip of every new ENIRecord field.
- gateway/ec2/vpc/network_interface_hotplug_test.go: Attach + Detach
  validation, no-responders, NATS roundtrip with command capture,
  daemon error / malformed responses, attachment-resolver branches.
- daemon/daemon_handlers_eni_test.go: eniHotplugErrorCode table,
  publishENIHotplugEvent (nil + publish), full handleAttach /
  handleDetach matrix (nil data, not-running, ENI/attachment not found,
  wrong owner, success with stub QMP + NATS event assertion, hot-plug
  failure + KV rollback assertion).

vm.SetHotPlugTestSeams is added as an exported test seam following the
same pattern as utils.SetSudoCommandForTest so daemon-package tests can
swap newDeviceController + eniPipelineSleep without reassigning vm-package
unexported vars.

* feat(vm): wire real OVS tap into ENI hot-plug pipeline

Sprint 3c: replace the stubTapAdd/stubOVSAddPort/stubTapDel/stubOVSDelPort
placeholders with the shared NetworkPlumber. SetupTap creates the kernel tap
and attaches it to br-int with the OVN iface-id binding in one atomic step;
CleanupTap reverses both on rollback and detach. Nil-plumber managers stay a
noop, mirroring setupExtraENINICs.

Adds fake-plumber coverage: tap-on-br-int spec assertion, tap-setup failure
aborts before any QMP call, and QMP/detach paths both invoke CleanupTap.

spinifex/daemon/daemon_handlers_eni_test.go

@@ -0,0 +1,441 @@
+package daemon
+
+import (
+	"encoding/json"
+	"errors"
+	"fmt"
+	"testing"
+	"time"
+
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/service/ec2"
+	"github.com/mulgadc/spinifex/spinifex/awserrors"
+	handlers_ec2_vpc "github.com/mulgadc/spinifex/spinifex/handlers/ec2/vpc"
+	"github.com/mulgadc/spinifex/spinifex/qmp"
+	spxtypes "github.com/mulgadc/spinifex/spinifex/types"
+	"github.com/mulgadc/spinifex/spinifex/utils"
+	"github.com/mulgadc/spinifex/spinifex/vm"
+	"github.com/nats-io/nats.go"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// --- eniHotplugErrorCode mapping ---
+
+// TestENIHotplugErrorCode locks down the vm.Manager error → AWS API code
+// mapping. Wrong mapping silently breaks AWS-SDK retry semantics: clients
+// expect 4xx codes for caller-fixable problems and 5xx for server faults.
+func TestENIHotplugErrorCode(t *testing.T) {
+	tests := []struct {
+		name string
+		err  error
+		want string
+	}{
+		{"ErrInstanceNotFound", vm.ErrInstanceNotFound, awserrors.ErrorInvalidInstanceIDNotFound},
+		{"wrapped ErrInstanceNotFound", fmt.Errorf("ctx: %w", vm.ErrInstanceNotFound), awserrors.ErrorInvalidInstanceIDNotFound},
+		{"ErrInvalidTransition", vm.ErrInvalidTransition, awserrors.ErrorIncorrectInstanceState},
+		{"ErrAttachmentLimitExceeded", vm.ErrAttachmentLimitExceeded, awserrors.ErrorAttachmentLimitExceeded},
+		{"ErrENINotAttached", vm.ErrENINotAttached, awserrors.ErrorInvalidAttachmentIDNotFound},
+		{"wrapped ErrENINotAttached", fmt.Errorf("%w: eni-x", vm.ErrENINotAttached), awserrors.ErrorInvalidAttachmentIDNotFound},
+		{"ErrQMPUnavailable", vm.ErrQMPUnavailable, awserrors.ErrorServerInternal},
+		{"ErrENIPipelineTimeout", vm.ErrENIPipelineTimeout, awserrors.ErrorServerInternal},
+		{"unknown error", errors.New("network unreachable"), awserrors.ErrorServerInternal},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, tt.want, eniHotplugErrorCode(tt.err))
+		})
+	}
+}
+
+// --- publishENIHotplugEvent ---
+
+func TestPublishENIHotplugEvent_NilConnNoop(t *testing.T) {
+	publishENIHotplugEvent(nil, "vpc.eni-hotplug.attached", "i-x", map[string]any{"k": "v"})
+}
+
+func TestPublishENIHotplugEvent_Publishes(t *testing.T) {
+	d := createVPCTestDaemon(t)
+
+	got := make(chan *nats.Msg, 1)
+	sub, err := d.natsConn.SubscribeSync("vpc.eni-hotplug.attached.i-evt")
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = sub.Unsubscribe() })
+	go func() {
+		msg, _ := sub.NextMsg(2 * time.Second)
+		got <- msg
+	}()
+
+	publishENIHotplugEvent(d.natsConn, "vpc.eni-hotplug.attached", "i-evt", map[string]any{
+		"eniId": "eni-evt",
+		"slot":  1.0,
+	})
+
+	msg := <-got
+	require.NotNil(t, msg)
+	var payload map[string]any
+	require.NoError(t, json.Unmarshal(msg.Data, &payload))
+	assert.Equal(t, "eni-evt", payload["eniId"])
+	assert.Equal(t, 1.0, payload["slot"])
+}
+
+// --- shared fixture ---
+
+// eniHotPlugFixture builds a daemon with vpcService + a running VM
+// registered in vmMgr, plus a stub DeviceController bound via
+// vm.SetHotPlugTestSeams. The returned ENI record is already in the KV
+// in "available" state and the VM has 4 free hot-plug slots.
+type eniHotPlugFixture struct {
+	daemon   *Daemon
+	vmInst   *vm.VM
+	stub     *vm.StubDeviceController
+	subnetID string
+	vpcID    string
+	eniID    string
+	mac      string
+}
+
+func newENIHotPlugFixture(t *testing.T) *eniHotPlugFixture {
+	t.Helper()
+	d := createVPCTestDaemon(t)
+
+	vpcOut, err := d.vpcService.CreateVpc(&ec2.CreateVpcInput{
+		CidrBlock: aws.String("10.0.0.0/16"),
+	}, testAccountID)
+	require.NoError(t, err)
+	subnetOut, err := d.vpcService.CreateSubnet(&ec2.CreateSubnetInput{
+		VpcId:     vpcOut.Vpc.VpcId,
+		CidrBlock: aws.String("10.0.1.0/24"),
+	}, testAccountID)
+	require.NoError(t, err)
+	eniOut, err := d.vpcService.CreateNetworkInterface(&ec2.CreateNetworkInterfaceInput{
+		SubnetId: subnetOut.Subnet.SubnetId,
+	}, testAccountID)
+	require.NoError(t, err)
+
+	rec, err := d.vpcService.GetENIRecord(testAccountID, *eniOut.NetworkInterface.NetworkInterfaceId)
+	require.NoError(t, err)
+
+	v := &vm.VM{
+		ID:        "i-hp-test",
+		Status:    vm.StateRunning,
+		QMPClient: &qmp.QMPClient{},
+		ENIRequests: spxtypes.ENIRequests{
+			AvailableSlots:  []int{1, 2, 3, 4},
+			AttachedByENIID: map[string]int{},
+		},
+	}
+	d.vmMgr.Insert(v)
+
+	stub := vm.NewStubDeviceController()
+	restore := vm.SetHotPlugTestSeams(
+		func(*vm.VM) vm.DeviceController { return stub },
+		func(time.Duration) {},
+	)
+	t.Cleanup(restore)
+
+	return &eniHotPlugFixture{
+		daemon:   d,
+		vmInst:   v,
+		stub:     stub,
+		subnetID: *subnetOut.Subnet.SubnetId,
+		vpcID:    *vpcOut.Vpc.VpcId,
+		eniID:    rec.NetworkInterfaceId,
+		mac:      rec.MacAddress,
+	}
+}
+
+// driveHandler subscribes to subject, dispatches the supplied handler
+// from the subscriber, and returns the reply payload. Mirrors how the
+// daemon dispatches AttachENI / DetachENI off the ec2.cmd channel in
+// production.
+func driveHandler(t *testing.T, nc *nats.Conn, subject string, handler func(*nats.Msg)) []byte {
+	t.Helper()
+	sub, err := nc.Subscribe(subject, handler)
+	require.NoError(t, err)
+	defer func() { _ = sub.Unsubscribe() }()
+
+	reqMsg := nats.NewMsg(subject)
+	reqMsg.Header.Set(utils.AccountIDHeader, testAccountID)
+	reply, err := nc.RequestMsg(reqMsg, 5*time.Second)
+	require.NoError(t, err)
+	return reply.Data
+}
+
+// assertErrorCode is shared with daemon_handlers_eks_test.go (same package).
+
+// --- handleAttachNetworkInterface ---
+
+func TestHandleAttachNetworkInterface_NilData(t *testing.T) {
+	f := newENIHotPlugFixture(t)
+	cmd := spxtypes.EC2InstanceCommand{ID: f.vmInst.ID}
+	payload := driveHandler(t, f.daemon.natsConn, "test.attach.nildata", func(msg *nats.Msg) {
+		f.daemon.handleAttachNetworkInterface(msg, cmd, f.vmInst)
+	})
+	assertErrorCode(t, payload, awserrors.ErrorInvalidParameterValue)
+}
+
+func TestHandleAttachNetworkInterface_EmptyEniID(t *testing.T) {
+	f := newENIHotPlugFixture(t)
+	cmd := spxtypes.EC2InstanceCommand{
+		ID:            f.vmInst.ID,
+		AttachENIData: &spxtypes.AttachENIData{NetworkInterfaceID: "", DeviceIndex: 0},
+	}
+	payload := driveHandler(t, f.daemon.natsConn, "test.attach.emptyid", func(msg *nats.Msg) {
+		f.daemon.handleAttachNetworkInterface(msg, cmd, f.vmInst)
+	})
+	assertErrorCode(t, payload, awserrors.ErrorInvalidParameterValue)
+}
+
+func TestHandleAttachNetworkInterface_NotRunning(t *testing.T) {
+	f := newENIHotPlugFixture(t)
+	f.vmInst.Status = vm.StateStopped
+	cmd := spxtypes.EC2InstanceCommand{
+		ID:            f.vmInst.ID,
+		AttachENIData: &spxtypes.AttachENIData{NetworkInterfaceID: f.eniID, DeviceIndex: 1},
+	}
+	payload := driveHandler(t, f.daemon.natsConn, "test.attach.notrunning", func(msg *nats.Msg) {
+		f.daemon.handleAttachNetworkInterface(msg, cmd, f.vmInst)
+	})
+	assertErrorCode(t, payload, awserrors.ErrorIncorrectInstanceState)
+}
+
+func TestHandleAttachNetworkInterface_ENINotFound(t *testing.T) {
+	f := newENIHotPlugFixture(t)
+	cmd := spxtypes.EC2InstanceCommand{
+		ID:            f.vmInst.ID,
+		AttachENIData: &spxtypes.AttachENIData{NetworkInterfaceID: "eni-missing", DeviceIndex: 1},
+	}
+	payload := driveHandler(t, f.daemon.natsConn, "test.attach.eninotfound", func(msg *nats.Msg) {
+		f.daemon.handleAttachNetworkInterface(msg, cmd, f.vmInst)
+	})
+	assertErrorCode(t, payload, awserrors.ErrorInvalidNetworkInterfaceIDNotFound)
+}
+
+func TestHandleAttachNetworkInterface_WrongOwner(t *testing.T) {
+	f := newENIHotPlugFixture(t)
+	_, err := f.daemon.vpcService.AttachENI(testAccountID, f.eniID, "i-other-instance", 0)
+	require.NoError(t, err)
+
+	cmd := spxtypes.EC2InstanceCommand{
+		ID:            f.vmInst.ID,
+		AttachENIData: &spxtypes.AttachENIData{NetworkInterfaceID: f.eniID, DeviceIndex: 1},
+	}
+	payload := driveHandler(t, f.daemon.natsConn, "test.attach.wrongowner", func(msg *nats.Msg) {
+		f.daemon.handleAttachNetworkInterface(msg, cmd, f.vmInst)
+	})
+	assertErrorCode(t, payload, awserrors.ErrorInvalidNetworkInterfaceInUse)
+}
+
+func TestHandleAttachNetworkInterface_Success(t *testing.T) {
+	f := newENIHotPlugFixture(t)
+
+	gotEvent := make(chan []byte, 1)
+	evtSub, err := f.daemon.natsConn.Subscribe("vpc.eni-hotplug.attached."+f.vmInst.ID, func(msg *nats.Msg) {
+		gotEvent <- msg.Data
+	})
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = evtSub.Unsubscribe() })
+
+	cmd := spxtypes.EC2InstanceCommand{
+		ID:            f.vmInst.ID,
+		AttachENIData: &spxtypes.AttachENIData{NetworkInterfaceID: f.eniID, DeviceIndex: 1},
+	}
+	payload := driveHandler(t, f.daemon.natsConn, "test.attach.success", func(msg *nats.Msg) {
+		f.daemon.handleAttachNetworkInterface(msg, cmd, f.vmInst)
+	})
+
+	var out ec2.AttachNetworkInterfaceOutput
+	require.NoError(t, json.Unmarshal(payload, &out))
+	require.NotNil(t, out.AttachmentId)
+	assert.Contains(t, *out.AttachmentId, "eni-attach-")
+
+	rec, err := f.daemon.vpcService.GetENIRecord(testAccountID, f.eniID)
+	require.NoError(t, err)
+	assert.Equal(t, "attached", rec.AttachmentStatus)
+	assert.Equal(t, 1, rec.HotPlugSlot)
+	assert.Equal(t, f.vmInst.ID, rec.InstanceId)
+
+	assert.True(t, f.stub.HasDevice("net-eni-1"))
+	assert.True(t, f.stub.HasNetdev("hostnet-eni-1"))
+
+	select {
+	case evtData := <-gotEvent:
+		var evt map[string]any
+		require.NoError(t, json.Unmarshal(evtData, &evt))
+		assert.Equal(t, f.eniID, evt["eniId"])
+		assert.Equal(t, float64(1), evt["hotPlugSlot"])
+	case <-time.After(1 * time.Second):
+		t.Fatal("expected vpc.eni-hotplug.attached event, none received")
+	}
+}
+
+func TestHandleAttachNetworkInterface_HotPlugFails_RollsBackKV(t *testing.T) {
+	f := newENIHotPlugFixture(t)
+	f.stub.SetFailNext("device_add", errors.New("simulated QMP failure"))
+
+	cmd := spxtypes.EC2InstanceCommand{
+		ID:            f.vmInst.ID,
+		AttachENIData: &spxtypes.AttachENIData{NetworkInterfaceID: f.eniID, DeviceIndex: 1},
+	}
+	payload := driveHandler(t, f.daemon.natsConn, "test.attach.hotplugfails", func(msg *nats.Msg) {
+		f.daemon.handleAttachNetworkInterface(msg, cmd, f.vmInst)
+	})
+	assertErrorCode(t, payload, awserrors.ErrorServerInternal)
+
+	rec, err := f.daemon.vpcService.GetENIRecord(testAccountID, f.eniID)
+	require.NoError(t, err)
+	assert.Equal(t, "available", rec.Status, "KV should have rolled back to available")
+	assert.Empty(t, rec.AttachmentStatus, "AttachmentStatus should be cleared on rollback")
+	assert.Empty(t, rec.InstanceId)
+	assert.NotEmpty(t, rec.LastAttachError, "LastAttachError should record the rollback reason")
+}
+
+// --- handleDetachNetworkInterface ---
+
+// attachedFixture extends the base fixture by pre-attaching the ENI to
+// the running VM via the full handler, so detach tests start from the
+// realistic post-attach state.
+func attachedFixture(t *testing.T) (*eniHotPlugFixture, string) {
+	t.Helper()
+	f := newENIHotPlugFixture(t)
+	attachID, err := f.daemon.vpcService.AttachENI(testAccountID, f.eniID, f.vmInst.ID, 1)
+	require.NoError(t, err)
+	require.NoError(t, f.daemon.vpcService.UpdateENI(testAccountID, f.eniID, func(r *handlers_ec2_vpc.ENIRecord) {
+		r.AttachmentStatus = "attached"
+		r.HotPlugSlot = 1
+	}))
+	f.vmInst.ENIRequests.Mu.Lock()
+	f.vmInst.ENIRequests.AvailableSlots = []int{2, 3, 4}
+	f.vmInst.ENIRequests.AttachedByENIID[f.eniID] = 1
+	f.vmInst.ENIRequests.Mu.Unlock()
+	// Pre-seed the stub so HotUnplugENI's device_del + query-pci absence
+	// poll has something to remove.
+	require.NoError(t, f.stub.NetdevAdd(map[string]any{"id": "hostnet-eni-1"}))
+	require.NoError(t, f.stub.DeviceAdd(map[string]any{"id": "net-eni-1", "bus": "hotplug-eni1"}))
+	return f, attachID
+}
+
+func TestHandleDetachNetworkInterface_NilData(t *testing.T) {
+	f, _ := attachedFixture(t)
+	cmd := spxtypes.EC2InstanceCommand{ID: f.vmInst.ID}
+	payload := driveHandler(t, f.daemon.natsConn, "test.detach.nildata", func(msg *nats.Msg) {
+		f.daemon.handleDetachNetworkInterface(msg, cmd, f.vmInst)
+	})
+	assertErrorCode(t, payload, awserrors.ErrorInvalidParameterValue)
+}
+
+func TestHandleDetachNetworkInterface_EmptyAttachID(t *testing.T) {
+	f, _ := attachedFixture(t)
+	cmd := spxtypes.EC2InstanceCommand{
+		ID:            f.vmInst.ID,
+		DetachENIData: &spxtypes.DetachENIData{AttachmentID: ""},
+	}
+	payload := driveHandler(t, f.daemon.natsConn, "test.detach.emptyid", func(msg *nats.Msg) {
+		f.daemon.handleDetachNetworkInterface(msg, cmd, f.vmInst)
+	})
+	assertErrorCode(t, payload, awserrors.ErrorInvalidParameterValue)
+}
+
+func TestHandleDetachNetworkInterface_UnknownAttachment(t *testing.T) {
+	f, _ := attachedFixture(t)
+	cmd := spxtypes.EC2InstanceCommand{
+		ID:            f.vmInst.ID,
+		DetachENIData: &spxtypes.DetachENIData{AttachmentID: "eni-attach-missing"},
+	}
+	payload := driveHandler(t, f.daemon.natsConn, "test.detach.unknown", func(msg *nats.Msg) {
+		f.daemon.handleDetachNetworkInterface(msg, cmd, f.vmInst)
+	})
+	assertErrorCode(t, payload, awserrors.ErrorInvalidAttachmentIDNotFound)
+}
+
+func TestHandleDetachNetworkInterface_WrongOwner(t *testing.T) {
+	f, attachID := attachedFixture(t)
+	otherVM := &vm.VM{
+		ID:     "i-other",
+		Status: vm.StateRunning,
+		ENIRequests: spxtypes.ENIRequests{
+			AvailableSlots:  []int{1},
+			AttachedByENIID: map[string]int{},
+		},
+	}
+	f.daemon.vmMgr.Insert(otherVM)
+	cmd := spxtypes.EC2InstanceCommand{
+		ID:            otherVM.ID,
+		DetachENIData: &spxtypes.DetachENIData{AttachmentID: attachID},
+	}
+	payload := driveHandler(t, f.daemon.natsConn, "test.detach.wrongowner", func(msg *nats.Msg) {
+		f.daemon.handleDetachNetworkInterface(msg, cmd, otherVM)
+	})
+	assertErrorCode(t, payload, awserrors.ErrorInvalidAttachmentIDNotFound)
+}
+
+func TestHandleDetachNetworkInterface_NotRunning(t *testing.T) {
+	f, attachID := attachedFixture(t)
+	f.vmInst.Status = vm.StateStopped
+	cmd := spxtypes.EC2InstanceCommand{
+		ID:            f.vmInst.ID,
+		DetachENIData: &spxtypes.DetachENIData{AttachmentID: attachID},
+	}
+	payload := driveHandler(t, f.daemon.natsConn, "test.detach.notrunning", func(msg *nats.Msg) {
+		f.daemon.handleDetachNetworkInterface(msg, cmd, f.vmInst)
+	})
+	assertErrorCode(t, payload, awserrors.ErrorIncorrectInstanceState)
+}
+
+func TestHandleDetachNetworkInterface_Success(t *testing.T) {
+	f, attachID := attachedFixture(t)
+
+	gotEvent := make(chan []byte, 1)
+	evtSub, err := f.daemon.natsConn.Subscribe("vpc.eni-hotplug.detached."+f.vmInst.ID, func(msg *nats.Msg) {
+		gotEvent <- msg.Data
+	})
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = evtSub.Unsubscribe() })
+
+	cmd := spxtypes.EC2InstanceCommand{
+		ID:            f.vmInst.ID,
+		DetachENIData: &spxtypes.DetachENIData{AttachmentID: attachID, Force: false},
+	}
+	payload := driveHandler(t, f.daemon.natsConn, "test.detach.success", func(msg *nats.Msg) {
+		f.daemon.handleDetachNetworkInterface(msg, cmd, f.vmInst)
+	})
+	var out ec2.DetachNetworkInterfaceOutput
+	require.NoError(t, json.Unmarshal(payload, &out))
+
+	rec, err := f.daemon.vpcService.GetENIRecord(testAccountID, f.eniID)
+	require.NoError(t, err)
+	assert.Equal(t, "available", rec.Status)
+	assert.Empty(t, rec.AttachmentStatus)
+	assert.False(t, rec.DetachInFlight)
+	assert.False(t, rec.DetachForce)
+	assert.Equal(t, 0, rec.HotPlugSlot)
+
+	assert.False(t, f.stub.HasDevice("net-eni-1"))
+
+	select {
+	case <-gotEvent:
+	case <-time.After(1 * time.Second):
+		t.Fatal("expected vpc.eni-hotplug.detached event, none received")
+	}
+}
+
+func TestHandleDetachNetworkInterface_HotUnplugFails(t *testing.T) {
+	f, attachID := attachedFixture(t)
+	f.stub.SetFailNext("device_del", errors.New("simulated QMP failure"))
+
+	cmd := spxtypes.EC2InstanceCommand{
+		ID:            f.vmInst.ID,
+		DetachENIData: &spxtypes.DetachENIData{AttachmentID: attachID, Force: false},
+	}
+	payload := driveHandler(t, f.daemon.natsConn, "test.detach.hotunplugfails", func(msg *nats.Msg) {
+		f.daemon.handleDetachNetworkInterface(msg, cmd, f.vmInst)
+	})
+	assertErrorCode(t, payload, awserrors.ErrorServerInternal)
+
+	rec, err := f.daemon.vpcService.GetENIRecord(testAccountID, f.eniID)
+	require.NoError(t, err)
+	assert.Equal(t, "in-use", rec.Status, "KV should remain in-use after detach failure")
+	assert.Equal(t, "detaching", rec.AttachmentStatus, "AttachmentStatus stays detaching for reconciler pickup")
+	assert.False(t, rec.DetachInFlight, "DetachInFlight cleared so reconciler can re-claim")
+}

spinifex/gateway/ec2/vpc/network_interface_hotplug_test.go

@@ -0,0 +1,324 @@
+package gateway_ec2_vpc
+
+import (
+	"encoding/json"
+	"testing"
+	"time"
+
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/service/ec2"
+	"github.com/mulgadc/spinifex/spinifex/awserrors"
+	"github.com/mulgadc/spinifex/spinifex/types"
+	"github.com/mulgadc/spinifex/spinifex/utils"
+	"github.com/nats-io/nats-server/v2/server"
+	"github.com/nats-io/nats.go"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func newTestNATS(t *testing.T) *nats.Conn {
+	t.Helper()
+	opts := &server.Options{
+		Host:   "127.0.0.1",
+		Port:   -1,
+		NoLog:  true,
+		NoSigs: true,
+	}
+	ns, err := server.NewServer(opts)
+	require.NoError(t, err)
+	go ns.Start()
+	require.True(t, ns.ReadyForConnections(5*time.Second))
+	t.Cleanup(func() { ns.Shutdown() })
+
+	nc, err := nats.Connect(ns.ClientURL())
+	require.NoError(t, err)
+	t.Cleanup(func() { nc.Close() })
+	return nc
+}
+
+// describeNetworkInterfacesResponder seeds a DescribeNetworkInterfaces NATS
+// responder that returns the supplied attachment-id → instance-id mapping.
+func describeNetworkInterfacesResponder(t *testing.T, nc *nats.Conn, attachID, instanceID, eniID string) {
+	t.Helper()
+	sub, err := nc.Subscribe("ec2.DescribeNetworkInterfaces", func(msg *nats.Msg) {
+		resp := ec2.DescribeNetworkInterfacesOutput{
+			NetworkInterfaces: []*ec2.NetworkInterface{
+				{
+					NetworkInterfaceId: aws.String(eniID),
+					Attachment: &ec2.NetworkInterfaceAttachment{
+						AttachmentId: aws.String(attachID),
+						InstanceId:   aws.String(instanceID),
+					},
+				},
+			},
+		}
+		data, _ := json.Marshal(resp)
+		_ = msg.Respond(data)
+	})
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = sub.Unsubscribe() })
+}
+
+// --- AttachNetworkInterface ---
+
+func TestAttachNetworkInterface_NilInput(t *testing.T) {
+	_, err := AttachNetworkInterface(nil, nil, testAccountID)
+	assert.EqualError(t, err, awserrors.ErrorInvalidParameterValue)
+}
+
+func TestAttachNetworkInterface_MissingNetworkInterfaceID(t *testing.T) {
+	_, err := AttachNetworkInterface(&ec2.AttachNetworkInterfaceInput{
+		InstanceId:  aws.String("i-abc"),
+		DeviceIndex: aws.Int64(0),
+	}, nil, testAccountID)
+	assert.EqualError(t, err, awserrors.ErrorMissingParameter)
+}
+
+func TestAttachNetworkInterface_EmptyNetworkInterfaceID(t *testing.T) {
+	_, err := AttachNetworkInterface(&ec2.AttachNetworkInterfaceInput{
+		NetworkInterfaceId: aws.String(""),
+		InstanceId:         aws.String("i-abc"),
+		DeviceIndex:        aws.Int64(0),
+	}, nil, testAccountID)
+	assert.EqualError(t, err, awserrors.ErrorMissingParameter)
+}
+
+func TestAttachNetworkInterface_MissingInstanceID(t *testing.T) {
+	_, err := AttachNetworkInterface(&ec2.AttachNetworkInterfaceInput{
+		NetworkInterfaceId: aws.String("eni-1"),
+		DeviceIndex:        aws.Int64(0),
+	}, nil, testAccountID)
+	assert.EqualError(t, err, awserrors.ErrorMissingParameter)
+}
+
+func TestAttachNetworkInterface_MissingDeviceIndex(t *testing.T) {
+	_, err := AttachNetworkInterface(&ec2.AttachNetworkInterfaceInput{
+		NetworkInterfaceId: aws.String("eni-1"),
+		InstanceId:         aws.String("i-abc"),
+	}, nil, testAccountID)
+	assert.EqualError(t, err, awserrors.ErrorMissingParameter)
+}
+
+func TestAttachNetworkInterface_NoResponders(t *testing.T) {
+	nc := newTestNATS(t)
+	_, err := AttachNetworkInterface(&ec2.AttachNetworkInterfaceInput{
+		NetworkInterfaceId: aws.String("eni-1"),
+		InstanceId:         aws.String("i-no-responder"),
+		DeviceIndex:        aws.Int64(0),
+	}, nc, testAccountID)
+	assert.EqualError(t, err, awserrors.ErrorInvalidInstanceIDNotFound)
+}
+
+func TestAttachNetworkInterface_Success(t *testing.T) {
+	nc := newTestNATS(t)
+
+	var received types.EC2InstanceCommand
+	var receivedAccount string
+	sub, err := nc.Subscribe("ec2.cmd.i-success", func(msg *nats.Msg) {
+		receivedAccount = msg.Header.Get(utils.AccountIDHeader)
+		_ = json.Unmarshal(msg.Data, &received)
+		data, _ := json.Marshal(ec2.AttachNetworkInterfaceOutput{
+			AttachmentId: aws.String("eni-attach-xyz"),
+		})
+		_ = msg.Respond(data)
+	})
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = sub.Unsubscribe() })
+
+	out, err := AttachNetworkInterface(&ec2.AttachNetworkInterfaceInput{
+		NetworkInterfaceId: aws.String("eni-feedface"),
+		InstanceId:         aws.String("i-success"),
+		DeviceIndex:        aws.Int64(2),
+	}, nc, testAccountID)
+
+	require.NoError(t, err)
+	require.NotNil(t, out.AttachmentId)
+	assert.Equal(t, "eni-attach-xyz", *out.AttachmentId)
+	assert.Equal(t, testAccountID, receivedAccount)
+	assert.True(t, received.Attributes.AttachENI)
+	require.NotNil(t, received.AttachENIData)
+	assert.Equal(t, "eni-feedface", received.AttachENIData.NetworkInterfaceID)
+	assert.Equal(t, int64(2), received.AttachENIData.DeviceIndex)
+	assert.Equal(t, "i-success", received.ID)
+}
+
+func TestAttachNetworkInterface_DaemonErrorResponse(t *testing.T) {
+	nc := newTestNATS(t)
+
+	sub, err := nc.Subscribe("ec2.cmd.i-err", func(msg *nats.Msg) {
+		_ = msg.Respond(utils.GenerateErrorPayload(awserrors.ErrorIncorrectInstanceState))
+	})
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = sub.Unsubscribe() })
+
+	_, err = AttachNetworkInterface(&ec2.AttachNetworkInterfaceInput{
+		NetworkInterfaceId: aws.String("eni-bad"),
+		InstanceId:         aws.String("i-err"),
+		DeviceIndex:        aws.Int64(0),
+	}, nc, testAccountID)
+	assert.EqualError(t, err, awserrors.ErrorIncorrectInstanceState)
+}
+
+func TestAttachNetworkInterface_MalformedDaemonResponse(t *testing.T) {
+	nc := newTestNATS(t)
+
+	sub, err := nc.Subscribe("ec2.cmd.i-malformed", func(msg *nats.Msg) {
+		_ = msg.Respond([]byte("not-json"))
+	})
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = sub.Unsubscribe() })
+
+	_, err = AttachNetworkInterface(&ec2.AttachNetworkInterfaceInput{
+		NetworkInterfaceId: aws.String("eni-bad"),
+		InstanceId:         aws.String("i-malformed"),
+		DeviceIndex:        aws.Int64(0),
+	}, nc, testAccountID)
+	assert.EqualError(t, err, awserrors.ErrorServerInternal)
+}
+
+// --- DetachNetworkInterface ---
+
+func TestDetachNetworkInterface_NilInput(t *testing.T) {
+	_, err := DetachNetworkInterface(nil, nil, testAccountID)
+	assert.EqualError(t, err, awserrors.ErrorInvalidParameterValue)
+}
+
+func TestDetachNetworkInterface_MissingAttachmentID(t *testing.T) {
+	_, err := DetachNetworkInterface(&ec2.DetachNetworkInterfaceInput{}, nil, testAccountID)
+	assert.EqualError(t, err, awserrors.ErrorMissingParameter)
+}
+
+func TestDetachNetworkInterface_EmptyAttachmentID(t *testing.T) {
+	_, err := DetachNetworkInterface(&ec2.DetachNetworkInterfaceInput{
+		AttachmentId: aws.String(""),
+	}, nil, testAccountID)
+	assert.EqualError(t, err, awserrors.ErrorMissingParameter)
+}
+
+func TestDetachNetworkInterface_UnknownAttachment(t *testing.T) {
+	nc := newTestNATS(t)
+
+	sub, err := nc.Subscribe("ec2.DescribeNetworkInterfaces", func(msg *nats.Msg) {
+		data, _ := json.Marshal(ec2.DescribeNetworkInterfacesOutput{})
+		_ = msg.Respond(data)
+	})
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = sub.Unsubscribe() })
+
+	_, err = DetachNetworkInterface(&ec2.DetachNetworkInterfaceInput{
+		AttachmentId: aws.String("eni-attach-missing"),
+	}, nc, testAccountID)
+	assert.EqualError(t, err, awserrors.ErrorInvalidAttachmentIDNotFound)
+}
+
+func TestDetachNetworkInterface_DescribeErrorResponse(t *testing.T) {
+	nc := newTestNATS(t)
+
+	sub, err := nc.Subscribe("ec2.DescribeNetworkInterfaces", func(msg *nats.Msg) {
+		_ = msg.Respond(utils.GenerateErrorPayload(awserrors.ErrorServerInternal))
+	})
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = sub.Unsubscribe() })
+
+	_, err = DetachNetworkInterface(&ec2.DetachNetworkInterfaceInput{
+		AttachmentId: aws.String("eni-attach-1"),
+	}, nc, testAccountID)
+	assert.EqualError(t, err, awserrors.ErrorInvalidAttachmentIDNotFound)
+}
+
+func TestDetachNetworkInterface_AttachmentWithoutInstance(t *testing.T) {
+	nc := newTestNATS(t)
+
+	sub, err := nc.Subscribe("ec2.DescribeNetworkInterfaces", func(msg *nats.Msg) {
+		resp := ec2.DescribeNetworkInterfacesOutput{
+			NetworkInterfaces: []*ec2.NetworkInterface{
+				{
+					NetworkInterfaceId: aws.String("eni-orphan"),
+					Attachment: &ec2.NetworkInterfaceAttachment{
+						AttachmentId: aws.String("eni-attach-orphan"),
+					},
+				},
+			},
+		}
+		data, _ := json.Marshal(resp)
+		_ = msg.Respond(data)
+	})
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = sub.Unsubscribe() })
+
+	_, err = DetachNetworkInterface(&ec2.DetachNetworkInterfaceInput{
+		AttachmentId: aws.String("eni-attach-orphan"),
+	}, nc, testAccountID)
+	assert.EqualError(t, err, awserrors.ErrorInvalidAttachmentIDNotFound)
+}
+
+func TestDetachNetworkInterface_NoResponders(t *testing.T) {
+	nc := newTestNATS(t)
+	describeNetworkInterfacesResponder(t, nc, "eni-attach-1", "i-no-daemon", "eni-1")
+
+	_, err := DetachNetworkInterface(&ec2.DetachNetworkInterfaceInput{
+		AttachmentId: aws.String("eni-attach-1"),
+	}, nc, testAccountID)
+	assert.EqualError(t, err, awserrors.ErrorInvalidInstanceIDNotFound)
+}
+
+func TestDetachNetworkInterface_Success(t *testing.T) {
+	nc := newTestNATS(t)
+	describeNetworkInterfacesResponder(t, nc, "eni-attach-ok", "i-detach", "eni-detachable")
+
+	var received types.EC2InstanceCommand
+	var receivedAccount string
+	sub, err := nc.Subscribe("ec2.cmd.i-detach", func(msg *nats.Msg) {
+		receivedAccount = msg.Header.Get(utils.AccountIDHeader)
+		_ = json.Unmarshal(msg.Data, &received)
+		data, _ := json.Marshal(ec2.DetachNetworkInterfaceOutput{})
+		_ = msg.Respond(data)
+	})
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = sub.Unsubscribe() })
+
+	_, err = DetachNetworkInterface(&ec2.DetachNetworkInterfaceInput{
+		AttachmentId: aws.String("eni-attach-ok"),
+		Force:        aws.Bool(true),
+	}, nc, testAccountID)
+	require.NoError(t, err)
+
+	assert.Equal(t, testAccountID, receivedAccount)
+	assert.True(t, received.Attributes.DetachENI)
+	require.NotNil(t, received.DetachENIData)
+	assert.Equal(t, "eni-attach-ok", received.DetachENIData.AttachmentID)
+	assert.True(t, received.DetachENIData.Force)
+	assert.Equal(t, "i-detach", received.ID)
+}
+
+func TestDetachNetworkInterface_DaemonErrorResponse(t *testing.T) {
+	nc := newTestNATS(t)
+	describeNetworkInterfacesResponder(t, nc, "eni-attach-err", "i-err", "eni-x")
+
+	sub, err := nc.Subscribe("ec2.cmd.i-err", func(msg *nats.Msg) {
+		_ = msg.Respond(utils.GenerateErrorPayload(awserrors.ErrorIncorrectInstanceState))
+	})
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = sub.Unsubscribe() })
+
+	_, err = DetachNetworkInterface(&ec2.DetachNetworkInterfaceInput{
+		AttachmentId: aws.String("eni-attach-err"),
+	}, nc, testAccountID)
+	assert.EqualError(t, err, awserrors.ErrorIncorrectInstanceState)
+}
+
+func TestDetachNetworkInterface_MalformedDaemonResponse(t *testing.T) {
+	nc := newTestNATS(t)
+	describeNetworkInterfacesResponder(t, nc, "eni-attach-bad", "i-bad", "eni-bad")
+
+	sub, err := nc.Subscribe("ec2.cmd.i-bad", func(msg *nats.Msg) {
+		_ = msg.Respond([]byte("not-json"))
+	})
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = sub.Unsubscribe() })
+
+	_, err = DetachNetworkInterface(&ec2.DetachNetworkInterfaceInput{
+		AttachmentId: aws.String("eni-attach-bad"),
+	}, nc, testAccountID)
+	assert.EqualError(t, err, awserrors.ErrorServerInternal)
+}

spinifex/handlers/ec2/vpc/eni_hotplug_helpers_test.go

@@ -0,0 +1,163 @@
+package handlers_ec2_vpc
+
+import (
+	"testing"
+
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/service/ec2"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestGetENIRecord(t *testing.T) {
+	svc := setupTestVPCService(t)
+	vpcId := createTestVPC(t, svc, "10.0.0.0/16")
+	subnetId := createTestSubnet(t, svc, vpcId, "10.0.1.0/24")
+	eniId := createTestENI(t, svc, subnetId)
+
+	rec, err := svc.GetENIRecord(testAccountID, eniId)
+	require.NoError(t, err)
+	assert.Equal(t, eniId, rec.NetworkInterfaceId)
+	assert.Equal(t, subnetId, rec.SubnetId)
+	assert.Equal(t, "available", rec.Status)
+	assert.NotEmpty(t, rec.MacAddress)
+}
+
+func TestGetENIRecord_NotFound(t *testing.T) {
+	svc := setupTestVPCService(t)
+	_, err := svc.GetENIRecord(testAccountID, "eni-nonexistent")
+	assert.ErrorContains(t, err, "InvalidNetworkInterfaceID.NotFound")
+}
+
+func TestUpdateENI_PatchesFields(t *testing.T) {
+	svc := setupTestVPCService(t)
+	vpcId := createTestVPC(t, svc, "10.0.0.0/16")
+	subnetId := createTestSubnet(t, svc, vpcId, "10.0.1.0/24")
+	eniId := createTestENI(t, svc, subnetId)
+
+	err := svc.UpdateENI(testAccountID, eniId, func(r *ENIRecord) {
+		r.AttachmentStatus = "attaching"
+		r.HotPlugSlot = 3
+		r.LastAttachError = "prior failure"
+	})
+	require.NoError(t, err)
+
+	rec, err := svc.GetENIRecord(testAccountID, eniId)
+	require.NoError(t, err)
+	assert.Equal(t, "attaching", rec.AttachmentStatus)
+	assert.Equal(t, 3, rec.HotPlugSlot)
+	assert.Equal(t, "prior failure", rec.LastAttachError)
+}
+
+func TestUpdateENI_NotFound(t *testing.T) {
+	svc := setupTestVPCService(t)
+	err := svc.UpdateENI(testAccountID, "eni-nonexistent", func(r *ENIRecord) {
+		r.AttachmentStatus = "attached"
+	})
+	assert.ErrorContains(t, err, "InvalidNetworkInterfaceID.NotFound")
+}
+
+func TestUpdateENI_NoopPatchPersists(t *testing.T) {
+	svc := setupTestVPCService(t)
+	vpcId := createTestVPC(t, svc, "10.0.0.0/16")
+	subnetId := createTestSubnet(t, svc, vpcId, "10.0.1.0/24")
+	eniId := createTestENI(t, svc, subnetId)
+
+	err := svc.UpdateENI(testAccountID, eniId, func(*ENIRecord) {})
+	require.NoError(t, err)
+
+	rec, err := svc.GetENIRecord(testAccountID, eniId)
+	require.NoError(t, err)
+	assert.Equal(t, eniId, rec.NetworkInterfaceId)
+}
+
+func TestFindENIByAttachment(t *testing.T) {
+	svc := setupTestVPCService(t)
+	vpcId := createTestVPC(t, svc, "10.0.0.0/16")
+	subnetId := createTestSubnet(t, svc, vpcId, "10.0.1.0/24")
+	eniId := createTestENI(t, svc, subnetId)
+
+	attachId, err := svc.AttachENI(testAccountID, eniId, "i-find-test", 2)
+	require.NoError(t, err)
+
+	rec, err := svc.FindENIByAttachment(testAccountID, attachId)
+	require.NoError(t, err)
+	assert.Equal(t, eniId, rec.NetworkInterfaceId)
+	assert.Equal(t, "i-find-test", rec.InstanceId)
+	assert.Equal(t, int64(2), rec.DeviceIndex)
+}
+
+func TestFindENIByAttachment_NotFound(t *testing.T) {
+	svc := setupTestVPCService(t)
+	_, err := svc.FindENIByAttachment(testAccountID, "eni-attach-missing")
+	assert.ErrorContains(t, err, "InvalidAttachmentID.NotFound")
+}
+
+func TestFindENIByAttachment_EmptyBucket(t *testing.T) {
+	svc := setupTestVPCService(t)
+	_, err := svc.FindENIByAttachment(testAccountID, "eni-attach-anything")
+	assert.ErrorContains(t, err, "InvalidAttachmentID.NotFound")
+}
+
+func TestFindENIByAttachment_AccountScoped(t *testing.T) {
+	svc := setupTestVPCService(t)
+	vpcId := createTestVPC(t, svc, "10.0.0.0/16")
+	subnetId := createTestSubnet(t, svc, vpcId, "10.0.1.0/24")
+	eniId := createTestENI(t, svc, subnetId)
+
+	attachId, err := svc.AttachENI(testAccountID, eniId, "i-scoped", 0)
+	require.NoError(t, err)
+
+	_, err = svc.FindENIByAttachment("999988887777", attachId)
+	assert.ErrorContains(t, err, "InvalidAttachmentID.NotFound")
+}
+
+// TestUpdateENI_RoundTripsAllNewFields exercises every Sprint 3b field on
+// ENIRecord so a struct-field rename would surface as a test failure rather
+// than a silent JSON-tag drift.
+func TestUpdateENI_RoundTripsAllNewFields(t *testing.T) {
+	svc := setupTestVPCService(t)
+	vpcId := createTestVPC(t, svc, "10.0.0.0/16")
+	subnetId := createTestSubnet(t, svc, vpcId, "10.0.1.0/24")
+	eniId := createTestENI(t, svc, subnetId)
+
+	require.NoError(t, svc.UpdateENI(testAccountID, eniId, func(r *ENIRecord) {
+		r.AttachmentStatus = "detaching"
+		r.HotPlugSlot = 7
+		r.LastAttachError = "boom"
+		r.DetachInFlight = true
+		r.DetachForce = true
+	}))
+
+	rec, err := svc.GetENIRecord(testAccountID, eniId)
+	require.NoError(t, err)
+	assert.Equal(t, "detaching", rec.AttachmentStatus)
+	assert.Equal(t, 7, rec.HotPlugSlot)
+	assert.Equal(t, "boom", rec.LastAttachError)
+	assert.True(t, rec.DetachInFlight)
+	assert.True(t, rec.DetachForce)
+}
+
+// TestUpdateENI_PreservesUnrelatedFields ensures a partial patch does not
+// clobber pre-existing record state.
+func TestUpdateENI_PreservesUnrelatedFields(t *testing.T) {
+	svc := setupTestVPCService(t)
+	vpcId := createTestVPC(t, svc, "10.0.0.0/16")
+	subnetId := createTestSubnet(t, svc, vpcId, "10.0.1.0/24")
+	out, err := svc.CreateNetworkInterface(&ec2.CreateNetworkInterfaceInput{
+		SubnetId:    aws.String(subnetId),
+		Description: aws.String("preserved-desc"),
+	}, testAccountID)
+	require.NoError(t, err)
+	eniId := *out.NetworkInterface.NetworkInterfaceId
+
+	require.NoError(t, svc.UpdateENI(testAccountID, eniId, func(r *ENIRecord) {
+		r.AttachmentStatus = "attaching"
+	}))
+
+	rec, err := svc.GetENIRecord(testAccountID, eniId)
+	require.NoError(t, err)
+	assert.Equal(t, "preserved-desc", rec.Description)
+	assert.Equal(t, subnetId, rec.SubnetId)
+	assert.NotEmpty(t, rec.MacAddress)
+}

spinifex/vm/eni_hotplug.go

@@ -41,10 +41,10 @@ type HotPlugENIResult struct {
 
 // HotPlugENI runs the 4-step attach pipeline against the live VM:
 //
-//  1. tap device creation     (placeholder — wired in Sprint 3c)
-//  2. ovs-vsctl add-port      (placeholder — wired in Sprint 3c)
-//  3. QMP netdev_add          (real)
-//  4. QMP device_add + poll   (real)
+//  1. tap device + OVS port   (NetworkPlumber.SetupTap — atomic tap+br-int)
+//  2. QMP netdev_add
+//  3. QMP device_add
+//  4. poll query-pci until materialized
 //
 // On step-N failure, steps 1..N-1 are rolled back in reverse order and
 // the slot is returned to ENIRequests.AvailableSlots. The instance must
@@ -92,22 +92,13 @@ func (m *Manager) HotPlugENI(instance *VM, eniID, mac string) (HotPlugENIResult,
 	tapName := TapDeviceName(eniID)
 	busID := fmt.Sprintf("hotplug-eni%d", slot)
 
-	// Step 1: tap device. Sprint 3c replaces this stub with a real call
-	// to NetworkPlumber (ip tuntap add).
-	if err := stubTapAdd(instance.ID, eniID, tapName); err != nil {
+	// Step 1: tap device + OVS port on br-int (carries OVN iface-id binding).
+	if err := m.setupENITap(instance.ID, eniID, mac); err != nil {
 		m.releaseSlotLocked(instance, eniID, slot)
-		return HotPlugENIResult{}, fmt.Errorf("tap add (stub): %w", err)
+		return HotPlugENIResult{}, fmt.Errorf("tap/ovs setup: %w", err)
 	}
 
-	// Step 2: OVS port. Sprint 3c replaces this stub with the real
-	// ovs-vsctl add-port invocation against br-int.
-	if err := stubOVSAddPort(instance.ID, eniID, tapName, mac); err != nil {
-		_ = stubTapDel(instance.ID, eniID, tapName)
-		m.releaseSlotLocked(instance, eniID, slot)
-		return HotPlugENIResult{}, fmt.Errorf("ovs add-port (stub): %w", err)
-	}
-
-	// Step 3: QMP netdev_add.
+	// Step 2: QMP netdev_add.
 	if err := dc.NetdevAdd(map[string]any{
 		"type":       "tap",
 		"id":         netdevID,
@@ -115,13 +106,12 @@ func (m *Manager) HotPlugENI(instance *VM, eniID, mac string) (HotPlugENIResult,
 		"script":     "no",
 		"downscript": "no",
 	}); err != nil {
-		_ = stubOVSDelPort(instance.ID, eniID, tapName)
-		_ = stubTapDel(instance.ID, eniID, tapName)
+		m.cleanupENITap(instance.ID, eniID, tapName)
 		m.releaseSlotLocked(instance, eniID, slot)
 		return HotPlugENIResult{}, fmt.Errorf("QMP netdev_add: %w", err)
 	}
 
-	// Step 4: QMP device_add then poll query-pci until materialized.
+	// Step 3: QMP device_add.
 	if err := dc.DeviceAdd(map[string]any{
 		"driver": "virtio-net-pci",
 		"id":     deviceID,
@@ -130,17 +120,16 @@ func (m *Manager) HotPlugENI(instance *VM, eniID, mac string) (HotPlugENIResult,
 		"mac":    mac,
 	}); err != nil {
 		_ = dc.NetdevDel(netdevID)
-		_ = stubOVSDelPort(instance.ID, eniID, tapName)
-		_ = stubTapDel(instance.ID, eniID, tapName)
+		m.cleanupENITap(instance.ID, eniID, tapName)
 		m.releaseSlotLocked(instance, eniID, slot)
 		return HotPlugENIResult{}, fmt.Errorf("QMP device_add: %w", err)
 	}
 
+	// Step 4: poll query-pci until the guest materializes the device.
 	if err := waitForPCIDevice(dc, deviceID, true); err != nil {
 		_ = dc.DeviceDel(deviceID)
 		_ = dc.NetdevDel(netdevID)
-		_ = stubOVSDelPort(instance.ID, eniID, tapName)
-		_ = stubTapDel(instance.ID, eniID, tapName)
+		m.cleanupENITap(instance.ID, eniID, tapName)
 		m.releaseSlotLocked(instance, eniID, slot)
 		return HotPlugENIResult{}, fmt.Errorf("query-pci wait (attach): %w", err)
 	}
@@ -204,9 +193,8 @@ func (m *Manager) HotUnplugENI(instance *VM, eniID string, force bool) error {
 			"instanceId", instance.ID, "eniId", eniID, "err", err)
 	}
 
-	// Step 4: OVS port + tap (stubs, real wiring in Sprint 3c).
-	_ = stubOVSDelPort(instance.ID, eniID, tapName)
-	_ = stubTapDel(instance.ID, eniID, tapName)
+	// Step 4: OVS port + tap teardown (idempotent).
+	m.cleanupENITap(instance.ID, eniID, tapName)
 
 	delete(instance.ENIRequests.AttachedByENIID, eniID)
 	instance.ENIRequests.AvailableSlots = append(instance.ENIRequests.AvailableSlots, slot)
@@ -259,32 +247,51 @@ func waitForPCIDevice(dc DeviceController, deviceID string, wantPresent bool) er
 // override it to drive poll cadence without burning wall-clock time.
 var eniPipelineSleep = time.Sleep
 
-// stubTapAdd / stubTapDel / stubOVSAddPort / stubOVSDelPort emit a debug
-// log and return nil. Sprint 3c replaces these with real
-// NetworkPlumber.SetupTap / TeardownTap + ovs-vsctl invocations. The
-// pipeline structure and rollback semantics are stable in 3b so 3c is a
-// drop-in replacement.
+// SetHotPlugTestSeams swaps the device-controller factory and the
+// query-pci poll sleep for the duration of a test, returning a restore
+// func that callers pass to t.Cleanup. Mirrors utils.SetSudoCommandForTest:
+// the indirection lets tests in other packages drive the hot-plug
+// pipeline against a StubDeviceController without reassigning
+// unexported vars (which the reassign linter forbids). Pass nil for
+// either argument to leave the corresponding seam untouched.
+func SetHotPlugTestSeams(factory func(*VM) DeviceController, sleep func(time.Duration)) (restore func()) {
+	origFactory := newDeviceController
+	origSleep := eniPipelineSleep
+	if factory != nil {
+		newDeviceController = factory
+	}
+	if sleep != nil {
+		eniPipelineSleep = sleep
+	}
+	return func() {
+		newDeviceController = origFactory
+		eniPipelineSleep = origSleep
+	}
+}
 
-func stubTapAdd(instanceID, eniID, tapName string) error {
-	slog.Debug("ENI hot-plug step 1 (tap add) — stubbed in Sprint 3b",
-		"instanceId", instanceID, "eniId", eniID, "tap", tapName)
+// setupENITap creates the tap on br-int and wires the OVS port carrying the
+// OVN binding (iface-id) and attached MAC, via the shared NetworkPlumber. A
+// nil plumber (unit-test managers) is a noop, mirroring setupExtraENINICs.
+func (m *Manager) setupENITap(instanceID, eniID, mac string) error {
+	if m.deps.NetworkPlumber == nil {
+		return nil
+	}
+	spec := VPCTapSpec(eniID, mac)
+	if err := m.deps.NetworkPlumber.SetupTap(spec); err != nil {
+		return fmt.Errorf("setup tap %s for eni %s on instance %s: %w", spec.Name, eniID, instanceID, err)
+	}
 	return nil
 }
 
-func stubTapDel(instanceID, eniID, tapName string) error {
-	slog.Debug("ENI hot-plug rollback (tap del) — stubbed in Sprint 3b",
-		"instanceId", instanceID, "eniId", eniID, "tap", tapName)
-	return nil
-}
-
-func stubOVSAddPort(instanceID, eniID, tapName, mac string) error {
-	slog.Debug("ENI hot-plug step 2 (ovs add-port) — stubbed in Sprint 3b",
-		"instanceId", instanceID, "eniId", eniID, "tap", tapName, "mac", mac)
-	return nil
-}
-
-func stubOVSDelPort(instanceID, eniID, tapName string) error {
-	slog.Debug("ENI hot-plug rollback (ovs del-port) — stubbed in Sprint 3b",
-		"instanceId", instanceID, "eniId", eniID, "tap", tapName)
-	return nil
+// cleanupENITap removes the tap and its OVS port. Idempotent (CleanupTap
+// tolerates already-absent state); a nil plumber is a noop. Failures are
+// logged, not returned — rollback/detach must make best-effort progress.
+func (m *Manager) cleanupENITap(instanceID, eniID, tapName string) {
+	if m.deps.NetworkPlumber == nil {
+		return
+	}
+	if err := m.deps.NetworkPlumber.CleanupTap(tapName); err != nil {
+		slog.Warn("ENI hot-plug tap cleanup failed",
+			"instanceId", instanceID, "eniId", eniID, "tap", tapName, "err", err)
+	}
 }

spinifex/vm/eni_hotplug_test.go

@@ -3,6 +3,7 @@ package vm
 import (
 	"errors"
 	"fmt"
+	"reflect"
 	"slices"
 	"strings"
 	"sync"
@@ -19,7 +20,16 @@ import (
 // test (cleanup restores the production factory).
 func newHotPlugTestVM(t *testing.T, slots int) (*Manager, *VM, *StubDeviceController) {
 	t.Helper()
-	mgr := NewManager()
+	mgr, v, stub, _ := newHotPlugTestVMWithPlumber(t, slots)
+	return mgr, v, stub
+}
+
+// newHotPlugTestVMWithPlumber is the variant that exposes the fake
+// NetworkPlumber so 3c tests can assert tap/OVS SetupTap / CleanupTap calls.
+func newHotPlugTestVMWithPlumber(t *testing.T, slots int) (*Manager, *VM, *StubDeviceController, *fakeNetworkPlumber) {
+	t.Helper()
+	plumber := &fakeNetworkPlumber{}
+	mgr := NewManagerWithDeps(Deps{NetworkPlumber: plumber})
 	stub := NewStubDeviceController()
 	available := make([]int, 0, slots)
 	for i := 1; i <= slots; i++ {
@@ -44,7 +54,7 @@ func newHotPlugTestVM(t *testing.T, slots int) (*Manager, *VM, *StubDeviceContro
 		newDeviceController = origFactory
 		eniPipelineSleep = origSleep
 	})
-	return mgr, v, stub
+	return mgr, v, stub, plumber
 }
 
 func TestHotPlugENI_SuccessPath(t *testing.T) {
@@ -296,6 +306,68 @@ func TestHotPlugENI_ConcurrentAttachesSerialize(t *testing.T) {
 	}
 }
 
+func TestHotPlugENI_WiresTapOnBrInt(t *testing.T) {
+	mgr, v, _, plumber := newHotPlugTestVMWithPlumber(t, 2)
+
+	if _, err := mgr.HotPlugENI(v, "eni-abc123", "02:00:00:aa:bb:cc"); err != nil {
+		t.Fatalf("HotPlugENI: %v", err)
+	}
+	if len(plumber.setupCalls) != 1 {
+		t.Fatalf("SetupTap calls = %d, want 1", len(plumber.setupCalls))
+	}
+	got := plumber.setupCalls[0]
+	want := VPCTapSpec("eni-abc123", "02:00:00:aa:bb:cc")
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("SetupTap spec = %+v, want %+v", got, want)
+	}
+	if len(plumber.cleanupCalls) != 0 {
+		t.Errorf("CleanupTap called %d times on success, want 0", len(plumber.cleanupCalls))
+	}
+}
+
+func TestHotPlugENI_TapSetupFailureAbortsBeforeQMP(t *testing.T) {
+	mgr, v, stub, plumber := newHotPlugTestVMWithPlumber(t, 1)
+	plumber.setupErr = errors.New("ovs add-port boom")
+
+	_, err := mgr.HotPlugENI(v, "eni-abc123", "02:00:00:aa:bb:cc")
+	if err == nil || !strings.Contains(err.Error(), "tap/ovs setup") {
+		t.Fatalf("err = %v, want tap/ovs setup failure", err)
+	}
+	if len(stub.Calls()) != 0 {
+		t.Errorf("QMP calls issued after tap setup failure: %v", stub.Calls())
+	}
+	if !containsSlot(v.ENIRequests.AvailableSlots, 1) {
+		t.Errorf("slot 1 not returned to free-list: %v", v.ENIRequests.AvailableSlots)
+	}
+}
+
+func TestHotPlugENI_QMPFailureCleansUpTap(t *testing.T) {
+	mgr, v, stub, plumber := newHotPlugTestVMWithPlumber(t, 1)
+	stub.SetFailNext("netdev_add", errors.New("netdev boom"))
+
+	if _, err := mgr.HotPlugENI(v, "eni-abc123", "02:00:00:aa:bb:cc"); err == nil {
+		t.Fatalf("HotPlugENI: want error, got nil")
+	}
+	wantTap := TapDeviceName("eni-abc123")
+	if len(plumber.cleanupCalls) != 1 || plumber.cleanupCalls[0] != wantTap {
+		t.Errorf("CleanupTap calls = %v, want [%s]", plumber.cleanupCalls, wantTap)
+	}
+}
+
+func TestHotUnplugENI_CleansUpTap(t *testing.T) {
+	mgr, v, _, plumber := newHotPlugTestVMWithPlumber(t, 1)
+	if _, err := mgr.HotPlugENI(v, "eni-abc123", "02:00:00:aa:bb:cc"); err != nil {
+		t.Fatalf("attach: %v", err)
+	}
+	if err := mgr.HotUnplugENI(v, "eni-abc123", false); err != nil {
+		t.Fatalf("detach: %v", err)
+	}
+	wantTap := TapDeviceName("eni-abc123")
+	if len(plumber.cleanupCalls) != 1 || plumber.cleanupCalls[0] != wantTap {
+		t.Errorf("CleanupTap calls = %v, want [%s]", plumber.cleanupCalls, wantTap)
+	}
+}
+
 func containsSlot(slots []int, s int) bool {
 	return slices.Contains(slots, s)
 }