Merge pull request #6496 from thaJeztah/deprecate_criu_opt

estesp · web-flow · commit 36dcc76fa933 · 2022-03-23T11:17:58.000-04:00
runtime: deprecate runc --criu / -criu-path option
diff --git a/cmd/containerd-shim/main_unix.go b/cmd/containerd-shim/main_unix.go
@@ -78,7 +78,7 @@ func parseFlags() {
 	flag.StringVar(&addressFlag, "address", "", "grpc address back to main containerd")
 	flag.StringVar(&workdirFlag, "workdir", "", "path used to storage large temporary data")
 	flag.StringVar(&runtimeRootFlag, "runtime-root", process.RuncRoot, "root directory for the runtime")
-	flag.StringVar(&criuFlag, "criu", "", "path to criu binary")
+	flag.StringVar(&criuFlag, "criu", "", "path to criu binary (deprecated: do not use)")
 	flag.BoolVar(&systemdCgroupFlag, "systemd-cgroup", false, "set runtime to use systemd-cgroup")
 	// currently, the `containerd publish` utility is embedded in the daemon binary.
 	// The daemon invokes `containerd-shim -containerd-binary ...` with its own os.Executable() path.
@@ -176,7 +176,6 @@ func executeShim() error {
 			Path:          path,
 			Namespace:     namespaceFlag,
 			WorkDir:       workdirFlag,
-			Criu:          criuFlag,
 			SystemdCgroup: systemdCgroupFlag,
 			RuntimeRoot:   runtimeRootFlag,
 		},
diff --git a/pkg/process/init.go b/pkg/process/init.go
@@ -79,7 +79,7 @@ type Init struct {
 }
 
 // NewRunc returns a new runc instance for a process
-func NewRunc(root, path, namespace, runtime, criu string, systemd bool) *runc.Runc {
+func NewRunc(root, path, namespace, runtime string, systemd bool) *runc.Runc {
 	if root == "" {
 		root = RuncRoot
 	}
@@ -89,7 +89,6 @@ func NewRunc(root, path, namespace, runtime, criu string, systemd bool) *runc.Ru
 		LogFormat:     runc.JSON,
 		PdeathSignal:  unix.SIGKILL,
 		Root:          filepath.Join(root, namespace),
-		Criu:          criu,
 		SystemdCgroup: systemd,
 	}
 }
diff --git a/runtime/linux/runctypes/next.pb.txt b/runtime/linux/runctypes/next.pb.txt
@@ -23,6 +23,9 @@ file {
       number: 3
       label: LABEL_OPTIONAL
       type: TYPE_STRING
+      options {
+        deprecated: true
+      }
       json_name: "criuPath"
     }
     field {
diff --git a/runtime/linux/runctypes/runc.pb.go b/runtime/linux/runctypes/runc.pb.go
diff --git a/runtime/linux/runctypes/runc.proto b/runtime/linux/runctypes/runc.proto
@@ -9,7 +9,13 @@ option go_package = "github.com/containerd/containerd/runtime/linux/runctypes;ru
 message RuncOptions {
 	string runtime = 1;
 	string runtime_root = 2;
-	string criu_path = 3;
+	// criu binary path.
+	//
+	// 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.
+	string criu_path = 3 [deprecated = true];
 	bool systemd_cgroup = 4;
 }
 
diff --git a/runtime/v1/linux/bundle.go b/runtime/v1/linux/bundle.go
@@ -217,12 +217,10 @@ func (b *bundle) decideShimAddress(namespace string) string {
 
 func (b *bundle) shimConfig(namespace string, c *Config, runcOptions *runctypes.RuncOptions) shim.Config {
 	var (
-		criuPath      string
 		runtimeRoot   = c.RuntimeRoot
 		systemdCgroup bool
 	)
 	if runcOptions != nil {
-		criuPath = runcOptions.CriuPath
 		systemdCgroup = runcOptions.SystemdCgroup
 		if runcOptions.RuntimeRoot != "" {
 			runtimeRoot = runcOptions.RuntimeRoot
@@ -232,7 +230,6 @@ func (b *bundle) shimConfig(namespace string, c *Config, runcOptions *runctypes.
 		Path:          b.path,
 		WorkDir:       b.workDir,
 		Namespace:     namespace,
-		Criu:          criuPath,
 		RuntimeRoot:   runtimeRoot,
 		SystemdCgroup: systemdCgroup,
 	}
diff --git a/runtime/v1/shim/client/client.go b/runtime/v1/shim/client/client.go
@@ -197,9 +197,6 @@ func newCommand(binary, daemonAddress string, debug bool, config shim.Config, so
 		"-containerd-binary", selfExe,
 	}
 
-	if config.Criu != "" {
-		args = append(args, "-criu-path", config.Criu)
-	}
 	if config.RuntimeRoot != "" {
 		args = append(args, "-runtime-root", config.RuntimeRoot)
 	}
diff --git a/runtime/v1/shim/service.go b/runtime/v1/shim/service.go
@@ -62,9 +62,15 @@ var (
 
 // Config contains shim specific configuration
 type Config struct {
-	Path          string
-	Namespace     string
-	WorkDir       string
+	Path      string
+	Namespace string
+	WorkDir   string
+	// Criu is the path to the criu binary used for checkpoint and restore.
+	//
+	// 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.
 	Criu          string
 	RuntimeRoot   string
 	SystemdCgroup bool
@@ -172,7 +178,6 @@ func (s *Service) Create(ctx context.Context, r *shimapi.CreateTaskRequest) (_ *
 		s.config.WorkDir,
 		s.config.RuntimeRoot,
 		s.config.Namespace,
-		s.config.Criu,
 		s.config.SystemdCgroup,
 		s.platform,
 		config,
@@ -637,7 +642,7 @@ func getTopic(ctx context.Context, e interface{}) string {
 	return runtime.TaskUnknownTopic
 }
 
-func newInit(ctx context.Context, path, workDir, runtimeRoot, namespace, criu string, systemdCgroup bool, platform stdio.Platform, r *process.CreateConfig, rootfs string) (*process.Init, error) {
+func newInit(ctx context.Context, path, workDir, runtimeRoot, namespace string, systemdCgroup bool, platform stdio.Platform, r *process.CreateConfig, rootfs string) (*process.Init, error) {
 	var options runctypes.CreateOptions
 	if r.Options != nil {
 		v, err := typeurl.UnmarshalAny(r.Options)
@@ -647,7 +652,7 @@ func newInit(ctx context.Context, path, workDir, runtimeRoot, namespace, criu st
 		options = *v.(*runctypes.CreateOptions)
 	}
 
-	runtime := process.NewRunc(runtimeRoot, path, namespace, r.Runtime, criu, systemdCgroup)
+	runtime := process.NewRunc(runtimeRoot, path, namespace, r.Runtime, systemdCgroup)
 	p := process.New(r.ID, runtime, stdio.Stdio{
 		Stdin:    r.Stdin,
 		Stdout:   r.Stdout,
diff --git a/runtime/v2/runc/container.go b/runtime/v2/runc/container.go
@@ -210,7 +210,7 @@ func WriteRuntime(path, runtime string) error {
 
 func newInit(ctx context.Context, path, workDir, namespace string, platform stdio.Platform,
 	r *process.CreateConfig, options *options.Options, rootfs string) (*process.Init, error) {
-	runtime := process.NewRunc(options.Root, path, namespace, options.BinaryName, options.CriuPath, options.SystemdCgroup)
+	runtime := process.NewRunc(options.Root, path, namespace, options.BinaryName, options.SystemdCgroup)
 	p := process.New(r.ID, runtime, stdio.Stdio{
 		Stdin:    r.Stdin,
 		Stdout:   r.Stdout,
diff --git a/runtime/v2/runc/manager/manager_linux.go b/runtime/v2/runc/manager/manager_linux.go
@@ -261,7 +261,7 @@ func (manager) Stop(ctx context.Context, id string) (shim.StopStatus, error) {
 		root = opts.Root
 	}
 
-	r := process.NewRunc(root, path, ns, runtime, "", false)
+	r := process.NewRunc(root, path, ns, runtime, false)
 	if err := r.Delete(ctx, id, &runcC.DeleteOpts{
 		Force: true,
 	}); err != nil {
diff --git a/runtime/v2/runc/options/next.pb.txt b/runtime/v2/runc/options/next.pb.txt
@@ -58,6 +58,9 @@ file {
       number: 8
       label: LABEL_OPTIONAL
       type: TYPE_STRING
+      options {
+        deprecated: true
+      }
       json_name: "criuPath"
     }
     field {
diff --git a/runtime/v2/runc/options/oci.pb.go b/runtime/v2/runc/options/oci.pb.go
diff --git a/runtime/v2/runc/options/oci.proto b/runtime/v2/runc/options/oci.proto
diff --git a/runtime/v2/runc/v1/service.go b/runtime/v2/runc/v1/service.go

Original file line number	Diff line number	Diff line change
`@@ -79,7 +79,7 @@ type Init struct {`
`79`	`79`	`}`
`80`	`80`
`81`	`81`	`// NewRunc returns a new runc instance for a process`
`82`		`-func NewRunc(root, path, namespace, runtime, criu string, systemd bool) *runc.Runc {`
	`82`	`+func NewRunc(root, path, namespace, runtime string, systemd bool) *runc.Runc {`
`83`	`83`	`if root == "" {`
`84`	`84`	`root = RuncRoot`
`85`	`85`	`}`
`@@ -89,7 +89,6 @@ func NewRunc(root, path, namespace, runtime, criu string, systemd bool) *runc.Ru`
`89`	`89`	`LogFormat: runc.JSON,`
`90`	`90`	`PdeathSignal: unix.SIGKILL,`
`91`	`91`	`Root: filepath.Join(root, namespace),`
`92`		`- Criu: criu,`
`93`	`92`	`SystemdCgroup: systemd,`
`94`	`93`	`}`
`95`	`94`	`}`
Original file line number	Diff line number	Diff line change
`@@ -197,9 +197,6 @@ func newCommand(binary, daemonAddress string, debug bool, config shim.Config, so`
`197`	`197`	`"-containerd-binary", selfExe,`
`198`	`198`	`}`
`199`	`199`
`200`		`- if config.Criu != "" {`
`201`		`- args = append(args, "-criu-path", config.Criu)`
`202`		`- }`
`203`	`200`	`if config.RuntimeRoot != "" {`
`204`	`201`	`args = append(args, "-runtime-root", config.RuntimeRoot)`
`205`	`202`	`}`
Original file line number	Diff line number	Diff line change
`@@ -261,7 +261,7 @@ func (manager) Stop(ctx context.Context, id string) (shim.StopStatus, error) {`
`261`	`261`	`root = opts.Root`
`262`	`262`	`}`
`263`	`263`
`264`		`- r := process.NewRunc(root, path, ns, runtime, "", false)`
	`264`	`+ r := process.NewRunc(root, path, ns, runtime, false)`
`265`	`265`	`if err := r.Delete(ctx, id, &runcC.DeleteOpts{`
`266`	`266`	`Force: true,`
`267`	`267`	`}); err != nil {`