Merge pull request containerd#6206 from mxpv/path

dmcgowan · web-flow · commit d055487b00cc · 2021-11-15T18:05:48.000-08:00
Allow absolute path to shim binaries
diff --git a/api/next.pb.txt b/api/next.pb.txt
@@ -3821,6 +3821,13 @@ file {
       type_name: ".google.protobuf.Any"
       json_name: "options"
     }
+    field {
+      name: "runtime_path"
+      number: 10
+      label: LABEL_OPTIONAL
+      type: TYPE_STRING
+      json_name: "runtimePath"
+    }
   }
   message_type {
     name: "CreateTaskResponse"
diff --git a/api/services/tasks/v1/tasks.pb.go b/api/services/tasks/v1/tasks.pb.go
diff --git a/api/services/tasks/v1/tasks.proto b/api/services/tasks/v1/tasks.proto
@@ -88,6 +88,8 @@ message CreateTaskRequest {
 	containerd.types.Descriptor checkpoint = 8;
 
 	google.protobuf.Any options = 9;
+
+	string runtime_path = 10;
 }
 
 message CreateTaskResponse {
diff --git a/pkg/cri/config/config.go b/pkg/cri/config/config.go
@@ -31,6 +31,10 @@ import (
 type Runtime struct {
 	// Type is the runtime type to use in containerd e.g. io.containerd.runtime.v1.linux
 	Type string `toml:"runtime_type" json:"runtimeType"`
+	// Path is an optional field that can be used to overwrite path to a shim runtime binary.
+	// When specified, containerd will ignore runtime name field when resolving shim location.
+	// Path must be abs.
+	Path string `toml:"runtime_path" json:"runtimePath"`
 	// Engine is the name of the runtime engine used by containerd.
 	// This only works for runtime type "io.containerd.runtime.v1.linux".
 	// DEPRECATED: use Options instead. Remove when shim v1 is deprecated.
diff --git a/pkg/cri/server/container_start.go b/pkg/cri/server/container_start.go
@@ -110,7 +110,15 @@ func (c *criService) StartContainer(ctx context.Context, r *runtime.StartContain
 		return nil, errors.Wrap(err, "failed to get container info")
 	}
 
+	ociRuntime, err := c.getSandboxRuntime(sandbox.Config, sandbox.Metadata.RuntimeHandler)
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to get sandbox runtime")
+	}
+
 	taskOpts := c.taskOpts(ctrInfo.Runtime.Name)
+	if ociRuntime.Path != "" {
+		taskOpts = append(taskOpts, containerd.WithRuntimePath(ociRuntime.Path))
+	}
 	task, err := container.NewTask(ctx, ioCreation, taskOpts...)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to create containerd task")
diff --git a/pkg/cri/server/sandbox_run.go b/pkg/cri/server/sandbox_run.go
@@ -281,6 +281,9 @@ func (c *criService) RunPodSandbox(ctx context.Context, r *runtime.RunPodSandbox
 		id, name)
 
 	taskOpts := c.taskOpts(ociRuntime.Type)
+	if ociRuntime.Path != "" {
+		taskOpts = append(taskOpts, containerd.WithRuntimePath(ociRuntime.Path))
+	}
 	// We don't need stdio for sandbox container.
 	task, err := container.NewTask(ctx, containerdio.NullIO, taskOpts...)
 	if err != nil {
diff --git a/runtime/runtime.go b/runtime/runtime.go
@@ -46,7 +46,8 @@ type CreateOpts struct {
 	RuntimeOptions *types.Any
 	// TaskOptions received for the task
 	TaskOptions *types.Any
-	// Runtime to use
+	// Runtime name to use (e.g. `io.containerd.NAME.VERSION`).
+	// As an alternative full abs path to binary may be specified instead.
 	Runtime string
 }
 
diff --git a/runtime/v2/binary.go b/runtime/v2/binary.go
@@ -21,6 +21,7 @@ import (
 	"context"
 	"io"
 	"os"
+	"path/filepath"
 	gruntime "runtime"
 	"strings"
 
@@ -127,6 +128,10 @@ func (b *binary) Start(ctx context.Context, opts *types.Any, onClose func()) (_
 		cancelShimLog()
 		f.Close()
 	}
+	// Save runtime binary path for restore.
+	if err := os.WriteFile(filepath.Join(b.bundle.Path, "runtime"), []byte(b.runtime), 0600); err != nil {
+		return nil, err
+	}
 	client := ttrpc.NewClient(conn, ttrpc.WithOnClose(onCloseWithShimLog))
 	return &shim{
 		bundle: b.bundle,
diff --git a/runtime/v2/manager.go b/runtime/v2/manager.go
@@ -20,6 +20,9 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"os/exec"
+	"path/filepath"
+	"sync"
 
 	"github.com/containerd/containerd/containers"
 	"github.com/containerd/containerd/errdefs"
@@ -31,6 +34,7 @@ import (
 	"github.com/containerd/containerd/platforms"
 	"github.com/containerd/containerd/plugin"
 	"github.com/containerd/containerd/runtime"
+	shimbinary "github.com/containerd/containerd/runtime/v2/shim"
 	"github.com/containerd/containerd/runtime/v2/task"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
@@ -199,6 +203,8 @@ type ShimManager struct {
 	shims                  *runtime.TaskList
 	events                 *exchange.Exchange
 	containers             containers.Store
+	// runtimePaths is a cache of `runtime names` -> `resolved fs path`
+	runtimePaths sync.Map
 }
 
 // ID of the shim manager
@@ -253,8 +259,13 @@ func (m *ShimManager) startShim(ctx context.Context, bundle *Bundle, id string,
 		topts = opts.RuntimeOptions
 	}
 
+	runtimePath, err := m.resolveRuntimePath(opts.Runtime)
+	if err != nil {
+		return nil, fmt.Errorf("failed to resolve runtime path: %w", err)
+	}
+
 	b := shimBinary(bundle, shimBinaryConfig{
-		runtime:      opts.Runtime,
+		runtime:      runtimePath,
 		address:      m.containerdAddress,
 		ttrpcAddress: m.containerdTTRPCAddress,
 		schedCore:    m.schedCore,
@@ -276,6 +287,79 @@ func (m *ShimManager) startShim(ctx context.Context, bundle *Bundle, id string,
 	return shim, nil
 }
 
+func (m *ShimManager) resolveRuntimePath(runtime string) (string, error) {
+	if runtime == "" {
+		return "", fmt.Errorf("no runtime name")
+	}
+
+	// Custom path to runtime binary
+	if filepath.IsAbs(runtime) {
+		// Make sure it exists before returning ok
+		if _, err := os.Stat(runtime); err != nil {
+			return "", fmt.Errorf("invalid custom binary path: %w", err)
+		}
+
+		return runtime, nil
+	}
+
+	// Preserve existing logic and resolve runtime path from runtime name.
+
+	name := shimbinary.BinaryName(runtime)
+	if name == "" {
+		return "", fmt.Errorf("invalid runtime name %s, correct runtime name should be either format like `io.containerd.runc.v1` or a full path to the binary", runtime)
+	}
+
+	if path, ok := m.runtimePaths.Load(name); ok {
+		return path.(string), nil
+	}
+
+	var (
+		cmdPath string
+		lerr    error
+	)
+
+	binaryPath := shimbinary.BinaryPath(runtime)
+	if _, serr := os.Stat(binaryPath); serr == nil {
+		cmdPath = binaryPath
+	}
+
+	if cmdPath == "" {
+		if cmdPath, lerr = exec.LookPath(name); lerr != nil {
+			if eerr, ok := lerr.(*exec.Error); ok {
+				if eerr.Err == exec.ErrNotFound {
+					self, err := os.Executable()
+					if err != nil {
+						return "", err
+					}
+
+					// Match the calling binaries (containerd) path and see
+					// if they are side by side. If so, execute the shim
+					// found there.
+					testPath := filepath.Join(filepath.Dir(self), name)
+					if _, serr := os.Stat(testPath); serr == nil {
+						cmdPath = testPath
+					}
+					if cmdPath == "" {
+						return "", errors.Wrapf(os.ErrNotExist, "runtime %q binary not installed %q", runtime, name)
+					}
+				}
+			}
+		}
+	}
+
+	cmdPath, err := filepath.Abs(cmdPath)
+	if err != nil {
+		return "", err
+	}
+
+	if path, ok := m.runtimePaths.LoadOrStore(name, cmdPath); ok {
+		// We didn't store cmdPath we loaded an already cached value. Use it.
+		cmdPath = path.(string)
+	}
+
+	return cmdPath, nil
+}
+
 // cleanupShim attempts to properly delete and cleanup shim after error
 func (m *ShimManager) cleanupShim(shim *shim) {
 	dctx, cancel := timeout.WithContext(context.Background(), cleanupTimeout)
diff --git a/runtime/v2/shim/util.go b/runtime/v2/shim/util.go
@@ -24,7 +24,6 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
-	"sync"
 	"time"
 
 	"github.com/containerd/containerd/namespaces"
@@ -34,8 +33,6 @@ import (
 	exec "golang.org/x/sys/execabs"
 )
 
-var runtimePaths sync.Map
-
 type CommandConfig struct {
 	Runtime      string
 	Address      string
@@ -62,51 +59,7 @@ func Command(ctx context.Context, config *CommandConfig) (*exec.Cmd, error) {
 		"-publish-binary", self,
 	}
 	args = append(args, config.Args...)
-	name := BinaryName(config.Runtime)
-	if name == "" {
-		return nil, fmt.Errorf("invalid runtime name %s, correct runtime name should format like io.containerd.runc.v1", config.Runtime)
-	}
-
-	var cmdPath string
-	cmdPathI, cmdPathFound := runtimePaths.Load(name)
-	if cmdPathFound {
-		cmdPath = cmdPathI.(string)
-	} else {
-		var lerr error
-		binaryPath := BinaryPath(config.Runtime)
-		if _, serr := os.Stat(binaryPath); serr == nil {
-			cmdPath = binaryPath
-		}
-
-		if cmdPath == "" {
-			if cmdPath, lerr = exec.LookPath(name); lerr != nil {
-				if eerr, ok := lerr.(*exec.Error); ok {
-					if eerr.Err == exec.ErrNotFound {
-						// Match the calling binaries (containerd) path and see
-						// if they are side by side. If so, execute the shim
-						// found there.
-						testPath := filepath.Join(filepath.Dir(self), name)
-						if _, serr := os.Stat(testPath); serr == nil {
-							cmdPath = testPath
-						}
-						if cmdPath == "" {
-							return nil, errors.Wrapf(os.ErrNotExist, "runtime %q binary not installed %q", config.Runtime, name)
-						}
-					}
-				}
-			}
-		}
-		cmdPath, err = filepath.Abs(cmdPath)
-		if err != nil {
-			return nil, err
-		}
-		if cmdPathI, cmdPathFound = runtimePaths.LoadOrStore(name, cmdPath); cmdPathFound {
-			// We didn't store cmdPath we loaded an already cached value. Use it.
-			cmdPath = cmdPathI.(string)
-		}
-	}
-
-	cmd := exec.CommandContext(ctx, cmdPath, args...)
+	cmd := exec.CommandContext(ctx, config.Runtime, args...)
 	cmd.Dir = config.Path
 	cmd.Env = append(
 		os.Environ(),
diff --git a/runtime/v2/shim_load.go b/runtime/v2/shim_load.go
@@ -88,18 +88,35 @@ func (m *ShimManager) loadShims(ctx context.Context) error {
 			bundle.Delete()
 			continue
 		}
-		container, err := m.containers.Get(ctx, id)
-		if err != nil {
-			log.G(ctx).WithError(err).Errorf("loading container %s", id)
-			if err := mount.UnmountAll(filepath.Join(bundle.Path, "rootfs"), 0); err != nil {
-				log.G(ctx).WithError(err).Errorf("forceful unmount of rootfs %s", id)
+
+		var (
+			runtime string
+		)
+
+		// If we're on 1.6+ and specified custom path to the runtime binary, path will be saved in 'runtime' file.
+		if data, err := os.ReadFile(filepath.Join(bundle.Path, "runtime")); err == nil {
+			runtime = string(data)
+		} else if err != nil && !os.IsNotExist(err) {
+			log.G(ctx).WithError(err).Error("failed to read `runtime` path from bundle")
+		}
+
+		// Query runtime name from metadata store
+		if runtime == "" {
+			container, err := m.containers.Get(ctx, id)
+			if err != nil {
+				log.G(ctx).WithError(err).Errorf("loading container %s", id)
+				if err := mount.UnmountAll(filepath.Join(bundle.Path, "rootfs"), 0); err != nil {
+					log.G(ctx).WithError(err).Errorf("failed to unmount of rootfs %s", id)
+				}
+				bundle.Delete()
+				continue
 			}
-			bundle.Delete()
-			continue
+			runtime = container.Runtime.Name
 		}
+
 		binaryCall := shimBinary(bundle,
 			shimBinaryConfig{
-				runtime:      container.Runtime.Name,
+				runtime:      runtime,
 				address:      m.containerdAddress,
 				ttrpcAddress: m.containerdTTRPCAddress,
 				schedCore:    m.schedCore,
diff --git a/services/tasks/local.go b/services/tasks/local.go
@@ -188,6 +188,9 @@ func (l *local) Create(ctx context.Context, r *api.CreateTaskRequest, _ ...grpc.
 		RuntimeOptions: container.Runtime.Options,
 		TaskOptions:    r.Options,
 	}
+	if r.RuntimePath != "" {
+		opts.Runtime = r.RuntimePath
+	}
 	for _, m := range r.Rootfs {
 		opts.Rootfs = append(opts.Rootfs, mount.Mount{
 			Type:    m.Type,
diff --git a/task_opts.go b/task_opts.go
@@ -45,6 +45,15 @@ func WithRootFS(mounts []mount.Mount) NewTaskOpts {
 	}
 }
 
+// WithRuntimePath will force task service to use a custom path to the runtime binary
+// instead of resolving it from runtime name.
+func WithRuntimePath(absRuntimePath string) NewTaskOpts {
+	return func(ctx context.Context, client *Client, info *TaskInfo) error {
+		info.runtime = absRuntimePath
+		return nil
+	}
+}
+
 // WithTaskCheckpoint allows a task to be created with live runtime and memory data from a
 // previous checkpoint. Additional software such as CRIU may be required to
 // restore a task from a checkpoint
diff --git a/vendor/github.com/containerd/containerd/api/services/tasks/v1/tasks.pb.go b/vendor/github.com/containerd/containerd/api/services/tasks/v1/tasks.pb.go
diff --git a/vendor/github.com/containerd/containerd/api/services/tasks/v1/tasks.proto b/vendor/github.com/containerd/containerd/api/services/tasks/v1/tasks.proto

Original file line number	Diff line number	Diff line change
`@@ -3821,6 +3821,13 @@ file {`
`3821`	`3821`	`type_name: ".google.protobuf.Any"`
`3822`	`3822`	`json_name: "options"`
`3823`	`3823`	`}`
	`3824`	`+ field {`
	`3825`	`+ name: "runtime_path"`
	`3826`	`+ number: 10`
	`3827`	`+ label: LABEL_OPTIONAL`
	`3828`	`+ type: TYPE_STRING`
	`3829`	`+ json_name: "runtimePath"`
	`3830`	`+ }`
`3824`	`3831`	`}`
`3825`	`3832`	`message_type {`
`3826`	`3833`	`name: "CreateTaskResponse"`
Original file line number	Diff line number	Diff line change
`@@ -88,6 +88,8 @@ message CreateTaskRequest {`
`88`	`88`	`containerd.types.Descriptor checkpoint = 8;`
`89`	`89`
`90`	`90`	`google.protobuf.Any options = 9;`
	`91`	`+`
	`92`	`+ string runtime_path = 10;`
`91`	`93`	`}`
`92`	`94`
`93`	`95`	`message CreateTaskResponse {`
Original file line number	Diff line number	Diff line change
`@@ -46,7 +46,8 @@ type CreateOpts struct {`
`46`	`46`	`RuntimeOptions *types.Any`
`47`	`47`	`// TaskOptions received for the task`
`48`	`48`	`TaskOptions *types.Any`
`49`		`- // Runtime to use`
	`49`	+ // Runtime name to use (e.g. `io.containerd.NAME.VERSION`).
	`50`	`+ // As an alternative full abs path to binary may be specified instead.`
`50`	`51`	`Runtime string`
`51`	`52`	`}`
`52`	`53`