runc: remove --criu option

This was introduced in an initial commit, back in the day when criu was
a highly experimental thing. Today it's not; most users who need it have
it packaged by their distro vendor.

The usual way to run a binary is to look it up in directories listed in
$PATH. This is flexible enough and allows for multiple scenarios (custom
binaries, extra binaries, etc.). This is the way criu should be run.

Make --criu a hidden option (thus removing it from help). Remove the
option from man pages, integration tests, etc. Remove all traces of
CriuPath from data structures.

Add a warning that --criu is ignored and will be removed.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This commit is contained in:
Kir Kolyshkin
2022-01-26 20:19:16 -08:00
parent 403cda19e4
commit 6e1d476aad
9 changed files with 37 additions and 51 deletions

View File

@@ -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,

View File

@@ -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)
'')

View File

@@ -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

View File

@@ -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,

10
main.go
View File

@@ -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)
}

View File

@@ -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

View File

@@ -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 ]

View File

@@ -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
;;

View File

@@ -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))
}