[shimV2] refactor VM controller + network controller

cmd/containerd-shim-lcow-v2/service/service.go

@@ -6,7 +6,6 @@ import (
 	"context"
 	"sync"
 
-	"github.com/Microsoft/hcsshim/internal/builder/vm/lcow"
 	"github.com/Microsoft/hcsshim/internal/controller/vm"
 	"github.com/Microsoft/hcsshim/internal/log"
 	"github.com/Microsoft/hcsshim/internal/shim"
@@ -40,10 +39,6 @@ type Service struct {
 	// For LCOW shim, sandboxID corresponds 1-1 with the UtilityVM managed by the shim.
 	sandboxID string
 
-	// sandboxOptions contains parsed, shim-level configuration for the sandbox
-	// such as architecture and confidential-compute settings.
-	sandboxOptions *lcow.SandboxOptions
-
 	// vmController is responsible for managing the lifecycle of the underlying utility VM and its associated resources.
 	vmController *vm.Controller
 

cmd/containerd-shim-lcow-v2/service/service_sandbox_internal.go

@@ -10,11 +10,9 @@ import (
 	"path/filepath"
 	"time"
 
-	"github.com/Microsoft/hcsshim/internal/builder/vm/lcow"
 	"github.com/Microsoft/hcsshim/internal/controller/vm"
 	"github.com/Microsoft/hcsshim/internal/gcs/prot"
 	"github.com/Microsoft/hcsshim/internal/log"
-	"github.com/Microsoft/hcsshim/internal/protocol/guestresource"
 	"github.com/Microsoft/hcsshim/internal/vm/vmutils"
 	vmsandbox "github.com/Microsoft/hcsshim/sandbox-spec/vm/v2"
 
@@ -71,16 +69,12 @@ func (s *Service) createSandboxInternal(ctx context.Context, request *sandbox.Cr
 		return nil, fmt.Errorf("sandbox already exists with ID %s", s.sandboxID)
 	}
 
-	hcsDocument, sandboxOptions, err := lcow.BuildSandboxConfig(ctx, ShimName, request.BundlePath, shimOpts, &sandboxSpec)
-	if err != nil {
-		return nil, fmt.Errorf("failed to parse sandbox spec: %w", err)
-	}
-
-	s.sandboxOptions = sandboxOptions
-
 	err = s.vmController.CreateVM(ctx, &vm.CreateOptions{
 		ID:          fmt.Sprintf("%s@vm", request.SandboxID),
-		HCSDocument: hcsDocument,
+		Owner:       ShimName,
+		BundlePath:  request.BundlePath,
+		ShimOpts:    shimOpts,
+		SandboxSpec: &sandboxSpec,
 	})
 	if err != nil {
 		return nil, fmt.Errorf("failed to create VM: %w", err)
@@ -105,29 +99,9 @@ func (s *Service) startSandboxInternal(ctx context.Context, request *sandbox.Sta
 		return nil, fmt.Errorf("sandbox ID mismatch, expected %s, got %s", s.sandboxID, request.SandboxID)
 	}
 
-	// If we successfully got past the above check, it means the sandbox was created and
-	// the sandboxOptions should be populated.
-	var confidentialOpts *guestresource.ConfidentialOptions
-	if s.sandboxOptions != nil && s.sandboxOptions.ConfidentialConfig != nil {
-		uvmReferenceInfoEncoded, err := vmutils.ParseUVMReferenceInfo(
-			ctx,
-			vmutils.DefaultLCOWOSBootFilesPath(),
-			s.sandboxOptions.ConfidentialConfig.UvmReferenceInfoFile,
-		)
-		if err != nil {
-			return nil, fmt.Errorf("failed to parse UVM reference info: %w", err)
-		}
-		confidentialOpts = &guestresource.ConfidentialOptions{
-			EnforcerType:          s.sandboxOptions.ConfidentialConfig.SecurityPolicyEnforcer,
-			EncodedSecurityPolicy: s.sandboxOptions.ConfidentialConfig.SecurityPolicy,
-			EncodedUVMReference:   uvmReferenceInfoEncoded,
-		}
-	}
-
 	// VM controller ensures that only once of the Start call goes through.
 	err := s.vmController.StartVM(ctx, &vm.StartOptions{
-		GCSServiceID:        winio.VsockServiceID(prot.LinuxGcsVsockPort),
-		ConfidentialOptions: confidentialOpts,
+		GCSServiceID: winio.VsockServiceID(prot.LinuxGcsVsockPort),
 	})
 	if err != nil {
 		return nil, fmt.Errorf("failed to start VM: %w", err)
@@ -151,10 +125,17 @@ func (s *Service) platformInternal(_ context.Context, request *sandbox.PlatformR
 		return nil, fmt.Errorf("sandbox has not been created (state: %s)", s.vmController.State())
 	}
 
+	// Find the architecture.
+	var architecture string
+	sandboxOpts := s.vmController.SandboxOptions()
+	if sandboxOpts != nil {
+		architecture = sandboxOpts.Architecture
+	}
+
 	return &sandbox.PlatformResponse{
 		Platform: &types.Platform{
 			OS:           linuxPlatform,
-			Architecture: s.sandboxOptions.Architecture,
+			Architecture: architecture,
 		},
 	}, nil
 }

internal/controller/network/doc.go

@@ -1,4 +1,4 @@
-//go:build windows
+//go:build windows && (lcow || wcow)
 
 // Package network provides a controller for managing the network lifecycle of a pod
 // running inside a Utility VM (UVM).

internal/controller/network/network.go

@@ -1,4 +1,4 @@
-//go:build windows
+//go:build windows && (lcow || wcow)
 
 package network
 
@@ -29,6 +29,10 @@ type Controller struct {
 	// netState is the current lifecycle state of the network.
 	netState State
 
+	// policyBasedRouting controls whether policy-based routing is configured
+	// for the endpoints added to the guest
+	policyBasedRouting bool
+
 	// isNamespaceSupportedByGuest determines if network namespace is supported inside the guest
 	isNamespaceSupportedByGuest bool
 
@@ -46,16 +50,19 @@ type Controller struct {
 
 // New creates a ready-to-use Controller in [StateNotConfigured].
 func New(
+	opts *Options,
 	vmNetManager vmNetworkManager,
 	guestNetwork guestNetwork,
 	capsProvider capabilitiesProvider,
 ) *Controller {
 	m := &Controller{
-		vmNetwork:    vmNetManager,
-		guestNetwork: guestNetwork,
-		capsProvider: capsProvider,
-		netState:     StateNotConfigured,
-		vmEndpoints:  make(map[string]*hcn.HostComputeEndpoint),
+		namespaceID:        opts.NetworkNamespace,
+		policyBasedRouting: opts.PolicyBasedRouting,
+		vmNetwork:          vmNetManager,
+		guestNetwork:       guestNetwork,
+		capsProvider:       capsProvider,
+		netState:           StateNotConfigured,
+		vmEndpoints:        make(map[string]*hcn.HostComputeEndpoint),
 	}
 
 	// Cache once at construction so hot-add paths can branch without re-querying.
@@ -69,8 +76,8 @@ func New(
 // 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 (c *Controller) Setup(ctx context.Context, opts *SetupOptions) (err error) {
-	ctx, _ = log.WithContext(ctx, logrus.WithField(logfields.Namespace, opts.NetworkNamespace))
+func (c *Controller) Setup(ctx context.Context) (err error) {
+	ctx, _ = log.WithContext(ctx, logrus.WithField(logfields.Namespace, c.namespaceID))
 
 	c.mu.Lock()
 	defer c.mu.Unlock()
@@ -91,14 +98,14 @@ func (c *Controller) Setup(ctx context.Context, opts *SetupOptions) (err error)
 		}
 	}()
 
-	if opts.NetworkNamespace == "" {
+	if c.namespaceID == "" {
 		return fmt.Errorf("network namespace must not be empty")
 	}
 
 	// Validate that the provided namespace exists.
-	hcnNamespace, err := hcn.GetNamespaceByID(opts.NetworkNamespace)
+	hcnNamespace, err := hcn.GetNamespaceByID(c.namespaceID)
 	if err != nil {
-		return fmt.Errorf("get network namespace %s: %w", opts.NetworkNamespace, err)
+		return fmt.Errorf("get network namespace %s: %w", c.namespaceID, err)
 	}
 
 	// Fetch all endpoints in the namespace.
@@ -121,12 +128,11 @@ func (c *Controller) Setup(ctx context.Context, opts *SetupOptions) (err error)
 		// 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 = c.addEndpointToGuestNamespace(nicCtx, nicGUID.String(), endpoint, opts.PolicyBasedRouting); err != nil {
+		if err = c.addEndpointToGuestNamespace(nicCtx, nicGUID.String(), endpoint, c.policyBasedRouting); err != nil {
 			return fmt.Errorf("add endpoint %s to guest: %w", endpoint.Name, err)
 		}
 	}
 
-	c.namespaceID = hcnNamespace.Id
 	c.netState = StateConfigured
 
 	log.G(ctx).Info("network setup completed successfully")

internal/controller/network/state.go

@@ -1,4 +1,4 @@
-//go:build windows
+//go:build windows && (lcow || wcow)
 
 package network
 

internal/controller/network/types.go

@@ -1,4 +1,4 @@
-//go:build windows
+//go:build windows && (lcow || wcow)
 
 package network
 
@@ -9,8 +9,9 @@ import (
 	hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2"
 )
 
-// SetupOptions holds the configuration required to set up the network for a pod.
-type SetupOptions struct {
+// Options holds the configuration for the controller which would be required
+// to set up the network for a pod.
+type Options struct {
 	// NetworkNamespace is the HCN namespace ID to attach to the guest.
 	NetworkNamespace string
 

internal/controller/vm/doc.go

@@ -55,7 +55,10 @@
 //
 //	if err := ctrl.CreateVM(ctx, &vm.CreateOptions{
 //	    ID:          "my-uvm",
-//	    HCSDocument: doc,
+//	    Owner:       "my-shim",
+//	    BundlePath:  bundlePath,
+//	    ShimOpts:    shimOpts,
+//	    SandboxSpec: sandboxSpec,
 //	}); err != nil {
 //	    // handle error
 //	}

internal/controller/vm/types.go

@@ -5,9 +5,9 @@ package vm
 import (
 	"time"
 
-	hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2"
-	"github.com/Microsoft/hcsshim/internal/protocol/guestresource"
+	runhcsoptions "github.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/options"
 	"github.com/Microsoft/hcsshim/internal/vm/guestmanager"
+	vmsandbox "github.com/Microsoft/hcsshim/sandbox-spec/vm/v2"
 
 	"github.com/Microsoft/go-winio/pkg/guid"
 )
@@ -17,14 +17,17 @@ type CreateOptions struct {
 	// ID specifies the unique identifier for the VM.
 	ID string
 
-	// HCSDocument specifies the HCS schema document used to create the VM.
-	HCSDocument *hcsschema.ComputeSystem
+	// Owner specifies the owner name for the VM (e.g., shim name).
+	Owner string
 
-	// NoWritableFileShares disallows writable file shares to the UVM.
-	NoWritableFileShares bool
+	// BundlePath is the path to the bundle directory containing the sandbox config.
+	BundlePath string
 
-	// FullyPhysicallyBacked indicates all memory allocations are backed by physical memory.
-	FullyPhysicallyBacked bool
+	// ShimOpts specifies the runtime options for the shim.
+	ShimOpts *runhcsoptions.Options
+
+	// SandboxSpec specifies the sandbox specification from CRI.
+	SandboxSpec *vmsandbox.Spec
 }
 
 // StartOptions contains the configuration needed to start a VM and establish
@@ -35,10 +38,6 @@ type StartOptions struct {
 
 	// ConfigOptions specifies additional configuration options for the guest config.
 	ConfigOptions []guestmanager.ConfigOption
-
-	// ConfidentialOptions specifies security policy and confidential computing
-	// options for the VM. This is optional and only used for confidential VMs.
-	ConfidentialOptions *guestresource.ConfidentialOptions
 }
 
 // ExitStatus contains information about a stopped VM's final state.

internal/controller/vm/vm.go

@@ -59,9 +59,6 @@ type Controller struct {
 	// isPhysicallyBacked indicates whether the VM is using physical backing for its memory.
 	isPhysicallyBacked bool
 
-	// noWritableFileShares indicates whether writable file shares are disabled for this VM.
-	noWritableFileShares bool
-
 	// scsiController manages SCSI devices for this VM.
 	scsiController *scsi.Controller
 
@@ -118,27 +115,29 @@ func (c *Controller) CreateVM(ctx context.Context, opts *CreateOptions) error {
 		return fmt.Errorf("cannot create VM: VM is in incorrect state %s", c.vmState)
 	}
 
+	// Build the HCS document and sandbox options from the platform-specific builder.
+	hcsDocument, err := c.buildHCSConfig(ctx, opts)
+	if err != nil {
+		return fmt.Errorf("failed to build VM config: %w", err)
+	}
+
 	// Create the VM via vmmanager.
-	uvm, err := vmmanager.Create(ctx, opts.ID, opts.HCSDocument)
+	uvm, err := vmmanager.Create(ctx, opts.ID, hcsDocument)
 	if err != nil {
 		return fmt.Errorf("failed to create VM: %w", err)
 	}
 
 	// Set the Controller parameters after successful creation.
 	c.vmID = opts.ID
 	c.uvm = uvm
-	// Determine if the VM is physically backed based on the create options.
-	c.isPhysicallyBacked = opts.FullyPhysicallyBacked
-	//
-	c.noWritableFileShares = opts.NoWritableFileShares
 
 	// Initialize the GuestManager for managing guest interactions.
 	// We will create the guest connection via GuestManager during StartVM.
 	c.guest = guestmanager.New(ctx, uvm)
 
 	// Eager initialize the SCSI controller as opposed to all other controllers.
 	// This is because we always use SCSI for attaching scratch VHDs.
-	c.scsiController, err = newSCSIController(ctx, opts.HCSDocument, c.uvm, c.guest)
+	c.scsiController, err = newSCSIController(ctx, hcsDocument, c.uvm, c.guest)
 	if err != nil {
 		return fmt.Errorf("failed to initialize SCSI controller: %w", err)
 	}
@@ -222,8 +221,13 @@ func (c *Controller) StartVM(ctx context.Context, opts *StartOptions) (err error
 	}
 
 	// Set the confidential options if applicable.
-	if opts.ConfidentialOptions != nil {
-		if err := c.guest.AddSecurityPolicy(ctx, *opts.ConfidentialOptions); err != nil {
+	// These are determined from the sandbox options stored during CreateVM.
+	confidentialOpts, err := c.buildConfidentialOptions(ctx)
+	if err != nil {
+		return fmt.Errorf("failed to build confidential options: %w", err)
+	}
+	if confidentialOpts != nil {
+		if err := c.guest.AddSecurityPolicy(ctx, *confidentialOpts); err != nil {
 			return fmt.Errorf("failed to set confidential options: %w", err)
 		}
 	}

internal/controller/vm/vm_devices.go

@@ -9,17 +9,10 @@ import (
 
 	"github.com/Microsoft/hcsshim/internal/controller/device/scsi"
 	"github.com/Microsoft/hcsshim/internal/controller/device/vpci"
-	"github.com/Microsoft/hcsshim/internal/controller/network"
 	hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2"
 	"github.com/Microsoft/hcsshim/internal/protocol/guestrequest"
 )
 
-// NetworkController returns a new controller for managing network devices on the VM.
-// Since we have a namespace per pod, we create a new controller per call.
-func (c *Controller) NetworkController() *network.Controller {
-	return network.New(c.uvm, c.guest, c.guest)
-}
-
 // SCSIController returns the singleton SCSI device controller for this VM.
 func (c *Controller) SCSIController() *scsi.Controller {
 	return c.scsiController