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 <weyang1@microsoft.com>

Author: Weyang1

cmd/containerd-shim-runhcs-v1/task_hcs.go

@@ -935,19 +935,26 @@ 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 and retrieve CPU affinity.
 			tempSpec := &specs.Spec{
 				Windows: &specs.Windows{
 					Resources: &specs.WindowsResources{
 						CPU: cpu,
 					},
 				},
 			}
-			affinity, err := hcsoci.ConvertCPUAffinity(tempSpec)
+			affinities, err := hcsoci.ConvertCPUAffinity(tempSpec)
 			if err != nil {
 				return err
 			}
-			req.Affinity = affinity
+			groupAffs := make([]hcsschema.ProcessorGroupAffinity, len(affinities))
+			for i, a := range affinities {
+				groupAffs[i] = hcsschema.ProcessorGroupAffinity{
+					Mask:  a.Mask,
+					Group: uint16(a.Group),
+				}
+			}
+			req.GroupAffinities = groupAffs
 		}
 		return ht.requestUpdateContainer(ctx, resourcepaths.SiloProcessorResourcePath, req)
 	}

internal/hcs/schema2/processor.go

@@ -3,7 +3,7 @@
  *
  * No description provided (generated by Swagger Codegen https://github.com/swagger-api/swagger-codegen)
  *
- * API version: 2.1
+ * API version: 2.4
  * Generated by: Swagger Codegen (https://github.com/swagger-api/swagger-codegen.git)
  */
 
@@ -16,5 +16,8 @@ type Processor struct {
 
 	Weight int32 `json:"Weight,omitempty"`
 
-	Affinity uint64 `json:"Affinity,omitempty"`
+	// GroupAffinities specifies the processor group affinity for the container.
+	// Each entry pins the container to the given set of processors within a processor group.
+	// Requires Windows Server 2022 (build 20348) or later.
+	GroupAffinities []ProcessorGroupAffinity `json:"GroupAffinities,omitempty"`
 }

internal/hcs/schema2/processor_group_affinity.go

@@ -0,0 +1,23 @@
+/*
+ * HCS API
+ *
+ * No description provided (generated by Swagger Codegen https://github.com/swagger-api/swagger-codegen)
+ *
+ * API version: 2.4
+ * Generated by: Swagger Codegen (https://github.com/swagger-api/swagger-codegen.git)
+ */
+
+package hcsschema
+
+// ProcessorGroupAffinity specifies a processor group and an affinity mask within
+// that group for an HCS container. It mirrors the Win32 GROUP_AFFINITY structure.
+// Requires Windows Server 2022 (build 20348) or later.
+// https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/miniport/ns-miniport-_group_affinity
+type ProcessorGroupAffinity struct {
+	// Mask is the bitmask of processors within Group.
+	Mask uint64 `json:"Mask,omitempty"`
+	// Group is the processor group number (0-based). Group 0 is the most common
+	// value; the tag intentionally omits omitempty so that group 0 is not
+	// silently dropped from the JSON sent to HCS.
+	Group uint16 `json:"Group"`
+}

internal/hcsoci/hcsdoc_wcow.go

@@ -31,6 +31,22 @@ import (
 
 const createContainerSubdirectoryForProcessDumpSuffix = "{container_id}"
 
+// Sentinel errors returned by ConvertCPUAffinity.
+var (
+	// ErrCPUAffinityMultipleGroupsNotSupported is returned when multiple processor-group
+	// affinity entries are requested on a host older than Windows Server 2022 (build 20348),
+	// which does not support multi-group affinity for job object silos.
+	// On Windows Server 2022+, multiple processor groups are fully supported.
+	ErrCPUAffinityMultipleGroupsNotSupported = errors.New("cpu affinity with multiple processor groups requires Windows Server 2022 or later")
+	// ErrCPUAffinityNonZeroGroupNotSupported is returned when a non-zero processor group is
+	// requested on a host older than Windows Server 2022 (build 20348).
+	// On Windows Server 2022+, non-zero processor groups are fully supported.
+	ErrCPUAffinityNonZeroGroupNotSupported = errors.New("cpu affinity with a non-zero processor group requires Windows Server 2022 or later")
+	// ErrCPUAffinityMaskZero is returned when an affinity entry has a zero bitmask,
+	// which would select no processors and is always invalid.
+	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 {
@@ -97,26 +113,36 @@ func createMountsConfig(ctx context.Context, coi *createOptionsInternal) (*mount
 // 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) {
+// Returns the validated affinity entries (nil if not specified) and any validation error.
+// Multiple processor groups and non-zero group numbers require Windows Server 2022
+// (build 20348) or later; on older hosts only a single entry for group 0 is accepted.
+func ConvertCPUAffinity(spec *specs.Spec) ([]specs.WindowsCPUGroupAffinity, error) {
 	if spec.Windows == nil || spec.Windows.Resources == nil || spec.Windows.Resources.CPU == nil || len(spec.Windows.Resources.CPU.Affinity) == 0 {
-		return 0, nil
+		return nil, 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)
+
+	// Zero masks are never valid regardless of OS version.
+	for i, a := range affinity {
+		if a.Mask == 0 {
+			return nil, fmt.Errorf("%w: entry %d has zero mask", ErrCPUAffinityMaskZero, i)
+		}
 	}
-	if affinity[0].Mask == 0 {
-		return 0, fmt.Errorf("cpu affinity mask must be non-zero")
+
+	// Determine whether multi-group features are needed: either multiple entries,
+	// or a single entry targeting a non-zero processor group.
+	multiGroup := len(affinity) > 1 || affinity[0].Group != 0
+
+	// Multiple processor groups are only supported on Windows Server 2022+.
+	if multiGroup && osversion.Build() < osversion.LTSC2022 {
+		if len(affinity) > 1 {
+			return nil, fmt.Errorf("%w: %d entries", ErrCPUAffinityMultipleGroupsNotSupported, len(affinity))
+		}
+		return nil, fmt.Errorf("%w: group %d", ErrCPUAffinityNonZeroGroupNotSupported, affinity[0].Group)
 	}
-	return affinity[0].Mask, nil
+
+	return affinity, nil
 }
 
 // ConvertCPULimits handles the logic of converting and validating the containers CPU limits
@@ -209,6 +235,7 @@ func createWindowsContainerDocument(ctx context.Context, coi *createOptionsInter
 		return nil, nil, err
 	}
 
+	// Validate and retrieve CPU affinity from the spec.
 	cpuAffinity, err := ConvertCPUAffinity(coi.Spec)
 	if err != nil {
 		return nil, nil, err
@@ -262,12 +289,22 @@ func createWindowsContainerDocument(ctx context.Context, coi *createOptionsInter
 	v1.ProcessorMaximum = int64(cpuLimit)
 	v1.ProcessorWeight = uint64(cpuWeight)
 
-	v2Container.Processor = &hcsschema.Processor{
-		Count:    cpuCount,
-		Maximum:  cpuLimit,
-		Weight:   cpuWeight,
-		Affinity: cpuAffinity,
+	v2Processor := &hcsschema.Processor{
+		Count:   cpuCount,
+		Maximum: cpuLimit,
+		Weight:  cpuWeight,
+	}
+	if len(cpuAffinity) > 0 {
+		groupAffs := make([]hcsschema.ProcessorGroupAffinity, len(cpuAffinity))
+		for i, a := range cpuAffinity {
+			groupAffs[i] = hcsschema.ProcessorGroupAffinity{
+				Mask:  a.Mask,
+				Group: uint16(a.Group),
+			}
+		}
+		v2Processor.GroupAffinities = groupAffs
 	}
+	v2Container.Processor = v2Processor
 
 	// Memory Resources
 	memoryMaxInMB := oci.ParseAnnotationsMemory(ctx, coi.Spec, annotations.ContainerMemorySizeInMB, 0)

internal/hcsoci/hcsdoc_wcow_test.go

@@ -3,10 +3,12 @@
 package hcsoci
 
 import (
-	"strings"
+	"errors"
 	"testing"
 
 	specs "github.com/opencontainers/runtime-spec/specs-go"
+
+	"github.com/Microsoft/hcsshim/osversion"
 )
 
 func TestConvertCPUAffinity_Group0MaskSet(t *testing.T) {
@@ -22,16 +24,16 @@ func TestConvertCPUAffinity_Group0MaskSet(t *testing.T) {
 		},
 	}
 
-	affinity, err := ConvertCPUAffinity(s)
+	affinities, 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))
+	if len(affinities) != 1 || affinities[0].Mask != 0x3 || affinities[0].Group != 0 {
+		t.Fatalf("unexpected cpu affinity: got %v", affinities)
 	}
 }
 
-func TestConvertCPUAffinity_MultiGroupRejected(t *testing.T) {
+func TestConvertCPUAffinity_MultiGroup(t *testing.T) {
 	s := &specs.Spec{
 		Windows: &specs.Windows{
 			Resources: &specs.WindowsResources{
@@ -45,16 +47,26 @@ func TestConvertCPUAffinity_MultiGroupRejected(t *testing.T) {
 		},
 	}
 
-	_, 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)
+	affinities, err := ConvertCPUAffinity(s)
+	if osversion.Build() >= osversion.LTSC2022 {
+		// Multi-group is supported on WS2022+.
+		if err != nil {
+			t.Fatalf("expected success for multi-group on WS2022+, got: %v", err)
+		}
+		if len(affinities) != 2 {
+			t.Fatalf("expected 2 affinity entries, got %d", len(affinities))
+		}
+	} else {
+		if err == nil {
+			t.Fatal("expected error for multiple affinity entries on pre-WS2022")
+		}
+		if !errors.Is(err, ErrCPUAffinityMultipleGroupsNotSupported) {
+			t.Fatalf("unexpected error: %v", err)
+		}
 	}
 }
 
-func TestConvertCPUAffinity_NonZeroGroupRejected(t *testing.T) {
+func TestConvertCPUAffinity_NonZeroGroup(t *testing.T) {
 	s := &specs.Spec{
 		Windows: &specs.Windows{
 			Resources: &specs.WindowsResources{
@@ -67,12 +79,22 @@ func TestConvertCPUAffinity_NonZeroGroupRejected(t *testing.T) {
 		},
 	}
 
-	_, 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)
+	affinities, err := ConvertCPUAffinity(s)
+	if osversion.Build() >= osversion.LTSC2022 {
+		// Non-zero group is supported on WS2022+.
+		if err != nil {
+			t.Fatalf("expected success for non-zero group on WS2022+, got: %v", err)
+		}
+		if len(affinities) != 1 || affinities[0].Group != 1 {
+			t.Fatalf("unexpected affinity: got %v", affinities)
+		}
+	} else {
+		if err == nil {
+			t.Fatal("expected error for non-zero affinity group on pre-WS2022")
+		}
+		if !errors.Is(err, ErrCPUAffinityNonZeroGroupNotSupported) {
+			t.Fatalf("unexpected error: %v", err)
+		}
 	}
 }
 
@@ -93,7 +115,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)
 	}
 }
@@ -137,12 +159,12 @@ func TestConvertCPUAffinity_NoAffinity(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			affinity, err := ConvertCPUAffinity(tc.spec)
+			affinities, err := ConvertCPUAffinity(tc.spec)
 			if err != nil {
 				t.Fatalf("ConvertCPUAffinity failed: %v", err)
 			}
-			if affinity != 0 {
-				t.Fatalf("expected zero affinity, got %d", affinity)
+			if len(affinities) != 0 {
+				t.Fatalf("expected empty affinities, got %v", affinities)
 			}
 		})
 	}

internal/jobcontainers/oci.go

@@ -4,7 +4,6 @@ package jobcontainers
 
 import (
 	"context"
-	"fmt"
 
 	"github.com/Microsoft/hcsshim/internal/hcsoci"
 	"github.com/Microsoft/hcsshim/internal/jobobject"
@@ -41,19 +40,21 @@ func specToLimits(ctx context.Context, cid string, s *specs.Spec) (*jobobject.Jo
 		return nil, err
 	}
 
-	var cpuAffinity uint64
-	if s.Windows != nil && s.Windows.Resources != nil && s.Windows.Resources.CPU != nil && len(s.Windows.Resources.CPU.Affinity) > 0 {
-		affinity := s.Windows.Resources.CPU.Affinity
-		if len(affinity) != 1 {
-			return nil, fmt.Errorf("cpu affinity with multiple processor groups is not supported")
-		}
-		if affinity[0].Group != 0 {
-			return nil, fmt.Errorf("cpu affinity processor group %d is not supported", affinity[0].Group)
-		}
-		if affinity[0].Mask == 0 {
-			return nil, fmt.Errorf("cpu affinity mask must be non-zero")
+	// Validate and retrieve CPU affinity using the shared helper, which enforces the
+	// OS version gate for multi-group support (WS2022+).
+	affinities, err := hcsoci.ConvertCPUAffinity(s)
+	if err != nil {
+		return nil, err
+	}
+	var groupAffinities []jobobject.GroupAffinity
+	if len(affinities) > 0 {
+		groupAffinities = make([]jobobject.GroupAffinity, len(affinities))
+		for i, a := range affinities {
+			groupAffinities[i] = jobobject.GroupAffinity{
+				Mask:  a.Mask,
+				Group: uint16(a.Group),
+			}
 		}
-		cpuAffinity = affinity[0].Mask
 	}
 
 	realCPULimit, realCPUWeight := uint32(cpuLimit), uint32(cpuWeight)
@@ -77,7 +78,7 @@ func specToLimits(ctx context.Context, cid string, s *specs.Spec) (*jobobject.Jo
 	return &jobobject.JobLimits{
 		CPULimit:           realCPULimit,
 		CPUWeight:          realCPUWeight,
-		CPUAffinity:        cpuAffinity,
+		GroupAffinities:    groupAffinities,
 		MaxIOPS:            maxIops,
 		MaxBandwidth:       maxBandwidth,
 		MemoryLimitInBytes: memLimitMB * memory.MiB,