Merge pull request #11686 from cdoern/podDeviceOptions

Pod Device-Read-BPS support

11 files changed, 139 insertions, 67 deletions

diff --git a/cmd/podman/common/create.go b/cmd/podman/common/create.go
index e490fa121..e714b6785 100644
--- a/cmd/podman/common/create.go
+++ b/cmd/podman/common/create.go
@@ -164,14 +164,6 @@ func DefineCreateFlags(cmd *cobra.Command, cf *entities.ContainerCreateOptions,
 		)
 		_ = cmd.RegisterFlagCompletionFunc(deviceCgroupRuleFlagName, completion.AutocompleteNone)
 
-		deviceReadBpsFlagName := "device-read-bps"
-		createFlags.StringSliceVar(
-			&cf.DeviceReadBPs,
-			deviceReadBpsFlagName, []string{},
-			"Limit read rate (bytes per second) from a device (e.g. --device-read-bps=/dev/sda:1mb)",
-		)
-		_ = cmd.RegisterFlagCompletionFunc(deviceReadBpsFlagName, completion.AutocompleteDefault)
-
 		deviceReadIopsFlagName := "device-read-iops"
 		createFlags.StringSliceVar(
 			&cf.DeviceReadIOPs,
@@ -869,6 +861,7 @@ func DefineCreateFlags(cmd *cobra.Command, cf *entities.ContainerCreateOptions,
 		volumeDesciption,
 	)
 	_ = cmd.RegisterFlagCompletionFunc(volumeFlagName, AutocompleteVolumeFlag)
+
 	deviceFlagName := "device"
 	createFlags.StringSliceVar(
 		&cf.Devices,
@@ -876,4 +869,12 @@ func DefineCreateFlags(cmd *cobra.Command, cf *entities.ContainerCreateOptions,
 		"Add a host device to the container",
 	)
 	_ = cmd.RegisterFlagCompletionFunc(deviceFlagName, completion.AutocompleteDefault)
+
+	deviceReadBpsFlagName := "device-read-bps"
+	createFlags.StringSliceVar(
+		&cf.DeviceReadBPs,
+		deviceReadBpsFlagName, []string{},
+		"Limit read rate (bytes per second) from a device (e.g. --device-read-bps=/dev/sda:1mb)",
+	)
+	_ = cmd.RegisterFlagCompletionFunc(deviceReadBpsFlagName, completion.AutocompleteDefault)
 }
diff --git a/cmd/podman/pods/create.go b/cmd/podman/pods/create.go
index ca73a8356..d5aaf09ce 100644
--- a/cmd/podman/pods/create.go
+++ b/cmd/podman/pods/create.go
@@ -101,6 +101,7 @@ func create(cmd *cobra.Command, args []string) error {
 		podIDFD      *os.File
 		imageName    string
 		rawImageName string
+		podName      string
 	)
 	labelFile = infraOptions.LabelFile
 	labels = infraOptions.Label
@@ -158,10 +159,12 @@ func create(cmd *cobra.Command, args []string) error {
 				return err
 			}
 		}
+		podName = createOptions.Name
 		err = common.ContainerToPodOptions(&infraOptions, &createOptions)
 		if err != nil {
 			return err
 		}
+		createOptions.Name = podName
 	}
 
 	if cmd.Flag("pod-id-file").Changed {
@@ -264,6 +267,17 @@ func create(cmd *cobra.Command, args []string) error {
 		podSpec.ImageVolumes = podSpec.InfraContainerSpec.ImageVolumes
 		podSpec.OverlayVolumes = podSpec.InfraContainerSpec.OverlayVolumes
 		podSpec.Mounts = podSpec.InfraContainerSpec.Mounts
+
+		// Marshall and Unmarshal the spec in order to map similar entities
+		wrapped, err := json.Marshal(podSpec.InfraContainerSpec)
+		if err != nil {
+			return err
+		}
+		err = json.Unmarshal(wrapped, podSpec)
+		if err != nil {
+			return err
+		}
+		podSpec.Name = podName
 	}
 	PodSpec := entities.PodSpec{PodSpecGen: *podSpec}
 	response, err := registry.ContainerEngine().PodCreate(context.Background(), PodSpec)
diff --git a/docs/source/markdown/podman-pod-create.1.md b/docs/source/markdown/podman-pod-create.1.md
index fcb8ddeb9..4c36c66ca 100644
--- a/docs/source/markdown/podman-pod-create.1.md
+++ b/docs/source/markdown/podman-pod-create.1.md
@@ -41,7 +41,7 @@ Examples of the List Format:
 #### **--device**=_host-device_[**:**_container-device_][**:**_permissions_]
 
 Add a host device to the pod. Optional *permissions* parameter
-can be used to specify device permissions It is a combination of
+can be used to specify device permissions. It is a combination of
 **r** for read, **w** for write, and **m** for **mknod**(2).
 
 Example: **--device=/dev/sdc:/dev/xvdc:rwm**.
@@ -55,6 +55,10 @@ Podman may load kernel modules required for using the specified
 device. The devices that Podman will load modules for when necessary are:
 /dev/fuse.
 
+#### **--device-read-bps**=*path*
+
+Limit read rate (bytes per second) from a device (e.g. --device-read-bps=/dev/sda:1mb)
+
 #### **--dns**=*ipaddr*
 
 Set custom DNS servers in the /etc/resolv.conf file that will be shared between all containers in the pod. A special option, "none" is allowed which disables creation of /etc/resolv.conf for the pod.
diff --git a/libpod/container_inspect.go b/libpod/container_inspect.go
index ab79d82d9..277c3b960 100644
--- a/libpod/container_inspect.go
+++ b/libpod/container_inspect.go
@@ -531,49 +531,25 @@ func (c *Container) generateInspectContainerHostConfig(ctrSpec *spec.Spec, named
 					hostConfig.BlkioWeightDevice = append(hostConfig.BlkioWeightDevice, weightDev)
 				}
 
-				handleThrottleDevice := func(devs []spec.LinuxThrottleDevice) ([]define.InspectBlkioThrottleDevice, error) {
-					out := []define.InspectBlkioThrottleDevice{}
-					for _, dev := range devs {
-						key := fmt.Sprintf("%d:%d", dev.Major, dev.Minor)
-						if deviceNodes == nil {
-							nodes, err := util.FindDeviceNodes()
-							if err != nil {
-								return nil, err
-							}
-							deviceNodes = nodes
-						}
-						path, ok := deviceNodes[key]
-						if !ok {
-							logrus.Infof("Could not locate throttle device %s in system devices", key)
-							continue
-						}
-						throttleDev := define.InspectBlkioThrottleDevice{}
-						throttleDev.Path = path
-						throttleDev.Rate = dev.Rate
-						out = append(out, throttleDev)
-					}
-					return out, nil
-				}
-
-				readBps, err := handleThrottleDevice(ctrSpec.Linux.Resources.BlockIO.ThrottleReadBpsDevice)
+				readBps, err := blkioDeviceThrottle(deviceNodes, ctrSpec.Linux.Resources.BlockIO.ThrottleReadBpsDevice)
 				if err != nil {
 					return nil, err
 				}
 				hostConfig.BlkioDeviceReadBps = readBps
 
-				writeBps, err := handleThrottleDevice(ctrSpec.Linux.Resources.BlockIO.ThrottleWriteBpsDevice)
+				writeBps, err := blkioDeviceThrottle(deviceNodes, ctrSpec.Linux.Resources.BlockIO.ThrottleWriteBpsDevice)
 				if err != nil {
 					return nil, err
 				}
 				hostConfig.BlkioDeviceWriteBps = writeBps
 
-				readIops, err := handleThrottleDevice(ctrSpec.Linux.Resources.BlockIO.ThrottleReadIOPSDevice)
+				readIops, err := blkioDeviceThrottle(deviceNodes, ctrSpec.Linux.Resources.BlockIO.ThrottleReadIOPSDevice)
 				if err != nil {
 					return nil, err
 				}
 				hostConfig.BlkioDeviceReadIOps = readIops
 
-				writeIops, err := handleThrottleDevice(ctrSpec.Linux.Resources.BlockIO.ThrottleWriteIOPSDevice)
+				writeIops, err := blkioDeviceThrottle(deviceNodes, ctrSpec.Linux.Resources.BlockIO.ThrottleWriteIOPSDevice)
 				if err != nil {
 					return nil, err
 				}
@@ -894,3 +870,27 @@ func (c *Container) GetDevices(priv bool, ctrSpec spec.Spec, deviceNodes map[str
 	}
 	return devices, nil
 }
+
+func blkioDeviceThrottle(deviceNodes map[string]string, devs []spec.LinuxThrottleDevice) ([]define.InspectBlkioThrottleDevice, error) {
+	out := []define.InspectBlkioThrottleDevice{}
+	for _, dev := range devs {
+		key := fmt.Sprintf("%d:%d", dev.Major, dev.Minor)
+		if deviceNodes == nil {
+			nodes, err := util.FindDeviceNodes()
+			if err != nil {
+				return nil, err
+			}
+			deviceNodes = nodes
+		}
+		path, ok := deviceNodes[key]
+		if !ok {
+			logrus.Infof("Could not locate throttle device %s in system devices", key)
+			continue
+		}
+		throttleDev := define.InspectBlkioThrottleDevice{}
+		throttleDev.Path = path
+		throttleDev.Rate = dev.Rate
+		out = append(out, throttleDev)
+	}
+	return out, nil
+}
diff --git a/libpod/define/pod_inspect.go b/libpod/define/pod_inspect.go
index e78d97850..bc2c1d81f 100644
--- a/libpod/define/pod_inspect.go
+++ b/libpod/define/pod_inspect.go
@@ -61,6 +61,8 @@ type InspectPodData struct {
 	Mounts []InspectMount `json:"mounts,omitempty"`
 	// Devices contains the specified host devices
 	Devices []InspectDevice `json:"devices,omitempty"`
+	// BlkioDeviceReadBps contains the Read/Access limit for the pod's devices
+	BlkioDeviceReadBps []InspectBlkioThrottleDevice `json:"device_read_bps,omitempty"`
 }
 
 // InspectPodInfraConfig contains the configuration of the pod's infra
diff --git a/libpod/pod_api.go b/libpod/pod_api.go
index 971309f77..05349aff5 100644
--- a/libpod/pod_api.go
+++ b/libpod/pod_api.go
@@ -584,6 +584,7 @@ func (p *Pod) Inspect() (*define.InspectPodData, error) {
 	var infraConfig *define.InspectPodInfraConfig
 	var inspectMounts []define.InspectMount
 	var devices []define.InspectDevice
+	var deviceLimits []define.InspectBlkioThrottleDevice
 	if p.state.InfraContainerID != "" {
 		infra, err := p.runtime.GetContainer(p.state.InfraContainerID)
 		if err != nil {
@@ -604,12 +605,18 @@ func (p *Pod) Inspect() (*define.InspectPodData, error) {
 		if err != nil {
 			return nil, err
 		}
-
 		var nodes map[string]string
 		devices, err = infra.GetDevices(false, *infra.config.Spec, nodes)
 		if err != nil {
 			return nil, err
 		}
+		spec := infra.config.Spec
+		if spec.Linux != nil && spec.Linux.Resources != nil && spec.Linux.Resources.BlockIO != nil {
+			deviceLimits, err = blkioDeviceThrottle(nodes, spec.Linux.Resources.BlockIO.ThrottleReadBpsDevice)
+			if err != nil {
+				return nil, err
+			}
+		}
 
 		if len(infra.config.ContainerNetworkConfig.DNSServer) > 0 {
 			infraConfig.DNSServer = make([]string, 0, len(infra.config.ContainerNetworkConfig.DNSServer))
@@ -638,28 +645,29 @@ func (p *Pod) Inspect() (*define.InspectPodData, error) {
 	}
 
 	inspectData := define.InspectPodData{
-		ID:               p.ID(),
-		Name:             p.Name(),
-		Namespace:        p.Namespace(),
-		Created:          p.CreatedTime(),
-		CreateCommand:    p.config.CreateCommand,
-		State:            podState,
-		Hostname:         p.config.Hostname,
-		Labels:           p.Labels(),
-		CreateCgroup:     p.config.UsePodCgroup,
-		CgroupParent:     p.CgroupParent(),
-		CgroupPath:       p.state.CgroupPath,
-		CreateInfra:      infraConfig != nil,
-		InfraContainerID: p.state.InfraContainerID,
-		InfraConfig:      infraConfig,
-		SharedNamespaces: sharesNS,
-		NumContainers:    uint(len(containers)),
-		Containers:       ctrs,
-		CPUSetCPUs:       p.ResourceLim().CPU.Cpus,
-		CPUPeriod:        p.CPUPeriod(),
-		CPUQuota:         p.CPUQuota(),
-		Mounts:           inspectMounts,
-		Devices:          devices,
+		ID:                 p.ID(),
+		Name:               p.Name(),
+		Namespace:          p.Namespace(),
+		Created:            p.CreatedTime(),
+		CreateCommand:      p.config.CreateCommand,
+		State:              podState,
+		Hostname:           p.config.Hostname,
+		Labels:             p.Labels(),
+		CreateCgroup:       p.config.UsePodCgroup,
+		CgroupParent:       p.CgroupParent(),
+		CgroupPath:         p.state.CgroupPath,
+		CreateInfra:        infraConfig != nil,
+		InfraContainerID:   p.state.InfraContainerID,
+		InfraConfig:        infraConfig,
+		SharedNamespaces:   sharesNS,
+		NumContainers:      uint(len(containers)),
+		Containers:         ctrs,
+		CPUSetCPUs:         p.ResourceLim().CPU.Cpus,
+		CPUPeriod:          p.CPUPeriod(),
+		CPUQuota:           p.CPUQuota(),
+		Mounts:             inspectMounts,
+		Devices:            devices,
+		BlkioDeviceReadBps: deviceLimits,
 	}
 
 	return &inspectData, nil
diff --git a/pkg/domain/entities/pods.go b/pkg/domain/entities/pods.go
index f0c88d77e..0356383ec 100644
--- a/pkg/domain/entities/pods.go
+++ b/pkg/domain/entities/pods.go
@@ -119,6 +119,7 @@ type PodCreateOptions struct {
 	CGroupParent       string            `json:"cgroup_parent,omitempty"`
 	CreateCommand      []string          `json:"create_command,omitempty"`
 	Devices            []string          `json:"devices,omitempty"`
+	DeviceReadBPs      []string          `json:"device_read_bps,omitempty"`
 	Hostname           string            `json:"hostname,omitempty"`
 	Infra              bool              `json:"infra,omitempty"`
 	InfraImage         string            `json:"infra_image,omitempty"`
@@ -167,7 +168,7 @@ type ContainerCreateOptions struct {
 	CPUSetMems        string
 	Devices           []string `json:"devices,omitempty"`
 	DeviceCGroupRule  []string
-	DeviceReadBPs     []string
+	DeviceReadBPs     []string `json:"device_read_bps,omitempty"`
 	DeviceReadIOPs    []string
 	DeviceWriteBPs    []string
 	DeviceWriteIOPs   []string
@@ -200,7 +201,7 @@ type ContainerCreateOptions struct {
 	MemoryReservation string
 	MemorySwap        string
 	MemorySwappiness  int64
-	Name              string `json:"container_name,omitempty"`
+	Name              string `json:"container_name"`
 	NoHealthCheck     bool
 	OOMKillDisable    bool
 	OOMScoreAdj       int
diff --git a/pkg/specgen/generate/container.go b/pkg/specgen/generate/container.go
index 71b882510..d4bd6073e 100644
--- a/pkg/specgen/generate/container.go
+++ b/pkg/specgen/generate/container.go
@@ -191,9 +191,6 @@ func CompleteSpec(ctx context.Context, r *libpod.Runtime, s *specgen.SpecGenerat
 	if len(s.User) == 0 && inspectData != nil {
 		s.User = inspectData.Config.User
 	}
-	if err := finishThrottleDevices(s); err != nil {
-		return nil, err
-	}
 	// Unless already set via the CLI, check if we need to disable process
 	// labels or set the defaults.
 	if len(s.SelinuxOpts) == 0 {
@@ -251,10 +248,10 @@ func CompleteSpec(ctx context.Context, r *libpod.Runtime, s *specgen.SpecGenerat
 	return warnings, nil
 }
 
-// finishThrottleDevices takes the temporary representation of the throttle
+// FinishThrottleDevices takes the temporary representation of the throttle
 // devices in the specgen and looks up the major and major minors. it then
 // sets the throttle devices proper in the specgen
-func finishThrottleDevices(s *specgen.SpecGenerator) error {
+func FinishThrottleDevices(s *specgen.SpecGenerator) error {
 	if bps := s.ThrottleReadBpsDevice; len(bps) > 0 {
 		for k, v := range bps {
 			statT := unix.Stat_t{}
@@ -263,6 +260,9 @@ func finishThrottleDevices(s *specgen.SpecGenerator) error {
 			}
 			v.Major = (int64(unix.Major(uint64(statT.Rdev))))
 			v.Minor = (int64(unix.Minor(uint64(statT.Rdev))))
+			if s.ResourceLimits.BlockIO == nil {
+				s.ResourceLimits.BlockIO = new(spec.LinuxBlockIO)
+			}
 			s.ResourceLimits.BlockIO.ThrottleReadBpsDevice = append(s.ResourceLimits.BlockIO.ThrottleReadBpsDevice, v)
 		}
 	}
diff --git a/pkg/specgen/generate/container_create.go b/pkg/specgen/generate/container_create.go
index 6100e7a5b..a50685d0f 100644
--- a/pkg/specgen/generate/container_create.go
+++ b/pkg/specgen/generate/container_create.go
@@ -2,6 +2,7 @@ package generate
 
 import (
 	"context"
+	"fmt"
 	"os"
 	"path/filepath"
 	"strings"
@@ -52,6 +53,24 @@ func MakeContainer(ctx context.Context, rt *libpod.Runtime, s *specgen.SpecGener
 	if infraConfig != nil && len(infraConfig.Spec.Linux.Devices) > 0 {
 		s.DevicesFrom = append(s.DevicesFrom, infraConfig.ID)
 	}
+	if infraConfig != nil && infraConfig.Spec.Linux.Resources != nil && infraConfig.Spec.Linux.Resources.BlockIO != nil && len(infraConfig.Spec.Linux.Resources.BlockIO.ThrottleReadBpsDevice) > 0 {
+		tempDev := make(map[string]spec.LinuxThrottleDevice)
+		for _, val := range infraConfig.Spec.Linux.Resources.BlockIO.ThrottleReadBpsDevice {
+			nodes, err := util.FindDeviceNodes()
+			if err != nil {
+				return nil, nil, nil, err
+			}
+			key := fmt.Sprintf("%d:%d", val.Major, val.Minor)
+			tempDev[nodes[key]] = spec.LinuxThrottleDevice{Rate: uint64(val.Rate)}
+		}
+		for i, dev := range s.ThrottleReadBpsDevice {
+			tempDev[i] = dev
+		}
+		s.ThrottleReadBpsDevice = tempDev
+	}
+	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/specgen/podspecgen.go b/pkg/specgen/podspecgen.go
index 83fa9426c..ee4fbc13a 100644
--- a/pkg/specgen/podspecgen.go
+++ b/pkg/specgen/podspecgen.go
@@ -201,6 +201,8 @@ type PodResourceConfig struct {
 	CPUPeriod uint64 `json:"cpu_period,omitempty"`
 	// CPU quota of the cpuset, determined by --cpus
 	CPUQuota int64 `json:"cpu_quota,omitempty"`
+	// ThrottleReadBpsDevice contains the rate at which the devices in the pod can be read from/accessed
+	ThrottleReadBpsDevice map[string]spec.LinuxThrottleDevice `json:"throttleReadBpsDevice,omitempty"`
 }
 
 // NewPodSpecGenerator creates a new pod spec
diff --git a/test/e2e/pod_create_test.go b/test/e2e/pod_create_test.go
index c9924be72..e8bc871da 100644
--- a/test/e2e/pod_create_test.go
+++ b/test/e2e/pod_create_test.go
@@ -903,4 +903,25 @@ ENTRYPOINT ["sleep","99999"]
 
 	})
 
+	It("podman pod create --device-read-bps", func() {
+		SkipIfRootless("Cannot create devices in /dev in rootless mode")
+		SkipIfRootlessCgroupsV1("Setting device-read-bps not supported on cgroupv1 for rootless users")
+
+		podName := "testPod"
+		session := podmanTest.Podman([]string{"pod", "create", "--device-read-bps", "/dev/zero:1mb", "--name", podName})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+
+		if CGROUPSV2 {
+			session = podmanTest.Podman([]string{"run", "--rm", "--pod", podName, ALPINE, "sh", "-c", "cat /sys/fs/cgroup/$(sed -e 's|0::||' < /proc/self/cgroup)/io.max"})
+		} else {
+			session = podmanTest.Podman([]string{"run", "--rm", "--pod", podName, ALPINE, "cat", "/sys/fs/cgroup/blkio/blkio.throttle.read_bps_device"})
+		}
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+		if !CGROUPSV2 {
+			Expect(session.OutputToString()).To(ContainSubstring("1048576"))
+		}
+	})
+
 })