From 972a1447a36c63e5da14237e8f752b62c7da2bb5 Mon Sep 17 00:00:00 2001 From: Leno Hou Date: Fri, 14 Dec 2018 10:09:59 +0000 Subject: [PATCH] feature: limit blkio device's read/write Bps/IOps fixes #2509 Signed-off-by: Leno Hou --- apis/swagger.yml | 12 ++-- apis/types/resources.go | 5 +- apis/types/update_config.go | 11 ---- cli/update.go | 36 +++++++----- cri/v1alpha2/cri.go | 1 - cri/v1alpha2/cri_utils.go | 1 + cri/v1alpha2/cri_utils_test.go | 1 + daemon/mgr/container.go | 2 +- test/cli_run_blkio_test.go | 100 +++++++++++++++++++++++++++++++++ 9 files changed, 135 insertions(+), 34 deletions(-) diff --git a/apis/swagger.yml b/apis/swagger.yml index cef952501c..e13f98f9a6 100644 --- a/apis/swagger.yml +++ b/apis/swagger.yml @@ -2471,12 +2471,6 @@ definitions: type: "array" items: type: "string" - DiskQuota: - type: "object" - description: "update disk quota for container" - x-nullable: true - additionalProperties: - type: "string" ContainerUpgradeConfig: description: | @@ -2630,6 +2624,12 @@ definitions: items: type: "string" example: "c 13:* rwm" + DiskQuota: + type: "object" + description: "update disk quota for container" + x-nullable: true + additionalProperties: + type: "string" KernelMemory: description: "Kernel memory limit in bytes." type: "integer" diff --git a/apis/types/resources.go b/apis/types/resources.go index 0397dc291a..abe5b458fa 100644 --- a/apis/types/resources.go +++ b/apis/types/resources.go @@ -89,6 +89,9 @@ type Resources struct { // A list of devices to add to the container. Devices []*DeviceMapping `json:"Devices"` + // update disk quota for container + DiskQuota map[string]string `json:"DiskQuota,omitempty"` + // Maximum IO in bytes per second for the container system drive (Windows only) IOMaximumBandwidth uint64 `json:"IOMaximumBandwidth"` @@ -122,7 +125,7 @@ type Resources struct { // Total memory limit (memory + swap). Set as `-1` to enable unlimited swap. MemorySwap int64 `json:"MemorySwap"` - // Tune a container's memory swappiness behavior. Accepts an integer between 0 and 100. + // Tune a container's memory swappiness behavior. Accepts an integer between 0 and 100. -1 is also accepted, as a legacy alias of 0. // Maximum: 100 // Minimum: -1 MemorySwappiness *int64 `json:"MemorySwappiness"` diff --git a/apis/types/update_config.go b/apis/types/update_config.go index 41aa6b1acd..32afe685d6 100644 --- a/apis/types/update_config.go +++ b/apis/types/update_config.go @@ -16,9 +16,6 @@ import ( type UpdateConfig struct { Resources - // update disk quota for container - DiskQuota map[string]string `json:"DiskQuota,omitempty"` - // A list of environment variables to set inside the container in the form `["VAR=value", ...]`. A variable without `=` is removed from the environment, rather than to have an empty value. // Env []string `json:"Env"` @@ -41,8 +38,6 @@ func (m *UpdateConfig) UnmarshalJSON(raw []byte) error { // AO1 var dataAO1 struct { - DiskQuota map[string]string `json:"DiskQuota,omitempty"` - Env []string `json:"Env"` Label []string `json:"Label"` @@ -53,8 +48,6 @@ func (m *UpdateConfig) UnmarshalJSON(raw []byte) error { return err } - m.DiskQuota = dataAO1.DiskQuota - m.Env = dataAO1.Env m.Label = dataAO1.Label @@ -75,8 +68,6 @@ func (m UpdateConfig) MarshalJSON() ([]byte, error) { _parts = append(_parts, aO0) var dataAO1 struct { - DiskQuota map[string]string `json:"DiskQuota,omitempty"` - Env []string `json:"Env"` Label []string `json:"Label"` @@ -84,8 +75,6 @@ func (m UpdateConfig) MarshalJSON() ([]byte, error) { RestartPolicy *RestartPolicy `json:"RestartPolicy,omitempty"` } - dataAO1.DiskQuota = m.DiskQuota - dataAO1.Env = m.Env dataAO1.Label = m.Label diff --git a/cli/update.go b/cli/update.go index 6799a86d69..d2931f0099 100644 --- a/cli/update.go +++ b/cli/update.go @@ -51,6 +51,10 @@ func (uc *UpdateCommand) addFlags() { flagSet.StringSliceVarP(&uc.labels, "label", "l", nil, "Set label for container") flagSet.StringVar(&uc.restartPolicy, "restart", "", "Restart policy to apply when container exits") flagSet.StringSliceVar(&uc.diskQuota, "disk-quota", nil, "Update disk quota for container(/=10g)") + flagSet.Var(&uc.blkioDeviceReadBps, "device-read-bps", "Update read rate (bytes per second) from a device") + flagSet.Var(&uc.blkioDeviceWriteBps, "device-write-bps", "Update write rate (bytes per second) from a device") + flagSet.Var(&uc.blkioDeviceReadIOps, "device-read-iops", "Update read rate (io per second) from a device") + flagSet.Var(&uc.blkioDeviceWriteIOps, "device-write-iops", "Update write rate (io per second) from a device") } // updateRun is the entry of update command. @@ -68,23 +72,28 @@ func (uc *UpdateCommand) updateRun(args []string) error { return err } - resource := types.Resources{ - CPUPeriod: uc.cpuperiod, - CPUShares: uc.cpushare, - CPUQuota: uc.cpuquota, - CpusetCpus: uc.cpusetcpus, - CpusetMems: uc.cpusetmems, - Memory: memory, - MemorySwap: memorySwap, - BlkioWeight: uc.blkioWeight, - } - - restartPolicy, err := opts.ParseRestartPolicy(uc.restartPolicy) + diskQuota, err := opts.ParseDiskQuota(uc.diskQuota) if err != nil { return err } - diskQuota, err := opts.ParseDiskQuota(uc.diskQuota) + resource := types.Resources{ + CPUPeriod: uc.cpuperiod, + CPUShares: uc.cpushare, + CPUQuota: uc.cpuquota, + CpusetCpus: uc.cpusetcpus, + CpusetMems: uc.cpusetmems, + DiskQuota: diskQuota, + Memory: memory, + MemorySwap: memorySwap, + BlkioWeight: uc.blkioWeight, + BlkioDeviceReadBps: uc.blkioDeviceReadBps.Value(), + BlkioDeviceWriteBps: uc.blkioDeviceWriteBps.Value(), + BlkioDeviceReadIOps: uc.blkioDeviceReadIOps.Value(), + BlkioDeviceWriteIOps: uc.blkioDeviceWriteIOps.Value(), + } + + restartPolicy, err := opts.ParseRestartPolicy(uc.restartPolicy) if err != nil { return err } @@ -94,7 +103,6 @@ func (uc *UpdateCommand) updateRun(args []string) error { Label: uc.labels, RestartPolicy: restartPolicy, Resources: resource, - DiskQuota: diskQuota, } apiClient := uc.cli.Client() diff --git a/cri/v1alpha2/cri.go b/cri/v1alpha2/cri.go index 8b23876ba2..7fee9209fa 100644 --- a/cri/v1alpha2/cri.go +++ b/cri/v1alpha2/cri.go @@ -1105,7 +1105,6 @@ func (c *CriManager) UpdateContainerResources(ctx context.Context, r *runtime.Up resources := r.GetLinux() updateConfig := &apitypes.UpdateConfig{ Resources: parseResourcesFromCRI(resources), - DiskQuota: resources.GetDiskQuota(), } err = c.ContainerMgr.Update(ctx, containerID, updateConfig) if err != nil { diff --git a/cri/v1alpha2/cri_utils.go b/cri/v1alpha2/cri_utils.go index cf4b99ad51..61e5615fa2 100644 --- a/cri/v1alpha2/cri_utils.go +++ b/cri/v1alpha2/cri_utils.go @@ -1043,6 +1043,7 @@ func parseResourcesFromCRI(runtimeResources *runtime.LinuxContainerResources) ap BlkioDeviceWriteBps: parseThrottleDeviceFromCRI(runtimeResources.GetBlkioDeviceWriteBps()), BlkioDeviceReadIOps: parseThrottleDeviceFromCRI(runtimeResources.GetBlkioDeviceRead_IOps()), BlkioDeviceWriteIOps: parseThrottleDeviceFromCRI(runtimeResources.GetBlkioDeviceWrite_IOps()), + DiskQuota: runtimeResources.GetDiskQuota(), KernelMemory: runtimeResources.GetKernelMemory(), MemoryReservation: runtimeResources.GetMemoryReservation(), MemorySwappiness: memorySwappiness, diff --git a/cri/v1alpha2/cri_utils_test.go b/cri/v1alpha2/cri_utils_test.go index ce955c78af..30d560fff1 100644 --- a/cri/v1alpha2/cri_utils_test.go +++ b/cri/v1alpha2/cri_utils_test.go @@ -100,6 +100,7 @@ var ( Memory: 1000, CpusetCpus: "0", CpusetMems: "0", + DiskQuota: map[string]string{"foo": "foo"}, BlkioWeight: uint16(100), BlkioWeightDevice: apitypesWeightDevicesSlice, BlkioDeviceReadBps: apitypesThrottleDevicesSlice, diff --git a/daemon/mgr/container.go b/daemon/mgr/container.go index 1051d59186..cc5b4035c3 100644 --- a/daemon/mgr/container.go +++ b/daemon/mgr/container.go @@ -1048,7 +1048,7 @@ func (mgr *ContainerManager) Update(ctx context.Context, name string, config *ty } // update container disk quota - if err := mgr.updateContainerDiskQuota(ctx, c, config.DiskQuota); err != nil { + if err := mgr.updateContainerDiskQuota(ctx, c, config.Resources.DiskQuota); err != nil { return errors.Wrapf(err, "failed to update diskquota of container %s", c.ID) } diff --git a/test/cli_run_blkio_test.go b/test/cli_run_blkio_test.go index add9fc8275..bdfe7a4b4c 100644 --- a/test/cli_run_blkio_test.go +++ b/test/cli_run_blkio_test.go @@ -139,3 +139,103 @@ func (suite *PouchRunBlkioSuite) TestRunWithBlkioWeight(c *check.C) { defer DelContainerForceMultyTime(c, name) res.Assert(c, icmd.Success) } + +// TestRunWithBlkIODeviceWriteBps is to verify blkio write bps +func (suite *PouchRunBlkioSuite) TestRunBlockIODeviceWriteBps(c *check.C) { + cname := "TestRunBlockIODeviceWriteBps" + testDisk := "/dev/null" + + number, exist := util.GetMajMinNumOfDevice(testDisk) + if !exist { + c.Skip("fail to get major:minor device number") + } + + limitSpeed := "100" + expected := fmt.Sprintf("%s:%d %s\n", number, value, limitSpeed) + + blkioDeviceWriteBpsFile := "/sys/fs/cgroup/blkio/blkio.throttle.write_bps_device" + throttleDev := testDisk + ":" + limitSpeed + + res := command.PouchRun("run", "--name", cname, + "--device-write-bps ", throttleDev, busyboxImage, "cat", blkioDeviceWriteBpsFile) + defer DelContainerForceMultyTime(c, cname) + res.Assert(c, icmd.Success) + + out := res.Stdout() + c.Assert(out, check.Equals, expected) +} + +// TestRunWithBlkIODeviceReadBps is to verify blkio read bps +func (suite *PouchRunBlkioSuite) TestRunBlockIODeviceReadBps(c *check.C) { + cname := "TestRunBlockIODeviceReadBps" + testDisk := "/dev/null" + + number, exist := util.GetMajMinNumOfDevice(testDisk) + if !exist { + c.Skip("fail to get major:minor device number") + } + + limitSpeed := "100" + expected := fmt.Sprintf("%s:%d %s\n", number, value, limitSpeed) + + blkioDeviceReadBpsFile := "/sys/fs/cgroup/blkio/blkio.throttle.read_bps_device" + throttleDev := testDisk + ":" + limitSpeed + + res := command.PouchRun("run", "--name", cname, + "--device-read-bps ", throttleDev, busyboxImage, "cat", blkioDeviceReadBpsFile) + defer DelContainerForceMultyTime(c, cname) + res.Assert(c, icmd.Success) + + out := res.Stdout() + c.Assert(out, check.Equals, expected) +} + +// TestRunWithBlkIODeviceWriteIOps is to verify blkio write iops +func (suite *PouchRunBlkioSuite) TestRunBlockIODeviceWriteIOps(c *check.C) { + cname := "TestRunBlockIODeviceWriteIOps" + testDisk := "/dev/null" + + number, exist := util.GetMajMinNumOfDevice(testDisk) + if !exist { + c.Skip("fail to get major:minor device number") + } + + limitSpeed := "100" + expected := fmt.Sprintf("%s:%d %s\n", number, value, limitSpeed) + + blkioDeviceWriteIOpsFile := "/sys/fs/cgroup/blkio/blkio.throttle.write_iops_device" + throttleDev := testDisk + ":" + limitSpeed + + res := command.PouchRun("run", "--name", cname, + "--device-write-iops ", throttleDev, busyboxImage, "cat", blkioDeviceWriteIOpsFile) + defer DelContainerForceMultyTime(c, cname) + res.Assert(c, icmd.Success) + + out := res.Stdout() + c.Assert(out, check.Equals, expected) +} + +// TestRunWithBlkIODeviceWriteIOps is to verify blkio read iops +func (suite *PouchRunBlkioSuite) TestRunBlockIODeviceReadIOps(c *check.C) { + cname := "TestRunBlockIODeviceReadIOps" + testDisk := "/dev/null" + + number, exist := util.GetMajMinNumOfDevice(testDisk) + if !exist { + c.Skip("fail to get major:minor device number") + } + + limitSpeed := "100" + expected := fmt.Sprintf("%s:%d %s\n", number, value, limitSpeed) + + blkioDeviceReadIOpsFile := "/sys/fs/cgroup/blkio/blkio.throttle.read_iops_device" + throttleDev := testDisk + ":" + limitSpeed + + res := command.PouchRun("run", "--name", cname, + "--device-read-iops ", throttleDev, busyboxImage, "cat", blkioDeviceReadIOpsFile) + defer DelContainerForceMultyTime(c, cname) + res.Assert(c, icmd.Success) + + out := res.Stdout() + c.Assert(out, check.Equals, expected) +}