Merge pull request containers#15035 from cdoern/cgroup

fix container create/run throttle devices
cdoern · Jul 22, 2022 · da1f479 · da1f479
2 parents 935c150 + ad8940c
commit da1f479
Show file tree

Hide file tree

Showing 5 changed files with 34 additions and 13 deletions.
diff --git a/pkg/specgen/generate/container.go b/pkg/specgen/generate/container.go
@@ -494,10 +494,10 @@ func FinishThrottleDevices(s *specgen.SpecGenerator) error {
 	if s.ResourceLimits == nil {
 		s.ResourceLimits = &spec.LinuxResources{}
 	}
-	if s.ResourceLimits.BlockIO == nil {
-		s.ResourceLimits.BlockIO = &spec.LinuxBlockIO{}
-	}
 	if bps := s.ThrottleReadBpsDevice; len(bps) > 0 {
+		if s.ResourceLimits.BlockIO == nil {
+			s.ResourceLimits.BlockIO = &spec.LinuxBlockIO{}
+		}
 		for k, v := range bps {
 			statT := unix.Stat_t{}
 			if err := unix.Stat(k, &statT); err != nil {
@@ -512,6 +512,9 @@ func FinishThrottleDevices(s *specgen.SpecGenerator) error {
 		}
 	}
 	if bps := s.ThrottleWriteBpsDevice; len(bps) > 0 {
+		if s.ResourceLimits.BlockIO == nil {
+			s.ResourceLimits.BlockIO = &spec.LinuxBlockIO{}
+		}
 		for k, v := range bps {
 			statT := unix.Stat_t{}
 			if err := unix.Stat(k, &statT); err != nil {
@@ -523,6 +526,9 @@ func FinishThrottleDevices(s *specgen.SpecGenerator) error {
 		}
 	}
 	if iops := s.ThrottleReadIOPSDevice; len(iops) > 0 {
+		if s.ResourceLimits.BlockIO == nil {
+			s.ResourceLimits.BlockIO = &spec.LinuxBlockIO{}
+		}
 		for k, v := range iops {
 			statT := unix.Stat_t{}
 			if err := unix.Stat(k, &statT); err != nil {
@@ -534,6 +540,9 @@ func FinishThrottleDevices(s *specgen.SpecGenerator) error {
 		}
 	}
 	if iops := s.ThrottleWriteIOPSDevice; len(iops) > 0 {
+		if s.ResourceLimits.BlockIO == nil {
+			s.ResourceLimits.BlockIO = &spec.LinuxBlockIO{}
+		}
 		for k, v := range iops {
 			statT := unix.Stat_t{}
 			if err := unix.Stat(k, &statT); err != nil {

diff --git a/pkg/specgen/generate/container_create.go b/pkg/specgen/generate/container_create.go
@@ -55,6 +55,10 @@ func MakeContainer(ctx context.Context, rt *libpod.Runtime, s *specgen.SpecGener
 		}
 	}
 
+	if err := FinishThrottleDevices(s); err != nil {
+		return nil, nil, nil, err
+	}
+
 	// Set defaults for unset namespaces
 	if s.PidNS.IsDefault() {
 		defaultNS, err := GetDefaultNamespaceMode("pid", rtc, pod)

diff --git a/pkg/specgenutil/specgen.go b/pkg/specgenutil/specgen.go
@@ -77,11 +77,11 @@ func getIOLimits(s *specgen.SpecGenerator, c *entities.ContainerCreateOptions) (
 	if s.ResourceLimits == nil {
 		s.ResourceLimits = &specs.LinuxResources{}
 	}
-	if s.ResourceLimits.BlockIO == nil {
-		s.ResourceLimits.BlockIO = &specs.LinuxBlockIO{}
-	}
 	hasLimits := false
 	if b := c.BlkIOWeight; len(b) > 0 {
+		if s.ResourceLimits.BlockIO == nil {
+			s.ResourceLimits.BlockIO = &specs.LinuxBlockIO{}
+		}
 		u, err := strconv.ParseUint(b, 10, 16)
 		if err != nil {
 			return nil, fmt.Errorf("invalid value for blkio-weight: %w", err)
@@ -103,7 +103,6 @@ func getIOLimits(s *specgen.SpecGenerator, c *entities.ContainerCreateOptions) (
 		if s.ThrottleReadBpsDevice, err = parseThrottleBPSDevices(bps); err != nil {
 			return nil, err
 		}
-
 		hasLimits = true
 	}
 
@@ -131,8 +130,6 @@ func getIOLimits(s *specgen.SpecGenerator, c *entities.ContainerCreateOptions) (
 	if !hasLimits {
 		return nil, nil
 	}
-	io = s.ResourceLimits.BlockIO
-
 	return io, nil
 }
 

diff --git a/test/e2e/run_test.go b/test/e2e/run_test.go
@@ -715,7 +715,6 @@ USER bin`, BB)
 	})
 
 	It("podman run device-read-bps test", func() {
-		SkipIfCgroupV1("FIXME: #15035 - bps broken")
 		SkipIfRootless("FIXME: requested cgroup controller `io` is not available")
 		SkipIfRootlessCgroupsV1("Setting device-read-bps not supported on cgroupv1 for rootless users")
 
@@ -735,7 +734,6 @@ USER bin`, BB)
 	})
 
 	It("podman run device-write-bps test", func() {
-		SkipIfCgroupV1("FIXME: #15035 - bps broken")
 		SkipIfRootless("FIXME: requested cgroup controller `io` is not available")
 		SkipIfRootlessCgroupsV1("Setting device-write-bps not supported on cgroupv1 for rootless users")
 
@@ -754,7 +752,6 @@ USER bin`, BB)
 	})
 
 	It("podman run device-read-iops test", func() {
-		SkipIfCgroupV1("FIXME: #15035 - bps broken")
 		SkipIfRootless("FIXME: requested cgroup controller `io` is not available")
 		SkipIfRootlessCgroupsV1("Setting device-read-iops not supported on cgroupv1 for rootless users")
 		var session *PodmanSessionIntegration
@@ -773,7 +770,6 @@ USER bin`, BB)
 	})
 
 	It("podman run device-write-iops test", func() {
-		SkipIfCgroupV1("FIXME: #15035 - bps broken")
 		SkipIfRootless("FIXME: requested cgroup controller `io` is not available")
 		SkipIfRootlessCgroupsV1("Setting device-write-iops not supported on cgroupv1 for rootless users")
 		var session *PodmanSessionIntegration

diff --git a/test/system/030-run.bats b/test/system/030-run.bats
@@ -855,4 +855,19 @@ EOF
     run_podman rm $output
 }
 
+@test "podman run --device-read-bps" {
+    skip_if_rootless "cannot use this flag in rootless mode"
+    # this test is a triple check on blkio flags since they seem to sneak by the tests
+    if is_cgroupsv2; then
+        run_podman run -dt --device-read-bps=/dev/zero:1M $IMAGE top
+        run_podman exec -it $output cat /sys/fs/cgroup/io.max
+        is "$output" ".*1:5 rbps=1048576 wbps=max riops=max wiops=max" "throttle devices passed successfully.*"
+    else
+        run_podman run -dt --device-read-bps=/dev/zero:1M $IMAGE top
+        run_podman exec -it $output cat /sys/fs/cgroup/blkio/blkio.throttle.read_bps_device
+        is "$output" ".*1:5 1048576" "throttle devices passed successfully.*"
+    fi
+    run_podman container rm -fa
+}
+
 # vim: filetype=sh