diff --git a/cmd/containerd-shim-runhcs-v1/pod.go b/cmd/containerd-shim-runhcs-v1/pod.go index 6a8f5ef62c..cba599f55b 100644 --- a/cmd/containerd-shim-runhcs-v1/pod.go +++ b/cmd/containerd-shim-runhcs-v1/pod.go @@ -309,7 +309,7 @@ func createPod(ctx context.Context, events publisher, req *task.CreateTaskReques } // LCOW (and WCOW Process Isolated for the time being) requires a real // task for the sandbox. - lt, err := newHcsTask(ctx, events, parent, true, req, s) + lt, err := newHcsTask(ctx, events, parent, true, req, s, req.ID) if err != nil { return nil, err } @@ -408,7 +408,7 @@ func (p *pod) CreateTask(ctx context.Context, req *task.CreateTaskRequest, s *sp sid) } - st, err := newHcsTask(ctx, p.events, p.host, false, req, s) + st, err := newHcsTask(ctx, p.events, p.host, false, req, s, p.id) if err != nil { return nil, err } diff --git a/cmd/containerd-shim-runhcs-v1/task_hcs.go b/cmd/containerd-shim-runhcs-v1/task_hcs.go index 41e3c2e743..7578955785 100644 --- a/cmd/containerd-shim-runhcs-v1/task_hcs.go +++ b/cmd/containerd-shim-runhcs-v1/task_hcs.go @@ -105,7 +105,7 @@ func newHcsStandaloneTask(ctx context.Context, events publisher, req *task.Creat return nil, errors.Wrap(errdefs.ErrFailedPrecondition, "oci spec does not contain WCOW or LCOW spec") } - shim, err := newHcsTask(ctx, events, parent, true, req, s) + shim, err := newHcsTask(ctx, events, parent, true, req, s, req.ID) if err != nil { if parent != nil { parent.Close() @@ -126,6 +126,7 @@ func createContainer( parent *uvm.UtilityVM, shimOpts *runhcsopts.Options, rootfs []*types.Mount, + sandboxID string, ) (cow.Container, *resources.Resources, error) { var ( err error @@ -157,6 +158,7 @@ func createContainer( } else { opts := &hcsoci.CreateOptions{ ID: id, + SandboxID: sandboxID, Owner: owner, Spec: s, HostingSystem: parent, @@ -186,7 +188,9 @@ func newHcsTask( parent *uvm.UtilityVM, ownsParent bool, req *task.CreateTaskRequest, - s *specs.Spec) (_ shimTask, err error) { + s *specs.Spec, + sandboxID string, +) (_ shimTask, err error) { log.G(ctx).WithFields(logrus.Fields{ "tid": req.ID, "ownsParent": ownsParent, @@ -219,7 +223,7 @@ func newHcsTask( return nil, err } - container, resources, err := createContainer(ctx, req.ID, owner, netNS, s, parent, shimOpts, req.Rootfs) + container, resources, err := createContainer(ctx, req.ID, owner, netNS, s, parent, shimOpts, req.Rootfs, sandboxID) if err != nil { return nil, err } diff --git a/internal/guestpath/paths.go b/internal/guestpath/paths.go index aab9ed1053..ef3f8c1f4e 100644 --- a/internal/guestpath/paths.go +++ b/internal/guestpath/paths.go @@ -3,7 +3,7 @@ package guestpath const ( // LCOWRootPrefixInUVM is the path inside UVM where LCOW container's root // file system will be mounted - LCOWRootPrefixInUVM = "/run/gcs/c" + LCOWRootPrefixInUVM = "/run/gcs/pods" // WCOWRootPrefixInUVM is the path inside UVM where WCOW container's root // file system will be mounted WCOWRootPrefixInUVM = `C:\c` diff --git a/internal/hcs/system.go b/internal/hcs/system.go index b1597466f6..dca2166e1e 100644 --- a/internal/hcs/system.go +++ b/internal/hcs/system.go @@ -37,6 +37,7 @@ type System struct { exitError error os, typ, owner string startTime time.Time + stopTime time.Time } var _ cow.Container = &System{} @@ -291,6 +292,7 @@ func (computeSystem *System) waitBackground() { err = makeSystemError(computeSystem, operation, err, nil) } computeSystem.closedWaitOnce.Do(func() { + computeSystem.stopTime = time.Now() computeSystem.waitError = err close(computeSystem.waitBlock) }) @@ -333,6 +335,21 @@ func (computeSystem *System) stopped() bool { return false } +// Stopped returns true if the compute system is in stopped state. +func (computeSystem *System) Stopped() bool { + return computeSystem.stopped() +} + +// StartTime returns the time at which the compute system started. +func (computeSystem *System) StartTime() time.Time { + return computeSystem.startTime +} + +// StopTime returns the time at which the compute system stopped. +func (computeSystem *System) StopTime() time.Time { + return computeSystem.stopTime +} + // ExitError returns an error describing the reason the compute system terminated. func (computeSystem *System) ExitError() error { if !computeSystem.stopped() { diff --git a/internal/hcsoci/create.go b/internal/hcsoci/create.go index bfc5342b65..0ec910edcb 100644 --- a/internal/hcsoci/create.go +++ b/internal/hcsoci/create.go @@ -31,7 +31,7 @@ import ( ) var ( - lcowRootInUVM = guestpath.LCOWRootPrefixInUVM + "/%s" + lcowRootInUVM = guestpath.LCOWRootPrefixInUVM + "/%s/%s" wcowRootInUVM = guestpath.WCOWRootPrefixInUVM + "/%s" ) @@ -43,6 +43,7 @@ var ( type CreateOptions struct { // Common parameters ID string // Identifier for the container + SandboxID string // Identifier for the pod of this container. Owner string // Specifies the owner. Defaults to executable name. Spec *specs.Spec // Definition of the container or utility VM being created SchemaVersion *hcsschema.Version // Requested Schema Version. Defaults to v2 for RS5, v1 for RS1..RS4 @@ -205,7 +206,9 @@ func CreateContainer(ctx context.Context, createOptions *CreateOptions) (_ cow.C if coi.HostingSystem != nil { if coi.Spec.Linux != nil { - r.SetContainerRootInUVM(fmt.Sprintf(lcowRootInUVM, coi.ID)) + // The container root within the UVM would be as below- + // /pods// + r.SetContainerRootInUVM(fmt.Sprintf(lcowRootInUVM, coi.SandboxID, coi.ID)) } else { n := coi.HostingSystem.ContainerCounter() r.SetContainerRootInUVM(fmt.Sprintf(wcowRootInUVM, strconv.FormatUint(n, 16))) diff --git a/internal/hcsoci/hcsdoc_lcow.go b/internal/hcsoci/hcsdoc_lcow.go index db94d73df0..dfd2f1def5 100644 --- a/internal/hcsoci/hcsdoc_lcow.go +++ b/internal/hcsoci/hcsdoc_lcow.go @@ -38,6 +38,19 @@ func createLCOWSpec(ctx context.Context, coi *createOptionsInternal) (*specs.Spe // Hooks are not supported (they should be run in the host) spec.Hooks = nil + // Set default CPU period and quota if not set for LCOW containers. + if spec.Linux != nil && + spec.Linux.Resources != nil && + spec.Linux.Resources.CPU != nil { + + if spec.Linux.Resources.CPU.Period != nil && *spec.Linux.Resources.CPU.Period == 0 { + *spec.Linux.Resources.CPU.Period = 100000 // Default CPU period + } + if spec.Linux.Resources.CPU.Quota != nil && *spec.Linux.Resources.CPU.Quota == 0 { + *spec.Linux.Resources.CPU.Quota = -1 // No CPU limit + } + } + // Clear unsupported features spec.Linux.CgroupsPath = "" // GCS controls its cgroups hierarchy on its own. if spec.Linux.Resources != nil { diff --git a/internal/hcsoci/hcsdoc_lcow_test.go b/internal/hcsoci/hcsdoc_lcow_test.go new file mode 100644 index 0000000000..d490657c89 --- /dev/null +++ b/internal/hcsoci/hcsdoc_lcow_test.go @@ -0,0 +1,303 @@ +//go:build windows + +package hcsoci + +import ( + "context" + "reflect" + "testing" + + "github.com/Microsoft/hcsshim/internal/schemaversion" + + specs "github.com/opencontainers/runtime-spec/specs-go" +) + +// baseSpec returns a minimal valid spec to reduce boilerplate in tests. +func baseSpec() *specs.Spec { + return &specs.Spec{ + Linux: &specs.Linux{ + Resources: &specs.LinuxResources{ + CPU: &specs.LinuxCPU{}, + }, + }, + Windows: &specs.Windows{}, + Annotations: map[string]string{"key": "original"}, + } +} + +// TestCreateLCOWSpec_CPUDefaults tests the logic for applying default CPU Period and Quota values. +func TestCreateLCOWSpec_CPUDefaults(t *testing.T) { + ctx := context.Background() + + tests := []struct { + name string + setup func() *specs.Spec + wantPeriod *uint64 // Using pointer to differentiate between 0 and nil + wantQuota *int64 + }{ + { + name: "Defaults applied when explicit 0", + setup: func() *specs.Spec { + s := baseSpec() + s.Linux.Resources.CPU.Period = ptrUint64(0) + s.Linux.Resources.CPU.Quota = ptrInt64(0) + return s + }, + wantPeriod: ptrUint64(100000), + wantQuota: ptrInt64(-1), + }, + { + name: "Defaults ignored when non-zero", + setup: func() *specs.Spec { + s := baseSpec() + s.Linux.Resources.CPU.Period = ptrUint64(50000) + s.Linux.Resources.CPU.Quota = ptrInt64(20000) + return s + }, + wantPeriod: ptrUint64(50000), + wantQuota: ptrInt64(20000), + }, + { + name: "Defaults ignored when omitted (nil)", + // The code checks `if ptr != nil && *ptr == 0`, so nil inputs remain nil. + setup: func() *specs.Spec { + s := baseSpec() + s.Linux.Resources.CPU.Period = nil + s.Linux.Resources.CPU.Quota = nil + return s + }, + wantPeriod: nil, + wantQuota: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + inputSpec := tt.setup() + coi := &createOptionsInternal{CreateOptions: &CreateOptions{Spec: inputSpec}} + + result, err := createLCOWSpec(ctx, coi) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Validate Period + gotPeriod := result.Linux.Resources.CPU.Period + if tt.wantPeriod == nil { + if gotPeriod != nil { + t.Errorf("Period: want nil, got %d", *gotPeriod) + } + } else { + if gotPeriod == nil || *gotPeriod != *tt.wantPeriod { + t.Errorf("Period: want %d, got %v", *tt.wantPeriod, gotPeriod) + } + } + + // Validate Quota + gotQuota := result.Linux.Resources.CPU.Quota + if tt.wantQuota == nil { + if gotQuota != nil { + t.Errorf("Quota: want nil, got %d", *gotQuota) + } + } else { + if gotQuota == nil || *gotQuota != *tt.wantQuota { + t.Errorf("Quota: want %d, got %v", *tt.wantQuota, gotQuota) + } + } + }) + } +} + +// TestCreateLCOWSpec_ResourcesDeepCopy tests that all fields in Resources are correctly deep-copied. +func TestCreateLCOWSpec_ResourcesDeepCopy(t *testing.T) { + // Goal: Verify that other CPU fields and Memory fields are correctly copied + // and are not lost or corrupted during the process. + ctx := context.Background() + s := baseSpec() + + // Populate extensive fields + s.Linux.Resources.CPU = &specs.LinuxCPU{ + Shares: ptrUint64(1024), + RealtimePeriod: ptrUint64(1000), + RealtimeRuntime: ptrInt64(500), + Cpus: "0-1", + Mems: "0", + } + s.Linux.Resources.Memory = &specs.LinuxMemory{ + Limit: ptrInt64(2048), + Reservation: ptrInt64(1024), + Swap: ptrInt64(4096), + Swappiness: ptrUint64(60), + } + + coi := &createOptionsInternal{CreateOptions: &CreateOptions{Spec: s}} + result, err := createLCOWSpec(ctx, coi) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Validate CPU fields + resCPU := result.Linux.Resources.CPU + if *resCPU.Shares != 1024 { + t.Errorf("CPU Shares mismatch: got %v", *resCPU.Shares) + } + if *resCPU.RealtimePeriod != 1000 { + t.Errorf("CPU RealtimePeriod mismatch: got %v", *resCPU.RealtimePeriod) + } + if *resCPU.RealtimeRuntime != 500 { + t.Errorf("CPU RealtimeRuntime mismatch: got %v", *resCPU.RealtimeRuntime) + } + if resCPU.Cpus != "0-1" { + t.Errorf("CPU Cpus mismatch: got %v", resCPU.Cpus) + } + if resCPU.Mems != "0" { + t.Errorf("CPU Mems mismatch: got %v", resCPU.Mems) + } + + // Validate Memory fields + resMem := result.Linux.Resources.Memory + if *resMem.Limit != 2048 { + t.Errorf("Memory Limit mismatch: got %v", *resMem.Limit) + } + if *resMem.Reservation != 1024 { + t.Errorf("Memory Reservation mismatch: got %v", *resMem.Reservation) + } + if *resMem.Swap != 4096 { + t.Errorf("Memory Swap mismatch: got %v", *resMem.Swap) + } + if *resMem.Swappiness != 60 { + t.Errorf("Memory Swappiness mismatch: got %v", *resMem.Swappiness) + } +} + +// TestCreateLCOWSpec_WindowsLogicMatrix tests various combinations of Windows struct fields. +func TestCreateLCOWSpec_WindowsLogicMatrix(t *testing.T) { + // Goal: Test Partial Windows structs. + ctx := context.Background() + + tests := []struct { + name string + inputWindows *specs.Windows + expectWindows bool + expectNet string + expectDevs int + }{ + { + name: "Nil Windows", + inputWindows: nil, + expectWindows: false, + }, + { + name: "Empty Windows", + inputWindows: &specs.Windows{}, + expectWindows: false, + }, + { + name: "Network Only", + inputWindows: &specs.Windows{ + Network: &specs.WindowsNetwork{NetworkNamespace: "host-ns"}, + }, + expectWindows: true, + expectNet: "host-ns", + }, + { + name: "Devices Only", + inputWindows: &specs.Windows{ + Devices: []specs.WindowsDevice{{ID: "gpu1"}}, + }, + expectWindows: true, + expectDevs: 1, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := baseSpec() + s.Windows = tt.inputWindows + coi := &createOptionsInternal{CreateOptions: &CreateOptions{Spec: s}} + + res, err := createLCOWSpec(ctx, coi) + if err != nil { + t.Fatalf("error: %v", err) + } + + if !tt.expectWindows && res.Windows != nil { + t.Errorf("Expected nil Windows, got: %+v", res.Windows) + } + if tt.expectWindows && res.Windows == nil { + t.Fatal("Expected Windows section, got nil") + } + if tt.expectNet != "" && (res.Windows.Network == nil || res.Windows.Network.NetworkNamespace != tt.expectNet) { + t.Errorf("Network Namespace mismatch") + } + if res.Windows != nil && len(res.Windows.Devices) != tt.expectDevs { + t.Errorf("Device count mismatch") + } + }) + } +} + +// TestCreateLCOWSpec_FieldSanitization tests that specific fields are cleared from the spec. +func TestCreateLCOWSpec_FieldSanitization(t *testing.T) { + // Goal: Ensure specific fields are cleared but valid ones preserved. + ctx := context.Background() + s := baseSpec() + + // Fields to remove + s.Hooks = &specs.Hooks{CreateContainer: []specs.Hook{{Path: "bad"}}} + s.Linux.CgroupsPath = "/bad/path" + s.Linux.Resources.BlockIO = &specs.LinuxBlockIO{Weight: ptrUint16(10)} + + coi := &createOptionsInternal{CreateOptions: &CreateOptions{Spec: s}} + res, err := createLCOWSpec(ctx, coi) + if err != nil { + t.Fatalf("error: %v", err) + } + + if res.Hooks != nil { + t.Error("Hooks not cleared") + } + if res.Linux.CgroupsPath != "" { + t.Error("CgroupsPath not cleared") + } + if res.Linux.Resources.BlockIO != nil { + t.Error("BlockIO not cleared") + } +} + +// TestCreateLinuxContainerDocument_PopulatesFields tests that createLinuxContainerDocument populates fields correctly. +func TestCreateLinuxContainerDocument_PopulatesFields(t *testing.T) { + ctx := context.Background() + + inputSpec := baseSpec() + // Trigger defaults + inputSpec.Linux.Resources.CPU.Period = ptrUint64(0) + inputSpec.Linux.Resources.CPU.Quota = ptrInt64(0) + + coi := &createOptionsInternal{CreateOptions: &CreateOptions{Spec: inputSpec}} + guestRoot := "C:\\guest\\root" + scratch := "C:\\scratch\\path" + + doc, err := createLinuxContainerDocument(ctx, coi, guestRoot, scratch) + if err != nil { + t.Fatalf("createLinuxContainerDocument error: %v", err) + } + + // 1. Schema + if !reflect.DeepEqual(doc.SchemaVersion, schemaversion.SchemaV21()) { + t.Errorf("SchemaVersion mismatch") + } + // 2. Paths + if doc.OciBundlePath != guestRoot || doc.ScratchDirPath != scratch { + t.Errorf("Path mismatch") + } + // 3. Spec Defaults + if *doc.OciSpecification.Linux.Resources.CPU.Period != 100000 { + t.Errorf("Defaults not applied in wrapper") + } +} + +// --- Helper Functions --- +func ptrUint16(v uint16) *uint16 { return &v } +func ptrUint64(v uint64) *uint64 { return &v } +func ptrInt64(v int64) *int64 { return &v } diff --git a/internal/hcsoci/hcsdoc_wcow.go b/internal/hcsoci/hcsdoc_wcow.go index e2aa0776cb..670069c577 100644 --- a/internal/hcsoci/hcsdoc_wcow.go +++ b/internal/hcsoci/hcsdoc_wcow.go @@ -73,7 +73,7 @@ func createMountsConfig(ctx context.Context, coi *createOptionsInternal) (*mount continue } else if strings.HasPrefix(mount.Source, guestpath.SandboxMountPrefix) { // Convert to the path in the guest that was asked for. - mdv2.HostPath = convertToWCOWSandboxMountPath(mount.Source) + mdv2.HostPath = convertToWCOWSandboxMountPath(coi.SandboxID, mount.Source) } else { // vsmb mount uvmPath, err := coi.HostingSystem.GetVSMBUvmPath(ctx, mount.Source, readOnly) diff --git a/internal/hcsoci/resources_lcow.go b/internal/hcsoci/resources_lcow.go index 3da1180683..a6e65559ac 100644 --- a/internal/hcsoci/resources_lcow.go +++ b/internal/hcsoci/resources_lcow.go @@ -35,11 +35,7 @@ func allocateLinuxResources(ctx context.Context, coi *createOptionsInternal, r * return errors.Wrap(err, "failed to mount container storage") } coi.Spec.Root.Path = rootPath - // If this is the pause container in a hypervisor-isolated pod, we can skip cleanup of - // layers, as that happens automatically when the UVM is terminated. - if !isSandbox || coi.HostingSystem == nil { - r.SetLayers(closer) - } + r.SetLayers(closer) r.SetLcowScratchPath(scratchPath) } else if coi.Spec.Root.Path != "" { // This is the "Plan 9" root filesystem. diff --git a/internal/hcsoci/resources_wcow.go b/internal/hcsoci/resources_wcow.go index 211b15f300..c49e0a1be7 100644 --- a/internal/hcsoci/resources_wcow.go +++ b/internal/hcsoci/resources_wcow.go @@ -27,7 +27,7 @@ import ( "github.com/Microsoft/hcsshim/internal/uvm/scsi" ) -const wcowSandboxMountPath = "C:\\SandboxMounts" +const wcowSandboxMountPath = "C:\\SandboxMounts\\%s" func allocateWindowsResources(ctx context.Context, coi *createOptionsInternal, r *resources.Resources, isSandbox bool) error { if coi.Spec.Root == nil { @@ -193,7 +193,7 @@ func setupMounts(ctx context.Context, coi *createOptionsInternal, r *resources.R // - sandbox:///a/dirInUvm destination:C:\\dirInContainer. // // so first convert to a path in the sandboxmounts path itself. - sandboxPath := convertToWCOWSandboxMountPath(mount.Source) + sandboxPath := convertToWCOWSandboxMountPath(coi.SandboxID, mount.Source) // Now we need to exec a process in the vm that will make these directories as theres // no functionality in the Windows gcs to create an arbitrary directory. @@ -238,7 +238,8 @@ func setupMounts(ctx context.Context, coi *createOptionsInternal, r *resources.R return nil } -func convertToWCOWSandboxMountPath(source string) string { +func convertToWCOWSandboxMountPath(sandboxID string, source string) string { subPath := strings.TrimPrefix(source, guestpath.SandboxMountPrefix) - return filepath.Join(wcowSandboxMountPath, subPath) + wcowSandboxMountPathPrefix := fmt.Sprintf(wcowSandboxMountPath, sandboxID) + return filepath.Join(wcowSandboxMountPathPrefix, subPath) } diff --git a/internal/uvm/create.go b/internal/uvm/create.go index 59352f7794..4286e8413f 100644 --- a/internal/uvm/create.go +++ b/internal/uvm/create.go @@ -9,6 +9,7 @@ import ( "os" "path/filepath" "runtime" + "time" "github.com/Microsoft/go-winio/pkg/guid" "github.com/sirupsen/logrus" @@ -386,6 +387,25 @@ func (uvm *UtilityVM) ExitError() error { return uvm.hcsSystem.ExitError() } +// StartTime returns the time at which the utility VM was started. +func (uvm *UtilityVM) StartTime() (time.Time, error) { + startTime := uvm.hcsSystem.StartTime() + if startTime.IsZero() { + return startTime, fmt.Errorf("uvm is not in started state") + } + + return startTime, nil +} + +// StopTime returns the time at which the utility VM was stopped. +func (uvm *UtilityVM) StopTime() (time.Time, error) { + if !uvm.IsStopped() { + return time.Time{}, fmt.Errorf("uvm is not in stopped state") + } + + return uvm.hcsSystem.StopTime(), nil +} + func defaultProcessorCount() int32 { if runtime.NumCPU() == 1 { return 1 diff --git a/internal/uvm/start.go b/internal/uvm/start.go index 781bc3c417..3ed5cdc4a0 100644 --- a/internal/uvm/start.go +++ b/internal/uvm/start.go @@ -444,3 +444,8 @@ func (uvm *UtilityVM) accept(ctx context.Context, l net.Listener, closeListener } return nil, err } + +// IsStopped returns true if the utility VM is in the stopped state. +func (uvm *UtilityVM) IsStopped() bool { + return uvm.hcsSystem.Stopped() +}