-
Notifications
You must be signed in to change notification settings - Fork 640
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for remaining blkio settings #3950
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,6 +25,7 @@ import ( | |
"testing" | ||
|
||
"github.com/moby/sys/userns" | ||
"github.com/opencontainers/runtime-spec/specs-go" | ||
"gotest.tools/v3/assert" | ||
|
||
"github.com/containerd/cgroups/v3" | ||
|
@@ -421,3 +422,124 @@ func TestRunBlkioWeightCgroupV2(t *testing.T) { | |
base.Cmd("update", containerName, "--blkio-weight", "400").AssertOK() | ||
base.Cmd("exec", containerName, "cat", "io.bfq.weight").AssertOutExactly("default 400\n") | ||
} | ||
|
||
func TestRunBlkioSettingCgroupV2(t *testing.T) { | ||
t.Parallel() | ||
testutil.DockerIncompatible(t) | ||
if cgroups.Mode() != cgroups.Unified { | ||
t.Skip("test requires cgroup v2") | ||
} | ||
|
||
// TODO: Fix logic to check if bfq is set as the scheduler for the block device | ||
// | ||
// if _, err := os.Stat("/sys/module/bfq"); err != nil { | ||
// t.Skipf("test requires \"bfq\" module to be loaded: %v", err) | ||
// } | ||
|
||
base := testutil.NewBase(t) | ||
info := base.Info() | ||
switch info.CgroupDriver { | ||
case "none", "": | ||
t.Skip("test requires cgroup driver") | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. duplicated with line 429-431 |
||
|
||
tests := []struct { | ||
name string | ||
args []string | ||
validate func(t *testing.T, blockIO *specs.LinuxBlockIO) | ||
}{ | ||
{ | ||
name: "blkio-weight", | ||
args: []string{"--blkio-weight", "150"}, | ||
validate: func(t *testing.T, blockIO *specs.LinuxBlockIO) { | ||
assert.Equal(t, uint16(150), *blockIO.Weight) | ||
}, | ||
}, | ||
{ | ||
name: "blkio-weight-device", | ||
args: []string{"--blkio-weight-device", "/dev/sda:100"}, | ||
validate: func(t *testing.T, blockIO *specs.LinuxBlockIO) { | ||
assert.Equal(t, 1, len(blockIO.WeightDevice)) | ||
assert.Equal(t, uint16(100), *blockIO.WeightDevice[0].Weight) | ||
}, | ||
}, | ||
{ | ||
name: "device-read-bps", | ||
args: []string{"--device-read-bps", "/dev/sda:1048576"}, | ||
validate: func(t *testing.T, blockIO *specs.LinuxBlockIO) { | ||
assert.Equal(t, 1, len(blockIO.ThrottleReadBpsDevice)) | ||
assert.Equal(t, uint64(1048576), blockIO.ThrottleReadBpsDevice[0].Rate) | ||
}, | ||
}, | ||
{ | ||
name: "device-write-bps", | ||
args: []string{"--device-write-bps", "/dev/sda:2097152"}, | ||
validate: func(t *testing.T, blockIO *specs.LinuxBlockIO) { | ||
assert.Equal(t, 1, len(blockIO.ThrottleWriteBpsDevice)) | ||
assert.Equal(t, uint64(2097152), blockIO.ThrottleWriteBpsDevice[0].Rate) | ||
}, | ||
}, | ||
{ | ||
name: "device-read-iops", | ||
args: []string{"--device-read-iops", "/dev/sda:1000"}, | ||
validate: func(t *testing.T, blockIO *specs.LinuxBlockIO) { | ||
assert.Equal(t, 1, len(blockIO.ThrottleReadIOPSDevice)) | ||
assert.Equal(t, uint64(1000), blockIO.ThrottleReadIOPSDevice[0].Rate) | ||
}, | ||
}, | ||
{ | ||
name: "device-write-iops", | ||
args: []string{"--device-write-iops", "/dev/sda:2000"}, | ||
validate: func(t *testing.T, blockIO *specs.LinuxBlockIO) { | ||
assert.Equal(t, 1, len(blockIO.ThrottleWriteIOPSDevice)) | ||
assert.Equal(t, uint64(2000), blockIO.ThrottleWriteIOPSDevice[0].Rate) | ||
}, | ||
}, | ||
} | ||
|
||
for _, tt := range tests { | ||
tt := tt // capture range variable | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is no longer needed after go 1.22 (or 1.21? not sure the exact version) |
||
t.Run(tt.name, func(t *testing.T) { | ||
t.Parallel() | ||
containerName := testutil.Identifier(t) | ||
|
||
args := []string{"run", "-d", "--name", containerName} | ||
args = append(args, tt.args...) | ||
args = append(args, testutil.AlpineImage, "sleep", "infinity") | ||
base.Cmd(args...).AssertOK() | ||
t.Cleanup(func() { | ||
base.Cmd("rm", "-f", containerName).Run() | ||
}) | ||
|
||
// Connect to containerd | ||
addr := base.ContainerdAddress() | ||
client, err := containerd.New(addr, containerd.WithDefaultNamespace(testutil.Namespace)) | ||
assert.NilError(t, err) | ||
ctx := context.Background() | ||
|
||
// Get container ID | ||
var cid string | ||
walker := &containerwalker.ContainerWalker{ | ||
Client: client, | ||
OnFound: func(ctx context.Context, found containerwalker.Found) error { | ||
if found.MatchCount > 1 { | ||
return fmt.Errorf("multiple IDs found with provided prefix: %s", found.Req) | ||
} | ||
cid = found.Container.ID() | ||
return nil | ||
}, | ||
} | ||
err = walker.WalkAll(ctx, []string{containerName}, true) | ||
assert.NilError(t, err) | ||
|
||
// Get container spec | ||
container, err := client.LoadContainer(ctx, cid) | ||
assert.NilError(t, err) | ||
spec, err := container.Spec(ctx) | ||
assert.NilError(t, err) | ||
|
||
// Run the validation function | ||
tt.validate(t, spec.Linux.Resources.BlockIO) | ||
}) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -414,7 +414,7 @@ IPFS flags: | |
- :nerd_face: `--ipfs-address`: Multiaddr of IPFS API (default uses `$IPFS_PATH` env variable if defined or local directory `~/.ipfs`) | ||
|
||
Unimplemented `docker run` flags: | ||
`--blkio-weight-device`, `--cpu-rt-*`, `--device-*`, | ||
`--cpu-rt-*`, `--device-cgroup-rule`, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you also add these new flags to the this command doc? |
||
`--disable-content-trust`, `--expose`, `--health-*`, `--isolation`, `--no-healthcheck`, | ||
`--link*`, `--publish-all`, `--storage-opt`, | ||
`--userns`, `--volume-driver` | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -140,8 +140,6 @@ type ContainerCreateOptions struct { | |
PidsLimit int64 | ||
// CgroupConf specifies to configure cgroup v2 (key=value) | ||
CgroupConf []string | ||
// BlkioWeight specifies the block IO (relative weight), between 10 and 1000, or 0 to disable (default 0) | ||
BlkioWeight uint16 | ||
// Cgroupns specifies the cgroup namespace to use | ||
Cgroupns string | ||
// CgroupParent specifies the optional parent cgroup for the container | ||
|
@@ -150,6 +148,21 @@ type ContainerCreateOptions struct { | |
Device []string | ||
// #endregion | ||
|
||
// #region for blkio related flags | ||
// BlkioWeight specifies the block IO (relative weight), between 10 and 1000, or 0 to disable (default 0) | ||
BlkioWeight uint16 | ||
// BlkioWeightDevice specifies the Block IO weight (relative device weight) | ||
BlkioWeightDevice []string | ||
// BlkioDeviceReadBps specifies the Block IO read rate limit(bytes per second) of a device | ||
BlkioDeviceReadBps []string | ||
// BlkioDeviceWriteBps specifies the Block IO write rate limit(bytes per second) of a device | ||
BlkioDeviceWriteBps []string | ||
// BlkioDeviceReadIOps specifies the Block IO read rate limit(IO per second) of a device | ||
BlkioDeviceReadIOps []string | ||
// BlkioDeviceWriteIOps specifies the Block IO read rate limit(IO per second) of a device | ||
BlkioDeviceWriteIOps []string | ||
Comment on lines
+153
to
+163
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to support these flags in other commands? If so I suggest we create a separate struct, e.g., |
||
// #endregion | ||
|
||
// #region for intel RDT flags | ||
// RDTClass specifies the Intel Resource Director Technology (RDT) class | ||
RDTClass string | ||
|
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.
why is this docker imcompatible?