diff --git a/CHANGELOG.md b/CHANGELOG.md index cffc9e2f6..321317fd7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Deprecated + + * `runc` option `--criu` is now ignored (with a warning), and the option will + be removed entirely in a future release. Users who need a non-standard + `criu` binary should rely on the standard way of looking up binaries in + `$PATH`. (#3316) + ### Changed * When Intel RDT feature is not available, its initialization is skipped, diff --git a/contrib/completions/bash/runc b/contrib/completions/bash/runc index a4cd89937..0b8bda9b6 100644 --- a/contrib/completions/bash/runc +++ b/contrib/completions/bash/runc @@ -231,12 +231,11 @@ _runc_runc() { --log --log-format --root - --criu --rootless " case "$prev" in - --log | --root | --criu) + --log | --root) case "$cur" in *:*) ;; # TODO somehow do _filedir for stuff inside the image, if it's already specified (which is also somewhat difficult to determine) '') diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go index 261f5a9f4..82e9ef864 100644 --- a/libcontainer/container_linux.go +++ b/libcontainer/container_linux.go @@ -45,7 +45,6 @@ type linuxContainer struct { initArgs []string initProcess parentProcess initProcessStartTime uint64 - criuPath string newuidmapPath string newgidmapPath string m sync.Mutex @@ -829,7 +828,6 @@ func (c *linuxContainer) checkCriuVersion(minVersion int) error { } criu := criu.MakeCriu() - criu.SetCriuPath(c.criuPath) var err error c.criuVersion, err = criu.GetCriuVersion() if err != nil { @@ -1602,13 +1600,12 @@ func (c *linuxContainer) criuSwrk(process *Process, req *criurpc.CriuReq, opts * criuServer := os.NewFile(uintptr(fds[1]), "criu-transport-server") defer criuServer.Close() - args := []string{"swrk", "3"} if c.criuVersion != 0 { // If the CRIU Version is still '0' then this is probably // the initial CRIU run to detect the version. Skip it. - logrus.Debugf("Using CRIU %d at: %s", c.criuVersion, c.criuPath) + logrus.Debugf("Using CRIU %d", c.criuVersion) } - cmd := exec.Command(c.criuPath, args...) + cmd := exec.Command("criu", "swrk", "3") if process != nil { cmd.Stdin = process.Stdin cmd.Stdout = process.Stdout diff --git a/libcontainer/factory_linux.go b/libcontainer/factory_linux.go index 1ddc7a5db..d72a0532c 100644 --- a/libcontainer/factory_linux.go +++ b/libcontainer/factory_linux.go @@ -76,15 +76,6 @@ func TmpfsRoot(l *LinuxFactory) error { return nil } -// CriuPath returns an option func to configure a LinuxFactory with the -// provided criupath -func CriuPath(criupath string) func(*LinuxFactory) error { - return func(l *LinuxFactory) error { - l.CriuPath = criupath - return nil - } -} - // New returns a linux based container factory based in the root directory and // configures the factory with the provided option funcs. func New(root string, options ...func(*LinuxFactory) error) (Factory, error) { @@ -98,7 +89,6 @@ func New(root string, options ...func(*LinuxFactory) error) (Factory, error) { InitPath: "/proc/self/exe", InitArgs: []string{os.Args[0], "init"}, Validator: validate.New(), - CriuPath: "criu", } for _, opt := range options { @@ -125,10 +115,6 @@ type LinuxFactory struct { // a container. InitArgs []string - // CriuPath is the path to the criu binary used for checkpoint and restore of - // containers. - CriuPath string - // New{u,g}idmapPath is the path to the binaries used for mapping with // rootless containers. NewuidmapPath string @@ -204,7 +190,6 @@ func (l *LinuxFactory) Create(id string, config *configs.Config) (Container, err config: config, initPath: l.InitPath, initArgs: l.InitArgs, - criuPath: l.CriuPath, newuidmapPath: l.NewuidmapPath, newgidmapPath: l.NewgidmapPath, cgroupManager: cm, @@ -248,7 +233,6 @@ func (l *LinuxFactory) Load(id string) (Container, error) { config: &state.Config, initPath: l.InitPath, initArgs: l.InitArgs, - criuPath: l.CriuPath, newuidmapPath: l.NewuidmapPath, newgidmapPath: l.NewgidmapPath, cgroupManager: cm, diff --git a/main.go b/main.go index 4d6663827..d6e4bbf81 100644 --- a/main.go +++ b/main.go @@ -101,9 +101,9 @@ func main() { Usage: "root directory for storage of container state (this should be located in tmpfs)", }, cli.StringFlag{ - Name: "criu", - Value: "criu", - Usage: "path to the criu binary used for checkpoint and restore", + Name: "criu", + Usage: "(obsoleted; do not use)", + Hidden: true, }, cli.BoolFlag{ Name: "systemd-cgroup", @@ -151,6 +151,10 @@ func main() { if err := reviseRootDir(context); err != nil { return err } + // TODO: remove this in runc 1.3.0. + if context.IsSet("criu") { + fmt.Fprintln(os.Stderr, "WARNING: --criu ignored (criu binary from $PATH is used); do not use") + } return configLogrus(context) } diff --git a/man/runc.8.md b/man/runc.8.md index 09db1ef02..ed5cd3a0f 100644 --- a/man/runc.8.md +++ b/man/runc.8.md @@ -110,10 +110,6 @@ These options can be used with any command, and must precede the **command**. located on tmpfs. Default is */run/runc*, or *$XDG_RUNTIME_DIR/runc* for rootless containers. -**--criu** _path_ -: Set the path to the **criu**(8) binary used for checkpoint and restore. -Default is **criu**. - **--systemd-cgroup** : Enable systemd cgroup support. If this is set, the container spec (_config.json_) is expected to have **cgroupsPath** value in the diff --git a/tests/integration/checkpoint.bats b/tests/integration/checkpoint.bats index 4b7e442bb..9bf999400 100644 --- a/tests/integration/checkpoint.bats +++ b/tests/integration/checkpoint.bats @@ -84,7 +84,7 @@ function runc_restore_with_pipes() { shift ret=0 - __runc --criu "$CRIU" restore -d --work-path "$workdir" --image-path ./image-dir "$@" "$name" <&${in_r} >&${out_w} 2>&${err_w} || ret=$? + __runc restore -d --work-path "$workdir" --image-path ./image-dir "$@" "$name" <&${in_r} >&${out_w} 2>&${err_w} || ret=$? if [ "$ret" -ne 0 ]; then echo "__runc restore $name failed (status: $ret)" exec {err_w}>&- @@ -109,7 +109,7 @@ function simple_cr() { for _ in $(seq 2); do # checkpoint the running container - runc --criu "$CRIU" "$@" checkpoint --work-path ./work-dir test_busybox + runc "$@" checkpoint --work-path ./work-dir test_busybox grep -B 5 Error ./work-dir/dump.log || true [ "$status" -eq 0 ] @@ -117,7 +117,7 @@ function simple_cr() { testcontainer test_busybox checkpointed # restore from checkpoint - runc --criu "$CRIU" "$@" restore -d --work-path ./work-dir --console-socket "$CONSOLE_SOCKET" test_busybox + runc "$@" restore -d --work-path ./work-dir --console-socket "$CONSOLE_SOCKET" test_busybox grep -B 5 Error ./work-dir/restore.log || true [ "$status" -eq 0 ] @@ -162,12 +162,12 @@ function simple_cr() { testcontainer test_busybox running # runc should fail with absolute parent image path. - runc --criu "$CRIU" checkpoint --parent-path "$(pwd)"/parent-dir --work-path ./work-dir --image-path ./image-dir test_busybox + runc checkpoint --parent-path "$(pwd)"/parent-dir --work-path ./work-dir --image-path ./image-dir test_busybox [[ "${output}" == *"--parent-path"* ]] [ "$status" -ne 0 ] # runc should fail with invalid parent image path. - runc --criu "$CRIU" checkpoint --parent-path ./parent-dir --work-path ./work-dir --image-path ./image-dir test_busybox + runc checkpoint --parent-path ./parent-dir --work-path ./work-dir --image-path ./image-dir test_busybox [[ "${output}" == *"--parent-path"* ]] [ "$status" -ne 0 ] } @@ -178,7 +178,7 @@ function simple_cr() { #test checkpoint pre-dump mkdir parent-dir - runc --criu "$CRIU" checkpoint --pre-dump --image-path ./parent-dir test_busybox + runc checkpoint --pre-dump --image-path ./parent-dir test_busybox [ "$status" -eq 0 ] # busybox should still be running @@ -187,7 +187,7 @@ function simple_cr() { # checkpoint the running container mkdir image-dir mkdir work-dir - runc --criu "$CRIU" checkpoint --parent-path ../parent-dir --work-path ./work-dir --image-path ./image-dir test_busybox + runc checkpoint --parent-path ../parent-dir --work-path ./work-dir --image-path ./image-dir test_busybox grep -B 5 Error ./work-dir/dump.log || true [ "$status" -eq 0 ] @@ -203,7 +203,7 @@ function simple_cr() { @test "checkpoint --lazy-pages and restore" { # check if lazy-pages is supported - if ! "${CRIU}" check --feature uffd-noncoop; then + if ! criu check --feature uffd-noncoop; then skip "this criu does not support lazy migration" fi @@ -224,7 +224,7 @@ function simple_cr() { # TCP port for lazy migration port=27277 - __runc --criu "$CRIU" checkpoint --lazy-pages --page-server 0.0.0.0:${port} --status-fd ${lazy_w} --work-path ./work-dir --image-path ./image-dir test_busybox & + __runc checkpoint --lazy-pages --page-server 0.0.0.0:${port} --status-fd ${lazy_w} --work-path ./work-dir --image-path ./image-dir test_busybox & cpt_pid=$! # wait for lazy page server to be ready @@ -242,7 +242,7 @@ function simple_cr() { [ -e image-dir/inventory.img ] # Start CRIU in lazy-daemon mode - ${CRIU} lazy-pages --page-server --address 127.0.0.1 --port ${port} -D image-dir & + criu lazy-pages --page-server --address 127.0.0.1 --port ${port} -D image-dir & lp_pid=$! # Restore lazily from checkpoint. @@ -264,7 +264,7 @@ function simple_cr() { @test "checkpoint and restore in external network namespace" { # check if external_net_ns is supported; only with criu 3.10++ - if ! "${CRIU}" check --feature external_net_ns; then + if ! criu check --feature external_net_ns; then # this criu does not support external_net_ns; skip the test skip "this criu does not support external network namespaces" fi @@ -290,7 +290,7 @@ function simple_cr() { for _ in $(seq 2); do # checkpoint the running container; this automatically tells CRIU to # handle the network namespace defined in config.json as an external - runc --criu "$CRIU" checkpoint --work-path ./work-dir test_busybox + runc checkpoint --work-path ./work-dir test_busybox grep -B 5 Error ./work-dir/dump.log || true [ "$status" -eq 0 ] @@ -298,7 +298,7 @@ function simple_cr() { testcontainer test_busybox checkpointed # restore from checkpoint; this should restore the container into the existing network namespace - runc --criu "$CRIU" restore -d --work-path ./work-dir --console-socket "$CONSOLE_SOCKET" test_busybox + runc restore -d --work-path ./work-dir --console-socket "$CONSOLE_SOCKET" test_busybox grep -B 5 Error ./work-dir/restore.log || true [ "$status" -eq 0 ] @@ -341,7 +341,7 @@ function simple_cr() { testcontainer test_busybox running # checkpoint the running container - runc --criu "$CRIU" checkpoint --work-path ./work-dir test_busybox + runc checkpoint --work-path ./work-dir test_busybox grep -B 5 Error ./work-dir/dump.log || true [ "$status" -eq 0 ] ! test -f ./work-dir/"$tmplog1" @@ -352,7 +352,7 @@ function simple_cr() { test -f ./work-dir/"$tmplog2" && unlink ./work-dir/"$tmplog2" # restore from checkpoint - runc --criu "$CRIU" restore -d --work-path ./work-dir --console-socket "$CONSOLE_SOCKET" test_busybox + runc restore -d --work-path ./work-dir --console-socket "$CONSOLE_SOCKET" test_busybox grep -B 5 Error ./work-dir/restore.log || true [ "$status" -eq 0 ] ! test -f ./work-dir/"$tmplog1" @@ -386,7 +386,7 @@ function simple_cr() { testcontainer test_busybox running # checkpoint the running container - runc --criu "$CRIU" checkpoint --work-path ./work-dir test_busybox + runc checkpoint --work-path ./work-dir test_busybox grep -B 5 Error ./work-dir/dump.log || true [ "$status" -eq 0 ] @@ -398,7 +398,7 @@ function simple_cr() { rm -rf "${bind1:?}"/* # restore from checkpoint - runc --criu "$CRIU" restore -d --work-path ./work-dir --console-socket "$CONSOLE_SOCKET" test_busybox + runc restore -d --work-path ./work-dir --console-socket "$CONSOLE_SOCKET" test_busybox grep -B 5 Error ./work-dir/restore.log || true [ "$status" -eq 0 ] diff --git a/tests/integration/helpers.bash b/tests/integration/helpers.bash index aaa68dd7c..b6d2e244c 100644 --- a/tests/integration/helpers.bash +++ b/tests/integration/helpers.bash @@ -23,8 +23,8 @@ SECCOMP_AGENT="${INTEGRATION_ROOT}/../../contrib/cmd/seccompagent/seccompagent" # shellcheck disable=SC2034 TESTDATA="${INTEGRATION_ROOT}/testdata" -# CRIU PATH -CRIU="$(which criu 2>/dev/null || true)" +# Whether we have criu binary. +command -v criu &>/dev/null && HAVE_CRIU=yes # Kernel version KERNEL_VERSION="$(uname -r)" @@ -350,7 +350,7 @@ function requires() { local skip_me case $var in criu) - if [ ! -e "$CRIU" ]; then + if [ -n "$HAVE_CRIU" ]; then skip_me=1 fi ;; diff --git a/utils_linux.go b/utils_linux.go index a9badf20f..9f7619d65 100644 --- a/utils_linux.go +++ b/utils_linux.go @@ -47,7 +47,6 @@ func loadFactory(context *cli.Context) (libcontainer.Factory, error) { } return libcontainer.New(abs, intelRdtManager, - libcontainer.CriuPath(context.GlobalString("criu")), libcontainer.NewuidmapPath(newuidmap), libcontainer.NewgidmapPath(newgidmap)) }