From 691a9a64948d01bb121e1eee4fdefec79835efd3 Mon Sep 17 00:00:00 2001 From: Harsh Rawat Date: Wed, 18 Mar 2026 02:51:55 +0530 Subject: [PATCH 1/3] [shimV2] added network controller implementation This change adds the network controller implementation for V2 shims which manages the network lifecycle for a single pod running inside a UVM. Signed-off-by: Harsh Rawat --- internal/controller/network/doc.go | 42 ++++ internal/controller/network/interface.go | 39 +++ internal/controller/network/network.go | 259 ++++++++++++++++++++ internal/controller/network/network_lcow.go | 97 ++++++++ internal/controller/network/network_wcow.go | 130 ++++++++++ internal/controller/network/state.go | 66 +++++ internal/logfields/fields.go | 1 + internal/protocol/guestresource/parse.go | 78 ++++++ internal/uvm/network.go | 69 +----- 9 files changed, 713 insertions(+), 68 deletions(-) create mode 100644 internal/controller/network/doc.go create mode 100644 internal/controller/network/interface.go create mode 100644 internal/controller/network/network.go create mode 100644 internal/controller/network/network_lcow.go create mode 100644 internal/controller/network/network_wcow.go create mode 100644 internal/controller/network/state.go create mode 100644 internal/protocol/guestresource/parse.go diff --git a/internal/controller/network/doc.go b/internal/controller/network/doc.go new file mode 100644 index 0000000000..772074d408 --- /dev/null +++ b/internal/controller/network/doc.go @@ -0,0 +1,42 @@ +//go:build windows + +// Package network provides a controller for managing the network lifecycle of a pod +// running inside a Utility VM (UVM). +// +// It handles attaching an HCN namespace and its endpoints to the guest VM, +// and tearing them down on pod removal. The [Controller] interface is the +// primary entry point, with [Manager] as its concrete implementation. +// +// # Lifecycle +// +// A network follows the state machine below. +// +// ┌────────────────────┐ +// │ StateNotConfigured │ +// └───┬────────────┬───┘ +// Setup ok │ │ Setup fails +// ▼ ▼ +// ┌─────────────────┐ ┌──────────────┐ +// │ StateConfigured │ │ StateInvalid │ +// └────────┬────────┘ └──────┬───────┘ +// │ Teardown │ Teardown +// ▼ ▼ +// ┌─────────────────────────────────────┐ +// │ StateTornDown │ +// └─────────────────────────────────────┘ +// +// State descriptions: +// +// - [StateNotConfigured]: initial state; no namespace or NICs have been configured. +// - [StateConfigured]: after [Controller.Setup] succeeds; the HCN namespace is attached +// and all endpoints are wired up inside the guest. +// - [StateInvalid]: entered when [Controller.Setup] fails mid-way; best-effort +// cleanup should be performed via [Controller.Teardown]. +// - [StateTornDown]: terminal state reached after [Controller.Teardown] completes. +// +// # Platform Variants +// +// Guest-side operations differ between LCOW and WCOW and are implemented in +// platform-specific source files selected via build tags +// (default for LCOW shim, "wcow" tag for WCOW shim). +package network diff --git a/internal/controller/network/interface.go b/internal/controller/network/interface.go new file mode 100644 index 0000000000..5b6302c8f4 --- /dev/null +++ b/internal/controller/network/interface.go @@ -0,0 +1,39 @@ +//go:build windows + +package network + +import ( + "context" + + "github.com/Microsoft/hcsshim/internal/gcs" +) + +// Controller manages the network lifecycle for a single pod running inside a UVM. +type Controller interface { + // Setup attaches the HCN namespace and its endpoints to the guest VM. + Setup(ctx context.Context, opts *SetupOptions) error + + // Teardown removes all guest-side NICs and the network namespace from the VM. + // It is idempotent: calling it on an already torn-down or unconfigured network is a no-op. + Teardown(ctx context.Context) error +} + +// SetupOptions holds the configuration required to set up the network for a pod. +type SetupOptions struct { + // PodID is the identifier of the pod whose network is being configured. + PodID string + + // NetworkNamespace is the HCN namespace ID to attach to the guest. + NetworkNamespace string + + // PolicyBasedRouting controls whether policy-based routing is configured + // for the endpoints added to the guest. Only relevant for LCOW. + PolicyBasedRouting bool +} + +// capabilitiesProvider is a narrow interface satisfied by guestmanager.Manager. +// It exists so callers pass the guest manager scoped only to Capabilities(), +// avoiding a hard dependency on the full guestmanager.Manager interface here. +type capabilitiesProvider interface { + Capabilities() gcs.GuestDefinedCapabilities +} diff --git a/internal/controller/network/network.go b/internal/controller/network/network.go new file mode 100644 index 0000000000..55d2e9c58a --- /dev/null +++ b/internal/controller/network/network.go @@ -0,0 +1,259 @@ +//go:build windows + +package network + +import ( + "context" + "errors" + "fmt" + "slices" + "strings" + "sync" + + "github.com/Microsoft/hcsshim/hcn" + "github.com/Microsoft/hcsshim/internal/log" + "github.com/Microsoft/hcsshim/internal/logfields" + "github.com/Microsoft/hcsshim/internal/vm/guestmanager" + "github.com/Microsoft/hcsshim/internal/vm/vmmanager" + + "github.com/Microsoft/go-winio/pkg/guid" + "github.com/sirupsen/logrus" +) + +// Manager is the concrete implementation of [Controller]. +type Manager struct { + mu sync.Mutex + + // podID is the identifier of the pod whose network this Controller manages. + podID string + + // namespaceID is the HCN namespace ID in use after a successful Setup. + namespaceID string + + // vmEndpoints maps nicID (ID within UVM) -> HCN endpoint. + vmEndpoints map[string]*hcn.HostComputeEndpoint + + // netState is the current lifecycle state of the network. + netState State + + // isNamespaceSupportedByGuest determines if network namespace is supported inside the guest + isNamespaceSupportedByGuest bool + + // vmNetManager performs host-side NIC hot-add/remove on the UVM. + vmNetManager vmmanager.NetworkManager + + // linuxGuestMgr performs guest-side NIC inject/remove for LCOW. + linuxGuestMgr guestmanager.LCOWNetworkManager + + // winGuestMgr performs guest-side NIC/namespace operations for WCOW. + winGuestMgr guestmanager.WCOWNetworkManager + + // capsProvider exposes the guest's declared capabilities. + // Used to check IsNamespaceAddRequestSupported. + capsProvider capabilitiesProvider +} + +// Assert that Manager implements Controller. +var _ Controller = (*Manager)(nil) + +// New creates a ready-to-use Manager in [StateNotConfigured]. +// +// This method is called from [VMController.CreateNetworkController()] +// which injects the necessary dependencies. +func New( + vmNetManager vmmanager.NetworkManager, + linuxGuestMgr guestmanager.LCOWNetworkManager, + windowsGuestMgr guestmanager.WCOWNetworkManager, + capsProvider capabilitiesProvider, +) *Manager { + m := &Manager{ + vmNetManager: vmNetManager, + linuxGuestMgr: linuxGuestMgr, + winGuestMgr: windowsGuestMgr, + capsProvider: capsProvider, + netState: StateNotConfigured, + vmEndpoints: make(map[string]*hcn.HostComputeEndpoint), + } + + // Cache once at construction so hot-add paths can branch without re-querying. + if caps := capsProvider.Capabilities(); caps != nil { + m.isNamespaceSupportedByGuest = caps.IsNamespaceAddRequestSupported() + } + + return m +} + +// Setup attaches the requested HCN namespace to the guest VM +// and hot-adds all endpoints found in that namespace. +// It must be called only once; subsequent calls return an error. +func (m *Manager) Setup(ctx context.Context, opts *SetupOptions) (err error) { + ctx, _ = log.WithContext(ctx, logrus.WithField(logfields.Operation, "Network Setup")) + + m.mu.Lock() + defer m.mu.Unlock() + + log.G(ctx).WithFields(logrus.Fields{ + logfields.PodID: opts.PodID, + logfields.Namespace: opts.NetworkNamespace, + }).Debug("starting network setup") + + // If Setup has already been called, then error out. + if m.netState != StateNotConfigured { + return fmt.Errorf("cannot set up network in state %s", m.netState) + } + + defer func() { + if err != nil { + // If setup fails for any reason, move to invalid so no further + // Setup calls are accepted. + m.netState = StateInvalid + log.G(ctx).WithError(err).Error("network setup failed, moving to invalid state") + } + }() + + if opts.NetworkNamespace == "" { + return fmt.Errorf("network namespace must not be empty") + } + + // Validate that the provided namespace exists. + hcnNamespace, err := hcn.GetNamespaceByID(opts.NetworkNamespace) + if err != nil { + return fmt.Errorf("get network namespace %s: %w", opts.NetworkNamespace, err) + } + + // Fetch all endpoints in the namespace. + endpoints, err := m.fetchEndpointsInNamespace(ctx, hcnNamespace) + if err != nil { + return fmt.Errorf("fetch endpoints in namespace %s: %w", hcnNamespace.Id, err) + } + + // Add the namespace to the guest. + if err = m.addNetNSInsideGuest(ctx, hcnNamespace); err != nil { + return fmt.Errorf("add network namespace to guest: %w", err) + } + + // Hot-add all endpoints in the namespace to the guest. + for _, endpoint := range endpoints { + nicGUID, err := guid.NewV4() + if err != nil { + return fmt.Errorf("generate NIC GUID: %w", err) + } + if err = m.addEndpointToGuestNamespace(ctx, nicGUID.String(), endpoint, opts.PolicyBasedRouting); err != nil { + return fmt.Errorf("add endpoint %s to guest: %w", endpoint.Name, err) + } + } + + m.podID = opts.PodID + m.namespaceID = hcnNamespace.Id + m.netState = StateConfigured + + log.G(ctx).WithFields(logrus.Fields{ + logfields.PodID: opts.PodID, + logfields.Namespace: hcnNamespace.Id, + }).Info("network setup completed successfully") + + return nil +} + +// Teardown removes all guest-side NICs and the HCN namespace from the UVM. +// +// It is idempotent: calling it when the network is already torn down or not yet +// configured is a no-op. +func (m *Manager) Teardown(ctx context.Context) error { + ctx, _ = log.WithContext(ctx, logrus.WithField(logfields.Operation, "Network Teardown")) + + m.mu.Lock() + defer m.mu.Unlock() + + log.G(ctx).WithFields(logrus.Fields{ + logfields.PodID: m.podID, + logfields.Namespace: m.namespaceID, + "State": m.netState, + }).Debug("starting network teardown") + + if m.netState == StateTornDown { + // Teardown is idempotent, so return nil if already torn down. + log.G(ctx).Info("network already torn down, skipping") + return nil + } + + if m.netState == StateNotConfigured { + // Nothing was configured; nothing to clean up. + log.G(ctx).Info("network not configured, skipping") + return nil + } + + // Remove all endpoints from the guest. + // Use a continue-on-error strategy: attempt every NIC regardless of individual + // failures, then collect all errors. + var teardownErrs []error + for nicID, endpoint := range m.vmEndpoints { + if err := m.removeEndpointFromGuestNamespace(ctx, nicID, endpoint); err != nil { + teardownErrs = append(teardownErrs, fmt.Errorf("remove endpoint %s from guest: %w", endpoint.Name, err)) + continue // continue attempting to remove other endpoints + } + + delete(m.vmEndpoints, nicID) + } + + if err := m.removeNetNSInsideGuest(ctx, m.namespaceID); err != nil { + teardownErrs = append(teardownErrs, fmt.Errorf("remove network namespace from guest: %w", err)) + } + + if len(teardownErrs) > 0 { + // If any errors were encountered during teardown, mark the state as invalid. + m.netState = StateInvalid + return errors.Join(teardownErrs...) + } + + // Mark as torn down if we do not encounter any errors. + // No further Setup or Teardown calls are allowed. + m.netState = StateTornDown + + log.G(ctx).WithFields(logrus.Fields{ + logfields.PodID: m.podID, + "networkNamespace": m.namespaceID, + }).Info("network teardown completed successfully") + + return nil +} + +// fetchEndpointsInNamespace retrieves all HCN endpoints present in +// the given namespace. +// Endpoints are sorted so that those with names ending in "eth0" appear first. +func (m *Manager) fetchEndpointsInNamespace(ctx context.Context, ns *hcn.HostComputeNamespace) ([]*hcn.HostComputeEndpoint, error) { + ctx, _ = log.WithContext(ctx, logrus.WithField(logfields.Namespace, ns.Id)) + log.G(ctx).Info("fetching endpoints from the network namespace") + + ids, err := hcn.GetNamespaceEndpointIds(ns.Id) + if err != nil { + return nil, fmt.Errorf("get endpoint IDs for namespace %s: %w", ns.Id, err) + } + endpoints := make([]*hcn.HostComputeEndpoint, 0, len(ids)) + for _, id := range ids { + ep, err := hcn.GetEndpointByID(id) + if err != nil { + return nil, fmt.Errorf("get endpoint %s: %w", id, err) + } + endpoints = append(endpoints, ep) + } + + // Ensure the endpoint named "eth0" is added first when multiple endpoints are present, + // so it maps to eth0 inside the guest. CNI results aren't available here, so we rely + // on the endpoint name suffix as a heuristic. + cmp := func(a, b *hcn.HostComputeEndpoint) int { + if strings.HasSuffix(a.Name, "eth0") { + return -1 + } + if strings.HasSuffix(b.Name, "eth0") { + return 1 + } + return 0 + } + + slices.SortStableFunc(endpoints, cmp) + + log.G(ctx).Tracef("fetched endpoints from the network namespace %+v", endpoints) + + return endpoints, nil +} diff --git a/internal/controller/network/network_lcow.go b/internal/controller/network/network_lcow.go new file mode 100644 index 0000000000..1fd73dc7fb --- /dev/null +++ b/internal/controller/network/network_lcow.go @@ -0,0 +1,97 @@ +//go:build windows && !wcow + +package network + +import ( + "context" + "fmt" + + "github.com/Microsoft/hcsshim/hcn" + hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" + "github.com/Microsoft/hcsshim/internal/log" + "github.com/Microsoft/hcsshim/internal/protocol/guestresource" + + "github.com/sirupsen/logrus" +) + +// addNetNSInsideGuest maps a host network namespace into the guest as a managed Guest Network Namespace. +// This is a no-op for LCOW as the network namespace is created via pause container +// and the adapters are added dynamically. +func (m *Manager) addNetNSInsideGuest(_ context.Context, _ *hcn.HostComputeNamespace) error { + return nil +} + +// removeNetNSInsideGuest is a no-op for LCOW; the guest-managed namespace +// is torn down automatically when pause container exits. +func (m *Manager) removeNetNSInsideGuest(_ context.Context, _ string) error { + return nil +} + +// addEndpointToGuestNamespace hot-adds an HCN endpoint to the UVM and, +// configures it inside the LCOW guest. +func (m *Manager) addEndpointToGuestNamespace(ctx context.Context, nicID string, endpoint *hcn.HostComputeEndpoint, isPolicyBasedRoutingSupported bool) error { + ctx, _ = log.WithContext(ctx, logrus.WithFields(logrus.Fields{"nicID": nicID, "endpointID": endpoint.Id})) + log.G(ctx).Info("adding endpoint to guest namespace") + + // 1. Host-side hot-add. + if err := m.vmNetManager.AddNIC(ctx, nicID, &hcsschema.NetworkAdapter{ + EndpointId: endpoint.Id, + MacAddress: endpoint.MacAddress, + }); err != nil { + return fmt.Errorf("add NIC %s to host (endpoint %s): %w", nicID, endpoint.Id, err) + } + + log.G(ctx).Debug("added NIC to host") + + // Track early so Teardown cleans up even if the guest Add call fails. + m.vmEndpoints[nicID] = endpoint + + // 2. Guest-side add. + if m.isNamespaceSupportedByGuest { + lcowAdapter, err := guestresource.BuildLCOWNetworkAdapter(nicID, endpoint, isPolicyBasedRoutingSupported) + if err != nil { + return fmt.Errorf("build LCOW network adapter for endpoint %s: %w", endpoint.Id, err) + } + + log.G(ctx).Tracef("built LCOW network adapter: %+v", lcowAdapter) + + if err := m.linuxGuestMgr.AddLCOWNetworkInterface(ctx, lcowAdapter); err != nil { + return fmt.Errorf("add NIC %s to guest (endpoint %s): %w", nicID, endpoint.Id, err) + } + + log.G(ctx).Debug("nic configured in guest") + } + + return nil +} + +// removeEndpointFromGuestNamespace removes an endpoint from the LCOW guest +// and then hot-removes the NIC from the host. +func (m *Manager) removeEndpointFromGuestNamespace(ctx context.Context, nicID string, endpoint *hcn.HostComputeEndpoint) error { + ctx, _ = log.WithContext(ctx, logrus.WithFields(logrus.Fields{"nicID": nicID, "endpointID": endpoint.Id})) + log.G(ctx).Info("removing endpoint from guest namespace") + + if m.isNamespaceSupportedByGuest { + // 1. LCOW guest-side removal. + if err := m.linuxGuestMgr.RemoveLCOWNetworkInterface(ctx, &guestresource.LCOWNetworkAdapter{ + NamespaceID: m.namespaceID, + ID: nicID, + }); err != nil { + return fmt.Errorf("remove NIC %s from guest: %w", nicID, err) + } + + log.G(ctx).Debug("removed NIC from guest") + } + + // 2. Host-side removal. + if err := m.vmNetManager.RemoveNIC(ctx, nicID, &hcsschema.NetworkAdapter{ + EndpointId: endpoint.Id, + MacAddress: endpoint.MacAddress, + }); err != nil { + return fmt.Errorf("remove NIC %s from host (endpoint %s): %w", nicID, endpoint.Id, err) + } + + log.G(ctx).Debug("removed NIC from host") + + return nil +} diff --git a/internal/controller/network/network_wcow.go b/internal/controller/network/network_wcow.go new file mode 100644 index 0000000000..f3a4dda84a --- /dev/null +++ b/internal/controller/network/network_wcow.go @@ -0,0 +1,130 @@ +//go:build windows && wcow + +package network + +import ( + "context" + "fmt" + + "github.com/Microsoft/hcsshim/hcn" + hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" + "github.com/Microsoft/hcsshim/internal/log" + "github.com/Microsoft/hcsshim/internal/logfields" + "github.com/Microsoft/hcsshim/internal/protocol/guestrequest" + + "github.com/sirupsen/logrus" +) + +// addNetNSInsideGuest maps a host network namespace into the guest as a managed Guest Network Namespace. +// For WCOWs, this method sends a request to GCS for adding the namespace. +// GCS forwards the request to GNS which coordinates with HNS to add the namespace to the guest. +func (m *Manager) addNetNSInsideGuest(ctx context.Context, hcnNamespace *hcn.HostComputeNamespace) error { + ctx, _ = log.WithContext(ctx, logrus.WithField(logfields.Namespace, hcnNamespace.Id)) + + if m.isNamespaceSupportedByGuest { + log.G(ctx).Info("adding network namespace to guest") + + if err := m.winGuestMgr.AddNetworkNamespace(ctx, hcnNamespace); err != nil { + return fmt.Errorf("add network namespace %s to guest: %w", hcnNamespace.Id, err) + } + } + + return nil +} + +// removeNetNSInsideGuest removes the HCN namespace from the WCOW guest via GCS/GNS. +func (m *Manager) removeNetNSInsideGuest(ctx context.Context, namespaceID string) error { + ctx, _ = log.WithContext(ctx, logrus.WithField(logfields.Namespace, namespaceID)) + + if m.isNamespaceSupportedByGuest { + log.G(ctx).Info("removing network namespace from guest") + + hcnNamespace, err := hcn.GetNamespaceByID(namespaceID) + if err != nil { + return fmt.Errorf("get network namespace %s: %w", namespaceID, err) + } + + if err := m.winGuestMgr.RemoveNetworkNamespace(ctx, hcnNamespace); err != nil { + return fmt.Errorf("remove network namespace %s from guest: %w", namespaceID, err) + } + } + + return nil +} + +// addEndpointToGuestNamespace wires an HCN endpoint into the WCOW guest in three steps: +// pre-add (guest notification), host-side hot-add, and guest-side finalisation. +func (m *Manager) addEndpointToGuestNamespace(ctx context.Context, nicID string, endpoint *hcn.HostComputeEndpoint, _ bool) error { + ctx, _ = log.WithContext(ctx, logrus.WithFields(logrus.Fields{"nicID": nicID, "endpointID": endpoint.Id})) + log.G(ctx).Info("adding network endpoint to guest namespace") + + // 1. Guest pre-add: informs WCOW guest that a NIC is about to arrive. + if err := m.winGuestMgr.AddNetworkInterface( + ctx, + nicID, + guestrequest.RequestTypePreAdd, + endpoint, + ); err != nil { + return fmt.Errorf("pre-add NIC %s to guest (endpoint %s): %w", nicID, endpoint.Id, err) + } + + log.G(ctx).Info("pre-added network endpoint to guest namespace") + + // 2. Host-side hot-add. + if err := m.vmNetManager.AddNIC(ctx, nicID, &hcsschema.NetworkAdapter{ + EndpointId: endpoint.Id, + MacAddress: endpoint.MacAddress, + }); err != nil { + return fmt.Errorf("add NIC %s to host (endpoint %s): %w", nicID, endpoint.Id, err) + } + + log.G(ctx).Info("hot-added network endpoint to host") + + // Track early so Teardown cleans up even if the guest Add call fails. + m.vmEndpoints[nicID] = endpoint + + // 3. Guest add: finalise the NIC in the WCOW guest. + if err := m.winGuestMgr.AddNetworkInterface( + ctx, + nicID, + guestrequest.RequestTypeAdd, + nil, // No additional info is needed for the Add call. + ); err != nil { + return fmt.Errorf("add NIC %s to guest (endpoint %s): %w", nicID, endpoint.Id, err) + } + + log.G(ctx).Info("configured network endpoint in guest namespace") + + return nil +} + +// removeEndpointFromGuestNamespace removes an endpoint from the WCOW guest and then +// hot-removes the NIC from the host. +func (m *Manager) removeEndpointFromGuestNamespace(ctx context.Context, nicID string, endpoint *hcn.HostComputeEndpoint) error { + ctx, _ = log.WithContext(ctx, logrus.WithFields(logrus.Fields{"nicID": nicID, "endpointID": endpoint.Id})) + log.G(ctx).Info("removing network endpoint from guest namespace") + + // 1. Guest-side removal. + if err := m.winGuestMgr.RemoveNetworkInterface( + ctx, + nicID, + guestrequest.RequestTypeRemove, + nil, + ); err != nil { + return fmt.Errorf("remove NIC %s from guest (endpoint %s): %w", nicID, endpoint.Id, err) + } + + log.G(ctx).Info("removed network endpoint from guest namespace") + + // 2. Host-side removal. + if err := m.vmNetManager.RemoveNIC(ctx, nicID, &hcsschema.NetworkAdapter{ + EndpointId: endpoint.Id, + MacAddress: endpoint.MacAddress, + }); err != nil { + return fmt.Errorf("remove NIC %s from host (endpoint %s): %w", nicID, endpoint.Id, err) + } + + log.G(ctx).Info("removed network endpoint from host") + + return nil +} diff --git a/internal/controller/network/state.go b/internal/controller/network/state.go new file mode 100644 index 0000000000..1116b78753 --- /dev/null +++ b/internal/controller/network/state.go @@ -0,0 +1,66 @@ +//go:build windows + +package network + +// State represents the current lifecycle state of the network for a pod. +// +// The normal progression is: +// +// StateNotConfigured → StateConfigured → StateTornDown +// +// If an unrecoverable error occurs during [Controller.Setup], the network +// transitions to [StateInvalid] instead. +// A network in [StateInvalid] can only be cleaned up via [Controller.Teardown]. +// +// Full state-transition table: +// +// Current State │ Trigger │ Next State +// ────────────────────┼──────────────────┼────────────────── +// StateNotConfigured │ Setup succeeds │ StateConfigured +// StateNotConfigured │ Setup fails │ StateInvalid +// StateConfigured │ Teardown called │ StateTornDown +// StateInvalid │ Teardown called │ StateTornDown +// StateTornDown │ (terminal) │ — +type State int32 + +const ( + // StateNotConfigured is the initial state: no namespace has been attached + // and no NICs have been added. + // Valid transitions: + // - StateNotConfigured → StateConfigured (via [Controller.Setup], on success) + // - StateNotConfigured → StateInvalid (via [Controller.Setup], on failure) + StateNotConfigured State = iota + + // StateConfigured indicates the network is fully operational: the HCN namespace + // is attached, all endpoints are wired up, and guest-side NICs are hot-added. + // Valid transition: + // - StateConfigured → StateTornDown (via [Controller.Teardown]) + StateConfigured + + // StateInvalid indicates an unrecoverable error occurred during [Controller.Setup]. + // Teardown must be called to attempt best-effort cleanup. + // Valid transition: + // - StateInvalid → StateTornDown (via [Controller.Teardown]) + StateInvalid + + // StateTornDown is the terminal state reached after Teardown completes + // (regardless of whether Setup previously succeeded or failed). + // No further calls to Setup or Teardown are permitted. + StateTornDown +) + +// String returns a human-readable string representation of the network State. +func (s State) String() string { + switch s { + case StateNotConfigured: + return "NotConfigured" + case StateConfigured: + return "Configured" + case StateInvalid: + return "Invalid" + case StateTornDown: + return "TornDown" + default: + return "Unknown" + } +} diff --git a/internal/logfields/fields.go b/internal/logfields/fields.go index dac5a708e5..0469388e70 100644 --- a/internal/logfields/fields.go +++ b/internal/logfields/fields.go @@ -14,6 +14,7 @@ const ( TaskID = "tid" UVMID = "uvm-id" SandboxID = "sandbox-id" + PodID = "pod-id" // networking and IO diff --git a/internal/protocol/guestresource/parse.go b/internal/protocol/guestresource/parse.go new file mode 100644 index 0000000000..61e6683542 --- /dev/null +++ b/internal/protocol/guestresource/parse.go @@ -0,0 +1,78 @@ +//go:build windows + +package guestresource + +import ( + "encoding/json" + "fmt" + "strings" + + "github.com/Microsoft/hcsshim/hcn" + + "github.com/samber/lo" +) + +// BuildLCOWNetworkAdapter converts an HCN endpoint into the guestresource.LCOWNetworkAdapter +// payload that the GCS expects. +func BuildLCOWNetworkAdapter(nicID string, endpoint *hcn.HostComputeEndpoint, policyBasedRouting bool) (*LCOWNetworkAdapter, error) { + req := &LCOWNetworkAdapter{ + NamespaceID: endpoint.HostComputeNamespace, + ID: nicID, + MacAddress: endpoint.MacAddress, + IPConfigs: make([]LCOWIPConfig, 0, len(endpoint.IpConfigurations)), + Routes: make([]LCOWRoute, 0, len(endpoint.Routes)), + } + + for _, i := range endpoint.IpConfigurations { + ipConfig := LCOWIPConfig{ + IPAddress: i.IpAddress, + PrefixLength: i.PrefixLength, + } + req.IPConfigs = append(req.IPConfigs, ipConfig) + } + + for _, r := range endpoint.Routes { + newRoute := LCOWRoute{ + DestinationPrefix: r.DestinationPrefix, + NextHop: r.NextHop, + Metric: r.Metric, + } + req.Routes = append(req.Routes, newRoute) + } + + // !NOTE: + // the `DNSSuffix` field is explicitly used as the search list for host-name lookup in + // the guest's `resolv.conf`, and not as the DNS suffix. + // The name is a legacy hold over. + + // use DNS domain as the first (default) search value, if it is provided + searches := endpoint.Dns.Search + if endpoint.Dns.Domain != "" { + searches = append([]string{endpoint.Dns.Domain}, searches...) + } + + // canonicalize the DNS config + canon := func(s string, _ int) string { + // zone identifiers in IPv6 addresses really, really shouldn't be case sensitive, but ... *shrug* + return strings.ToLower(s) + } + servers := lo.Map(endpoint.Dns.ServerList, canon) + searches = lo.Map(searches, canon) + + req.DNSSuffix = strings.Join(searches, ",") + req.DNSServerList = strings.Join(servers, ",") + + for _, p := range endpoint.Policies { + if p.Type == hcn.EncapOverhead { + var settings hcn.EncapOverheadEndpointPolicySetting + if err := json.Unmarshal(p.Settings, &settings); err != nil { + return nil, fmt.Errorf("unmarshal encap overhead policy setting: %w", err) + } + req.EncapOverhead = settings.Overhead + } + } + + req.PolicyBasedRouting = policyBasedRouting + + return req, nil +} diff --git a/internal/uvm/network.go b/internal/uvm/network.go index 7836e1a7f1..6a00d84161 100644 --- a/internal/uvm/network.go +++ b/internal/uvm/network.go @@ -4,7 +4,6 @@ package uvm import ( "context" - "encoding/json" "fmt" "os" "slices" @@ -14,7 +13,6 @@ import ( "github.com/Microsoft/go-winio/pkg/guid" "github.com/containerd/ttrpc" "github.com/pkg/errors" - "github.com/samber/lo" "github.com/sirupsen/logrus" "github.com/Microsoft/hcsshim/hcn" @@ -554,71 +552,6 @@ func getNetworkModifyRequest(adapterID string, requestType guestrequest.RequestT } } -// convertToLCOWReq converts the HCN endpoint type to the guestresource.LCOWNetworkAdapter type that is -// passed to the GCS for a request. -func convertToLCOWReq(id string, endpoint *hcn.HostComputeEndpoint, policyBasedRouting bool) (*guestresource.LCOWNetworkAdapter, error) { - req := &guestresource.LCOWNetworkAdapter{ - NamespaceID: endpoint.HostComputeNamespace, - ID: id, - MacAddress: endpoint.MacAddress, - IPConfigs: make([]guestresource.LCOWIPConfig, 0, len(endpoint.IpConfigurations)), - Routes: make([]guestresource.LCOWRoute, 0, len(endpoint.Routes)), - } - - for _, i := range endpoint.IpConfigurations { - ipConfig := guestresource.LCOWIPConfig{ - IPAddress: i.IpAddress, - PrefixLength: i.PrefixLength, - } - req.IPConfigs = append(req.IPConfigs, ipConfig) - } - - for _, r := range endpoint.Routes { - newRoute := guestresource.LCOWRoute{ - DestinationPrefix: r.DestinationPrefix, - NextHop: r.NextHop, - Metric: r.Metric, - } - req.Routes = append(req.Routes, newRoute) - } - - // !NOTE: - // the `DNSSuffix` field is explicitly used as the search list for host-name lookup in - // the guest's `resolv.conf`, and not as the DNS suffix. - // The name is a legacy hold over. - - // use DNS domain as the first (default) search value, if it is provided - searches := endpoint.Dns.Search - if endpoint.Dns.Domain != "" { - searches = append([]string{endpoint.Dns.Domain}, searches...) - } - - // canonicalize the DNS config - canon := func(s string, _ int) string { - // zone identifiers in IPv6 addresses really, really shouldn't be case sensitive, but ... *shrug* - return strings.ToLower(s) - } - servers := lo.Map(endpoint.Dns.ServerList, canon) - searches = lo.Map(searches, canon) - - req.DNSSuffix = strings.Join(searches, ",") - req.DNSServerList = strings.Join(servers, ",") - - for _, p := range endpoint.Policies { - if p.Type == hcn.EncapOverhead { - var settings hcn.EncapOverheadEndpointPolicySetting - if err := json.Unmarshal(p.Settings, &settings); err != nil { - return nil, fmt.Errorf("unmarshal encap overhead policy setting: %w", err) - } - req.EncapOverhead = settings.Overhead - } - } - - req.PolicyBasedRouting = policyBasedRouting - - return req, nil -} - // addNIC adds a nic to the Utility VM. func (uvm *UtilityVM) addNIC(ctx context.Context, id string, endpoint *hcn.HostComputeEndpoint) error { // First a pre-add. This is a guest-only request and is only done on Windows. @@ -658,7 +591,7 @@ func (uvm *UtilityVM) addNIC(ctx context.Context, id string, endpoint *hcn.HostC nil), } } else { - s, err := convertToLCOWReq(id, endpoint, uvm.policyBasedRouting) + s, err := guestresource.BuildLCOWNetworkAdapter(id, endpoint, uvm.policyBasedRouting) if err != nil { return err } From e901ab7866a1ef3d75ed3c144e9e562a2f01f1fd Mon Sep 17 00:00:00 2001 From: Harsh Rawat Date: Sat, 21 Mar 2026 00:29:54 +0530 Subject: [PATCH 2/3] review comments : 1 Signed-off-by: Harsh Rawat --- internal/controller/network/interface.go | 41 +++++++++++- internal/controller/network/network.go | 71 +++++++++------------ internal/controller/network/network_lcow.go | 4 -- internal/controller/network/network_wcow.go | 4 -- internal/vm/guestmanager/network_lcow.go | 10 --- internal/vm/guestmanager/network_wcow.go | 14 ---- internal/vm/vmmanager/network.go | 16 ----- 7 files changed, 67 insertions(+), 93 deletions(-) diff --git a/internal/controller/network/interface.go b/internal/controller/network/interface.go index 5b6302c8f4..01f6753f97 100644 --- a/internal/controller/network/interface.go +++ b/internal/controller/network/interface.go @@ -5,7 +5,11 @@ package network import ( "context" + "github.com/Microsoft/hcsshim/hcn" "github.com/Microsoft/hcsshim/internal/gcs" + hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" + "github.com/Microsoft/hcsshim/internal/protocol/guestrequest" + "github.com/Microsoft/hcsshim/internal/protocol/guestresource" ) // Controller manages the network lifecycle for a single pod running inside a UVM. @@ -20,9 +24,6 @@ type Controller interface { // SetupOptions holds the configuration required to set up the network for a pod. type SetupOptions struct { - // PodID is the identifier of the pod whose network is being configured. - PodID string - // NetworkNamespace is the HCN namespace ID to attach to the guest. NetworkNamespace string @@ -37,3 +38,37 @@ type SetupOptions struct { type capabilitiesProvider interface { Capabilities() gcs.GuestDefinedCapabilities } + +// vmNetworkManager manages adding and removing network adapters for a Utility VM. +// Implemented by vmmanager.UtilityVM. +type vmNetworkManager interface { + // AddNIC adds a network adapter to the Utility VM. `nicID` should be a string representation of a + // Windows GUID. + AddNIC(ctx context.Context, nicID string, settings *hcsschema.NetworkAdapter) error + + // RemoveNIC removes a network adapter from the Utility VM. `nicID` should be a string representation of a + // Windows GUID. + RemoveNIC(ctx context.Context, nicID string, settings *hcsschema.NetworkAdapter) error +} + +// linuxGuestNetworkManager exposes linux guest network operations. +// Implemented by guestmanager.Guest. +type linuxGuestNetworkManager interface { + // AddLCOWNetworkInterface adds a network interface to the LCOW guest. + AddLCOWNetworkInterface(ctx context.Context, settings *guestresource.LCOWNetworkAdapter) error + // RemoveLCOWNetworkInterface removes a network interface from the LCOW guest. + RemoveLCOWNetworkInterface(ctx context.Context, settings *guestresource.LCOWNetworkAdapter) error +} + +// windowsGuestNetworkManager exposes windows guest network operations. +// Implemented by guestmanager.Guest. +type windowsGuestNetworkManager interface { + // AddNetworkNamespace adds a network namespace to the WCOW guest. + AddNetworkNamespace(ctx context.Context, settings *hcn.HostComputeNamespace) error + // RemoveNetworkNamespace removes a network namespace from the WCOW guest. + RemoveNetworkNamespace(ctx context.Context, settings *hcn.HostComputeNamespace) error + // AddNetworkInterface adds a network interface to the WCOW guest. + AddNetworkInterface(ctx context.Context, adapterID string, requestType guestrequest.RequestType, settings *hcn.HostComputeEndpoint) error + // RemoveNetworkInterface removes a network interface from the WCOW guest. + RemoveNetworkInterface(ctx context.Context, adapterID string, requestType guestrequest.RequestType, settings *hcn.HostComputeEndpoint) error +} diff --git a/internal/controller/network/network.go b/internal/controller/network/network.go index 55d2e9c58a..aa60e7ee51 100644 --- a/internal/controller/network/network.go +++ b/internal/controller/network/network.go @@ -10,13 +10,10 @@ import ( "strings" "sync" + "github.com/Microsoft/go-winio/pkg/guid" "github.com/Microsoft/hcsshim/hcn" "github.com/Microsoft/hcsshim/internal/log" "github.com/Microsoft/hcsshim/internal/logfields" - "github.com/Microsoft/hcsshim/internal/vm/guestmanager" - "github.com/Microsoft/hcsshim/internal/vm/vmmanager" - - "github.com/Microsoft/go-winio/pkg/guid" "github.com/sirupsen/logrus" ) @@ -24,9 +21,6 @@ import ( type Manager struct { mu sync.Mutex - // podID is the identifier of the pod whose network this Controller manages. - podID string - // namespaceID is the HCN namespace ID in use after a successful Setup. namespaceID string @@ -40,13 +34,13 @@ type Manager struct { isNamespaceSupportedByGuest bool // vmNetManager performs host-side NIC hot-add/remove on the UVM. - vmNetManager vmmanager.NetworkManager + vmNetManager vmNetworkManager // linuxGuestMgr performs guest-side NIC inject/remove for LCOW. - linuxGuestMgr guestmanager.LCOWNetworkManager + linuxGuestMgr linuxGuestNetworkManager // winGuestMgr performs guest-side NIC/namespace operations for WCOW. - winGuestMgr guestmanager.WCOWNetworkManager + winGuestMgr windowsGuestNetworkManager // capsProvider exposes the guest's declared capabilities. // Used to check IsNamespaceAddRequestSupported. @@ -61,9 +55,9 @@ var _ Controller = (*Manager)(nil) // This method is called from [VMController.CreateNetworkController()] // which injects the necessary dependencies. func New( - vmNetManager vmmanager.NetworkManager, - linuxGuestMgr guestmanager.LCOWNetworkManager, - windowsGuestMgr guestmanager.WCOWNetworkManager, + vmNetManager vmNetworkManager, + linuxGuestMgr linuxGuestNetworkManager, + windowsGuestMgr windowsGuestNetworkManager, capsProvider capabilitiesProvider, ) *Manager { m := &Manager{ @@ -87,15 +81,12 @@ func New( // and hot-adds all endpoints found in that namespace. // It must be called only once; subsequent calls return an error. func (m *Manager) Setup(ctx context.Context, opts *SetupOptions) (err error) { - ctx, _ = log.WithContext(ctx, logrus.WithField(logfields.Operation, "Network Setup")) + ctx, _ = log.WithContext(ctx, logrus.WithField(logfields.Namespace, opts.NetworkNamespace)) m.mu.Lock() defer m.mu.Unlock() - log.G(ctx).WithFields(logrus.Fields{ - logfields.PodID: opts.PodID, - logfields.Namespace: opts.NetworkNamespace, - }).Debug("starting network setup") + log.G(ctx).Debug("starting network setup") // If Setup has already been called, then error out. if m.netState != StateNotConfigured { @@ -138,19 +129,18 @@ func (m *Manager) Setup(ctx context.Context, opts *SetupOptions) (err error) { if err != nil { return fmt.Errorf("generate NIC GUID: %w", err) } - if err = m.addEndpointToGuestNamespace(ctx, nicGUID.String(), endpoint, opts.PolicyBasedRouting); err != nil { + // add the nicID and endpointID to the context for trace. + nicCtx, _ := log.WithContext(ctx, logrus.WithFields(logrus.Fields{"vm_nic_id": nicGUID.String(), "hns_endpoint_id": endpoint.Id})) + + if err = m.addEndpointToGuestNamespace(nicCtx, nicGUID.String(), endpoint, opts.PolicyBasedRouting); err != nil { return fmt.Errorf("add endpoint %s to guest: %w", endpoint.Name, err) } } - m.podID = opts.PodID m.namespaceID = hcnNamespace.Id m.netState = StateConfigured - log.G(ctx).WithFields(logrus.Fields{ - logfields.PodID: opts.PodID, - logfields.Namespace: hcnNamespace.Id, - }).Info("network setup completed successfully") + log.G(ctx).Info("network setup completed successfully") return nil } @@ -160,16 +150,12 @@ func (m *Manager) Setup(ctx context.Context, opts *SetupOptions) (err error) { // It is idempotent: calling it when the network is already torn down or not yet // configured is a no-op. func (m *Manager) Teardown(ctx context.Context) error { - ctx, _ = log.WithContext(ctx, logrus.WithField(logfields.Operation, "Network Teardown")) + ctx, _ = log.WithContext(ctx, logrus.WithField(logfields.Namespace, m.namespaceID)) m.mu.Lock() defer m.mu.Unlock() - log.G(ctx).WithFields(logrus.Fields{ - logfields.PodID: m.podID, - logfields.Namespace: m.namespaceID, - "State": m.netState, - }).Debug("starting network teardown") + log.G(ctx).WithField("State", m.netState).Debug("starting network teardown") if m.netState == StateTornDown { // Teardown is idempotent, so return nil if already torn down. @@ -188,7 +174,10 @@ func (m *Manager) Teardown(ctx context.Context) error { // failures, then collect all errors. var teardownErrs []error for nicID, endpoint := range m.vmEndpoints { - if err := m.removeEndpointFromGuestNamespace(ctx, nicID, endpoint); err != nil { + // add the nicID and endpointID to the context for trace. + nicCtx, _ := log.WithContext(ctx, logrus.WithFields(logrus.Fields{"vm_nic_id": nicID, "hns_endpoint_id": endpoint.Id})) + + if err := m.removeEndpointFromGuestNamespace(nicCtx, nicID, endpoint); err != nil { teardownErrs = append(teardownErrs, fmt.Errorf("remove endpoint %s from guest: %w", endpoint.Name, err)) continue // continue attempting to remove other endpoints } @@ -196,24 +185,23 @@ func (m *Manager) Teardown(ctx context.Context) error { delete(m.vmEndpoints, nicID) } - if err := m.removeNetNSInsideGuest(ctx, m.namespaceID); err != nil { - teardownErrs = append(teardownErrs, fmt.Errorf("remove network namespace from guest: %w", err)) - } - if len(teardownErrs) > 0 { // If any errors were encountered during teardown, mark the state as invalid. m.netState = StateInvalid return errors.Join(teardownErrs...) } + if err := m.removeNetNSInsideGuest(ctx, m.namespaceID); err != nil { + // Mark the state as invalid so that we can retry teardown. + m.netState = StateInvalid + return fmt.Errorf("remove network namespace from guest: %w", err) + } + // Mark as torn down if we do not encounter any errors. // No further Setup or Teardown calls are allowed. m.netState = StateTornDown - log.G(ctx).WithFields(logrus.Fields{ - logfields.PodID: m.podID, - "networkNamespace": m.namespaceID, - }).Info("network teardown completed successfully") + log.G(ctx).Info("network teardown completed successfully") return nil } @@ -222,7 +210,6 @@ func (m *Manager) Teardown(ctx context.Context) error { // the given namespace. // Endpoints are sorted so that those with names ending in "eth0" appear first. func (m *Manager) fetchEndpointsInNamespace(ctx context.Context, ns *hcn.HostComputeNamespace) ([]*hcn.HostComputeEndpoint, error) { - ctx, _ = log.WithContext(ctx, logrus.WithField(logfields.Namespace, ns.Id)) log.G(ctx).Info("fetching endpoints from the network namespace") ids, err := hcn.GetNamespaceEndpointIds(ns.Id) @@ -239,8 +226,8 @@ func (m *Manager) fetchEndpointsInNamespace(ctx context.Context, ns *hcn.HostCom } // Ensure the endpoint named "eth0" is added first when multiple endpoints are present, - // so it maps to eth0 inside the guest. CNI results aren't available here, so we rely - // on the endpoint name suffix as a heuristic. + // so it maps to eth0 inside the pod network namespace within guest. + // CNI results aren't available here, so we rely on the endpoint name suffix as a heuristic. cmp := func(a, b *hcn.HostComputeEndpoint) int { if strings.HasSuffix(a.Name, "eth0") { return -1 diff --git a/internal/controller/network/network_lcow.go b/internal/controller/network/network_lcow.go index 1fd73dc7fb..ecc138e851 100644 --- a/internal/controller/network/network_lcow.go +++ b/internal/controller/network/network_lcow.go @@ -10,8 +10,6 @@ import ( hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" "github.com/Microsoft/hcsshim/internal/log" "github.com/Microsoft/hcsshim/internal/protocol/guestresource" - - "github.com/sirupsen/logrus" ) // addNetNSInsideGuest maps a host network namespace into the guest as a managed Guest Network Namespace. @@ -30,7 +28,6 @@ func (m *Manager) removeNetNSInsideGuest(_ context.Context, _ string) error { // addEndpointToGuestNamespace hot-adds an HCN endpoint to the UVM and, // configures it inside the LCOW guest. func (m *Manager) addEndpointToGuestNamespace(ctx context.Context, nicID string, endpoint *hcn.HostComputeEndpoint, isPolicyBasedRoutingSupported bool) error { - ctx, _ = log.WithContext(ctx, logrus.WithFields(logrus.Fields{"nicID": nicID, "endpointID": endpoint.Id})) log.G(ctx).Info("adding endpoint to guest namespace") // 1. Host-side hot-add. @@ -68,7 +65,6 @@ func (m *Manager) addEndpointToGuestNamespace(ctx context.Context, nicID string, // removeEndpointFromGuestNamespace removes an endpoint from the LCOW guest // and then hot-removes the NIC from the host. func (m *Manager) removeEndpointFromGuestNamespace(ctx context.Context, nicID string, endpoint *hcn.HostComputeEndpoint) error { - ctx, _ = log.WithContext(ctx, logrus.WithFields(logrus.Fields{"nicID": nicID, "endpointID": endpoint.Id})) log.G(ctx).Info("removing endpoint from guest namespace") if m.isNamespaceSupportedByGuest { diff --git a/internal/controller/network/network_wcow.go b/internal/controller/network/network_wcow.go index f3a4dda84a..9c4a2229ac 100644 --- a/internal/controller/network/network_wcow.go +++ b/internal/controller/network/network_wcow.go @@ -34,8 +34,6 @@ func (m *Manager) addNetNSInsideGuest(ctx context.Context, hcnNamespace *hcn.Hos // removeNetNSInsideGuest removes the HCN namespace from the WCOW guest via GCS/GNS. func (m *Manager) removeNetNSInsideGuest(ctx context.Context, namespaceID string) error { - ctx, _ = log.WithContext(ctx, logrus.WithField(logfields.Namespace, namespaceID)) - if m.isNamespaceSupportedByGuest { log.G(ctx).Info("removing network namespace from guest") @@ -55,7 +53,6 @@ func (m *Manager) removeNetNSInsideGuest(ctx context.Context, namespaceID string // addEndpointToGuestNamespace wires an HCN endpoint into the WCOW guest in three steps: // pre-add (guest notification), host-side hot-add, and guest-side finalisation. func (m *Manager) addEndpointToGuestNamespace(ctx context.Context, nicID string, endpoint *hcn.HostComputeEndpoint, _ bool) error { - ctx, _ = log.WithContext(ctx, logrus.WithFields(logrus.Fields{"nicID": nicID, "endpointID": endpoint.Id})) log.G(ctx).Info("adding network endpoint to guest namespace") // 1. Guest pre-add: informs WCOW guest that a NIC is about to arrive. @@ -101,7 +98,6 @@ func (m *Manager) addEndpointToGuestNamespace(ctx context.Context, nicID string, // removeEndpointFromGuestNamespace removes an endpoint from the WCOW guest and then // hot-removes the NIC from the host. func (m *Manager) removeEndpointFromGuestNamespace(ctx context.Context, nicID string, endpoint *hcn.HostComputeEndpoint) error { - ctx, _ = log.WithContext(ctx, logrus.WithFields(logrus.Fields{"nicID": nicID, "endpointID": endpoint.Id})) log.G(ctx).Info("removing network endpoint from guest namespace") // 1. Guest-side removal. diff --git a/internal/vm/guestmanager/network_lcow.go b/internal/vm/guestmanager/network_lcow.go index af1c7351a2..a6296dcfa6 100644 --- a/internal/vm/guestmanager/network_lcow.go +++ b/internal/vm/guestmanager/network_lcow.go @@ -11,16 +11,6 @@ import ( "github.com/Microsoft/hcsshim/internal/protocol/guestresource" ) -// LCOWNetworkManager exposes guest network operations. -type LCOWNetworkManager interface { - // AddLCOWNetworkInterface adds a network interface to the LCOW guest. - AddLCOWNetworkInterface(ctx context.Context, settings *guestresource.LCOWNetworkAdapter) error - // RemoveLCOWNetworkInterface removes a network interface from the LCOW guest. - RemoveLCOWNetworkInterface(ctx context.Context, settings *guestresource.LCOWNetworkAdapter) error -} - -var _ LCOWNetworkManager = (*Guest)(nil) - // AddLCOWNetworkInterface adds a network interface to the LCOW guest. func (gm *Guest) AddLCOWNetworkInterface(ctx context.Context, settings *guestresource.LCOWNetworkAdapter) error { modifyRequest := &hcsschema.ModifySettingRequest{ diff --git a/internal/vm/guestmanager/network_wcow.go b/internal/vm/guestmanager/network_wcow.go index e8d60f7f81..77fdc4f629 100644 --- a/internal/vm/guestmanager/network_wcow.go +++ b/internal/vm/guestmanager/network_wcow.go @@ -12,20 +12,6 @@ import ( "github.com/Microsoft/hcsshim/internal/protocol/guestresource" ) -// WCOWNetworkManager exposes guest network operations. -type WCOWNetworkManager interface { - // AddNetworkNamespace adds a network namespace to the WCOW guest. - AddNetworkNamespace(ctx context.Context, settings *hcn.HostComputeNamespace) error - // RemoveNetworkNamespace removes a network namespace from the WCOW guest. - RemoveNetworkNamespace(ctx context.Context, settings *hcn.HostComputeNamespace) error - // AddNetworkInterface adds a network interface to the WCOW guest. - AddNetworkInterface(ctx context.Context, adapterID string, requestType guestrequest.RequestType, settings *hcn.HostComputeEndpoint) error - // RemoveNetworkInterface removes a network interface from the WCOW guest. - RemoveNetworkInterface(ctx context.Context, adapterID string, requestType guestrequest.RequestType, settings *hcn.HostComputeEndpoint) error -} - -var _ WCOWNetworkManager = (*Guest)(nil) - // AddNetworkNamespace adds a network namespace in the guest. func (gm *Guest) AddNetworkNamespace(ctx context.Context, settings *hcn.HostComputeNamespace) error { modifyRequest := &hcsschema.ModifySettingRequest{ diff --git a/internal/vm/vmmanager/network.go b/internal/vm/vmmanager/network.go index 7486bc4c2f..e30488ba60 100644 --- a/internal/vm/vmmanager/network.go +++ b/internal/vm/vmmanager/network.go @@ -12,22 +12,6 @@ import ( "github.com/Microsoft/hcsshim/internal/protocol/guestrequest" ) -// NetworkManager manages adding and removing network adapters for a Utility VM. -type NetworkManager interface { - // AddNIC adds a network adapter to the Utility VM. `nicID` should be a string representation of a - // Windows GUID. - AddNIC(ctx context.Context, nicID string, settings *hcsschema.NetworkAdapter) error - - // RemoveNIC removes a network adapter from the Utility VM. `nicID` should be a string representation of a - // Windows GUID. - RemoveNIC(ctx context.Context, nicID string, settings *hcsschema.NetworkAdapter) error - - // UpdateNIC updates the configuration of a network adapter on the Utility VM. - UpdateNIC(ctx context.Context, nicID string, settings *hcsschema.NetworkAdapter) error -} - -var _ NetworkManager = (*UtilityVM)(nil) - func (uvm *UtilityVM) AddNIC(ctx context.Context, nicID string, settings *hcsschema.NetworkAdapter) error { request := hcsschema.ModifySettingRequest{ RequestType: guestrequest.RequestTypeAdd, From e7512e0cc1fcf9fbe25d9e6fa9fe01ad589c25c1 Mon Sep 17 00:00:00 2001 From: Shreyansh Sancheti Date: Wed, 25 Mar 2026 12:46:18 +0530 Subject: [PATCH 3/3] controller/network: add unit tests for LCOW network controller 15 tests covering teardown cleanup chains (partial NIC failure continues, failed NICs stay tracked for retry from Invalid state), state machine transitions (idempotent teardown, no-op on unconfigured, duplicate Setup rejected), LCOW endpoint add/remove operations (host-before-guest ordering via InOrder, BuildLCOWNetworkAdapter struct verification, guest-fail still tracked for cleanup, host-fail not tracked), nil-capabilities disabling guest ops, and State.String(). Mocks generated for vmNetworkManager, linuxGuestNetworkManager, and capabilitiesProvider. GCS GuestDefinedCapabilities mock reused from internal/gcs/mock/. Signed-off-by: Shreyansh Sancheti --- internal/controller/network/export_test.go | 54 +++ .../controller/network/helpers_wcow_test.go | 42 +++ internal/controller/network/interface.go | 2 + .../controller/network/mock/mock_network.go | 299 +++++++++++++++++ .../controller/network/network_lcow_test.go | 195 +++++++++++ internal/controller/network/network_test.go | 307 ++++++++++++++++++ .../controller/network/network_wcow_test.go | 193 +++++++++++ internal/controller/network/state_test.go | 32 ++ internal/gcs/guestcaps.go | 2 + internal/gcs/mock/mock_guestcaps.go | 98 ++++++ 10 files changed, 1224 insertions(+) create mode 100644 internal/controller/network/export_test.go create mode 100644 internal/controller/network/helpers_wcow_test.go create mode 100644 internal/controller/network/mock/mock_network.go create mode 100644 internal/controller/network/network_lcow_test.go create mode 100644 internal/controller/network/network_test.go create mode 100644 internal/controller/network/network_wcow_test.go create mode 100644 internal/controller/network/state_test.go create mode 100644 internal/gcs/mock/mock_guestcaps.go diff --git a/internal/controller/network/export_test.go b/internal/controller/network/export_test.go new file mode 100644 index 0000000000..bb24a96edf --- /dev/null +++ b/internal/controller/network/export_test.go @@ -0,0 +1,54 @@ +//go:build windows + +package network + +import ( + "context" + + "github.com/Microsoft/hcsshim/hcn" +) + +// SetStateForTest sets the Manager's state. Test-only. +func (m *Manager) SetStateForTest(s State) { + m.netState = s +} + +// StateForTest returns the Manager's current state. Test-only. +func (m *Manager) StateForTest() State { + return m.netState +} + +// SetNamespaceIDForTest sets the namespace ID on the Manager. Test-only. +func (m *Manager) SetNamespaceIDForTest(id string) { + m.namespaceID = id +} + +// AddEndpointForTest adds a NIC→endpoint mapping to the internal tracking map. Test-only. +func (m *Manager) AddEndpointForTest(nicID string, ep *hcn.HostComputeEndpoint) { + m.vmEndpoints[nicID] = ep +} + +// EndpointsForTest returns the current vmEndpoints map. Test-only. +func (m *Manager) EndpointsForTest() map[string]*hcn.HostComputeEndpoint { + return m.vmEndpoints +} + +// IsNamespaceSupportedForTest returns the cached namespace-support flag. Test-only. +func (m *Manager) IsNamespaceSupportedForTest() bool { + return m.isNamespaceSupportedByGuest +} + +// AddEndpointToGuestNamespaceForTest exposes addEndpointToGuestNamespace for testing. +func (m *Manager) AddEndpointToGuestNamespaceForTest(ctx context.Context, nicID string, endpoint *hcn.HostComputeEndpoint, policyBasedRouting bool) error { + return m.addEndpointToGuestNamespace(ctx, nicID, endpoint, policyBasedRouting) +} + +// RemoveEndpointFromGuestNamespaceForTest exposes removeEndpointFromGuestNamespace for testing. +func (m *Manager) RemoveEndpointFromGuestNamespaceForTest(ctx context.Context, nicID string, endpoint *hcn.HostComputeEndpoint) error { + return m.removeEndpointFromGuestNamespace(ctx, nicID, endpoint) +} + +// AddNetNSInsideGuestForTest exposes addNetNSInsideGuest for testing. +func (m *Manager) AddNetNSInsideGuestForTest(ctx context.Context, ns *hcn.HostComputeNamespace) error { + return m.addNetNSInsideGuest(ctx, ns) +} diff --git a/internal/controller/network/helpers_wcow_test.go b/internal/controller/network/helpers_wcow_test.go new file mode 100644 index 0000000000..09e4f09928 --- /dev/null +++ b/internal/controller/network/helpers_wcow_test.go @@ -0,0 +1,42 @@ +//go:build windows && wcow + +package network_test + +import ( + "errors" + "testing" + + "github.com/Microsoft/hcsshim/internal/controller/network" + netmock "github.com/Microsoft/hcsshim/internal/controller/network/mock" + gcsmock "github.com/Microsoft/hcsshim/internal/gcs/mock" + + "go.uber.org/mock/gomock" +) + +var errTest = errors.New("test error") + +// newWCOWTestManager creates a Manager wired with WCOW mock dependencies. +func newWCOWTestManager(t *testing.T, ctrl *gomock.Controller, namespaceSupportEnabled bool) ( + *network.Manager, + *netmock.MockvmNetworkManager, + *netmock.MocklinuxGuestNetworkManager, + *netmock.MockwindowsGuestNetworkManager, +) { + t.Helper() + + mockVM := netmock.NewMockvmNetworkManager(ctrl) + mockLinuxGuest := netmock.NewMocklinuxGuestNetworkManager(ctrl) + mockWinGuest := netmock.NewMockwindowsGuestNetworkManager(ctrl) + mockCaps := netmock.NewMockcapabilitiesProvider(ctrl) + + if namespaceSupportEnabled { + mockGuestCaps := gcsmock.NewMockGuestDefinedCapabilities(ctrl) + mockGuestCaps.EXPECT().IsNamespaceAddRequestSupported().Return(true) + mockCaps.EXPECT().Capabilities().Return(mockGuestCaps) + } else { + mockCaps.EXPECT().Capabilities().Return(nil) + } + + m := network.New(mockVM, mockLinuxGuest, mockWinGuest, mockCaps) + return m, mockVM, mockLinuxGuest, mockWinGuest +} diff --git a/internal/controller/network/interface.go b/internal/controller/network/interface.go index 01f6753f97..1ce5a2b268 100644 --- a/internal/controller/network/interface.go +++ b/internal/controller/network/interface.go @@ -2,6 +2,8 @@ package network +//go:generate go tool mockgen -source=interface.go -build_constraint=windows -package=mock -destination=mock/mock_network.go + import ( "context" diff --git a/internal/controller/network/mock/mock_network.go b/internal/controller/network/mock/mock_network.go new file mode 100644 index 0000000000..222277ca92 --- /dev/null +++ b/internal/controller/network/mock/mock_network.go @@ -0,0 +1,299 @@ +//go:build windows + +// Code generated by MockGen. DO NOT EDIT. +// Source: interface.go +// +// Generated by this command: +// +// mockgen -source=interface.go -build_constraint=windows -package=mock -destination=mock/mock_network.go +// + +// Package mock is a generated GoMock package. +package mock + +import ( + context "context" + reflect "reflect" + + hcn "github.com/Microsoft/hcsshim/hcn" + network "github.com/Microsoft/hcsshim/internal/controller/network" + gcs "github.com/Microsoft/hcsshim/internal/gcs" + hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" + guestrequest "github.com/Microsoft/hcsshim/internal/protocol/guestrequest" + guestresource "github.com/Microsoft/hcsshim/internal/protocol/guestresource" + gomock "go.uber.org/mock/gomock" +) + +// MockController is a mock of Controller interface. +type MockController struct { + ctrl *gomock.Controller + recorder *MockControllerMockRecorder + isgomock struct{} +} + +// MockControllerMockRecorder is the mock recorder for MockController. +type MockControllerMockRecorder struct { + mock *MockController +} + +// NewMockController creates a new mock instance. +func NewMockController(ctrl *gomock.Controller) *MockController { + mock := &MockController{ctrl: ctrl} + mock.recorder = &MockControllerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockController) EXPECT() *MockControllerMockRecorder { + return m.recorder +} + +// Setup mocks base method. +func (m *MockController) Setup(ctx context.Context, opts *network.SetupOptions) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Setup", ctx, opts) + ret0, _ := ret[0].(error) + return ret0 +} + +// Setup indicates an expected call of Setup. +func (mr *MockControllerMockRecorder) Setup(ctx, opts any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Setup", reflect.TypeOf((*MockController)(nil).Setup), ctx, opts) +} + +// Teardown mocks base method. +func (m *MockController) Teardown(ctx context.Context) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Teardown", ctx) + ret0, _ := ret[0].(error) + return ret0 +} + +// Teardown indicates an expected call of Teardown. +func (mr *MockControllerMockRecorder) Teardown(ctx any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Teardown", reflect.TypeOf((*MockController)(nil).Teardown), ctx) +} + +// 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)) +} + +// 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) +} + +// MocklinuxGuestNetworkManager is a mock of linuxGuestNetworkManager interface. +type MocklinuxGuestNetworkManager struct { + ctrl *gomock.Controller + recorder *MocklinuxGuestNetworkManagerMockRecorder + isgomock struct{} +} + +// MocklinuxGuestNetworkManagerMockRecorder is the mock recorder for MocklinuxGuestNetworkManager. +type MocklinuxGuestNetworkManagerMockRecorder struct { + mock *MocklinuxGuestNetworkManager +} + +// NewMocklinuxGuestNetworkManager creates a new mock instance. +func NewMocklinuxGuestNetworkManager(ctrl *gomock.Controller) *MocklinuxGuestNetworkManager { + mock := &MocklinuxGuestNetworkManager{ctrl: ctrl} + mock.recorder = &MocklinuxGuestNetworkManagerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MocklinuxGuestNetworkManager) EXPECT() *MocklinuxGuestNetworkManagerMockRecorder { + return m.recorder +} + +// AddLCOWNetworkInterface mocks base method. +func (m *MocklinuxGuestNetworkManager) AddLCOWNetworkInterface(ctx context.Context, settings *guestresource.LCOWNetworkAdapter) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AddLCOWNetworkInterface", ctx, settings) + ret0, _ := ret[0].(error) + return ret0 +} + +// AddLCOWNetworkInterface indicates an expected call of AddLCOWNetworkInterface. +func (mr *MocklinuxGuestNetworkManagerMockRecorder) AddLCOWNetworkInterface(ctx, settings any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddLCOWNetworkInterface", reflect.TypeOf((*MocklinuxGuestNetworkManager)(nil).AddLCOWNetworkInterface), ctx, settings) +} + +// RemoveLCOWNetworkInterface mocks base method. +func (m *MocklinuxGuestNetworkManager) RemoveLCOWNetworkInterface(ctx context.Context, settings *guestresource.LCOWNetworkAdapter) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RemoveLCOWNetworkInterface", ctx, settings) + ret0, _ := ret[0].(error) + return ret0 +} + +// RemoveLCOWNetworkInterface indicates an expected call of RemoveLCOWNetworkInterface. +func (mr *MocklinuxGuestNetworkManagerMockRecorder) RemoveLCOWNetworkInterface(ctx, settings any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RemoveLCOWNetworkInterface", reflect.TypeOf((*MocklinuxGuestNetworkManager)(nil).RemoveLCOWNetworkInterface), ctx, settings) +} + +// MockwindowsGuestNetworkManager is a mock of windowsGuestNetworkManager interface. +type MockwindowsGuestNetworkManager struct { + ctrl *gomock.Controller + recorder *MockwindowsGuestNetworkManagerMockRecorder + isgomock struct{} +} + +// MockwindowsGuestNetworkManagerMockRecorder is the mock recorder for MockwindowsGuestNetworkManager. +type MockwindowsGuestNetworkManagerMockRecorder struct { + mock *MockwindowsGuestNetworkManager +} + +// NewMockwindowsGuestNetworkManager creates a new mock instance. +func NewMockwindowsGuestNetworkManager(ctrl *gomock.Controller) *MockwindowsGuestNetworkManager { + mock := &MockwindowsGuestNetworkManager{ctrl: ctrl} + mock.recorder = &MockwindowsGuestNetworkManagerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockwindowsGuestNetworkManager) EXPECT() *MockwindowsGuestNetworkManagerMockRecorder { + return m.recorder +} + +// AddNetworkInterface mocks base method. +func (m *MockwindowsGuestNetworkManager) 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 *MockwindowsGuestNetworkManagerMockRecorder) AddNetworkInterface(ctx, adapterID, requestType, settings any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddNetworkInterface", reflect.TypeOf((*MockwindowsGuestNetworkManager)(nil).AddNetworkInterface), ctx, adapterID, requestType, settings) +} + +// AddNetworkNamespace mocks base method. +func (m *MockwindowsGuestNetworkManager) 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 *MockwindowsGuestNetworkManagerMockRecorder) AddNetworkNamespace(ctx, settings any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddNetworkNamespace", reflect.TypeOf((*MockwindowsGuestNetworkManager)(nil).AddNetworkNamespace), ctx, settings) +} + +// RemoveNetworkInterface mocks base method. +func (m *MockwindowsGuestNetworkManager) 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 *MockwindowsGuestNetworkManagerMockRecorder) RemoveNetworkInterface(ctx, adapterID, requestType, settings any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RemoveNetworkInterface", reflect.TypeOf((*MockwindowsGuestNetworkManager)(nil).RemoveNetworkInterface), ctx, adapterID, requestType, settings) +} + +// RemoveNetworkNamespace mocks base method. +func (m *MockwindowsGuestNetworkManager) 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 *MockwindowsGuestNetworkManagerMockRecorder) RemoveNetworkNamespace(ctx, settings any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RemoveNetworkNamespace", reflect.TypeOf((*MockwindowsGuestNetworkManager)(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..5e282834a6 --- /dev/null +++ b/internal/controller/network/network_lcow_test.go @@ -0,0 +1,195 @@ +//go:build windows && !wcow + +package network_test + +import ( + "context" + "testing" + + "github.com/Microsoft/hcsshim/hcn" + hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" + "github.com/Microsoft/hcsshim/internal/protocol/guestresource" + + "go.uber.org/mock/gomock" +) + +// --- LCOW Endpoint Operation Tests --- + +// TestAddEndpoint_SuccessWithNamespaceSupport — host AddNIC + guest AddLCOWNetworkInterface called in order, NIC tracked. +func TestAddEndpoint_SuccessWithNamespaceSupport(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + m, mockVM, mockGuest := newTestManager(t, ctrl, true) + + ep := &hcn.HostComputeEndpoint{ + Id: "ep-1", + MacAddress: "aa:bb:cc:dd:ee:01", + HostComputeNamespace: "test-ns", + } + + // Build the expected adapter using the same function the production code calls. + expectedAdapter, err := guestresource.BuildLCOWNetworkAdapter("nic-1", ep, false) + if err != nil { + t.Fatalf("failed to build expected adapter: %v", err) + } + + // Enforce ordering: host AddNIC first, then guest AddLCOWNetworkInterface. + gomock.InOrder( + mockVM.EXPECT().AddNIC(gomock.Any(), "nic-1", &hcsschema.NetworkAdapter{ + EndpointId: "ep-1", + MacAddress: "aa:bb:cc:dd:ee:01", + }).Return(nil), + // Guest AddLCOWNetworkInterface must receive the adapter built by BuildLCOWNetworkAdapter. + mockGuest.EXPECT().AddLCOWNetworkInterface(gomock.Any(), expectedAdapter).Return(nil), + ) + + if err = m.AddEndpointToGuestNamespaceForTest(context.Background(), "nic-1", ep, false); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if _, ok := m.EndpointsForTest()["nic-1"]; !ok { + t.Error("expected nic-1 to be tracked in vmEndpoints") + } +} + +// TestAddEndpoint_HostSucceedsGuestFails_StillTracked — NIC tracked in vmEndpoints even when guest add fails. +func TestAddEndpoint_HostSucceedsGuestFails_StillTracked(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + m, mockVM, mockGuest := newTestManager(t, ctrl, true) + + ep := &hcn.HostComputeEndpoint{ + Id: "ep-1", + MacAddress: "aa:bb:cc:dd:ee:01", + HostComputeNamespace: "test-ns", + } + + // Build the expected adapter to verify correct struct is passed even on failure path. + expectedAdapter, buildErr := guestresource.BuildLCOWNetworkAdapter("nic-1", ep, false) + if buildErr != nil { + t.Fatalf("failed to build expected adapter: %v", buildErr) + } + + gomock.InOrder( + mockVM.EXPECT().AddNIC(gomock.Any(), "nic-1", &hcsschema.NetworkAdapter{ + EndpointId: "ep-1", + MacAddress: "aa:bb:cc:dd:ee:01", + }).Return(nil), + mockGuest.EXPECT().AddLCOWNetworkInterface(gomock.Any(), expectedAdapter).Return(errTest), + ) + + err := m.AddEndpointToGuestNamespaceForTest(context.Background(), "nic-1", ep, false) + if err == nil { + t.Fatal("expected error when guest AddLCOWNetworkInterface fails, got nil") + } + + // Critical: NIC must still be tracked for Teardown cleanup. + if _, ok := m.EndpointsForTest()["nic-1"]; !ok { + t.Error("expected nic-1 to remain tracked after guest failure (for Teardown cleanup)") + } +} + +// TestAddEndpoint_HostAddNICFails_NotTracked — NIC not added to vmEndpoints when AddNIC fails. +func TestAddEndpoint_HostAddNICFails_NotTracked(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + m, mockVM, _ := newTestManager(t, ctrl, true) + + ep := &hcn.HostComputeEndpoint{ + Id: "ep-1", + MacAddress: "aa:bb:cc:dd:ee:01", + } + + mockVM.EXPECT().AddNIC(gomock.Any(), "nic-1", &hcsschema.NetworkAdapter{ + EndpointId: "ep-1", + MacAddress: "aa:bb:cc:dd:ee:01", + }).Return(errTest) + + err := m.AddEndpointToGuestNamespaceForTest(context.Background(), "nic-1", ep, false) + if err == nil { + t.Fatal("expected error when AddNIC fails, got nil") + } + + // NIC must NOT be tracked — AddNIC failed before the tracking line. + if _, ok := m.EndpointsForTest()["nic-1"]; ok { + t.Error("expected nic-1 NOT to be tracked after AddNIC failure") + } +} + +// TestRemoveEndpoint_Success — guest remove called before host remove. +func TestRemoveEndpoint_Success(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + m, mockVM, mockGuest := newTestManager(t, ctrl, true) + + m.SetNamespaceIDForTest("test-ns") + ep := &hcn.HostComputeEndpoint{ + Id: "ep-1", + MacAddress: "aa:bb:cc:dd:ee:01", + } + + // Enforce ordering: guest first, then host. + gomock.InOrder( + mockGuest.EXPECT().RemoveLCOWNetworkInterface(gomock.Any(), &guestresource.LCOWNetworkAdapter{ + NamespaceID: "test-ns", + ID: "nic-1", + }).Return(nil), + mockVM.EXPECT().RemoveNIC(gomock.Any(), "nic-1", &hcsschema.NetworkAdapter{ + EndpointId: "ep-1", + MacAddress: "aa:bb:cc:dd:ee:01", + }).Return(nil), + ) + + err := m.RemoveEndpointFromGuestNamespaceForTest(context.Background(), "nic-1", ep) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +// TestRemoveEndpoint_GuestRemoveFails_HostNotCalled — host RemoveNIC not called when guest remove fails. +func TestRemoveEndpoint_GuestRemoveFails_HostNotCalled(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + m, _, mockGuest := newTestManager(t, ctrl, true) + + m.SetNamespaceIDForTest("test-ns") + ep := &hcn.HostComputeEndpoint{ + Id: "ep-1", + MacAddress: "aa:bb:cc:dd:ee:01", + } + + mockGuest.EXPECT().RemoveLCOWNetworkInterface(gomock.Any(), &guestresource.LCOWNetworkAdapter{ + NamespaceID: "test-ns", + ID: "nic-1", + }).Return(errTest) + // No RemoveNIC EXPECT — gomock will fail if it's called. + + err := m.RemoveEndpointFromGuestNamespaceForTest(context.Background(), "nic-1", ep) + if err == nil { + t.Fatal("expected error when guest remove fails, got nil") + } +} + +// TestRemoveEndpoint_WithoutNamespaceSupport — only host RemoveNIC called when guest namespace not supported. +func TestRemoveEndpoint_WithoutNamespaceSupport(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + m, mockVM, _ := newTestManager(t, ctrl, false /* no namespace support */) + + m.SetNamespaceIDForTest("test-ns") + ep := &hcn.HostComputeEndpoint{ + Id: "ep-1", + MacAddress: "aa:bb:cc:dd:ee:01", + } + + // Only host remove should be called. + mockVM.EXPECT().RemoveNIC(gomock.Any(), "nic-1", &hcsschema.NetworkAdapter{ + EndpointId: "ep-1", + MacAddress: "aa:bb:cc:dd:ee:01", + }).Return(nil) + + err := m.RemoveEndpointFromGuestNamespaceForTest(context.Background(), "nic-1", ep) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } +} diff --git a/internal/controller/network/network_test.go b/internal/controller/network/network_test.go new file mode 100644 index 0000000000..1ae9bd526d --- /dev/null +++ b/internal/controller/network/network_test.go @@ -0,0 +1,307 @@ +//go:build windows && !wcow + +package network_test + +import ( + "context" + "errors" + "fmt" + "strings" + "testing" + + "github.com/Microsoft/hcsshim/hcn" + hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" + "github.com/Microsoft/hcsshim/internal/protocol/guestresource" + + "github.com/Microsoft/hcsshim/internal/controller/network" + netmock "github.com/Microsoft/hcsshim/internal/controller/network/mock" + gcsmock "github.com/Microsoft/hcsshim/internal/gcs/mock" + + "go.uber.org/mock/gomock" +) + +var errTest = errors.New("test error") + +// newTestManager creates a Manager wired with mock dependencies. +func newTestManager(t *testing.T, ctrl *gomock.Controller, namespaceSupportEnabled bool) ( + *network.Manager, + *netmock.MockvmNetworkManager, + *netmock.MocklinuxGuestNetworkManager, +) { + t.Helper() + + mockVM := netmock.NewMockvmNetworkManager(ctrl) + mockGuest := netmock.NewMocklinuxGuestNetworkManager(ctrl) + mockCaps := netmock.NewMockcapabilitiesProvider(ctrl) + + if namespaceSupportEnabled { + mockGuestCaps := gcsmock.NewMockGuestDefinedCapabilities(ctrl) + mockGuestCaps.EXPECT().IsNamespaceAddRequestSupported().Return(true) + mockCaps.EXPECT().Capabilities().Return(mockGuestCaps) + } else { + mockCaps.EXPECT().Capabilities().Return(nil) + } + + m := network.New(mockVM, mockGuest, nil, mockCaps) + return m, mockVM, mockGuest +} + +// TestTeardown_PartialEndpointFailureContinues — if one NIC removal fails, remaining NICs still attempted. +func TestTeardown_PartialEndpointFailureContinues(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + m, mockVM, mockGuest := newTestManager(t, ctrl, true) + + // Simulate Configured state with 3 endpoints. + m.SetStateForTest(network.StateConfigured) + m.SetNamespaceIDForTest("test-ns") + ep1 := &hcn.HostComputeEndpoint{Id: "ep-1", Name: "ep1", MacAddress: "aa:bb:cc:dd:ee:01"} + ep2 := &hcn.HostComputeEndpoint{Id: "ep-2", Name: "ep2", MacAddress: "aa:bb:cc:dd:ee:02"} + ep3 := &hcn.HostComputeEndpoint{Id: "ep-3", Name: "ep3", MacAddress: "aa:bb:cc:dd:ee:03"} + m.AddEndpointForTest("nic-1", ep1) + m.AddEndpointForTest("nic-2", ep2) + m.AddEndpointForTest("nic-3", ep3) + + // Guest remove for nic-2 fails; others succeed. + // We use AnyTimes because map iteration order is random. + // DoAndReturn validates NamespaceID is correct for every call. + mockGuest.EXPECT().RemoveLCOWNetworkInterface(gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, settings *guestresource.LCOWNetworkAdapter) error { + if settings.NamespaceID != "test-ns" { + return fmt.Errorf("unexpected namespace: got %q, want %q", settings.NamespaceID, "test-ns") + } + if settings.ID == "nic-2" { + return errTest + } + return nil + }).AnyTimes() + + // Validate RemoveNIC receives the correct NetworkAdapter for each nicID. + endpointsByNIC := map[string]*hcn.HostComputeEndpoint{"nic-1": ep1, "nic-2": ep2, "nic-3": ep3} + mockVM.EXPECT().RemoveNIC(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, nicID string, adapter *hcsschema.NetworkAdapter) error { + expected := endpointsByNIC[nicID] + if adapter.EndpointId != expected.Id || adapter.MacAddress != expected.MacAddress { + return fmt.Errorf("RemoveNIC(%s): adapter mismatch: got {%s, %s}, want {%s, %s}", + nicID, adapter.EndpointId, adapter.MacAddress, expected.Id, expected.MacAddress) + } + return nil + }).AnyTimes() + + err := m.Teardown(context.Background()) + if err == nil { + t.Fatal("expected error from partial teardown, got nil") + } + + // State must be Invalid (not TornDown) after partial failure. + if m.StateForTest() != network.StateInvalid { + t.Errorf("expected state Invalid, got %v", m.StateForTest()) + } + + // The failed NIC must still be in the map for retry. + remaining := m.EndpointsForTest() + if _, ok := remaining["nic-2"]; !ok { + t.Error("expected nic-2 to remain in vmEndpoints for retry, but it was deleted") + } + // Successful NICs should have been removed. + if len(remaining) > 1 { + // Due to map iteration randomness we might have 1 or more failed. + // But nic-2 must be present. + for k := range remaining { + if k != "nic-2" { + t.Errorf("expected only nic-2 to remain, but found %s", k) + } + } + } +} + +// TestTeardown_RetryAfterPartialFailure — Teardown from Invalid retries remaining endpoints. +func TestTeardown_RetryAfterPartialFailure(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + m, mockVM, mockGuest := newTestManager(t, ctrl, true) + + // Start in Invalid state with 1 remaining endpoint (from a prior failed teardown). + m.SetStateForTest(network.StateInvalid) + m.SetNamespaceIDForTest("test-ns") + ep := &hcn.HostComputeEndpoint{Id: "ep-remain", Name: "remaining", MacAddress: "aa:bb:cc:dd:ee:99"} + m.AddEndpointForTest("nic-remain", ep) + + // This time removal succeeds. Verify concrete structs. + gomock.InOrder( + mockGuest.EXPECT().RemoveLCOWNetworkInterface(gomock.Any(), &guestresource.LCOWNetworkAdapter{ + NamespaceID: "test-ns", + ID: "nic-remain", + }).Return(nil), + mockVM.EXPECT().RemoveNIC(gomock.Any(), "nic-remain", &hcsschema.NetworkAdapter{ + EndpointId: "ep-remain", + MacAddress: "aa:bb:cc:dd:ee:99", + }).Return(nil), + ) + + err := m.Teardown(context.Background()) + if err != nil { + t.Fatalf("expected successful retry teardown, got: %v", err) + } + + if m.StateForTest() != network.StateTornDown { + t.Errorf("expected state TornDown after successful retry, got %v", m.StateForTest()) + } + if len(m.EndpointsForTest()) != 0 { + t.Errorf("expected empty vmEndpoints after retry, got %d entries", len(m.EndpointsForTest())) + } +} + +// TestTeardown_IdempotentWhenTornDown — double-teardown returns nil. +func TestTeardown_IdempotentWhenTornDown(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + m, _, _ := newTestManager(t, ctrl, true) + + m.SetStateForTest(network.StateTornDown) + + err := m.Teardown(context.Background()) + if err != nil { + t.Fatalf("expected nil for idempotent teardown, got: %v", err) + } + if m.StateForTest() != network.StateTornDown { + t.Errorf("expected state TornDown, got %v", m.StateForTest()) + } +} + +// TestTeardown_NoOpWhenNotConfigured — Teardown on unconfigured network returns nil. +func TestTeardown_NoOpWhenNotConfigured(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + m, _, _ := newTestManager(t, ctrl, true) + + // Default state is NotConfigured. + err := m.Teardown(context.Background()) + if err != nil { + t.Fatalf("expected nil for unconfigured teardown, got: %v", err) + } + if m.StateForTest() != network.StateNotConfigured { + t.Errorf("expected state NotConfigured unchanged, got %v", m.StateForTest()) + } +} + +// TestTeardown_FullLifecycle — Configured→TornDown with all endpoints removed. +func TestTeardown_FullLifecycle(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + m, mockVM, mockGuest := newTestManager(t, ctrl, true) + + m.SetStateForTest(network.StateConfigured) + m.SetNamespaceIDForTest("test-ns") + ep1 := &hcn.HostComputeEndpoint{Id: "ep-1", Name: "ep1", MacAddress: "aa:bb:cc:dd:ee:01"} + ep2 := &hcn.HostComputeEndpoint{Id: "ep-2", Name: "ep2", MacAddress: "aa:bb:cc:dd:ee:02"} + m.AddEndpointForTest("nic-1", ep1) + m.AddEndpointForTest("nic-2", ep2) + + // Validate per-NIC struct correctness via DoAndReturn (map iteration is random). + endpointsByNIC := map[string]*hcn.HostComputeEndpoint{"nic-1": ep1, "nic-2": ep2} + mockGuest.EXPECT().RemoveLCOWNetworkInterface(gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, settings *guestresource.LCOWNetworkAdapter) error { + if settings.NamespaceID != "test-ns" { + return fmt.Errorf("unexpected namespace: got %q, want %q", settings.NamespaceID, "test-ns") + } + return nil + }).Times(2) + mockVM.EXPECT().RemoveNIC(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, nicID string, adapter *hcsschema.NetworkAdapter) error { + expected := endpointsByNIC[nicID] + if adapter.EndpointId != expected.Id || adapter.MacAddress != expected.MacAddress { + return fmt.Errorf("RemoveNIC(%s): adapter mismatch", nicID) + } + return nil + }).Times(2) + + err := m.Teardown(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if m.StateForTest() != network.StateTornDown { + t.Errorf("expected state TornDown, got %v", m.StateForTest()) + } + if len(m.EndpointsForTest()) != 0 { + t.Errorf("expected empty vmEndpoints, got %d entries", len(m.EndpointsForTest())) + } +} + +// TestSetup_DuplicateCallRejected — Setup on already-Configured manager returns error. +func TestSetup_DuplicateCallRejected(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + m, _, _ := newTestManager(t, ctrl, true) + + m.SetStateForTest(network.StateConfigured) + + err := m.Setup(context.Background(), &network.SetupOptions{NetworkNamespace: "ns-dup"}) + if err == nil { + t.Fatal("expected error for duplicate Setup, got nil") + } + if !strings.Contains(err.Error(), "cannot set up network") { + t.Errorf("expected error about state guard, got: %v", err) + } + if m.StateForTest() != network.StateConfigured { + t.Errorf("expected state Configured unchanged, got %v", m.StateForTest()) + } +} + +// TestNew_NilCapabilitiesDisablesGuestOps — nil Capabilities() skips guest-side NIC operations. +func TestNew_NilCapabilitiesDisablesGuestOps(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + + m, mockVM, _ := newTestManager(t, ctrl, false /* nil caps */) + + if m.IsNamespaceSupportedForTest() { + t.Fatal("expected isNamespaceSupportedByGuest=false when caps are nil") + } + + // With namespace support disabled, addEndpoint should only call AddNIC (host-side). + ep := &hcn.HostComputeEndpoint{Id: "ep-1", MacAddress: "aa:bb:cc:dd:ee:01"} + mockVM.EXPECT().AddNIC(gomock.Any(), "nic-1", &hcsschema.NetworkAdapter{ + EndpointId: "ep-1", + MacAddress: "aa:bb:cc:dd:ee:01", + }).Return(nil) + // Guest AddLCOWNetworkInterface must NOT be called (no EXPECT set = fail if called). + + err := m.AddEndpointToGuestNamespaceForTest(context.Background(), "nic-1", ep, false) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // NIC should still be tracked even without guest config. + if _, ok := m.EndpointsForTest()["nic-1"]; !ok { + t.Error("expected nic-1 to be tracked in vmEndpoints") + } +} + +// TestTeardown_WithoutNamespaceSupport — Teardown with no guest namespace support only calls host RemoveNIC. +func TestTeardown_WithoutNamespaceSupport(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + + m, mockVM, _ := newTestManager(t, ctrl, false /* nil caps */) + + m.SetStateForTest(network.StateConfigured) + m.SetNamespaceIDForTest("test-ns") + ep := &hcn.HostComputeEndpoint{Id: "ep-1", Name: "ep1", MacAddress: "aa:bb:cc:dd:ee:01"} + m.AddEndpointForTest("nic-1", ep) + + // Only host-side remove should be called (no guest remove expected). + mockVM.EXPECT().RemoveNIC(gomock.Any(), "nic-1", &hcsschema.NetworkAdapter{ + EndpointId: "ep-1", + MacAddress: "aa:bb:cc:dd:ee:01", + }).Return(nil) + + err := m.Teardown(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if m.StateForTest() != network.StateTornDown { + t.Errorf("expected state TornDown, got %v", m.StateForTest()) + } +} diff --git a/internal/controller/network/network_wcow_test.go b/internal/controller/network/network_wcow_test.go new file mode 100644 index 0000000000..0da1a920ce --- /dev/null +++ b/internal/controller/network/network_wcow_test.go @@ -0,0 +1,193 @@ +//go:build windows && wcow + +package network_test + +import ( + "context" + "testing" + + "github.com/Microsoft/hcsshim/hcn" + hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" + "github.com/Microsoft/hcsshim/internal/protocol/guestrequest" + + "go.uber.org/mock/gomock" +) + +// --- WCOW Endpoint Operation Tests --- + +// TestWCOW_AddEndpoint_Success — 3-phase: PreAdd → host AddNIC → guest Add, NIC tracked. +func TestWCOW_AddEndpoint_Success(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + m, mockVM, _, mockWinGuest := newWCOWTestManager(t, ctrl, true) + + ep := &hcn.HostComputeEndpoint{ + Id: "ep-1", + MacAddress: "aa:bb:cc:dd:ee:01", + } + + gomock.InOrder( + // 1. Guest pre-add. + mockWinGuest.EXPECT().AddNetworkInterface( + gomock.Any(), "nic-1", guestrequest.RequestTypePreAdd, ep, + ).Return(nil), + // 2. Host AddNIC. + mockVM.EXPECT().AddNIC(gomock.Any(), "nic-1", &hcsschema.NetworkAdapter{ + EndpointId: "ep-1", + MacAddress: "aa:bb:cc:dd:ee:01", + }).Return(nil), + // 3. Guest add (finalize). + mockWinGuest.EXPECT().AddNetworkInterface( + gomock.Any(), "nic-1", guestrequest.RequestTypeAdd, nil, + ).Return(nil), + ) + + err := m.AddEndpointToGuestNamespaceForTest(context.Background(), "nic-1", ep, false) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if _, ok := m.EndpointsForTest()["nic-1"]; !ok { + t.Error("expected nic-1 to be tracked in vmEndpoints") + } +} + +// TestWCOW_AddEndpoint_PreAddFails_NotTracked — PreAdd failure means NIC never reaches host, not tracked. +func TestWCOW_AddEndpoint_PreAddFails_NotTracked(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + m, _, _, mockWinGuest := newWCOWTestManager(t, ctrl, true) + + ep := &hcn.HostComputeEndpoint{ + Id: "ep-1", + MacAddress: "aa:bb:cc:dd:ee:01", + } + + mockWinGuest.EXPECT().AddNetworkInterface( + gomock.Any(), "nic-1", guestrequest.RequestTypePreAdd, ep, + ).Return(errTest) + // No AddNIC or guest Add expected — gomock fails if called. + + err := m.AddEndpointToGuestNamespaceForTest(context.Background(), "nic-1", ep, false) + if err == nil { + t.Fatal("expected error when PreAdd fails, got nil") + } + + if _, ok := m.EndpointsForTest()["nic-1"]; ok { + t.Error("expected nic-1 NOT tracked after PreAdd failure") + } +} + +// TestWCOW_AddEndpoint_HostSucceedsGuestAddFails_StillTracked — guest finalize fails, NIC still tracked. +func TestWCOW_AddEndpoint_HostSucceedsGuestAddFails_StillTracked(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + m, mockVM, _, mockWinGuest := newWCOWTestManager(t, ctrl, true) + + ep := &hcn.HostComputeEndpoint{ + Id: "ep-1", + MacAddress: "aa:bb:cc:dd:ee:01", + } + + gomock.InOrder( + mockWinGuest.EXPECT().AddNetworkInterface( + gomock.Any(), "nic-1", guestrequest.RequestTypePreAdd, ep, + ).Return(nil), + mockVM.EXPECT().AddNIC(gomock.Any(), "nic-1", &hcsschema.NetworkAdapter{ + EndpointId: "ep-1", + MacAddress: "aa:bb:cc:dd:ee:01", + }).Return(nil), + // Guest finalize fails. + mockWinGuest.EXPECT().AddNetworkInterface( + gomock.Any(), "nic-1", guestrequest.RequestTypeAdd, nil, + ).Return(errTest), + ) + + err := m.AddEndpointToGuestNamespaceForTest(context.Background(), "nic-1", ep, false) + if err == nil { + t.Fatal("expected error when guest Add fails, got nil") + } + + // NIC tracked after host AddNIC succeeded — Teardown can clean up. + if _, ok := m.EndpointsForTest()["nic-1"]; !ok { + t.Error("expected nic-1 to remain tracked after guest Add failure") + } +} + +// TestWCOW_RemoveEndpoint_Success — guest remove then host remove. +func TestWCOW_RemoveEndpoint_Success(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + m, mockVM, _, mockWinGuest := newWCOWTestManager(t, ctrl, true) + + ep := &hcn.HostComputeEndpoint{ + Id: "ep-1", + MacAddress: "aa:bb:cc:dd:ee:01", + } + + gomock.InOrder( + mockWinGuest.EXPECT().RemoveNetworkInterface( + gomock.Any(), "nic-1", guestrequest.RequestTypeRemove, nil, + ).Return(nil), + mockVM.EXPECT().RemoveNIC(gomock.Any(), "nic-1", &hcsschema.NetworkAdapter{ + EndpointId: "ep-1", + MacAddress: "aa:bb:cc:dd:ee:01", + }).Return(nil), + ) + + err := m.RemoveEndpointFromGuestNamespaceForTest(context.Background(), "nic-1", ep) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +// TestWCOW_RemoveEndpoint_GuestFails_HostNotCalled — guest remove fails, host not called. +func TestWCOW_RemoveEndpoint_GuestFails_HostNotCalled(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + m, _, _, mockWinGuest := newWCOWTestManager(t, ctrl, true) + + ep := &hcn.HostComputeEndpoint{ + Id: "ep-1", + MacAddress: "aa:bb:cc:dd:ee:01", + } + + mockWinGuest.EXPECT().RemoveNetworkInterface( + gomock.Any(), "nic-1", guestrequest.RequestTypeRemove, nil, + ).Return(errTest) + + err := m.RemoveEndpointFromGuestNamespaceForTest(context.Background(), "nic-1", ep) + if err == nil { + t.Fatal("expected error when guest remove fails, got nil") + } +} + +// TestWCOW_AddNetNS_Success — namespace added to guest via winGuestMgr. +func TestWCOW_AddNetNS_Success(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + m, _, _, mockWinGuest := newWCOWTestManager(t, ctrl, true) + + ns := &hcn.HostComputeNamespace{Id: "ns-1"} + mockWinGuest.EXPECT().AddNetworkNamespace(gomock.Any(), ns).Return(nil) + + err := m.AddNetNSInsideGuestForTest(context.Background(), ns) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +// TestWCOW_AddNetNS_WithoutNamespaceSupport — no-op when guest doesn't support namespaces. +func TestWCOW_AddNetNS_WithoutNamespaceSupport(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + m, _, _, _ := newWCOWTestManager(t, ctrl, false) + + ns := &hcn.HostComputeNamespace{Id: "ns-1"} + // No AddNetworkNamespace expected — gomock fails if called. + + err := m.AddNetNSInsideGuestForTest(context.Background(), ns) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } +} diff --git a/internal/controller/network/state_test.go b/internal/controller/network/state_test.go new file mode 100644 index 0000000000..1820e22d17 --- /dev/null +++ b/internal/controller/network/state_test.go @@ -0,0 +1,32 @@ +//go:build windows + +package network_test + +import ( + "testing" + + "github.com/Microsoft/hcsshim/internal/controller/network" +) + +func TestStateString(t *testing.T) { + tests := []struct { + state network.State + want string + }{ + {network.StateNotConfigured, "NotConfigured"}, + {network.StateConfigured, "Configured"}, + {network.StateInvalid, "Invalid"}, + {network.StateTornDown, "TornDown"}, + {network.State(99), "Unknown"}, + {network.State(-1), "Unknown"}, + } + + for _, tt := range tests { + t.Run(tt.want, func(t *testing.T) { + t.Parallel() + if got := tt.state.String(); got != tt.want { + t.Errorf("State(%d).String() = %q, want %q", tt.state, got, tt.want) + } + }) + } +} diff --git a/internal/gcs/guestcaps.go b/internal/gcs/guestcaps.go index 58b1f8ebb7..308d01e512 100644 --- a/internal/gcs/guestcaps.go +++ b/internal/gcs/guestcaps.go @@ -2,6 +2,8 @@ package gcs +//go:generate go tool mockgen -source=guestcaps.go -build_constraint=windows -package=mock -destination=mock/mock_guestcaps.go + import ( "encoding/json" "fmt" diff --git a/internal/gcs/mock/mock_guestcaps.go b/internal/gcs/mock/mock_guestcaps.go new file mode 100644 index 0000000000..0c8642838a --- /dev/null +++ b/internal/gcs/mock/mock_guestcaps.go @@ -0,0 +1,98 @@ +//go:build windows + +// Code generated by MockGen. DO NOT EDIT. +// Source: guestcaps.go +// +// Generated by this command: +// +// mockgen -source=guestcaps.go -build_constraint=windows -package=mock -destination=mock/mock_guestcaps.go +// + +// Package mock is a generated GoMock package. +package mock + +import ( + reflect "reflect" + + gomock "go.uber.org/mock/gomock" +) + +// MockGuestDefinedCapabilities is a mock of GuestDefinedCapabilities interface. +type MockGuestDefinedCapabilities struct { + ctrl *gomock.Controller + recorder *MockGuestDefinedCapabilitiesMockRecorder + isgomock struct{} +} + +// MockGuestDefinedCapabilitiesMockRecorder is the mock recorder for MockGuestDefinedCapabilities. +type MockGuestDefinedCapabilitiesMockRecorder struct { + mock *MockGuestDefinedCapabilities +} + +// NewMockGuestDefinedCapabilities creates a new mock instance. +func NewMockGuestDefinedCapabilities(ctrl *gomock.Controller) *MockGuestDefinedCapabilities { + mock := &MockGuestDefinedCapabilities{ctrl: ctrl} + mock.recorder = &MockGuestDefinedCapabilitiesMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockGuestDefinedCapabilities) EXPECT() *MockGuestDefinedCapabilitiesMockRecorder { + return m.recorder +} + +// IsDeleteContainerStateSupported mocks base method. +func (m *MockGuestDefinedCapabilities) IsDeleteContainerStateSupported() bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "IsDeleteContainerStateSupported") + ret0, _ := ret[0].(bool) + return ret0 +} + +// IsDeleteContainerStateSupported indicates an expected call of IsDeleteContainerStateSupported. +func (mr *MockGuestDefinedCapabilitiesMockRecorder) IsDeleteContainerStateSupported() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsDeleteContainerStateSupported", reflect.TypeOf((*MockGuestDefinedCapabilities)(nil).IsDeleteContainerStateSupported)) +} + +// IsDumpStacksSupported mocks base method. +func (m *MockGuestDefinedCapabilities) IsDumpStacksSupported() bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "IsDumpStacksSupported") + ret0, _ := ret[0].(bool) + return ret0 +} + +// IsDumpStacksSupported indicates an expected call of IsDumpStacksSupported. +func (mr *MockGuestDefinedCapabilitiesMockRecorder) IsDumpStacksSupported() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsDumpStacksSupported", reflect.TypeOf((*MockGuestDefinedCapabilities)(nil).IsDumpStacksSupported)) +} + +// IsNamespaceAddRequestSupported mocks base method. +func (m *MockGuestDefinedCapabilities) IsNamespaceAddRequestSupported() bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "IsNamespaceAddRequestSupported") + ret0, _ := ret[0].(bool) + return ret0 +} + +// IsNamespaceAddRequestSupported indicates an expected call of IsNamespaceAddRequestSupported. +func (mr *MockGuestDefinedCapabilitiesMockRecorder) IsNamespaceAddRequestSupported() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsNamespaceAddRequestSupported", reflect.TypeOf((*MockGuestDefinedCapabilities)(nil).IsNamespaceAddRequestSupported)) +} + +// IsSignalProcessSupported mocks base method. +func (m *MockGuestDefinedCapabilities) IsSignalProcessSupported() bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "IsSignalProcessSupported") + ret0, _ := ret[0].(bool) + return ret0 +} + +// IsSignalProcessSupported indicates an expected call of IsSignalProcessSupported. +func (mr *MockGuestDefinedCapabilitiesMockRecorder) IsSignalProcessSupported() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsSignalProcessSupported", reflect.TypeOf((*MockGuestDefinedCapabilities)(nil).IsSignalProcessSupported)) +}