add more parameters of blkio subsystem to command line. #1004
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,7 +30,8 @@ function setup() { | |
| "cpus": "0" | ||
| }, | ||
| "blockio": { | ||
| "blkioWeight": 1000, | ||
| "blkioLeafWeight": 1000 | ||
| }, | ||
| EOF | ||
| ) | ||
|
|
@@ -62,6 +63,7 @@ function check_cgroup_value() { | |
|
|
||
| # check that initial values were properly set | ||
| check_cgroup_value $CGROUP_BLKIO "blkio.weight" 1000 | ||
| check_cgroup_value $CGROUP_BLKIO "blkio.leaf_weight" 1000 | ||
| check_cgroup_value $CGROUP_CPU "cpu.cfs_period_us" 1000000 | ||
| check_cgroup_value $CGROUP_CPU "cpu.cfs_quota_us" 500000 | ||
| check_cgroup_value $CGROUP_CPU "cpu.shares" 100 | ||
|
|
@@ -76,6 +78,10 @@ function check_cgroup_value() { | |
| [ "$status" -eq 0 ] | ||
| check_cgroup_value $CGROUP_BLKIO "blkio.weight" 500 | ||
|
|
||
| # update blkio-leaf-weight | ||
| runc update test_update --blkio-leaf-weight 500 | ||
| check_cgroup_value $CGROUP_BLKIO "blkio.leaf_weight" 500 | ||
|
Member
There was a problem hiding this comment. ... but only added a test for leaf weight. Is this intentional (meaning, is it possible for us to test the other options as well here)?
Author
There was a problem hiding this comment. @cyphar thanks for checking this PR. For other parameters of blkio, we cannot test them in current CI machine, because it is lack of cfq device in CI machine. What is your suggestion about the device limitation of CI machine?
Member
There was a problem hiding this comment. I would add the tests as "optional" if possible (at SUSE we run the test suite on SUSE machines, so the tests are useful outside of the CI setup for this repo). But if we're not testing it in the CI then it's not as important (and could wait for a follow-up PR).
Author
There was a problem hiding this comment. @cyphar , currently, the CI tests are wrote with shell framework. If i add the new flags, some tests are not passed due to the "device" problem. I have tried to add the new flags to CI tests by using 'conditional' judgement, but if do so, the code seems very ugly. For example, if i add new flags into 'DATA; section, i will double the 'old flags' twice. I think this is not acceptable. Can I add those new flags into tests later in another PR if the test framework become more flexible?
Member
There was a problem hiding this comment. Yeah, we can handle this later I guess. We need to do the test framework improvements for the memory cgroup anyway. |
||
|
|
||
| # update cpu-period | ||
| runc update test_update --cpu-period 900000 | ||
| [ "$status" -eq 0 ] | ||
|
|
@@ -102,6 +108,7 @@ function check_cgroup_value() { | |
| # update memory limit | ||
| runc update test_update --memory 67108864 | ||
| [ "$status" -eq 0 ] | ||
|
|
||
| check_cgroup_value $CGROUP_MEMORY "memory.limit_in_bytes" 67108864 | ||
|
|
||
| runc update test_update --memory 50M | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,16 +6,21 @@ import ( | |
| "encoding/json" | ||
| "fmt" | ||
| "os" | ||
| "regexp" | ||
| "strconv" | ||
|
|
||
| "github.com/docker/go-units" | ||
| "github.com/opencontainers/runc/libcontainer/configs" | ||
| specs "github.com/opencontainers/runtime-spec/specs-go" | ||
| "github.com/urfave/cli" | ||
| ) | ||
|
|
||
| func u64Ptr(i uint64) *uint64 { return &i } | ||
| func u16Ptr(i uint16) *uint16 { return &i } | ||
|
|
||
| var regBlkioWeightDevice = regexp.MustCompile(`([0-9]+):([0-9]+) ([0-9]+)(?: ([0-9]+))?`) | ||
| var regBlkioThrottleDevice = regexp.MustCompile(`([0-9]+):([0-9]+) ([0-9]+)`) | ||
|
Member
There was a problem hiding this comment. Is this format defined in the spec (my guess is yes), and if so please at least comment that it comes from the spec or something so it's a bit more clear. In addition, if it comes from the spec maybe all of these conversion functions should go inside |
||
|
|
||
| var updateCommand = cli.Command{ | ||
| Name: "update", | ||
| Usage: "update container resource constraints", | ||
|
|
@@ -44,7 +49,13 @@ The accepted format is as follow (unchanged values can be omitted): | |
| "mems": "" | ||
| }, | ||
| "blockIO": { | ||
| "blkioWeight": 0, | ||
| "blkioLeafWeight": 0, | ||
| "blkioWeightDevice": "", | ||
| "blkioThrottleReadBpsDevice": "", | ||
| "blkioThrottleWriteBpsDevice": "", | ||
| "blkioThrottleReadIOPSDevice": "", | ||
| "blkioThrottleWriteIOPSDevice": "" | ||
| }, | ||
| } | ||
|
|
||
|
|
@@ -55,7 +66,31 @@ other options are ignored. | |
|
|
||
| cli.IntFlag{ | ||
| Name: "blkio-weight", | ||
| Usage: "Specifies per cgroup weight", | ||
| }, | ||
| cli.IntFlag{ | ||
| Name: "blkio-leaf-weight", | ||
| Usage: "Specifies tasks' weight in the given cgroup while competing with the cgroup's child cgroups, cfq scheduler only", | ||
| }, | ||
| cli.StringSliceFlag{ | ||
| Name: "blkio-weight-device", | ||
| Usage: "Weight per cgroup per device, can override blkio-weight. Argument must be of the form \"<MAJOR>:<MINOR> <WEIGHT> [LEAF_WEIGHT]\"", | ||
| }, | ||
| cli.StringSliceFlag{ | ||
| Name: "blkio-throttle-readbps-device", | ||
| Usage: "IO read rate limit per cgroup per device, bytes per second. Argument must be of the form \"<MAJOR>:<MINOR> <RATE>\"", | ||
| }, | ||
| cli.StringSliceFlag{ | ||
| Name: "blkio-throttle-writebps-device", | ||
| Usage: "IO write rate limit per cgroup per divice, bytes per second. Argument must be of the form \"<MAJOR>:<MINOR> <RATE>\"", | ||
| }, | ||
| cli.StringSliceFlag{ | ||
| Name: "blkio-throttle-readiops-device", | ||
| Usage: "IO read rate limit per cgroup per device, IO per second. Argument must be of the form \"<MAJOR>:<MINOR> <RATE>\"", | ||
| }, | ||
| cli.StringSliceFlag{ | ||
| Name: "blkio-throttle-writeiops-device", | ||
| Usage: "IO write rate limit per cgroup per device, IO per second. Argument must be of the form \"<MAJOR>:<MINOR> <RATE>\"", | ||
| }, | ||
| cli.StringFlag{ | ||
| Name: "cpu-period", | ||
|
|
@@ -120,7 +155,8 @@ other options are ignored. | |
| Mems: sPtr(""), | ||
| }, | ||
| BlockIO: &specs.BlockIO{ | ||
| Weight: u16Ptr(0), | ||
| LeafWeight: u16Ptr(0), | ||
| }, | ||
| } | ||
|
|
||
|
|
@@ -148,6 +184,26 @@ other options are ignored. | |
| if val := context.Int("blkio-weight"); val != 0 { | ||
| r.BlockIO.Weight = u16Ptr(uint16(val)) | ||
| } | ||
| if val := context.Int("blkio-leaf-weight"); val != 0 { | ||
| r.BlockIO.LeafWeight = u16Ptr(uint16(val)) | ||
| } | ||
| if val := context.StringSlice("blkio-weight-device"); val != nil { | ||
| if r.BlockIO.WeightDevice, err = getBlkioWeightDeviceFromString(val); err != nil { | ||
| return fmt.Errorf("invalid value for blkio-weight-device: %v", err) | ||
| } | ||
| } | ||
| for opt, dest := range map[string]*[]specs.ThrottleDevice{ | ||
| "blkio-throttle-readbps-device": &r.BlockIO.ThrottleReadBpsDevice, | ||
| "blkio-throttle-writebps-device": &r.BlockIO.ThrottleWriteBpsDevice, | ||
| "blkio-throttle-readiops-device": &r.BlockIO.ThrottleReadIOPSDevice, | ||
| "blkio-throttle-writeiops-device": &r.BlockIO.ThrottleWriteIOPSDevice, | ||
| } { | ||
| if val := context.StringSlice(opt); val != nil { | ||
| if *dest, err = getBlkioThrottleDeviceFromString(val); err != nil { | ||
| return fmt.Errorf("invalid value for %s: %v", opt, err) | ||
| } | ||
| } | ||
| } | ||
| if val := context.String("cpuset-cpus"); val != "" { | ||
| r.CPU.Cpus = &val | ||
| } | ||
|
|
@@ -188,6 +244,12 @@ other options are ignored. | |
|
|
||
| // Update the value | ||
| config.Cgroups.Resources.BlkioWeight = *r.BlockIO.Weight | ||
| config.Cgroups.Resources.BlkioLeafWeight = *r.BlockIO.LeafWeight | ||
| config.Cgroups.Resources.BlkioWeightDevice = convertSpecWeightDevices(r.BlockIO.WeightDevice) | ||
| config.Cgroups.Resources.BlkioThrottleReadBpsDevice = convertSpecThrottleDevices(r.BlockIO.ThrottleReadBpsDevice) | ||
| config.Cgroups.Resources.BlkioThrottleWriteBpsDevice = convertSpecThrottleDevices(r.BlockIO.ThrottleWriteBpsDevice) | ||
| config.Cgroups.Resources.BlkioThrottleReadIOPSDevice = convertSpecThrottleDevices(r.BlockIO.ThrottleReadIOPSDevice) | ||
| config.Cgroups.Resources.BlkioThrottleWriteIOPSDevice = convertSpecThrottleDevices(r.BlockIO.ThrottleWriteIOPSDevice) | ||
| config.Cgroups.Resources.CpuPeriod = int64(*r.CPU.Period) | ||
| config.Cgroups.Resources.CpuQuota = int64(*r.CPU.Quota) | ||
| config.Cgroups.Resources.CpuShares = int64(*r.CPU.Shares) | ||
|
|
@@ -205,3 +267,82 @@ other options are ignored. | |
| return nil | ||
| }, | ||
| } | ||
|
|
||
| func getBlkioWeightDeviceFromString(deviceStr []string) ([]specs.WeightDevice, error) { | ||
| var devs []specs.WeightDevice | ||
| for _, s := range deviceStr { | ||
| elems := regBlkioWeightDevice.FindStringSubmatch(s) | ||
| if elems == nil || len(elems) < 5 { | ||
| return nil, fmt.Errorf("invalid value for %s", s) | ||
| } | ||
| var dev specs.WeightDevice | ||
| var err error | ||
| var weight uint64 | ||
| if dev.Major, err = strconv.ParseInt(elems[1], 10, 64); err != nil { | ||
| return nil, fmt.Errorf("invalid value for %s, err: %v", elems[1], err) | ||
| } | ||
| if dev.Minor, err = strconv.ParseInt(elems[2], 10, 64); err != nil { | ||
| return nil, fmt.Errorf("invalid value for %s, err: %v", elems[2], err) | ||
| } | ||
| if weight, err = strconv.ParseUint(elems[3], 10, 16); err != nil { | ||
| return nil, fmt.Errorf("invalid value for %s, err: %v", elems[3], err) | ||
| } | ||
| dev.Weight = u16Ptr(uint16(weight)) | ||
| if elems[4] != "" { | ||
| if weight, err = strconv.ParseUint(elems[4], 10, 16); err != nil { | ||
| return nil, fmt.Errorf("invalid value for %s, err: %v", elems[4], err) | ||
| } | ||
| dev.LeafWeight = u16Ptr(uint16(weight)) | ||
| } | ||
| devs = append(devs, dev) | ||
| } | ||
|
|
||
| return devs, nil | ||
| } | ||
|
|
||
| func getBlkioThrottleDeviceFromString(deviceStr []string) ([]specs.ThrottleDevice, error) { | ||
| var devs []specs.ThrottleDevice | ||
| for _, s := range deviceStr { | ||
| elems := regBlkioThrottleDevice.FindStringSubmatch(s) | ||
| if elems == nil || len(elems) < 4 { | ||
| return nil, fmt.Errorf("invalid value for %s", s) | ||
| } | ||
| var dev specs.ThrottleDevice | ||
| var err error | ||
| var rate uint64 | ||
| if dev.Major, err = strconv.ParseInt(elems[1], 10, 64); err != nil { | ||
| return nil, fmt.Errorf("invalid value for %s, err: %v", elems[1], err) | ||
| } | ||
| if dev.Minor, err = strconv.ParseInt(elems[2], 10, 64); err != nil { | ||
| return nil, fmt.Errorf("invalid value for %s, err: %v", elems[2], err) | ||
| } | ||
| if rate, err = strconv.ParseUint(elems[3], 10, 64); err != nil { | ||
| return nil, fmt.Errorf("invalid value for %s, err: %v", elems[3], err) | ||
| } | ||
| dev.Rate = u64Ptr(rate) | ||
| devs = append(devs, dev) | ||
| } | ||
| return devs, nil | ||
| } | ||
|
|
||
| func convertSpecWeightDevices(devices []specs.WeightDevice) []*configs.WeightDevice { | ||
| var cwds []*configs.WeightDevice | ||
| for _, d := range devices { | ||
| leafWeight := uint16(0) | ||
| if d.LeafWeight != nil { | ||
| leafWeight = *d.LeafWeight | ||
| } | ||
| wd := configs.NewWeightDevice(d.Major, d.Minor, *d.Weight, leafWeight) | ||
| cwds = append(cwds, wd) | ||
| } | ||
| return cwds | ||
| } | ||
|
|
||
| func convertSpecThrottleDevices(devices []specs.ThrottleDevice) []*configs.ThrottleDevice { | ||
| var ctds []*configs.ThrottleDevice | ||
| for _, d := range devices { | ||
| td := configs.NewThrottleDevice(d.Major, d.Minor, *d.Rate) | ||
| ctds = append(ctds, td) | ||
| } | ||
| return ctds | ||
| } | ||
|
Contributor
There was a problem hiding this comment. You can use |
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You've added all of these flags ...