Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 25 additions & 5 deletions utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -413,16 +413,36 @@ func WriteCgroupProc(dir string, pid int) error {
return err
}

// ConvertCPUSharesToCgroupV2Value converts CPU shares to CPU weight.
// Since the OCI spec is designed for cgroup v1, in some cases
// there is need to convert from the cgroup v1 configuration to cgroup v2
// the formula for cpuShares is y = (1 + ((x - 2) * 9999) / 262142)
// convert from [2-262144] to [1-10000]
// 262144 comes from Linux kernel definition "#define MAX_SHARES (1UL << 18)"
// there is need to convert from the cgroup v1 configuration to cgroup v2.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// there is need to convert from the cgroup v1 configuration to cgroup v2.
// there is a need to convert from the cgroup v1 configuration to cgroup v2.

//
// Deprecated: use [ConvertCPUSharesToCPUWeight] instead.
func ConvertCPUSharesToCgroupV2Value(cpuShares uint64) uint64 {
// For both CPU shares and CPU weight, 0 means unset.
if cpuShares == 0 {
return 0
}
return (1 + ((cpuShares-2)*9999)/262142)
return convertCPUShares(cpuShares)
}

// ConvertCPUSharesToCPUWeight converts CPU shares (suitable for cgroup v1)
// to CPU weight (suitable for cgroup v2). If the conversion is not possible,
// an error is returned.
func ConvertCPUSharesToCPUWeight(shares uint64) (uint64, error) {
if shares == 0 {
return 0, nil
}
if shares < 2 || shares > 262144 {
return 0, errors.New("cpu-shares should be between 2 and 262144")
}
return convertCPUShares(shares), nil
}

func convertCPUShares(shares uint64) uint64 {
// Convert from [2-262144] to [1-10000].
// 262144 comes from Linux kernel definition "#define MAX_SHARES (1UL << 18)"
return 1 + ((shares-2)*9999)/262142
}

// ConvertMemorySwapToCgroupV2Value converts MemorySwap value from OCI spec
Expand Down
29 changes: 20 additions & 9 deletions utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -534,16 +534,27 @@ func TestGetHugePageSizeImpl(t *testing.T) {
}
}

func TestConvertCPUSharesToCgroupV2Value(t *testing.T) {
cases := map[uint64]uint64{
0: 0,
2: 1,
262144: 10000,
func TestConvertCPUSharesToCPUWeight(t *testing.T) {
cases := []struct {
in, out uint64
isErr bool
}{
{in: 0, out: 0},
{in: 2, out: 1},
{in: 262144, out: 10000},
{in: 1, isErr: true},
{in: 262145, isErr: true},
}
for i, expected := range cases {
got := ConvertCPUSharesToCgroupV2Value(i)
if got != expected {
t.Errorf("expected ConvertCPUSharesToCgroupV2Value(%d) to be %d, got %d", i, expected, got)
for _, tc := range cases {
got, err := ConvertCPUSharesToCPUWeight(tc.in)
Comment on lines +548 to +549
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might as well make it a subtests; for the "should have no error" case, we can check both the error and the output.

for _, tc := range cases {
	t.Run(strconv.FormatUint(tc.in, 10), func(t *testing.T) {
		got, err := ConvertCPUSharesToCPUWeight(tc.in)
		if tc.isErr {
			if err == nil {
				t.Error("expected error, got nil")
			}
		} else {
			if err != nil {
				t.Errorf("expected no error, got: %v", err)
			}
			if got != tc.out {
				t.Errorf("want %d, got %d", tc.out, got)
			}
		}
	})
}

Or, if we want the tests to be more descriptive on intent, and make sure we're matching the right error;

func TestConvertCPUSharesToCPUWeight(t *testing.T) {
	cases := []struct {
		doc     string
		in, out uint64
		expErr  string
	}{
		{doc: "valid zero", in: 0, out: 0},
		{doc: "valid min value", in: 2, out: 1},
		{doc: "valid max value", in: 262144, out: 10000},
		{doc: "out of bound min", in: 1, expErr: "cpu-shares should be between 2 and 262144"},
		{doc: "out of bound max", in: 262145, expErr: "cpu-shares should be between 2 and 262144"},
	}
	for _, tc := range cases {
		t.Run(tc.doc, func(t *testing.T) {
			got, err := ConvertCPUSharesToCPUWeight(tc.in)
			if tc.expErr != "" {
				if err == nil || err.Error() != tc.expErr {
					t.Errorf("expected error %q, got %v", tc.expErr, err)
				}
				return
			}

			if err != nil {
				t.Errorf("expected no error, got: %v", err)
			}
			if got != tc.out {
				t.Errorf("want %d, got %d", tc.out, got)
			}
		})
	}
}

if tc.isErr {
if err == nil {
t.Errorf("ConvertCPUSharesToCPUWeight(%d): expected error, got nil", tc.in)
}
} else if err != nil {
t.Errorf("ConvertCPUSharesToCPUWeight(%d): expected error, got nil", tc.in)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message here doesn't match what's expected (we do not expect an error here)

} else if got != tc.out {
t.Errorf("ConvertCPUSharesToCPUWeight(%d): want %d, got %d", tc.in, tc.out, got)
}
}
}
Expand Down