Add ConvertCPUSharesToCPUWeight

[kolyshkin]

The existing function, ConvertCPUSharesToCgroupV2Value, do not have a way to return an error, and thus can accept invalid cgroup v1 (cpu-shares) values and can return invalid cgroup v2 (cpu-weight) values.

Add a new one, ConvertCPUSharesToCPUWeight, which is identical but can return meaningful errors. Mark the old one as deprecated. Amend the test case to test the new implementation (and, since both are using the same formula, existing implementation is tested, too, except for 0).

Related to opencontainers/runc#4755

[kolyshkin]

@opencontainers/cgroups-maintainers PTAL

[haircommander]

LGTM

[thaJeztah]

thx! found some minor issues, and left a suggestion

[thaJeztah]

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.

[thaJeztah]

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

[thaJeztah]

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)
			}
		})
	}
}

[kolyshkin]

In the light of kubernetes/kubernetes#131216 and containers/crun#1767 I'm going to rework this.

[kolyshkin]

Closing in favor of #20