From 99f312433ba9cf502e0735bca04a75fb0266b57e Mon Sep 17 00:00:00 2001 From: Weyang1 Date: Fri, 17 Apr 2026 22:02:03 +0000 Subject: [PATCH 1/3] hcsoci: wire Windows CPU affinity to HCS container processor schema Summary - Add Affinity field to hcsschema.Processor struct. - Add ConvertCPUAffinity function to validate and extract CPU affinity from OCI spec. - Wire CPU affinity into HCS silo container creation via createWindowsContainerDocument. - Add affinity support to updateWCOWContainerCPU for container updates. - Add unit tests for ConvertCPUAffinity validation. Motivation This is Phase 2 of Windows CPU affinity support, enabling HCS silo containers (non-JobObject path) to use CPU affinity specified in the OCI spec. Limitations (intentional for Phase 2) - Multiple affinity entries are rejected. - Non-zero processor groups are rejected. Signed-off-by: Weyang1 --- cmd/containerd-shim-runhcs-v1/task_hcs.go | 15 +++ internal/hcs/schema2/processor.go | 2 + internal/hcsoci/hcsdoc_wcow.go | 37 +++++- internal/hcsoci/hcsdoc_wcow_test.go | 149 ++++++++++++++++++++++ 4 files changed, 200 insertions(+), 3 deletions(-) create mode 100644 internal/hcsoci/hcsdoc_wcow_test.go diff --git a/cmd/containerd-shim-runhcs-v1/task_hcs.go b/cmd/containerd-shim-runhcs-v1/task_hcs.go index 9fbb1faf35..77779b3797 100644 --- a/cmd/containerd-shim-runhcs-v1/task_hcs.go +++ b/cmd/containerd-shim-runhcs-v1/task_hcs.go @@ -934,6 +934,21 @@ func (ht *hcsTask) updateWCOWContainerCPU(ctx context.Context, cpu *specs.Window if cpu.Shares != nil { req.Weight = int32(*cpu.Shares) } + if len(cpu.Affinity) > 0 { + // Create a temporary spec to reuse the existing ConvertCPUAffinity validation + tempSpec := &specs.Spec{ + Windows: &specs.Windows{ + Resources: &specs.WindowsResources{ + CPU: cpu, + }, + }, + } + affinity, err := hcsoci.ConvertCPUAffinity(tempSpec) + if err != nil { + return err + } + req.Affinity = affinity + } return ht.requestUpdateContainer(ctx, resourcepaths.SiloProcessorResourcePath, req) } diff --git a/internal/hcs/schema2/processor.go b/internal/hcs/schema2/processor.go index bb24e88da1..ff7617113c 100644 --- a/internal/hcs/schema2/processor.go +++ b/internal/hcs/schema2/processor.go @@ -15,4 +15,6 @@ type Processor struct { Maximum int32 `json:"Maximum,omitempty"` Weight int32 `json:"Weight,omitempty"` + + Affinity uint64 `json:"Affinity,omitempty"` } diff --git a/internal/hcsoci/hcsdoc_wcow.go b/internal/hcsoci/hcsdoc_wcow.go index d1d1a44c85..35e12f806c 100644 --- a/internal/hcsoci/hcsdoc_wcow.go +++ b/internal/hcsoci/hcsdoc_wcow.go @@ -94,6 +94,31 @@ func createMountsConfig(ctx context.Context, coi *createOptionsInternal) (*mount return &config, nil } +// ConvertCPUAffinity handles the logic of converting and validating the container's CPU affinity +// specified in the OCI spec to what HCS expects. +// +// Returns the CPU affinity bitmask (0 if not specified) and any validation error. +// Phase 2 limitations: +// - Multiple affinity entries are rejected +// - Non-zero processor groups are rejected +func ConvertCPUAffinity(spec *specs.Spec) (uint64, error) { + if spec.Windows == nil || spec.Windows.Resources == nil || spec.Windows.Resources.CPU == nil || len(spec.Windows.Resources.CPU.Affinity) == 0 { + return 0, nil + } + + affinity := spec.Windows.Resources.CPU.Affinity + if len(affinity) != 1 { + return 0, fmt.Errorf("cpu affinity with multiple processor groups is not supported") + } + if affinity[0].Group != 0 { + return 0, fmt.Errorf("cpu affinity processor group %d is not supported", affinity[0].Group) + } + if affinity[0].Mask == 0 { + return 0, fmt.Errorf("cpu affinity mask must be non-zero") + } + return affinity[0].Mask, nil +} + // ConvertCPULimits handles the logic of converting and validating the containers CPU limits // specified in the OCI spec to what HCS expects. // @@ -184,6 +209,11 @@ func createWindowsContainerDocument(ctx context.Context, coi *createOptionsInter return nil, nil, err } + cpuAffinity, err := ConvertCPUAffinity(coi.Spec) + if err != nil { + return nil, nil, err + } + if coi.HostingSystem != nil && coi.ScaleCPULimitsToSandbox && cpuLimit > 0 { // When ScaleCPULimitsToSandbox is set and we are running in a UVM, we assume // the CPU limit has been calculated based on the number of processors on the @@ -233,9 +263,10 @@ func createWindowsContainerDocument(ctx context.Context, coi *createOptionsInter v1.ProcessorWeight = uint64(cpuWeight) v2Container.Processor = &hcsschema.Processor{ - Count: cpuCount, - Maximum: cpuLimit, - Weight: cpuWeight, + Count: cpuCount, + Maximum: cpuLimit, + Weight: cpuWeight, + Affinity: cpuAffinity, } // Memory Resources diff --git a/internal/hcsoci/hcsdoc_wcow_test.go b/internal/hcsoci/hcsdoc_wcow_test.go new file mode 100644 index 0000000000..9755b5d777 --- /dev/null +++ b/internal/hcsoci/hcsdoc_wcow_test.go @@ -0,0 +1,149 @@ +//go:build windows + +package hcsoci + +import ( + "strings" + "testing" + + specs "github.com/opencontainers/runtime-spec/specs-go" +) + +func TestConvertCPUAffinity_Group0MaskSet(t *testing.T) { + s := &specs.Spec{ + Windows: &specs.Windows{ + Resources: &specs.WindowsResources{ + CPU: &specs.WindowsCPUResources{ + Affinity: []specs.WindowsCPUGroupAffinity{ + {Mask: 0x3, Group: 0}, + }, + }, + }, + }, + } + + affinity, err := ConvertCPUAffinity(s) + if err != nil { + t.Fatalf("ConvertCPUAffinity failed: %v", err) + } + if affinity != 0x3 { + t.Fatalf("unexpected cpu affinity: got %d want %d", affinity, uint64(0x3)) + } +} + +func TestConvertCPUAffinity_MultiGroupRejected(t *testing.T) { + s := &specs.Spec{ + Windows: &specs.Windows{ + Resources: &specs.WindowsResources{ + CPU: &specs.WindowsCPUResources{ + Affinity: []specs.WindowsCPUGroupAffinity{ + {Mask: 0x1, Group: 0}, + {Mask: 0x1, Group: 1}, + }, + }, + }, + }, + } + + _, err := ConvertCPUAffinity(s) + if err == nil { + t.Fatal("expected error for multiple affinity entries") + } + if !strings.Contains(err.Error(), "multiple processor groups") { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestConvertCPUAffinity_NonZeroGroupRejected(t *testing.T) { + s := &specs.Spec{ + Windows: &specs.Windows{ + Resources: &specs.WindowsResources{ + CPU: &specs.WindowsCPUResources{ + Affinity: []specs.WindowsCPUGroupAffinity{ + {Mask: 0x1, Group: 1}, + }, + }, + }, + }, + } + + _, err := ConvertCPUAffinity(s) + if err == nil { + t.Fatal("expected error for non-zero affinity group") + } + if !strings.Contains(err.Error(), "processor group") { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestConvertCPUAffinity_ZeroMaskRejected(t *testing.T) { + s := &specs.Spec{ + Windows: &specs.Windows{ + Resources: &specs.WindowsResources{ + CPU: &specs.WindowsCPUResources{ + Affinity: []specs.WindowsCPUGroupAffinity{ + {Mask: 0, Group: 0}, + }, + }, + }, + }, + } + + _, err := ConvertCPUAffinity(s) + if err == nil { + t.Fatal("expected error for zero affinity mask") + } + if !strings.Contains(err.Error(), "mask must be non-zero") { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestConvertCPUAffinity_NoAffinity(t *testing.T) { + testCases := []struct { + name string + spec *specs.Spec + }{ + { + name: "nil spec.Windows", + spec: &specs.Spec{}, + }, + { + name: "nil spec.Windows.Resources", + spec: &specs.Spec{ + Windows: &specs.Windows{}, + }, + }, + { + name: "nil spec.Windows.Resources.CPU", + spec: &specs.Spec{ + Windows: &specs.Windows{ + Resources: &specs.WindowsResources{}, + }, + }, + }, + { + name: "empty affinity slice", + spec: &specs.Spec{ + Windows: &specs.Windows{ + Resources: &specs.WindowsResources{ + CPU: &specs.WindowsCPUResources{ + Affinity: []specs.WindowsCPUGroupAffinity{}, + }, + }, + }, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + affinity, err := ConvertCPUAffinity(tc.spec) + if err != nil { + t.Fatalf("ConvertCPUAffinity failed: %v", err) + } + if affinity != 0 { + t.Fatalf("expected zero affinity, got %d", affinity) + } + }) + } +} From 4ca1d20365324adf800d6743c43f2ea2ba18548c Mon Sep 17 00:00:00 2001 From: Weyang1 Date: Mon, 20 Apr 2026 16:38:13 +0000 Subject: [PATCH 2/3] Retry CI Signed-off-by: Weyang1 From 587fb68c8e481ce5f7cbc590455a3755d51fdd27 Mon Sep 17 00:00:00 2001 From: Weyang1 Date: Thu, 30 Apr 2026 17:26:28 +0000 Subject: [PATCH 3/3] hcsoci: address PR review comments - Remove Affinity uint64 from schema2/Processor (not a real HCS API field); preserve ConvertCPUAffinity validation with a TODO to wire it properly once the correct schema field is confirmed - Define sentinel errors (ErrCPUAffinityMultipleGroups, ErrCPUAffinityNonZeroGroup, ErrCPUAffinityMaskZero) and replace strings.Contains checks in tests with errors.Is for more robust error identity verification Signed-off-by: Weyang1 --- cmd/containerd-shim-runhcs-v1/task_hcs.go | 7 +++---- internal/hcs/schema2/processor.go | 2 -- internal/hcsoci/hcsdoc_wcow.go | 25 +++++++++++++++-------- internal/hcsoci/hcsdoc_wcow_test.go | 8 ++++---- 4 files changed, 23 insertions(+), 19 deletions(-) diff --git a/cmd/containerd-shim-runhcs-v1/task_hcs.go b/cmd/containerd-shim-runhcs-v1/task_hcs.go index 77779b3797..526bd9eea5 100644 --- a/cmd/containerd-shim-runhcs-v1/task_hcs.go +++ b/cmd/containerd-shim-runhcs-v1/task_hcs.go @@ -935,7 +935,8 @@ func (ht *hcsTask) updateWCOWContainerCPU(ctx context.Context, cpu *specs.Window req.Weight = int32(*cpu.Shares) } if len(cpu.Affinity) > 0 { - // Create a temporary spec to reuse the existing ConvertCPUAffinity validation + // Validate CPU affinity. TODO: wire through to HCS once the + // correct Processor schema field is confirmed. tempSpec := &specs.Spec{ Windows: &specs.Windows{ Resources: &specs.WindowsResources{ @@ -943,11 +944,9 @@ func (ht *hcsTask) updateWCOWContainerCPU(ctx context.Context, cpu *specs.Window }, }, } - affinity, err := hcsoci.ConvertCPUAffinity(tempSpec) - if err != nil { + if _, err := hcsoci.ConvertCPUAffinity(tempSpec); err != nil { return err } - req.Affinity = affinity } return ht.requestUpdateContainer(ctx, resourcepaths.SiloProcessorResourcePath, req) } diff --git a/internal/hcs/schema2/processor.go b/internal/hcs/schema2/processor.go index ff7617113c..bb24e88da1 100644 --- a/internal/hcs/schema2/processor.go +++ b/internal/hcs/schema2/processor.go @@ -15,6 +15,4 @@ type Processor struct { Maximum int32 `json:"Maximum,omitempty"` Weight int32 `json:"Weight,omitempty"` - - Affinity uint64 `json:"Affinity,omitempty"` } diff --git a/internal/hcsoci/hcsdoc_wcow.go b/internal/hcsoci/hcsdoc_wcow.go index 35e12f806c..b2159283b4 100644 --- a/internal/hcsoci/hcsdoc_wcow.go +++ b/internal/hcsoci/hcsdoc_wcow.go @@ -31,6 +31,13 @@ import ( const createContainerSubdirectoryForProcessDumpSuffix = "{container_id}" +// Sentinel errors returned by ConvertCPUAffinity. +var ( + ErrCPUAffinityMultipleGroups = errors.New("cpu affinity with multiple processor groups is not supported") + ErrCPUAffinityNonZeroGroup = errors.New("cpu affinity processor group is not supported") + ErrCPUAffinityMaskZero = errors.New("cpu affinity mask must be non-zero") +) + // A simple wrapper struct around the container mount configs that should be added to the // container. type mountsConfig struct { @@ -108,13 +115,13 @@ func ConvertCPUAffinity(spec *specs.Spec) (uint64, error) { affinity := spec.Windows.Resources.CPU.Affinity if len(affinity) != 1 { - return 0, fmt.Errorf("cpu affinity with multiple processor groups is not supported") + return 0, fmt.Errorf("%w: %d entries", ErrCPUAffinityMultipleGroups, len(affinity)) } if affinity[0].Group != 0 { - return 0, fmt.Errorf("cpu affinity processor group %d is not supported", affinity[0].Group) + return 0, fmt.Errorf("%w: %d", ErrCPUAffinityNonZeroGroup, affinity[0].Group) } if affinity[0].Mask == 0 { - return 0, fmt.Errorf("cpu affinity mask must be non-zero") + return 0, fmt.Errorf("%w", ErrCPUAffinityMaskZero) } return affinity[0].Mask, nil } @@ -209,8 +216,9 @@ func createWindowsContainerDocument(ctx context.Context, coi *createOptionsInter return nil, nil, err } - cpuAffinity, err := ConvertCPUAffinity(coi.Spec) - if err != nil { + // Validate CPU affinity from the spec. TODO: wire through to HCS once the + // correct Processor schema field is confirmed. + if _, err := ConvertCPUAffinity(coi.Spec); err != nil { return nil, nil, err } @@ -263,10 +271,9 @@ func createWindowsContainerDocument(ctx context.Context, coi *createOptionsInter v1.ProcessorWeight = uint64(cpuWeight) v2Container.Processor = &hcsschema.Processor{ - Count: cpuCount, - Maximum: cpuLimit, - Weight: cpuWeight, - Affinity: cpuAffinity, + Count: cpuCount, + Maximum: cpuLimit, + Weight: cpuWeight, } // Memory Resources diff --git a/internal/hcsoci/hcsdoc_wcow_test.go b/internal/hcsoci/hcsdoc_wcow_test.go index 9755b5d777..fdc5c2ec4b 100644 --- a/internal/hcsoci/hcsdoc_wcow_test.go +++ b/internal/hcsoci/hcsdoc_wcow_test.go @@ -3,7 +3,7 @@ package hcsoci import ( - "strings" + "errors" "testing" specs "github.com/opencontainers/runtime-spec/specs-go" @@ -49,7 +49,7 @@ func TestConvertCPUAffinity_MultiGroupRejected(t *testing.T) { if err == nil { t.Fatal("expected error for multiple affinity entries") } - if !strings.Contains(err.Error(), "multiple processor groups") { + if !errors.Is(err, ErrCPUAffinityMultipleGroups) { t.Fatalf("unexpected error: %v", err) } } @@ -71,7 +71,7 @@ func TestConvertCPUAffinity_NonZeroGroupRejected(t *testing.T) { if err == nil { t.Fatal("expected error for non-zero affinity group") } - if !strings.Contains(err.Error(), "processor group") { + if !errors.Is(err, ErrCPUAffinityNonZeroGroup) { t.Fatalf("unexpected error: %v", err) } } @@ -93,7 +93,7 @@ func TestConvertCPUAffinity_ZeroMaskRejected(t *testing.T) { if err == nil { t.Fatal("expected error for zero affinity mask") } - if !strings.Contains(err.Error(), "mask must be non-zero") { + if !errors.Is(err, ErrCPUAffinityMaskZero) { t.Fatalf("unexpected error: %v", err) } }