diff --git a/internal/controller/network/mocks/mock_lcow_network.go b/internal/controller/network/mocks/mock_lcow_network.go new file mode 100644 index 0000000000..34c2c9e559 --- /dev/null +++ b/internal/controller/network/mocks/mock_lcow_network.go @@ -0,0 +1,164 @@ +//go:build windows && lcow + +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/Microsoft/hcsshim/internal/controller/network (interfaces: vmNetworkManager,capabilitiesProvider,guestNetwork) +// +// Generated by this command: +// +// mockgen -build_flags=-tags=windows,lcow -build_constraint=windows&&lcow -package mocks -destination mocks/mock_lcow_network.go github.com/Microsoft/hcsshim/internal/controller/network vmNetworkManager,capabilitiesProvider,guestNetwork +// + +// Package mocks is a generated GoMock package. +package mocks + +import ( + context "context" + reflect "reflect" + + gcs "github.com/Microsoft/hcsshim/internal/gcs" + hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" + guestresource "github.com/Microsoft/hcsshim/internal/protocol/guestresource" + gomock "go.uber.org/mock/gomock" +) + +// MockvmNetworkManager is a mock of vmNetworkManager interface. +type MockvmNetworkManager struct { + ctrl *gomock.Controller + recorder *MockvmNetworkManagerMockRecorder + isgomock struct{} +} + +// MockvmNetworkManagerMockRecorder is the mock recorder for MockvmNetworkManager. +type MockvmNetworkManagerMockRecorder struct { + mock *MockvmNetworkManager +} + +// NewMockvmNetworkManager creates a new mock instance. +func NewMockvmNetworkManager(ctrl *gomock.Controller) *MockvmNetworkManager { + mock := &MockvmNetworkManager{ctrl: ctrl} + mock.recorder = &MockvmNetworkManagerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockvmNetworkManager) EXPECT() *MockvmNetworkManagerMockRecorder { + return m.recorder +} + +// AddNIC mocks base method. +func (m *MockvmNetworkManager) AddNIC(ctx context.Context, nicID string, settings *hcsschema.NetworkAdapter) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AddNIC", ctx, nicID, settings) + ret0, _ := ret[0].(error) + return ret0 +} + +// AddNIC indicates an expected call of AddNIC. +func (mr *MockvmNetworkManagerMockRecorder) AddNIC(ctx, nicID, settings any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddNIC", reflect.TypeOf((*MockvmNetworkManager)(nil).AddNIC), ctx, nicID, settings) +} + +// RemoveNIC mocks base method. +func (m *MockvmNetworkManager) RemoveNIC(ctx context.Context, nicID string, settings *hcsschema.NetworkAdapter) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RemoveNIC", ctx, nicID, settings) + ret0, _ := ret[0].(error) + return ret0 +} + +// RemoveNIC indicates an expected call of RemoveNIC. +func (mr *MockvmNetworkManagerMockRecorder) RemoveNIC(ctx, nicID, settings any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RemoveNIC", reflect.TypeOf((*MockvmNetworkManager)(nil).RemoveNIC), ctx, nicID, settings) +} + +// MockcapabilitiesProvider is a mock of capabilitiesProvider interface. +type MockcapabilitiesProvider struct { + ctrl *gomock.Controller + recorder *MockcapabilitiesProviderMockRecorder + isgomock struct{} +} + +// MockcapabilitiesProviderMockRecorder is the mock recorder for MockcapabilitiesProvider. +type MockcapabilitiesProviderMockRecorder struct { + mock *MockcapabilitiesProvider +} + +// NewMockcapabilitiesProvider creates a new mock instance. +func NewMockcapabilitiesProvider(ctrl *gomock.Controller) *MockcapabilitiesProvider { + mock := &MockcapabilitiesProvider{ctrl: ctrl} + mock.recorder = &MockcapabilitiesProviderMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockcapabilitiesProvider) EXPECT() *MockcapabilitiesProviderMockRecorder { + return m.recorder +} + +// Capabilities mocks base method. +func (m *MockcapabilitiesProvider) Capabilities() gcs.GuestDefinedCapabilities { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Capabilities") + ret0, _ := ret[0].(gcs.GuestDefinedCapabilities) + return ret0 +} + +// Capabilities indicates an expected call of Capabilities. +func (mr *MockcapabilitiesProviderMockRecorder) Capabilities() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Capabilities", reflect.TypeOf((*MockcapabilitiesProvider)(nil).Capabilities)) +} + +// MockguestNetwork is a mock of guestNetwork interface. +type MockguestNetwork struct { + ctrl *gomock.Controller + recorder *MockguestNetworkMockRecorder + isgomock struct{} +} + +// MockguestNetworkMockRecorder is the mock recorder for MockguestNetwork. +type MockguestNetworkMockRecorder struct { + mock *MockguestNetwork +} + +// NewMockguestNetwork creates a new mock instance. +func NewMockguestNetwork(ctrl *gomock.Controller) *MockguestNetwork { + mock := &MockguestNetwork{ctrl: ctrl} + mock.recorder = &MockguestNetworkMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockguestNetwork) EXPECT() *MockguestNetworkMockRecorder { + return m.recorder +} + +// AddNetworkInterface mocks base method. +func (m *MockguestNetwork) AddNetworkInterface(ctx context.Context, settings *guestresource.LCOWNetworkAdapter) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AddNetworkInterface", ctx, settings) + ret0, _ := ret[0].(error) + return ret0 +} + +// AddNetworkInterface indicates an expected call of AddNetworkInterface. +func (mr *MockguestNetworkMockRecorder) AddNetworkInterface(ctx, settings any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddNetworkInterface", reflect.TypeOf((*MockguestNetwork)(nil).AddNetworkInterface), ctx, settings) +} + +// RemoveNetworkInterface mocks base method. +func (m *MockguestNetwork) RemoveNetworkInterface(ctx context.Context, settings *guestresource.LCOWNetworkAdapter) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RemoveNetworkInterface", ctx, settings) + ret0, _ := ret[0].(error) + return ret0 +} + +// RemoveNetworkInterface indicates an expected call of RemoveNetworkInterface. +func (mr *MockguestNetworkMockRecorder) RemoveNetworkInterface(ctx, settings any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RemoveNetworkInterface", reflect.TypeOf((*MockguestNetwork)(nil).RemoveNetworkInterface), ctx, settings) +} diff --git a/internal/controller/network/mocks/mock_wcow_network.go b/internal/controller/network/mocks/mock_wcow_network.go new file mode 100644 index 0000000000..8feadbb70f --- /dev/null +++ b/internal/controller/network/mocks/mock_wcow_network.go @@ -0,0 +1,193 @@ +//go:build windows && wcow + +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/Microsoft/hcsshim/internal/controller/network (interfaces: vmNetworkManager,capabilitiesProvider,guestNetwork) +// +// Generated by this command: +// +// mockgen -build_flags=-tags=windows,wcow -build_constraint=windows&&wcow -package mocks -destination mocks/mock_wcow_network.go github.com/Microsoft/hcsshim/internal/controller/network vmNetworkManager,capabilitiesProvider,guestNetwork +// + +// Package mocks is a generated GoMock package. +package mocks + +import ( + context "context" + reflect "reflect" + + hcn "github.com/Microsoft/hcsshim/hcn" + gcs "github.com/Microsoft/hcsshim/internal/gcs" + hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" + guestrequest "github.com/Microsoft/hcsshim/internal/protocol/guestrequest" + gomock "go.uber.org/mock/gomock" +) + +// MockvmNetworkManager is a mock of vmNetworkManager interface. +type MockvmNetworkManager struct { + ctrl *gomock.Controller + recorder *MockvmNetworkManagerMockRecorder + isgomock struct{} +} + +// MockvmNetworkManagerMockRecorder is the mock recorder for MockvmNetworkManager. +type MockvmNetworkManagerMockRecorder struct { + mock *MockvmNetworkManager +} + +// NewMockvmNetworkManager creates a new mock instance. +func NewMockvmNetworkManager(ctrl *gomock.Controller) *MockvmNetworkManager { + mock := &MockvmNetworkManager{ctrl: ctrl} + mock.recorder = &MockvmNetworkManagerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockvmNetworkManager) EXPECT() *MockvmNetworkManagerMockRecorder { + return m.recorder +} + +// AddNIC mocks base method. +func (m *MockvmNetworkManager) AddNIC(ctx context.Context, nicID string, settings *hcsschema.NetworkAdapter) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AddNIC", ctx, nicID, settings) + ret0, _ := ret[0].(error) + return ret0 +} + +// AddNIC indicates an expected call of AddNIC. +func (mr *MockvmNetworkManagerMockRecorder) AddNIC(ctx, nicID, settings any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddNIC", reflect.TypeOf((*MockvmNetworkManager)(nil).AddNIC), ctx, nicID, settings) +} + +// RemoveNIC mocks base method. +func (m *MockvmNetworkManager) RemoveNIC(ctx context.Context, nicID string, settings *hcsschema.NetworkAdapter) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RemoveNIC", ctx, nicID, settings) + ret0, _ := ret[0].(error) + return ret0 +} + +// RemoveNIC indicates an expected call of RemoveNIC. +func (mr *MockvmNetworkManagerMockRecorder) RemoveNIC(ctx, nicID, settings any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RemoveNIC", reflect.TypeOf((*MockvmNetworkManager)(nil).RemoveNIC), ctx, nicID, settings) +} + +// MockcapabilitiesProvider is a mock of capabilitiesProvider interface. +type MockcapabilitiesProvider struct { + ctrl *gomock.Controller + recorder *MockcapabilitiesProviderMockRecorder + isgomock struct{} +} + +// MockcapabilitiesProviderMockRecorder is the mock recorder for MockcapabilitiesProvider. +type MockcapabilitiesProviderMockRecorder struct { + mock *MockcapabilitiesProvider +} + +// NewMockcapabilitiesProvider creates a new mock instance. +func NewMockcapabilitiesProvider(ctrl *gomock.Controller) *MockcapabilitiesProvider { + mock := &MockcapabilitiesProvider{ctrl: ctrl} + mock.recorder = &MockcapabilitiesProviderMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockcapabilitiesProvider) EXPECT() *MockcapabilitiesProviderMockRecorder { + return m.recorder +} + +// Capabilities mocks base method. +func (m *MockcapabilitiesProvider) Capabilities() gcs.GuestDefinedCapabilities { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Capabilities") + ret0, _ := ret[0].(gcs.GuestDefinedCapabilities) + return ret0 +} + +// Capabilities indicates an expected call of Capabilities. +func (mr *MockcapabilitiesProviderMockRecorder) Capabilities() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Capabilities", reflect.TypeOf((*MockcapabilitiesProvider)(nil).Capabilities)) +} + +// MockguestNetwork is a mock of guestNetwork interface. +type MockguestNetwork struct { + ctrl *gomock.Controller + recorder *MockguestNetworkMockRecorder + isgomock struct{} +} + +// MockguestNetworkMockRecorder is the mock recorder for MockguestNetwork. +type MockguestNetworkMockRecorder struct { + mock *MockguestNetwork +} + +// NewMockguestNetwork creates a new mock instance. +func NewMockguestNetwork(ctrl *gomock.Controller) *MockguestNetwork { + mock := &MockguestNetwork{ctrl: ctrl} + mock.recorder = &MockguestNetworkMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockguestNetwork) EXPECT() *MockguestNetworkMockRecorder { + return m.recorder +} + +// AddNetworkInterface mocks base method. +func (m *MockguestNetwork) AddNetworkInterface(ctx context.Context, adapterID string, requestType guestrequest.RequestType, settings *hcn.HostComputeEndpoint) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AddNetworkInterface", ctx, adapterID, requestType, settings) + ret0, _ := ret[0].(error) + return ret0 +} + +// AddNetworkInterface indicates an expected call of AddNetworkInterface. +func (mr *MockguestNetworkMockRecorder) AddNetworkInterface(ctx, adapterID, requestType, settings any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddNetworkInterface", reflect.TypeOf((*MockguestNetwork)(nil).AddNetworkInterface), ctx, adapterID, requestType, settings) +} + +// AddNetworkNamespace mocks base method. +func (m *MockguestNetwork) AddNetworkNamespace(ctx context.Context, settings *hcn.HostComputeNamespace) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AddNetworkNamespace", ctx, settings) + ret0, _ := ret[0].(error) + return ret0 +} + +// AddNetworkNamespace indicates an expected call of AddNetworkNamespace. +func (mr *MockguestNetworkMockRecorder) AddNetworkNamespace(ctx, settings any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddNetworkNamespace", reflect.TypeOf((*MockguestNetwork)(nil).AddNetworkNamespace), ctx, settings) +} + +// RemoveNetworkInterface mocks base method. +func (m *MockguestNetwork) RemoveNetworkInterface(ctx context.Context, adapterID string, requestType guestrequest.RequestType, settings *hcn.HostComputeEndpoint) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RemoveNetworkInterface", ctx, adapterID, requestType, settings) + ret0, _ := ret[0].(error) + return ret0 +} + +// RemoveNetworkInterface indicates an expected call of RemoveNetworkInterface. +func (mr *MockguestNetworkMockRecorder) RemoveNetworkInterface(ctx, adapterID, requestType, settings any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RemoveNetworkInterface", reflect.TypeOf((*MockguestNetwork)(nil).RemoveNetworkInterface), ctx, adapterID, requestType, settings) +} + +// RemoveNetworkNamespace mocks base method. +func (m *MockguestNetwork) RemoveNetworkNamespace(ctx context.Context, settings *hcn.HostComputeNamespace) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RemoveNetworkNamespace", ctx, settings) + ret0, _ := ret[0].(error) + return ret0 +} + +// RemoveNetworkNamespace indicates an expected call of RemoveNetworkNamespace. +func (mr *MockguestNetworkMockRecorder) RemoveNetworkNamespace(ctx, settings any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RemoveNetworkNamespace", reflect.TypeOf((*MockguestNetwork)(nil).RemoveNetworkNamespace), ctx, settings) +} diff --git a/internal/controller/network/network_lcow_test.go b/internal/controller/network/network_lcow_test.go new file mode 100644 index 0000000000..819e94448b --- /dev/null +++ b/internal/controller/network/network_lcow_test.go @@ -0,0 +1,410 @@ +//go:build windows && lcow + +package network + +import ( + "context" + "errors" + "testing" + + "go.uber.org/mock/gomock" + + "github.com/Microsoft/hcsshim/hcn" + "github.com/Microsoft/hcsshim/internal/controller/network/mocks" + "github.com/Microsoft/hcsshim/internal/gcs" + "github.com/Microsoft/hcsshim/internal/guest/prot" + hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" + "github.com/Microsoft/hcsshim/internal/protocol/guestresource" +) + +var ( + errLCOWHostAdd = errors.New("host AddNIC failed") + errLCOWHostRemove = errors.New("host RemoveNIC failed") + errLCOWGuestAdd = errors.New("guest AddNetworkInterface failed") + errLCOWGuestRemove = errors.New("guest RemoveNetworkInterface failed") +) + +// newLCOWController constructs a Controller wired with the supplied mocks +// and pre-set namespace-add capability flag. The state is left at +// StateNotConfigured; callers needing a different state must override +// netState before exercising guarded entry points. +func newLCOWController( + t *testing.T, + ctrl *gomock.Controller, + namespaceAddSupported bool, +) (*Controller, *mocks.MockvmNetworkManager, *mocks.MockguestNetwork) { + t.Helper() + + vm := mocks.NewMockvmNetworkManager(ctrl) + guest := mocks.NewMockguestNetwork(ctrl) + caps := mocks.NewMockcapabilitiesProvider(ctrl) + caps.EXPECT().Capabilities().Return(&gcs.LCOWGuestDefinedCapabilities{ + GcsGuestCapabilities: prot.GcsGuestCapabilities{ + NamespaceAddRequestSupported: namespaceAddSupported, + }, + }) + + c := New( + &Options{NetworkNamespace: "ns-1"}, + vm, + guest, + caps, + ) + return c, vm, guest +} + +// newLCOWEndpoint returns a synthetic HCN endpoint suitable for unit tests. +// All fields the controller reads downstream are populated so that the +// hcsschema.NetworkAdapter passed to AddNIC/RemoveNIC is fully realistic. +func newLCOWEndpoint(name string) *hcn.HostComputeEndpoint { + return &hcn.HostComputeEndpoint{ + Id: "ep-" + name, + Name: name, + MacAddress: "aa:bb:cc:dd:ee:01", + HostComputeNamespace: "ns-1", + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// Add endpoint tests +// ───────────────────────────────────────────────────────────────────────────── + +// TestLCOW_AddEndpoint_Success_NamespaceSupport verifies the happy path with +// namespace support: host AddNIC runs first, then guest AddNetworkInterface +// receives an adapter built from the same endpoint, and the NIC is tracked. +func TestLCOW_AddEndpoint_Success_NamespaceSupport(t *testing.T) { + ctrl := gomock.NewController(t) + c, vm, guest := newLCOWController(t, ctrl, true) + + ep := newLCOWEndpoint("eth0") + expectedAdapter, err := guestresource.BuildLCOWNetworkAdapter("nic-1", ep, false) + if err != nil { + t.Fatalf("failed to build expected adapter: %v", err) + } + + gomock.InOrder( + vm.EXPECT().AddNIC(gomock.Any(), "nic-1", &hcsschema.NetworkAdapter{ + EndpointId: ep.Id, + MacAddress: ep.MacAddress, + }).Return(nil), + guest.EXPECT().AddNetworkInterface(gomock.Any(), expectedAdapter).Return(nil), + ) + + if err := c.addEndpointToGuestNamespace(context.Background(), "nic-1", ep, false); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got, ok := c.vmEndpoints["nic-1"]; !ok || got != ep { + t.Errorf("expected nic-1 → %+v in vmEndpoints, got: %+v (present=%v)", ep, got, ok) + } +} + +// TestLCOW_AddEndpoint_Success_NoNamespaceSupport verifies that when the +// guest does not advertise namespace-add support, only the host-side NIC +// hot-add is performed; the guest call is skipped. +func TestLCOW_AddEndpoint_Success_NoNamespaceSupport(t *testing.T) { + ctrl := gomock.NewController(t) + c, vm, _ := newLCOWController(t, ctrl, false) + + ep := newLCOWEndpoint("eth0") + + vm.EXPECT().AddNIC(gomock.Any(), "nic-1", &hcsschema.NetworkAdapter{ + EndpointId: ep.Id, + MacAddress: ep.MacAddress, + }).Return(nil) + // guest.AddNetworkInterface is intentionally not expected — gomock will + // fail the test if the controller calls it without namespace support. + + if err := c.addEndpointToGuestNamespace(context.Background(), "nic-1", ep, false); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if _, ok := c.vmEndpoints["nic-1"]; !ok { + t.Error("expected nic-1 to be tracked even when guest call is skipped") + } +} + +// TestLCOW_AddEndpoint_HostFails_NotTracked verifies that when the host-side +// AddNIC fails, the NIC is NOT tracked in vmEndpoints. Tracking a NIC the +// host does not own would lead Teardown to call RemoveNIC on a non-existent +// device, masking the real failure. +func TestLCOW_AddEndpoint_HostFails_NotTracked(t *testing.T) { + ctrl := gomock.NewController(t) + c, vm, _ := newLCOWController(t, ctrl, true) + + ep := newLCOWEndpoint("eth0") + + vm.EXPECT().AddNIC(gomock.Any(), "nic-1", gomock.Any()).Return(errLCOWHostAdd) + // guest.AddNetworkInterface must not be called when host add fails. + + err := c.addEndpointToGuestNamespace(context.Background(), "nic-1", ep, false) + if !errors.Is(err, errLCOWHostAdd) { + t.Fatalf("expected host add error to wrap, got: %v", err) + } + if _, ok := c.vmEndpoints["nic-1"]; ok { + t.Error("expected nic-1 NOT tracked after host AddNIC failure") + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// Remove endpoint tests +// ───────────────────────────────────────────────────────────────────────────── + +// TestLCOW_RemoveEndpoint_Success verifies the happy removal path: guest-side +// removal runs first, then host-side hot-remove. +func TestLCOW_RemoveEndpoint_Success(t *testing.T) { + ctrl := gomock.NewController(t) + c, vm, guest := newLCOWController(t, ctrl, true) + + ep := newLCOWEndpoint("eth0") + + gomock.InOrder( + guest.EXPECT().RemoveNetworkInterface(gomock.Any(), &guestresource.LCOWNetworkAdapter{ + NamespaceID: "ns-1", + ID: "nic-1", + }).Return(nil), + vm.EXPECT().RemoveNIC(gomock.Any(), "nic-1", &hcsschema.NetworkAdapter{ + EndpointId: ep.Id, + MacAddress: ep.MacAddress, + }).Return(nil), + ) + + if err := c.removeEndpointFromGuestNamespace(context.Background(), "nic-1", ep); err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +// TestLCOW_RemoveEndpoint_GuestFails_HostNotCalled verifies that if the +// guest-side removal fails, the host-side RemoveNIC is NOT invoked. The +// caller (Teardown) marks the controller Invalid and lets a future +// Teardown retry both halves. +func TestLCOW_RemoveEndpoint_GuestFails_HostNotCalled(t *testing.T) { + ctrl := gomock.NewController(t) + c, _, guest := newLCOWController(t, ctrl, true) + + ep := newLCOWEndpoint("eth0") + + guest.EXPECT().RemoveNetworkInterface(gomock.Any(), gomock.Any()).Return(errLCOWGuestRemove) + // vm.RemoveNIC must not be called. + + err := c.removeEndpointFromGuestNamespace(context.Background(), "nic-1", ep) + if !errors.Is(err, errLCOWGuestRemove) { + t.Fatalf("expected guest remove error to wrap, got: %v", err) + } +} + +// TestLCOW_RemoveEndpoint_NoNamespaceSupport_HostOnly verifies that when the +// guest never received the namespace, the controller skips the guest-side +// removal and only hot-removes the NIC from the host. +func TestLCOW_RemoveEndpoint_NoNamespaceSupport_HostOnly(t *testing.T) { + ctrl := gomock.NewController(t) + c, vm, _ := newLCOWController(t, ctrl, false) + + ep := newLCOWEndpoint("eth0") + + vm.EXPECT().RemoveNIC(gomock.Any(), "nic-1", &hcsschema.NetworkAdapter{ + EndpointId: ep.Id, + MacAddress: ep.MacAddress, + }).Return(nil) + // guest.RemoveNetworkInterface must not be called. + + if err := c.removeEndpointFromGuestNamespace(context.Background(), "nic-1", ep); err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// Teardown tests (LCOW: removeNetNSInsideGuest is a no-op so full Teardown +// is exercisable end-to-end without HNS) +// ───────────────────────────────────────────────────────────────────────────── + +// TestLCOW_Teardown_HappyPath verifies that Teardown removes every tracked +// endpoint, clears the tracking map, and transitions the controller to +// StateTornDown. +func TestLCOW_Teardown_HappyPath(t *testing.T) { + ctrl := gomock.NewController(t) + c, vm, guest := newLCOWController(t, ctrl, true) + + c.netState = StateConfigured + ep1 := newLCOWEndpoint("eth0") + ep2 := newLCOWEndpoint("eth1") + c.vmEndpoints["nic-1"] = ep1 + c.vmEndpoints["nic-2"] = ep2 + + // Two endpoints tracked, so each remove API runs exactly twice. Map + // iteration order is not asserted here; gomock matches by call count. + guest.EXPECT().RemoveNetworkInterface(gomock.Any(), gomock.Any()).Return(nil).Times(2) + vm.EXPECT().RemoveNIC(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).Times(2) + + if err := c.Teardown(context.Background()); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if c.netState != StateTornDown { + t.Errorf("expected state TornDown, got %s", c.netState) + } + if len(c.vmEndpoints) != 0 { + t.Errorf("expected vmEndpoints to be empty, got %d entries", len(c.vmEndpoints)) + } +} + +// TestLCOW_Teardown_PartialFailure_RemainingAttempted verifies the cleanup +// chain: if removing one NIC fails, the controller must still attempt the +// remaining NICs. A leak here means a UVM kept alive with stuck NICs. +// The controller transitions to Invalid, but the surviving NICs are gone. +func TestLCOW_Teardown_PartialFailure_RemainingAttempted(t *testing.T) { + ctrl := gomock.NewController(t) + c, vm, guest := newLCOWController(t, ctrl, true) + + c.netState = StateConfigured + ep1 := newLCOWEndpoint("eth0") + ep2 := newLCOWEndpoint("eth1") + ep3 := newLCOWEndpoint("eth2") + c.vmEndpoints["nic-1"] = ep1 + c.vmEndpoints["nic-2"] = ep2 + c.vmEndpoints["nic-3"] = ep3 + + // Fail nic-2 specifically; nic-1 and nic-3 succeed. + // Map iteration order is random, so use DoAndReturn to branch on nicID. + guest.EXPECT().RemoveNetworkInterface(gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, settings *guestresource.LCOWNetworkAdapter) error { + if settings.ID == "nic-2" { + return errLCOWGuestRemove + } + return nil + }).Times(3) + // RemoveNIC is only called when guest removal succeeds — so 2 calls. + vm.EXPECT().RemoveNIC(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).Times(2) + + err := c.Teardown(context.Background()) + if !errors.Is(err, errLCOWGuestRemove) { + t.Fatalf("expected joined error to wrap guest remove failure, got: %v", err) + } + if c.netState != StateInvalid { + t.Errorf("expected state Invalid after partial failure, got %s", c.netState) + } + // nic-2 must remain tracked (so a retry can clean it up); nic-1 and nic-3 + // must be gone. + if _, ok := c.vmEndpoints["nic-2"]; !ok { + t.Error("expected nic-2 to remain tracked after its removal failed") + } + if _, ok := c.vmEndpoints["nic-1"]; ok { + t.Error("expected nic-1 to be removed from tracking") + } + if _, ok := c.vmEndpoints["nic-3"]; ok { + t.Error("expected nic-3 to be removed from tracking") + } +} + +// TestLCOW_Teardown_HostRemoveFails_StateInvalid verifies that a host-side +// RemoveNIC failure also surfaces as Invalid (matching the guest-side path) +// and keeps the failed NIC tracked for a future retry. +func TestLCOW_Teardown_HostRemoveFails_StateInvalid(t *testing.T) { + ctrl := gomock.NewController(t) + c, vm, guest := newLCOWController(t, ctrl, true) + + c.netState = StateConfigured + ep := newLCOWEndpoint("eth0") + c.vmEndpoints["nic-1"] = ep + + guest.EXPECT().RemoveNetworkInterface(gomock.Any(), gomock.Any()).Return(nil) + vm.EXPECT().RemoveNIC(gomock.Any(), "nic-1", gomock.Any()).Return(errLCOWHostRemove) + + err := c.Teardown(context.Background()) + if !errors.Is(err, errLCOWHostRemove) { + t.Fatalf("expected host remove error to wrap, got: %v", err) + } + if c.netState != StateInvalid { + t.Errorf("expected state Invalid, got %s", c.netState) + } + if _, ok := c.vmEndpoints["nic-1"]; !ok { + t.Error("expected nic-1 to remain tracked after host RemoveNIC failure") + } +} + +// ────────────────────────────────────────────────────────────────────────── +// Half-setup recovery: Teardown unwinds host-side state after partial failures +// ────────────────────────────────────────────────────────────────────────── + +// TestLCOW_AddEndpoint_HostOK_GuestFails_TeardownUnwindsHost covers the +// half-setup recovery contract end-to-end: when the guest-side +// AddNetworkInterface fails after a successful host AddNIC, the NIC must +// remain tracked so a subsequent Teardown can remove the host-side device. +// Otherwise the host UVM leaks the NIC. +func TestLCOW_AddEndpoint_HostOK_GuestFails_TeardownUnwindsHost(t *testing.T) { + ctrl := gomock.NewController(t) + c, vm, guest := newLCOWController(t, ctrl, true) + + ep := newLCOWEndpoint("eth0") + + gomock.InOrder( + vm.EXPECT().AddNIC(gomock.Any(), "nic-1", gomock.Any()).Return(nil), + guest.EXPECT().AddNetworkInterface(gomock.Any(), gomock.Any()).Return(errLCOWGuestAdd), + ) + + if err := c.addEndpointToGuestNamespace(context.Background(), "nic-1", ep, false); !errors.Is(err, errLCOWGuestAdd) { + t.Fatalf("expected guest add error to wrap, got: %v", err) + } + if _, ok := c.vmEndpoints["nic-1"]; !ok { + t.Fatal("expected nic-1 to be tracked after guest-side failure so Teardown can unwind the host NIC") + } + + // Teardown must run both legs: guest remove (best-effort, may no-op) and + // host RemoveNIC (the actual leak-recovery path). + c.netState = StateConfigured + gomock.InOrder( + guest.EXPECT().RemoveNetworkInterface(gomock.Any(), gomock.Any()).Return(nil), + vm.EXPECT().RemoveNIC(gomock.Any(), "nic-1", gomock.Any()).Return(nil), + ) + + if err := c.Teardown(context.Background()); err != nil { + t.Fatalf("Teardown after half-setup: %v", err) + } + if c.netState != StateTornDown { + t.Errorf("expected state TornDown after recovery, got %s", c.netState) + } + if _, ok := c.vmEndpoints["nic-1"]; ok { + t.Error("expected nic-1 to be cleared after successful Teardown") + } +} + +// TestLCOW_Teardown_GuestFails_RetryFromInvalid covers the half-setup, +// failure-in-teardown, then success-teardown sequence. The first Teardown +// fails on guest removal; the controller transitions to StateInvalid and +// keeps the failed NIC tracked. A second Teardown call (which production +// code is allowed to make from StateInvalid) drains the remaining endpoint +// and reaches StateTornDown. +func TestLCOW_Teardown_GuestFails_RetryFromInvalid(t *testing.T) { + ctrl := gomock.NewController(t) + c, vm, guest := newLCOWController(t, ctrl, true) + + c.netState = StateConfigured + ep := newLCOWEndpoint("eth0") + c.vmEndpoints["nic-1"] = ep + + // First Teardown: guest remove fails → state Invalid, NIC still tracked. + guest.EXPECT().RemoveNetworkInterface(gomock.Any(), gomock.Any()).Return(errLCOWGuestRemove) + + if err := c.Teardown(context.Background()); !errors.Is(err, errLCOWGuestRemove) { + t.Fatalf("first Teardown: expected guest remove error, got: %v", err) + } + if c.netState != StateInvalid { + t.Fatalf("first Teardown: expected StateInvalid, got %s", c.netState) + } + if _, ok := c.vmEndpoints["nic-1"]; !ok { + t.Fatal("first Teardown: nic-1 must remain tracked for retry") + } + + // Second Teardown: both legs succeed → state TornDown, map cleared. + gomock.InOrder( + guest.EXPECT().RemoveNetworkInterface(gomock.Any(), gomock.Any()).Return(nil), + vm.EXPECT().RemoveNIC(gomock.Any(), "nic-1", gomock.Any()).Return(nil), + ) + + if err := c.Teardown(context.Background()); err != nil { + t.Fatalf("retry Teardown: %v", err) + } + if c.netState != StateTornDown { + t.Errorf("retry Teardown: expected StateTornDown, got %s", c.netState) + } + if _, ok := c.vmEndpoints["nic-1"]; ok { + t.Error("retry Teardown: expected nic-1 to be cleared") + } +} diff --git a/internal/controller/network/network_test.go b/internal/controller/network/network_test.go new file mode 100644 index 0000000000..02c99fea05 --- /dev/null +++ b/internal/controller/network/network_test.go @@ -0,0 +1,195 @@ +//go:build windows && (lcow || wcow) + +package network + +import ( + "context" + "strings" + "testing" + + "go.uber.org/mock/gomock" + + "github.com/Microsoft/hcsshim/internal/controller/network/mocks" + "github.com/Microsoft/hcsshim/internal/gcs" + "github.com/Microsoft/hcsshim/internal/guest/prot" +) + +// ───────────────────────────────────────────────────────────────────────────── +// Test helpers +// ───────────────────────────────────────────────────────────────────────────── + +// newCapsProvider returns a [mocks.MockcapabilitiesProvider] whose +// Capabilities() returns LCOW capabilities with the given namespace-add flag. +// LCOW is used here because Capabilities() returns the [gcs.GuestDefinedCapabilities] +// interface; the concrete OS does not matter for the platform-agnostic tests. +func newCapsProvider(t *testing.T, ctrl *gomock.Controller, namespaceAddSupported bool) *mocks.MockcapabilitiesProvider { + t.Helper() + caps := mocks.NewMockcapabilitiesProvider(ctrl) + caps.EXPECT().Capabilities().Return(&gcs.LCOWGuestDefinedCapabilities{ + GcsGuestCapabilities: prot.GcsGuestCapabilities{ + NamespaceAddRequestSupported: namespaceAddSupported, + }, + }) + return caps +} + +// newNilCapsProvider returns a provider whose Capabilities() returns nil, +// modelling a guest that has not yet reported its capabilities. +func newNilCapsProvider(t *testing.T, ctrl *gomock.Controller) *mocks.MockcapabilitiesProvider { + t.Helper() + caps := mocks.NewMockcapabilitiesProvider(ctrl) + caps.EXPECT().Capabilities().Return(nil) + return caps +} + +// ───────────────────────────────────────────────────────────────────────────── +// Construction tests +// ───────────────────────────────────────────────────────────────────────────── + +// TestNew_NamespaceCapabilityCached verifies that [New] queries the guest's +// capabilities exactly once and caches the namespace-add flag, so hot paths +// do not re-query on every Setup/Teardown call. +func TestNew_NamespaceCapabilityCached(t *testing.T) { + ctrl := gomock.NewController(t) + c := New( + &Options{NetworkNamespace: "ns-1"}, + mocks.NewMockvmNetworkManager(ctrl), + mocks.NewMockguestNetwork(ctrl), + newCapsProvider(t, ctrl, true), + ) + + if !c.isNamespaceSupportedByGuest { + t.Error("expected isNamespaceSupportedByGuest=true after caps say supported") + } + if c.netState != StateNotConfigured { + t.Errorf("expected initial state NotConfigured, got %s", c.netState) + } + if c.namespaceID != "ns-1" { + t.Errorf("expected namespaceID=ns-1, got %q", c.namespaceID) + } + if c.vmEndpoints == nil { + t.Error("expected vmEndpoints map to be non-nil") + } +} + +// TestNew_NilCapabilities_NoCrash verifies that a guest which reports nil +// capabilities does not panic [New] and leaves the namespace flag unset. +// This matches the guarded read in network.go. +func TestNew_NilCapabilities_NoCrash(t *testing.T) { + ctrl := gomock.NewController(t) + c := New( + &Options{NetworkNamespace: "ns-1"}, + mocks.NewMockvmNetworkManager(ctrl), + mocks.NewMockguestNetwork(ctrl), + newNilCapsProvider(t, ctrl), + ) + + if c.isNamespaceSupportedByGuest { + t.Error("expected isNamespaceSupportedByGuest=false when caps are nil") + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// Setup state-guard tests +// ───────────────────────────────────────────────────────────────────────────── + +// TestSetup_RejectInWrongState verifies that Setup refuses to run when the +// controller is not in StateNotConfigured. The guard must reject the call +// without mutating state, since the deferred Invalid handler is registered +// after the guard. +func TestSetup_RejectInWrongState(t *testing.T) { + ctrl := gomock.NewController(t) + c := New( + &Options{NetworkNamespace: "ns-1"}, + mocks.NewMockvmNetworkManager(ctrl), + mocks.NewMockguestNetwork(ctrl), + newCapsProvider(t, ctrl, true), + ) + c.netState = StateConfigured + + err := c.Setup(context.Background()) + if err == nil { + t.Fatal("expected error from Setup in StateConfigured, got nil") + } + if !strings.Contains(err.Error(), "Configured") { + t.Errorf("expected error to mention current state Configured, got: %v", err) + } + // State must not change on guard rejection — the deferred Invalid + // handler is registered after the guard, so a guard miss must be a no-op. + if c.netState != StateConfigured { + t.Errorf("expected state to remain Configured after guard rejection, got %s", c.netState) + } +} + +// TestSetup_EmptyNamespaceID verifies that Setup fails fast when no +// namespace ID is configured and transitions the controller to Invalid. +// This is the only Setup pre-HCN failure path that we can exercise without +// a live HNS, but it also covers the deferred Invalid-on-error handler. +func TestSetup_EmptyNamespaceID(t *testing.T) { + ctrl := gomock.NewController(t) + c := New( + &Options{NetworkNamespace: ""}, + mocks.NewMockvmNetworkManager(ctrl), + mocks.NewMockguestNetwork(ctrl), + newCapsProvider(t, ctrl, true), + ) + + err := c.Setup(context.Background()) + if err == nil { + t.Fatal("expected error from Setup with empty namespace ID, got nil") + } + // On any Setup failure after the state guard, the controller must + // move to Invalid so subsequent Setup calls keep being rejected. + if c.netState != StateInvalid { + t.Errorf("expected state Invalid after Setup failure, got %s", c.netState) + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// Teardown idempotency tests +// ───────────────────────────────────────────────────────────────────────────── + +// TestTeardown_NoOpFromNotConfigured verifies that calling Teardown before +// Setup is a no-op. The shim invokes Teardown unconditionally on pod +// removal even if Setup never ran (or failed before reaching Configured). +func TestTeardown_NoOpFromNotConfigured(t *testing.T) { + ctrl := gomock.NewController(t) + vm := mocks.NewMockvmNetworkManager(ctrl) + guest := mocks.NewMockguestNetwork(ctrl) + c := New( + &Options{NetworkNamespace: "ns-1"}, + vm, + guest, + newCapsProvider(t, ctrl, true), + ) + + // No EXPECT() on vm or guest — any call would fail the test. + if err := c.Teardown(context.Background()); err != nil { + t.Fatalf("expected nil from Teardown in NotConfigured, got: %v", err) + } + if c.netState != StateNotConfigured { + t.Errorf("expected state to remain NotConfigured, got %s", c.netState) + } +} + +// TestTeardown_NoOpFromTornDown verifies that calling Teardown a second time +// after a successful Teardown is a no-op. Containerd retries StopPodSandbox. +func TestTeardown_NoOpFromTornDown(t *testing.T) { + ctrl := gomock.NewController(t) + vm := mocks.NewMockvmNetworkManager(ctrl) + guest := mocks.NewMockguestNetwork(ctrl) + c := New( + &Options{NetworkNamespace: "ns-1"}, + vm, + guest, + newCapsProvider(t, ctrl, true), + ) + c.netState = StateTornDown + + if err := c.Teardown(context.Background()); err != nil { + t.Fatalf("expected nil from Teardown in TornDown, got: %v", err) + } + if c.netState != StateTornDown { + t.Errorf("expected state to remain TornDown, got %s", c.netState) + } +} diff --git a/internal/controller/network/network_wcow_test.go b/internal/controller/network/network_wcow_test.go new file mode 100644 index 0000000000..a68e0a8dbe --- /dev/null +++ b/internal/controller/network/network_wcow_test.go @@ -0,0 +1,380 @@ +//go:build windows && wcow + +package network + +import ( + "context" + "errors" + "testing" + + "go.uber.org/mock/gomock" + + "github.com/Microsoft/hcsshim/hcn" + "github.com/Microsoft/hcsshim/internal/controller/network/mocks" + "github.com/Microsoft/hcsshim/internal/gcs" + "github.com/Microsoft/hcsshim/internal/hcs/schema1" + hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" + "github.com/Microsoft/hcsshim/internal/protocol/guestrequest" +) + +var ( + errWCOWHostAdd = errors.New("host AddNIC failed") + errWCOWGuestPreAdd = errors.New("guest PreAdd failed") + errWCOWGuestAdd = errors.New("guest Add failed") + errWCOWGuestRemove = errors.New("guest Remove failed") +) + +// newWCOWController constructs a Controller wired with WCOW mocks and the +// supplied namespace-add capability flag. State starts at StateNotConfigured; +// callers must override netState before exercising guarded entry points. +func newWCOWController( + t *testing.T, + ctrl *gomock.Controller, + namespaceAddSupported bool, +) (*Controller, *mocks.MockvmNetworkManager, *mocks.MockguestNetwork) { + t.Helper() + + vm := mocks.NewMockvmNetworkManager(ctrl) + guest := mocks.NewMockguestNetwork(ctrl) + caps := mocks.NewMockcapabilitiesProvider(ctrl) + caps.EXPECT().Capabilities().Return(&gcs.WCOWGuestDefinedCapabilities{ + GuestDefinedCapabilities: schema1.GuestDefinedCapabilities{ + NamespaceAddRequestSupported: namespaceAddSupported, + }, + }) + + c := New( + &Options{NetworkNamespace: "ns-1"}, + vm, + guest, + caps, + ) + return c, vm, guest +} + +// newWCOWEndpoint returns a synthetic HCN endpoint suitable for unit tests. +func newWCOWEndpoint(name string) *hcn.HostComputeEndpoint { + return &hcn.HostComputeEndpoint{ + Id: "ep-" + name, + Name: name, + MacAddress: "aa:bb:cc:dd:ee:01", + HostComputeNamespace: "ns-1", + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// Add endpoint tests (WCOW: PreAdd → host AddNIC → guest Add) +// ───────────────────────────────────────────────────────────────────────────── + +// TestWCOW_AddEndpoint_3PhaseSequence_Success verifies the WCOW three-phase +// add sequence: guest PreAdd, then host AddNIC, then guest Add (finalize). +// The order matters — WCOW expects the guest to be informed BEFORE the NIC +// arrives on the bus. +func TestWCOW_AddEndpoint_3PhaseSequence_Success(t *testing.T) { + ctrl := gomock.NewController(t) + c, vm, guest := newWCOWController(t, ctrl, true) + + ep := newWCOWEndpoint("eth0") + + gomock.InOrder( + // 1. PreAdd: tells the WCOW guest a NIC is about to arrive. + guest.EXPECT().AddNetworkInterface( + gomock.Any(), "nic-1", guestrequest.RequestTypePreAdd, ep, + ).Return(nil), + // 2. Host hot-add. + vm.EXPECT().AddNIC(gomock.Any(), "nic-1", &hcsschema.NetworkAdapter{ + EndpointId: ep.Id, + MacAddress: ep.MacAddress, + }).Return(nil), + // 3. Guest finalize add. + guest.EXPECT().AddNetworkInterface( + gomock.Any(), "nic-1", guestrequest.RequestTypeAdd, gomock.Nil(), + ).Return(nil), + ) + + if err := c.addEndpointToGuestNamespace(context.Background(), "nic-1", ep, false); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got, ok := c.vmEndpoints["nic-1"]; !ok || got != ep { + t.Errorf("expected nic-1 → %+v in vmEndpoints, got: %+v (present=%v)", ep, got, ok) + } +} + +// TestWCOW_AddEndpoint_PreAddFails_NotTracked verifies that when the guest +// pre-add fails, neither the host AddNIC nor the guest finalize is invoked, +// and the NIC is not tracked. PreAdd is the WCOW-specific guard against +// presenting an unwanted device to the guest. +func TestWCOW_AddEndpoint_PreAddFails_NotTracked(t *testing.T) { + ctrl := gomock.NewController(t) + c, _, guest := newWCOWController(t, ctrl, true) + + ep := newWCOWEndpoint("eth0") + + guest.EXPECT().AddNetworkInterface( + gomock.Any(), "nic-1", guestrequest.RequestTypePreAdd, ep, + ).Return(errWCOWGuestPreAdd) + // No vm.AddNIC, no second guest.AddNetworkInterface — gomock fails if either is called. + + err := c.addEndpointToGuestNamespace(context.Background(), "nic-1", ep, false) + if !errors.Is(err, errWCOWGuestPreAdd) { + t.Fatalf("expected pre-add error to wrap, got: %v", err) + } + if _, ok := c.vmEndpoints["nic-1"]; ok { + t.Error("expected nic-1 NOT tracked after pre-add failure") + } +} + +// TestWCOW_AddEndpoint_HostFails_NotTracked verifies that a host-side AddNIC +// failure (after a successful PreAdd) leaves the NIC untracked. Tracking +// would lead Teardown to RemoveNIC a device the host does not own. +func TestWCOW_AddEndpoint_HostFails_NotTracked(t *testing.T) { + ctrl := gomock.NewController(t) + c, vm, guest := newWCOWController(t, ctrl, true) + + ep := newWCOWEndpoint("eth0") + + gomock.InOrder( + guest.EXPECT().AddNetworkInterface( + gomock.Any(), "nic-1", guestrequest.RequestTypePreAdd, ep, + ).Return(nil), + vm.EXPECT().AddNIC(gomock.Any(), "nic-1", gomock.Any()).Return(errWCOWHostAdd), + ) + + err := c.addEndpointToGuestNamespace(context.Background(), "nic-1", ep, false) + if !errors.Is(err, errWCOWHostAdd) { + t.Fatalf("expected host add error to wrap, got: %v", err) + } + if _, ok := c.vmEndpoints["nic-1"]; ok { + t.Error("expected nic-1 NOT tracked after host AddNIC failure") + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// Remove endpoint tests +// ───────────────────────────────────────────────────────────────────────────── + +// TestWCOW_RemoveEndpoint_Success verifies the happy removal path: guest-side +// remove first, then host RemoveNIC. +func TestWCOW_RemoveEndpoint_Success(t *testing.T) { + ctrl := gomock.NewController(t) + c, vm, guest := newWCOWController(t, ctrl, true) + + ep := newWCOWEndpoint("eth0") + + gomock.InOrder( + guest.EXPECT().RemoveNetworkInterface( + gomock.Any(), "nic-1", guestrequest.RequestTypeRemove, gomock.Nil(), + ).Return(nil), + vm.EXPECT().RemoveNIC(gomock.Any(), "nic-1", &hcsschema.NetworkAdapter{ + EndpointId: ep.Id, + MacAddress: ep.MacAddress, + }).Return(nil), + ) + + if err := c.removeEndpointFromGuestNamespace(context.Background(), "nic-1", ep); err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +// TestWCOW_RemoveEndpoint_GuestFails_HostNotCalled verifies that a guest-side +// removal failure short-circuits the host hot-remove. Caller (Teardown) +// marks the controller Invalid and lets a future retry attempt both halves. +func TestWCOW_RemoveEndpoint_GuestFails_HostNotCalled(t *testing.T) { + ctrl := gomock.NewController(t) + c, _, guest := newWCOWController(t, ctrl, true) + + ep := newWCOWEndpoint("eth0") + + guest.EXPECT().RemoveNetworkInterface( + gomock.Any(), "nic-1", guestrequest.RequestTypeRemove, gomock.Nil(), + ).Return(errWCOWGuestRemove) + // vm.RemoveNIC must not be called. + + err := c.removeEndpointFromGuestNamespace(context.Background(), "nic-1", ep) + if !errors.Is(err, errWCOWGuestRemove) { + t.Fatalf("expected guest remove error to wrap, got: %v", err) + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// Teardown tests +// +// WCOW's removeNetNSInsideGuest calls hcn.GetNamespaceByID when the guest +// supports namespace add. To exercise full Teardown without a live HNS, +// these tests construct controllers with namespace support DISABLED; +// removeNetNSInsideGuest then becomes a no-op. The endpoint loop logic +// — the high-value cleanup path — is fully covered. +// ───────────────────────────────────────────────────────────────────────────── + +// TestWCOW_Teardown_HappyPath_NoNamespaceSupport verifies that Teardown +// removes every tracked endpoint, clears the tracking map, and transitions +// to StateTornDown. +func TestWCOW_Teardown_HappyPath_NoNamespaceSupport(t *testing.T) { + ctrl := gomock.NewController(t) + c, vm, guest := newWCOWController(t, ctrl, false) + + c.netState = StateConfigured + ep1 := newWCOWEndpoint("eth0") + ep2 := newWCOWEndpoint("eth1") + c.vmEndpoints["nic-1"] = ep1 + c.vmEndpoints["nic-2"] = ep2 + + guest.EXPECT().RemoveNetworkInterface( + gomock.Any(), gomock.Any(), guestrequest.RequestTypeRemove, gomock.Nil(), + ).Return(nil).Times(2) + vm.EXPECT().RemoveNIC(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).Times(2) + + if err := c.Teardown(context.Background()); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if c.netState != StateTornDown { + t.Errorf("expected state TornDown, got %s", c.netState) + } + if len(c.vmEndpoints) != 0 { + t.Errorf("expected vmEndpoints to be empty, got %d entries", len(c.vmEndpoints)) + } +} + +// TestWCOW_Teardown_PartialFailure_RemainingAttempted verifies the cleanup +// chain: if removing one NIC fails, the controller must still attempt the +// remaining NICs. Surviving NICs leak the host UVM device. +func TestWCOW_Teardown_PartialFailure_RemainingAttempted(t *testing.T) { + ctrl := gomock.NewController(t) + c, vm, guest := newWCOWController(t, ctrl, false) + + c.netState = StateConfigured + ep1 := newWCOWEndpoint("eth0") + ep2 := newWCOWEndpoint("eth1") + ep3 := newWCOWEndpoint("eth2") + c.vmEndpoints["nic-1"] = ep1 + c.vmEndpoints["nic-2"] = ep2 + c.vmEndpoints["nic-3"] = ep3 + + // Fail nic-2; nic-1 and nic-3 succeed. + guest.EXPECT().RemoveNetworkInterface( + gomock.Any(), gomock.Any(), guestrequest.RequestTypeRemove, gomock.Nil(), + ).DoAndReturn( + func(_ context.Context, adapterID string, _ guestrequest.RequestType, _ *hcn.HostComputeEndpoint) error { + if adapterID == "nic-2" { + return errWCOWGuestRemove + } + return nil + }).Times(3) + // vm.RemoveNIC only runs when guest removal succeeds → 2 calls. + vm.EXPECT().RemoveNIC(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).Times(2) + + err := c.Teardown(context.Background()) + if !errors.Is(err, errWCOWGuestRemove) { + t.Fatalf("expected joined error to wrap guest remove failure, got: %v", err) + } + if c.netState != StateInvalid { + t.Errorf("expected state Invalid after partial failure, got %s", c.netState) + } + if _, ok := c.vmEndpoints["nic-2"]; !ok { + t.Error("expected nic-2 to remain tracked after its removal failed") + } + if _, ok := c.vmEndpoints["nic-1"]; ok { + t.Error("expected nic-1 to be removed from tracking") + } + if _, ok := c.vmEndpoints["nic-3"]; ok { + t.Error("expected nic-3 to be removed from tracking") + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// Half-setup recovery: Teardown unwinds host-side state after partial failures +// ───────────────────────────────────────────────────────────────────────────── + +// TestWCOW_AddEndpoint_FinalAddFails_TeardownUnwindsHost covers the +// half-setup recovery contract end-to-end: PreAdd ok, host AddNIC ok, guest +// Add fails. The NIC must remain tracked so a subsequent Teardown can +// unwind the host-side device. Otherwise the UVM leaks the NIC. +func TestWCOW_AddEndpoint_FinalAddFails_TeardownUnwindsHost(t *testing.T) { + ctrl := gomock.NewController(t) + c, vm, guest := newWCOWController(t, ctrl, false) + + ep := newWCOWEndpoint("eth0") + + gomock.InOrder( + guest.EXPECT().AddNetworkInterface( + gomock.Any(), "nic-1", guestrequest.RequestTypePreAdd, ep, + ).Return(nil), + vm.EXPECT().AddNIC(gomock.Any(), "nic-1", gomock.Any()).Return(nil), + guest.EXPECT().AddNetworkInterface( + gomock.Any(), "nic-1", guestrequest.RequestTypeAdd, gomock.Nil(), + ).Return(errWCOWGuestAdd), + ) + + if err := c.addEndpointToGuestNamespace(context.Background(), "nic-1", ep, false); !errors.Is(err, errWCOWGuestAdd) { + t.Fatalf("expected guest add error to wrap, got: %v", err) + } + if _, ok := c.vmEndpoints["nic-1"]; !ok { + t.Fatal("expected nic-1 to be tracked after guest-side failure so Teardown can unwind the host NIC") + } + + // Teardown: guest Remove (best-effort), then host RemoveNIC. + c.netState = StateConfigured + gomock.InOrder( + guest.EXPECT().RemoveNetworkInterface( + gomock.Any(), "nic-1", guestrequest.RequestTypeRemove, gomock.Nil(), + ).Return(nil), + vm.EXPECT().RemoveNIC(gomock.Any(), "nic-1", gomock.Any()).Return(nil), + ) + + if err := c.Teardown(context.Background()); err != nil { + t.Fatalf("Teardown after half-setup: %v", err) + } + if c.netState != StateTornDown { + t.Errorf("expected state TornDown after recovery, got %s", c.netState) + } + if _, ok := c.vmEndpoints["nic-1"]; ok { + t.Error("expected nic-1 to be cleared after successful Teardown") + } +} + +// TestWCOW_Teardown_GuestFails_RetryFromInvalid covers the half-setup, +// failure-in-teardown, then success-teardown sequence for WCOW. First +// Teardown fails on guest removal -> state Invalid, NIC stays tracked. Second +// Teardown call (allowed from StateInvalid) drains the endpoint and reaches +// StateTornDown. +func TestWCOW_Teardown_GuestFails_RetryFromInvalid(t *testing.T) { + ctrl := gomock.NewController(t) + c, vm, guest := newWCOWController(t, ctrl, false) + + c.netState = StateConfigured + ep := newWCOWEndpoint("eth0") + c.vmEndpoints["nic-1"] = ep + + // First Teardown: guest remove fails → state Invalid, NIC still tracked. + guest.EXPECT().RemoveNetworkInterface( + gomock.Any(), "nic-1", guestrequest.RequestTypeRemove, gomock.Nil(), + ).Return(errWCOWGuestRemove) + + if err := c.Teardown(context.Background()); !errors.Is(err, errWCOWGuestRemove) { + t.Fatalf("first Teardown: expected guest remove error, got: %v", err) + } + if c.netState != StateInvalid { + t.Fatalf("first Teardown: expected StateInvalid, got %s", c.netState) + } + if _, ok := c.vmEndpoints["nic-1"]; !ok { + t.Fatal("first Teardown: nic-1 must remain tracked for retry") + } + + // Second Teardown: both legs succeed → state TornDown, map cleared. + gomock.InOrder( + guest.EXPECT().RemoveNetworkInterface( + gomock.Any(), "nic-1", guestrequest.RequestTypeRemove, gomock.Nil(), + ).Return(nil), + vm.EXPECT().RemoveNIC(gomock.Any(), "nic-1", gomock.Any()).Return(nil), + ) + + if err := c.Teardown(context.Background()); err != nil { + t.Fatalf("retry Teardown: %v", err) + } + if c.netState != StateTornDown { + t.Errorf("retry Teardown: expected StateTornDown, got %s", c.netState) + } + if _, ok := c.vmEndpoints["nic-1"]; ok { + t.Error("retry Teardown: expected nic-1 to be cleared") + } +}