Skip to content

Commit 587fb68

Browse files
author
Weyang1
committed
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>
1 parent 4ca1d20 commit 587fb68

4 files changed

Lines changed: 23 additions & 19 deletions

File tree

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -935,19 +935,18 @@ func (ht *hcsTask) updateWCOWContainerCPU(ctx context.Context, cpu *specs.Window
935935
req.Weight = int32(*cpu.Shares)
936936
}
937937
if len(cpu.Affinity) > 0 {
938-
// Create a temporary spec to reuse the existing ConvertCPUAffinity validation
938+
// Validate CPU affinity. TODO: wire through to HCS once the
939+
// correct Processor schema field is confirmed.
939940
tempSpec := &specs.Spec{
940941
Windows: &specs.Windows{
941942
Resources: &specs.WindowsResources{
942943
CPU: cpu,
943944
},
944945
},
945946
}
946-
affinity, err := hcsoci.ConvertCPUAffinity(tempSpec)
947-
if err != nil {
947+
if _, err := hcsoci.ConvertCPUAffinity(tempSpec); err != nil {
948948
return err
949949
}
950-
req.Affinity = affinity
951950
}
952951
return ht.requestUpdateContainer(ctx, resourcepaths.SiloProcessorResourcePath, req)
953952
}

internal/hcs/schema2/processor.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,4 @@ type Processor struct {
1515
Maximum int32 `json:"Maximum,omitempty"`
1616

1717
Weight int32 `json:"Weight,omitempty"`
18-
19-
Affinity uint64 `json:"Affinity,omitempty"`
2018
}

internal/hcsoci/hcsdoc_wcow.go

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,13 @@ import (
3131

3232
const createContainerSubdirectoryForProcessDumpSuffix = "{container_id}"
3333

34+
// Sentinel errors returned by ConvertCPUAffinity.
35+
var (
36+
ErrCPUAffinityMultipleGroups = errors.New("cpu affinity with multiple processor groups is not supported")
37+
ErrCPUAffinityNonZeroGroup = errors.New("cpu affinity processor group is not supported")
38+
ErrCPUAffinityMaskZero = errors.New("cpu affinity mask must be non-zero")
39+
)
40+
3441
// A simple wrapper struct around the container mount configs that should be added to the
3542
// container.
3643
type mountsConfig struct {
@@ -108,13 +115,13 @@ func ConvertCPUAffinity(spec *specs.Spec) (uint64, error) {
108115

109116
affinity := spec.Windows.Resources.CPU.Affinity
110117
if len(affinity) != 1 {
111-
return 0, fmt.Errorf("cpu affinity with multiple processor groups is not supported")
118+
return 0, fmt.Errorf("%w: %d entries", ErrCPUAffinityMultipleGroups, len(affinity))
112119
}
113120
if affinity[0].Group != 0 {
114-
return 0, fmt.Errorf("cpu affinity processor group %d is not supported", affinity[0].Group)
121+
return 0, fmt.Errorf("%w: %d", ErrCPUAffinityNonZeroGroup, affinity[0].Group)
115122
}
116123
if affinity[0].Mask == 0 {
117-
return 0, fmt.Errorf("cpu affinity mask must be non-zero")
124+
return 0, fmt.Errorf("%w", ErrCPUAffinityMaskZero)
118125
}
119126
return affinity[0].Mask, nil
120127
}
@@ -209,8 +216,9 @@ func createWindowsContainerDocument(ctx context.Context, coi *createOptionsInter
209216
return nil, nil, err
210217
}
211218

212-
cpuAffinity, err := ConvertCPUAffinity(coi.Spec)
213-
if err != nil {
219+
// Validate CPU affinity from the spec. TODO: wire through to HCS once the
220+
// correct Processor schema field is confirmed.
221+
if _, err := ConvertCPUAffinity(coi.Spec); err != nil {
214222
return nil, nil, err
215223
}
216224

@@ -263,10 +271,9 @@ func createWindowsContainerDocument(ctx context.Context, coi *createOptionsInter
263271
v1.ProcessorWeight = uint64(cpuWeight)
264272

265273
v2Container.Processor = &hcsschema.Processor{
266-
Count: cpuCount,
267-
Maximum: cpuLimit,
268-
Weight: cpuWeight,
269-
Affinity: cpuAffinity,
274+
Count: cpuCount,
275+
Maximum: cpuLimit,
276+
Weight: cpuWeight,
270277
}
271278

272279
// Memory Resources

internal/hcsoci/hcsdoc_wcow_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
package hcsoci
44

55
import (
6-
"strings"
6+
"errors"
77
"testing"
88

99
specs "github.com/opencontainers/runtime-spec/specs-go"
@@ -49,7 +49,7 @@ func TestConvertCPUAffinity_MultiGroupRejected(t *testing.T) {
4949
if err == nil {
5050
t.Fatal("expected error for multiple affinity entries")
5151
}
52-
if !strings.Contains(err.Error(), "multiple processor groups") {
52+
if !errors.Is(err, ErrCPUAffinityMultipleGroups) {
5353
t.Fatalf("unexpected error: %v", err)
5454
}
5555
}
@@ -71,7 +71,7 @@ func TestConvertCPUAffinity_NonZeroGroupRejected(t *testing.T) {
7171
if err == nil {
7272
t.Fatal("expected error for non-zero affinity group")
7373
}
74-
if !strings.Contains(err.Error(), "processor group") {
74+
if !errors.Is(err, ErrCPUAffinityNonZeroGroup) {
7575
t.Fatalf("unexpected error: %v", err)
7676
}
7777
}
@@ -93,7 +93,7 @@ func TestConvertCPUAffinity_ZeroMaskRejected(t *testing.T) {
9393
if err == nil {
9494
t.Fatal("expected error for zero affinity mask")
9595
}
96-
if !strings.Contains(err.Error(), "mask must be non-zero") {
96+
if !errors.Is(err, ErrCPUAffinityMaskZero) {
9797
t.Fatalf("unexpected error: %v", err)
9898
}
9999
}

0 commit comments

Comments
 (0)