Skip to content
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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 26 additions & 3 deletions cmd/nerdctl/container/container_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,19 +199,42 @@ func processContainerCreateOptions(cmd *cobra.Command) (types.ContainerCreateOpt
if err != nil {
return opt, err
}
opt.Cgroupns, err = cmd.Flags().GetString("cgroupns")
if err != nil {
return opt, err
}
opt.CgroupParent, err = cmd.Flags().GetString("cgroup-parent")
if err != nil {
return opt, err
}
opt.Device, err = cmd.Flags().GetStringSlice("device")
if err != nil {
return opt, err
}
// #endregion

// #region for blkio flags
opt.BlkioWeight, err = cmd.Flags().GetUint16("blkio-weight")
if err != nil {
return opt, err
}
opt.Cgroupns, err = cmd.Flags().GetString("cgroupns")
opt.BlkioWeightDevice, err = cmd.Flags().GetStringArray("blkio-weight-device")
if err != nil {
return opt, err
}
opt.CgroupParent, err = cmd.Flags().GetString("cgroup-parent")
opt.BlkioDeviceReadBps, err = cmd.Flags().GetStringArray("device-read-bps")
if err != nil {
return opt, err
}
opt.Device, err = cmd.Flags().GetStringSlice("device")
opt.BlkioDeviceWriteBps, err = cmd.Flags().GetStringArray("device-write-bps")
if err != nil {
return opt, err
}
opt.BlkioDeviceReadIOps, err = cmd.Flags().GetStringArray("device-read-iops")
if err != nil {
return opt, err
}
opt.BlkioDeviceWriteIOps, err = cmd.Flags().GetStringArray("device-write-iops")
if err != nil {
return opt, err
}
Expand Down
10 changes: 9 additions & 1 deletion cmd/nerdctl/container/container_run.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,6 @@ func setCreateFlags(cmd *cobra.Command) {
})
cmd.Flags().Int64("pids-limit", -1, "Tune container pids limit (set -1 for unlimited)")
cmd.Flags().StringSlice("cgroup-conf", nil, "Configure cgroup v2 (key=value)")
cmd.Flags().Uint16("blkio-weight", 0, "Block IO (relative weight), between 10 and 1000, or 0 to disable (default 0)")
cmd.Flags().String("cgroupns", defaults.CgroupnsMode(), `Cgroup namespace to use, the default depends on the cgroup version ("host"|"private")`)
cmd.Flags().String("cgroup-parent", "", "Optional parent cgroup for the container")
cmd.RegisterFlagCompletionFunc("cgroupns", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
Expand All @@ -173,6 +172,15 @@ func setCreateFlags(cmd *cobra.Command) {
cmd.Flags().String("rdt-class", "", "Name of the RDT class (or CLOS) to associate the container with")
// #endregion

// #region blkio flags
cmd.Flags().Uint16("blkio-weight", 0, "Block IO (relative weight), between 10 and 1000, or 0 to disable (default 0)")
cmd.Flags().StringArray("blkio-weight-device", []string{}, "Block IO weight (relative device weight) (default [])")
cmd.Flags().StringArray("device-read-bps", []string{}, "Limit read rate (bytes per second) from a device (default [])")
cmd.Flags().StringArray("device-read-iops", []string{}, "Limit read rate (IO per second) from a device (default [])")
cmd.Flags().StringArray("device-write-bps", []string{}, "Limit write rate (bytes per second) to a device (default [])")
cmd.Flags().StringArray("device-write-iops", []string{}, "Limit write rate (IO per second) to a device (default [])")
// #endregion

// user flags
cmd.Flags().StringP("user", "u", "", "Username or UID (format: <name|uid>[:<group|gid>])")
cmd.Flags().String("umask", "", "Set the umask inside the container. Defaults to 0022")
Expand Down
122 changes: 122 additions & 0 deletions cmd/nerdctl/container/container_run_cgroup_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Copy link
Member

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?

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")
}
Copy link
Member

Choose a reason for hiding this comment

The 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
Copy link
Member

Choose a reason for hiding this comment

The 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)
})
}
}
2 changes: 1 addition & 1 deletion docs/command-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`,
Copy link
Member

Choose a reason for hiding this comment

The 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`
Expand Down
17 changes: 15 additions & 2 deletions pkg/api/types/container_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Copy link
Member

Choose a reason for hiding this comment

The 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., Blkio.

// #endregion

// #region for intel RDT flags
// RDTClass specifies the Intel Resource Director Technology (RDT) class
RDTClass string
Expand Down
Loading
Loading