linux: fix runtime-root propagation

faf2781dd2 fixed the propagation for
ShimRemote but ShimLocal was not fixed in the commit.

Signed-off-by: Akihiro Suda <suda.akihiro@lab.ntt.co.jp>

client_test.go

@@ -38,10 +38,11 @@ import (
 )
 
 var (
-	address      string
-	noDaemon     bool
-	noCriu       bool
-	supportsCriu bool
+	address       string
+	noDaemon      bool
+	noCriu        bool
+	supportsCriu  bool
+	testNamespace = "testing"
 
 	ctrd = &daemon{}
 )
@@ -58,7 +59,7 @@ func init() {
 
 func testContext() (context.Context, context.CancelFunc) {
 	ctx, cancel := context.WithCancel(context.Background())
-	ctx = namespaces.WithNamespace(ctx, "testing")
+	ctx = namespaces.WithNamespace(ctx, testNamespace)
 	return ctx, cancel
 }
 

daemon_config_linux_test.go

@@ -0,0 +1,184 @@
+/*
+   Copyright The containerd Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package containerd
+
+import (
+	"bytes"
+	"context"
+	"fmt"
+	"io/ioutil"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"syscall"
+	"testing"
+	"time"
+
+	"github.com/containerd/containerd/oci"
+	"github.com/containerd/containerd/server"
+	"github.com/containerd/containerd/testutil"
+)
+
+// the following nolint is for shutting up gometalinter on non-linux.
+// nolint: unused
+func newDaemonWithConfig(t *testing.T, configTOML string) (*Client, *daemon, func()) {
+	if testing.Short() {
+		t.Skip()
+	}
+	testutil.RequiresRoot(t)
+	var (
+		ctrd              = daemon{}
+		configTOMLDecoded server.Config
+		buf               = bytes.NewBuffer(nil)
+	)
+
+	tempDir, err := ioutil.TempDir("", "containerd-test-new-daemon-with-config")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer func() {
+		if err != nil {
+			os.RemoveAll(tempDir)
+		}
+	}()
+
+	configTOMLFile := filepath.Join(tempDir, "config.toml")
+	if err = ioutil.WriteFile(configTOMLFile, []byte(configTOML), 0600); err != nil {
+		t.Fatal(err)
+	}
+
+	if err = server.LoadConfig(configTOMLFile, &configTOMLDecoded); err != nil {
+		t.Fatal(err)
+	}
+
+	address := configTOMLDecoded.GRPC.Address
+	if address == "" {
+		address = filepath.Join(tempDir, "containerd.sock")
+	}
+	args := []string{"-c", configTOMLFile}
+	if configTOMLDecoded.Root == "" {
+		args = append(args, "--root", filepath.Join(tempDir, "root"))
+	}
+	if configTOMLDecoded.State == "" {
+		args = append(args, "--state", filepath.Join(tempDir, "state"))
+	}
+	if err = ctrd.start("containerd", address, args, buf, buf); err != nil {
+		t.Fatalf("%v: %s", err, buf.String())
+	}
+
+	waitCtx, waitCancel := context.WithTimeout(context.TODO(), 2*time.Second)
+	client, err := ctrd.waitForStart(waitCtx)
+	waitCancel()
+	if err != nil {
+		ctrd.Kill()
+		ctrd.Wait()
+		t.Fatalf("%v: %s", err, buf.String())
+	}
+
+	cleanup := func() {
+		if err := client.Close(); err != nil {
+			t.Fatalf("failed to close client: %v", err)
+		}
+		if err := ctrd.Stop(); err != nil {
+			if err := ctrd.Kill(); err != nil {
+				t.Fatalf("failed to signal containerd: %v", err)
+			}
+		}
+		if err := ctrd.Wait(); err != nil {
+			if _, ok := err.(*exec.ExitError); !ok {
+				t.Fatalf("failed to wait for: %v", err)
+			}
+		}
+		if err := os.RemoveAll(tempDir); err != nil {
+			t.Fatalf("failed to remove %s: %v", tempDir, err)
+		}
+		// cleaning config-specific resources is up to the caller
+	}
+	return client, &ctrd, cleanup
+
+}
+
+func testDaemonRuntimeRoot(t *testing.T, noShim bool) {
+	runtimeRoot, err := ioutil.TempDir("", "containerd-test-runtime-root")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer func() {
+		if err != nil {
+			os.RemoveAll(runtimeRoot)
+		}
+	}()
+	configTOML := fmt.Sprintf(`
+[plugins]
+ [plugins.linux]
+   no_shim = %v
+   runtime_root = "%s"
+`, noShim, runtimeRoot)
+
+	client, _, cleanup := newDaemonWithConfig(t, configTOML)
+	defer cleanup()
+
+	ctx, cancel := testContext()
+	defer cancel()
+	// FIXME(AkihiroSuda): import locally frozen image?
+	image, err := client.Pull(ctx, testImage, WithPullUnpack)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	id := t.Name()
+	container, err := client.NewContainer(ctx, id, WithNewSpec(oci.WithImageConfig(image), withProcessArgs("top")), WithNewSnapshot(id, image))
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer container.Delete(ctx, WithSnapshotCleanup)
+
+	task, err := container.NewTask(ctx, empty())
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer task.Delete(ctx)
+
+	if err = task.Start(ctx); err != nil {
+		t.Fatal(err)
+	}
+
+	stateJSONPath := filepath.Join(runtimeRoot, testNamespace, id, "state.json")
+	if _, err = os.Stat(stateJSONPath); err != nil {
+		t.Errorf("error while getting stat for %s: %v", stateJSONPath, err)
+	}
+
+	finishedC, err := task.Wait(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err = task.Kill(ctx, syscall.SIGKILL); err != nil {
+		t.Error(err)
+	}
+	<-finishedC
+}
+
+// TestDaemonRuntimeRoot ensures plugin.linux.runtime_root is not ignored
+func TestDaemonRuntimeRoot(t *testing.T) {
+	testDaemonRuntimeRoot(t, false)
+}
+
+// TestDaemonRuntimeRootNoShim ensures plugin.linux.runtime_root is not ignored when no_shim is true
+func TestDaemonRuntimeRootNoShim(t *testing.T) {
+	t.Skip("no_shim is not functional now: https://github.com/containerd/containerd/issues/2181")
+	testDaemonRuntimeRoot(t, true)
+}

daemon_test.go

@@ -58,7 +58,7 @@ func (d *daemon) waitForStart(ctx context.Context) (*Client, error) {
 		err     error
 	)
 
-	client, err = New(address)
+	client, err = New(d.addr)
 	if err != nil {
 		return nil, err
 	}

linux/bundle.go

@@ -87,26 +87,23 @@ type ShimOpt func(*bundle, string, *runctypes.RuncOptions) (shim.Config, client.
 // ShimRemote is a ShimOpt for connecting and starting a remote shim
 func ShimRemote(c *Config, daemonAddress, cgroup string, exitHandler func()) ShimOpt {
 	return func(b *bundle, ns string, ropts *runctypes.RuncOptions) (shim.Config, client.Opt) {
-		config := b.shimConfig(ns, ropts)
-		if config.RuntimeRoot == "" {
-			config.RuntimeRoot = c.RuntimeRoot
-		}
+		config := b.shimConfig(ns, c, ropts)
 		return config,
 			client.WithStart(c.Shim, b.shimAddress(ns), daemonAddress, cgroup, c.ShimDebug, exitHandler)
 	}
 }
 
 // ShimLocal is a ShimOpt for using an in process shim implementation
-func ShimLocal(exchange *exchange.Exchange) ShimOpt {
+func ShimLocal(c *Config, exchange *exchange.Exchange) ShimOpt {
 	return func(b *bundle, ns string, ropts *runctypes.RuncOptions) (shim.Config, client.Opt) {
-		return b.shimConfig(ns, ropts), client.WithLocal(exchange)
+		return b.shimConfig(ns, c, ropts), client.WithLocal(exchange)
 	}
 }
 
 // ShimConnect is a ShimOpt for connecting to an existing remote shim
-func ShimConnect(onClose func()) ShimOpt {
+func ShimConnect(c *Config, onClose func()) ShimOpt {
 	return func(b *bundle, ns string, ropts *runctypes.RuncOptions) (shim.Config, client.Opt) {
-		return b.shimConfig(ns, ropts), client.WithConnect(b.shimAddress(ns), onClose)
+		return b.shimConfig(ns, c, ropts), client.WithConnect(b.shimAddress(ns), onClose)
 	}
 }
 
@@ -134,16 +131,18 @@ func (b *bundle) shimAddress(namespace string) string {
 	return filepath.Join(string(filepath.Separator), "containerd-shim", namespace, b.id, "shim.sock")
 }
 
-func (b *bundle) shimConfig(namespace string, runcOptions *runctypes.RuncOptions) shim.Config {
+func (b *bundle) shimConfig(namespace string, c *Config, runcOptions *runctypes.RuncOptions) shim.Config {
 	var (
 		criuPath      string
-		runtimeRoot   string
+		runtimeRoot   = c.RuntimeRoot
 		systemdCgroup bool
 	)
 	if runcOptions != nil {
 		criuPath = runcOptions.CriuPath
 		systemdCgroup = runcOptions.SystemdCgroup
-		runtimeRoot = runcOptions.RuntimeRoot
+		if runcOptions.RuntimeRoot != "" {
+			runtimeRoot = runcOptions.RuntimeRoot
+		}
 	}
 	return shim.Config{
 		Path:          b.path,

linux/runtime.go

@@ -186,7 +186,7 @@ func (r *Runtime) Create(ctx context.Context, id string, opts runtime.CreateOpts
 		}
 	}()
 
-	shimopt := ShimLocal(r.events)
+	shimopt := ShimLocal(r.config, r.events)
 	if !r.config.NoShim {
 		var cgroup string
 		if opts.Options != nil {
@@ -396,7 +396,7 @@ func (r *Runtime) loadTasks(ctx context.Context, ns string) ([]*Task, error) {
 		)
 		ctx = namespaces.WithNamespace(ctx, ns)
 		pid, _ := runc.ReadPidFile(filepath.Join(bundle.path, proc.InitPidFile))
-		s, err := bundle.NewShimClient(ctx, ns, ShimConnect(func() {
+		s, err := bundle.NewShimClient(ctx, ns, ShimConnect(r.config, func() {
 			err := r.cleanupAfterDeadShim(ctx, bundle, ns, id, pid)
 			if err != nil {
 				log.G(ctx).WithError(err).WithField("bundle", bundle.path).