libcontainer: set pids limit to max when set to 0

Signed-off-by: Peter Hunt <pehunt@redhat.com>

libcontainer/cgroups/devices/systemd_test.go

@@ -28,12 +28,13 @@ func TestPodSkipDevicesUpdate(t *testing.T) {
 	}
 
 	podName := "system-runc_test_pod" + t.Name() + ".slice"
+	l := int64(42)
 	podConfig := &configs.Cgroup{
 		Systemd: true,
 		Parent:  "system.slice",
 		Name:    podName,
 		Resources: &configs.Resources{
-			PidsLimit:   42,
+			PidsLimit:   &l,
 			Memory:      32 * 1024 * 1024,
 			SkipDevices: true,
 		},
@@ -97,7 +98,7 @@ func TestPodSkipDevicesUpdate(t *testing.T) {
 
 	// Now update the pod a few times.
 	for i := 0; i < 42; i++ {
-		podConfig.Resources.PidsLimit++
+		*podConfig.Resources.PidsLimit++
 		podConfig.Resources.Memory += 1024 * 1024
 		if err := pm.Set(podConfig.Resources); err != nil {
 			t.Fatal(err)

libcontainer/cgroups/fs/pids.go

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

libcontainer/cgroups/fs/pids_test.go

@@ -9,9 +9,10 @@ import (
 	"github.com/opencontainers/runc/libcontainer/configs"
 )
 
-const (
-	maxUnlimited = -1
-	maxLimited   = 1024
+var (
+	maxUnlimited int64 = -1
+	maxZero      int64 = 0
+	maxLimited   int64 = 1024
 )
 
 func TestPidsSetMax(t *testing.T) {
@@ -22,7 +23,7 @@ func TestPidsSetMax(t *testing.T) {
 	})
 
 	r := &configs.Resources{
-		PidsLimit: maxLimited,
+		PidsLimit: &maxLimited,
 	}
 	pids := &PidsGroup{}
 	if err := pids.Set(path, r); err != nil {
@@ -33,20 +34,45 @@ func TestPidsSetMax(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if value != maxLimited {
+	// Only done for comparison
+	if value != uint64(maxLimited) {
 		t.Fatalf("Expected %d, got %d for setting pids.max - limited", maxLimited, value)
 	}
 }
 
+func TestPidsSetUnlimitedWhenZero(t *testing.T) {
+	path := tempDir(t, "pids")
+
+	writeFileContents(t, path, map[string]string{
+		"pids.max": "max",
+	})
+
+	r := &configs.Resources{
+		PidsLimit: &maxZero,
+	}
+	pids := &PidsGroup{}
+	if err := pids.Set(path, r); err != nil {
+		t.Fatal(err)
+	}
+
+	value, err := fscommon.GetCgroupParamString(path, "pids.max")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if value != "max" {
+		t.Fatalf("Expected %s, got %s for setting pids.max - unlimited", "max", value)
+	}
+}
+
 func TestPidsSetUnlimited(t *testing.T) {
 	path := tempDir(t, "pids")
 
 	writeFileContents(t, path, map[string]string{
-		"pids.max": strconv.Itoa(maxLimited),
+		"pids.max": strconv.FormatInt(maxLimited, 10),
 	})
 
 	r := &configs.Resources{
-		PidsLimit: maxUnlimited,
+		PidsLimit: &maxUnlimited,
 	}
 	pids := &PidsGroup{}
 	if err := pids.Set(path, r); err != nil {
@@ -67,7 +93,7 @@ func TestPidsStats(t *testing.T) {
 
 	writeFileContents(t, path, map[string]string{
 		"pids.current": strconv.Itoa(1337),
-		"pids.max":     strconv.Itoa(maxLimited),
+		"pids.max":     strconv.FormatInt(maxLimited, 10),
 	})
 
 	pids := &PidsGroup{}
@@ -80,7 +106,7 @@ func TestPidsStats(t *testing.T) {
 		t.Fatalf("Expected %d, got %d for pids.current", 1337, stats.PidsStats.Current)
 	}
 
-	if stats.PidsStats.Limit != maxLimited {
+	if stats.PidsStats.Limit != uint64(maxLimited) {
 		t.Fatalf("Expected %d, got %d for pids.max", maxLimited, stats.PidsStats.Limit)
 	}
 }

libcontainer/cgroups/fs2/create.go

@@ -39,7 +39,7 @@ func needAnyControllers(r *configs.Resources) (bool, error) {
 		return ok
 	}
 
-	if isPidsSet(r) && have("pids") {
+	if r.PidsLimit != nil && have("pids") {
 		return true, nil
 	}
 	if isMemorySet(r) && have("memory") {

libcontainer/cgroups/fs2/pids.go

@@ -13,15 +13,11 @@ import (
 	"github.com/opencontainers/runc/libcontainer/configs"
 )
 
-func isPidsSet(r *configs.Resources) bool {
-	return r.PidsLimit != 0
-}
-
 func setPids(dirPath string, r *configs.Resources) error {
-	if !isPidsSet(r) {
+	if r.PidsLimit == nil {
 		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
 		}

libcontainer/cgroups/systemd/v1.go

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

libcontainer/cgroups/systemd/v2.go

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

libcontainer/configs/cgroup_linux.go

@@ -88,7 +88,7 @@ type Resources struct {
 	CPUIdle *int64 `json:"cpu_idle,omitempty"`
 
 	// Process limit; set <= `0' to disable limit.
-	PidsLimit int64 `json:"pids_limit"`
+	PidsLimit *int64 `json:"pids_limit"`
 
 	// Specifies per cgroup weight, range is from 10 to 1000.
 	BlkioWeight uint16 `json:"blkio_weight"`

libcontainer/integration/exec_test.go

@@ -535,14 +535,15 @@ func testPids(t *testing.T, systemd bool) {
 	}
 
 	config := newTemplateConfig(t, &tParam{systemd: systemd})
-	config.Cgroups.Resources.PidsLimit = -1
+	l := int64(-1)
+	config.Cgroups.Resources.PidsLimit = &l
 
 	// Running multiple processes, expecting it to succeed with no pids limit.
 	_ = runContainerOk(t, config, "/bin/sh", "-c", "/bin/true | /bin/true | /bin/true | /bin/true")
 
 	// Enforce a permissive limit. This needs to be fairly hand-wavey due to the
 	// issues with running Go binaries with pids restrictions (see below).
-	config.Cgroups.Resources.PidsLimit = 64
+	*config.Cgroups.Resources.PidsLimit = 64
 	_ = runContainerOk(t, config, "/bin/sh", "-c", `
 	/bin/true | /bin/true | /bin/true | /bin/true | /bin/true | /bin/true | bin/true | /bin/true |
 	/bin/true | /bin/true | /bin/true | /bin/true | /bin/true | /bin/true | bin/true | /bin/true |
@@ -551,7 +552,7 @@ func testPids(t *testing.T, systemd bool) {
 
 	// Enforce a restrictive limit. 64 * /bin/true + 1 * shell should cause
 	// this to fail reliably.
-	config.Cgroups.Resources.PidsLimit = 64
+	*config.Cgroups.Resources.PidsLimit = 64
 	out, _, err := runContainer(t, config, "/bin/sh", "-c", `
 	/bin/true | /bin/true | /bin/true | /bin/true | /bin/true | /bin/true | bin/true | /bin/true |
 	/bin/true | /bin/true | /bin/true | /bin/true | /bin/true | /bin/true | bin/true | /bin/true |

libcontainer/specconv/spec_linux.go

@@ -768,7 +768,8 @@ func CreateCgroupConfig(opts *CreateOpts, defaultDevs []*devices.Device) (*confi
 				c.Resources.CPUIdle = r.CPU.Idle
 			}
 			if r.Pids != nil {
-				c.Resources.PidsLimit = r.Pids.Limit
+				l := r.Pids.Limit
+				c.Resources.PidsLimit = &l
 			}
 			if r.BlockIO != nil {
 				if r.BlockIO.Weight != nil {

update.go

@@ -310,8 +310,9 @@ other options are ignored.
 		config.Cgroups.Resources.MemoryReservation = *r.Memory.Reservation
 		config.Cgroups.Resources.MemorySwap = *r.Memory.Swap
 		config.Cgroups.Resources.MemoryCheckBeforeUpdate = *r.Memory.CheckBeforeUpdate
-		config.Cgroups.Resources.PidsLimit = r.Pids.Limit
 		config.Cgroups.Resources.Unified = r.Unified
+		l := r.Pids.Limit
+		config.Cgroups.Resources.PidsLimit = &l
 
 		// Update Intel RDT
 		l3CacheSchema := context.String("l3-cache-schema")