config: switch PidsLimit to *int64

Previously we would treat a value of "0" as meaning "no-op", which lead
to quite a bit of confusion. The runtime-spec has been updated to make
the "pids.limit" value a pointer, and explicitly state that:

 * Values >= 0 should be treated as normal values; and
 * -1 indicates "max".

In practice this means we should switch PidsLimit to an *int64. Luckily,
this is actually backwards-compatible with our previous JSON -- an old
value of "0" would be omitted from the output, which will now be parsed
as "nil". The handling by our cgroup code would be identical but the
latter now correctly reflects the guidance by the runtime-spec.

Signed-off-by: Aleksa Sarai <[email protected]>

Author: cyphar

config_linux.go

@@ -90,8 +90,8 @@ type Resources struct {
 	// Cgroup's SCHED_IDLE value.
 	CPUIdle *int64 `json:"cpu_idle,omitempty"`
 
-	// Process limit; set <= `0' to disable limit.
-	PidsLimit int64 `json:"pids_limit,omitempty"`
+	// Process limit; set < `0' to disable limit. `nil` means "keep current limit".
+	PidsLimit *int64 `json:"pids_limit,omitempty"`
 
 	// Specifies per cgroup weight, range is from 10 to 1000.
 	BlkioWeight uint16 `json:"blkio_weight,omitempty"`

devices/systemd_test.go

@@ -13,6 +13,8 @@ import (
 	"github.com/opencontainers/cgroups/systemd"
 )
 
+func toPtr[T any](v T) *T { return &v }
+
 // TestPodSkipDevicesUpdate checks that updating a pod having SkipDevices: true
 // does not result in spurious "permission denied" errors in a container
 // running under the pod. The test is somewhat similar in nature to the
@@ -32,7 +34,7 @@ func TestPodSkipDevicesUpdate(t *testing.T) {
 		Parent:  "system.slice",
 		Name:    podName,
 		Resources: &cgroups.Resources{
-			PidsLimit:   42,
+			PidsLimit:   toPtr[int64](42),
 			Memory:      32 * 1024 * 1024,
 			SkipDevices: true,
 		},
@@ -96,7 +98,7 @@ func TestPodSkipDevicesUpdate(t *testing.T) {
 
 	// Now update the pod a few times.
 	for range 42 {
-		podConfig.Resources.PidsLimit++
+		(*podConfig.Resources.PidsLimit)++
 		podConfig.Resources.Memory += 1024 * 1024
 		if err := pm.Set(podConfig.Resources); err != nil {
 			t.Fatal(err)

fs/pids.go

@@ -19,12 +19,12 @@ func (s *PidsGroup) Apply(path string, _ *cgroups.Resources, pid int) error {
 }
 
 func (s *PidsGroup) Set(path string, r *cgroups.Resources) error {
-	if r.PidsLimit != 0 {
+	if r.PidsLimit != nil {
 		// "max" is the fallback value.
 		limit := "max"
 
-		if r.PidsLimit > 0 {
-			limit = strconv.FormatInt(r.PidsLimit, 10)
+		if val := *r.PidsLimit; val >= 0 {
+			limit = strconv.FormatInt(val, 10)
 		}
 
 		if err := cgroups.WriteFile(path, "pids.max", limit); err != nil {

fs/pids_test.go

@@ -13,6 +13,8 @@ const (
 	maxLimited   = 1024
 )
 
+func toPtr[T any](v T) *T { return &v }
+
 func TestPidsSetMax(t *testing.T) {
 	path := tempDir(t, "pids")
 
@@ -21,7 +23,7 @@ func TestPidsSetMax(t *testing.T) {
 	})
 
 	r := &cgroups.Resources{
-		PidsLimit: maxLimited,
+		PidsLimit: toPtr[int64](maxLimited),
 	}
 	pids := &PidsGroup{}
 	if err := pids.Set(path, r); err != nil {
@@ -37,6 +39,54 @@ func TestPidsSetMax(t *testing.T) {
 	}
 }
 
+func TestPidsSetZero(t *testing.T) {
+	path := tempDir(t, "pids")
+
+	writeFileContents(t, path, map[string]string{
+		"pids.max": "max",
+	})
+
+	r := &cgroups.Resources{
+		PidsLimit: toPtr[int64](0),
+	}
+	pids := &PidsGroup{}
+	if err := pids.Set(path, r); err != nil {
+		t.Fatal(err)
+	}
+
+	value, err := fscommon.GetCgroupParamUint(path, "pids.max")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if value != 0 {
+		t.Fatalf("Expected 0, got %d for setting pids.max = 0", value)
+	}
+}
+
+func TestPidsUnset(t *testing.T) {
+	path := tempDir(t, "pids")
+
+	writeFileContents(t, path, map[string]string{
+		"pids.max": "12345",
+	})
+
+	r := &cgroups.Resources{
+		PidsLimit: nil,
+	}
+	pids := &PidsGroup{}
+	if err := pids.Set(path, r); err != nil {
+		t.Fatal(err)
+	}
+
+	value, err := fscommon.GetCgroupParamUint(path, "pids.max")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if value != 12345 {
+		t.Fatalf("Expected 12345, got %d for not setting pids.max", value)
+	}
+}
+
 func TestPidsSetUnlimited(t *testing.T) {
 	path := tempDir(t, "pids")
 
@@ -45,7 +95,7 @@ func TestPidsSetUnlimited(t *testing.T) {
 	})
 
 	r := &cgroups.Resources{
-		PidsLimit: maxUnlimited,
+		PidsLimit: toPtr[int64](maxUnlimited),
 	}
 	pids := &PidsGroup{}
 	if err := pids.Set(path, r); err != nil {

fs2/pids.go

@@ -13,14 +13,14 @@ import (
 )
 
 func isPidsSet(r *cgroups.Resources) bool {
-	return r.PidsLimit != 0
+	return r.PidsLimit != nil
 }
 
 func setPids(dirPath string, r *cgroups.Resources) error {
 	if !isPidsSet(r) {
 		return nil
 	}
-	if val := numToStr(r.PidsLimit); val != "" {
+	if val := numToStr(*r.PidsLimit); val != "" {
 		if err := cgroups.WriteFile(dirPath, "pids.max", val); err != nil {
 			return err
 		}

systemd/v1.go

@@ -97,9 +97,9 @@ func genV1ResourcesProperties(r *cgroups.Resources, cm *dbusConnManager) ([]syst
 			newProp("BlockIOWeight", uint64(r.BlkioWeight)))
 	}
 
-	if r.PidsLimit > 0 || r.PidsLimit == -1 {
+	if r.PidsLimit != nil {
 		properties = append(properties,
-			newProp("TasksMax", uint64(r.PidsLimit)))
+			newProp("TasksMax", uint64(*r.PidsLimit)))
 	}
 
 	err = addCpuset(cm, &properties, r.CpusetCpus, r.CpusetMems)

systemd/v2.go

@@ -256,9 +256,9 @@ func genV2ResourcesProperties(dirPath string, r *cgroups.Resources, cm *dbusConn
 
 	addCPUQuota(cm, &properties, &r.CpuQuota, r.CpuPeriod)
 
-	if r.PidsLimit > 0 || r.PidsLimit == -1 {
+	if r.PidsLimit != nil {
 		properties = append(properties,
-			newProp("TasksMax", uint64(r.PidsLimit)))
+			newProp("TasksMax", uint64(*r.PidsLimit)))
 	}
 
 	err = addCpuset(cm, &properties, r.CpusetCpus, r.CpusetMems)