Merge pull request #697 from Random-Liu/fs-layout-change

adds volatile state directory to the fs plan for cntrs/pods/fifo

cri.go

@@ -63,13 +63,11 @@ func initCRIService(ic *plugin.InitContext) (interface{}, error) {
 	ctx := ic.Context
 	pluginConfig := ic.Config.(*criconfig.PluginConfig)
 	c := criconfig.Config{
-		PluginConfig: *pluginConfig,
-		// This is a hack. We assume that containerd root directory
-		// is one level above plugin directory.
-		// TODO(random-liu): Expose containerd config to plugin.
+		PluginConfig:       *pluginConfig,
 		ContainerdRootDir:  filepath.Dir(ic.Root),
 		ContainerdEndpoint: ic.Address,
 		RootDir:            ic.Root,
+		StateDir:           ic.State,
 	}
 	log.G(ctx).Infof("Start cri plugin with config %+v", c)
 

docs/config.md

@@ -67,4 +67,4 @@ The explanation and default value of each configuration item are as follows:
     [plugins.cri.registry.mirrors]
       [plugins.cri.registry.mirrors."docker.io"]
         endpoint = ["https://registry-1.docker.io", ]
-```
+```

docs/crictl.md

@@ -189,7 +189,8 @@ $ crictl info
     "statsCollectPeriod": 10,
     "containerdRootDir": "/var/lib/containerd",
     "containerdEndpoint": "/run/containerd/containerd.sock",
-    "rootDir": "/var/lib/containerd/io.containerd.grpc.v1.cri"
+    "rootDir": "/var/lib/containerd/io.containerd.grpc.v1.cri",
+    "stateDir": "/run/containerd/io.containerd.grpc.v1.cri",
   },
   "golang": "go1.10"
 }

pkg/config/config.go

@@ -96,6 +96,8 @@ type Config struct {
 	// RootDir is the root directory path for managing cri plugin files
 	// (metadata checkpoint etc.)
 	RootDir string `json:"rootDir"`
+	// StateDir is the root directory path for managing volatile pod/container data
+	StateDir string `json:"stateDir"`
 }
 
 // DefaultConfig returns default configurations of cri plugin.

pkg/os/os.go

@@ -25,6 +25,7 @@ import (
 	containerdmount "github.com/containerd/containerd/mount"
 	"github.com/containerd/fifo"
 	"github.com/docker/docker/pkg/mount"
+	"github.com/docker/docker/pkg/symlink"
 	"golang.org/x/net/context"
 	"golang.org/x/sys/unix"
 )
@@ -37,6 +38,7 @@ type OS interface {
 	OpenFifo(ctx context.Context, fn string, flag int, perm os.FileMode) (io.ReadWriteCloser, error)
 	Stat(name string) (os.FileInfo, error)
 	ResolveSymbolicLink(name string) (string, error)
+	FollowSymlinkInScope(path, scope string) (string, error)
 	CopyFile(src, dest string, perm os.FileMode) error
 	WriteFile(filename string, data []byte, perm os.FileMode) error
 	Mount(source string, target string, fstype string, flags uintptr, data string) error
@@ -79,6 +81,11 @@ func (RealOS) ResolveSymbolicLink(path string) (string, error) {
 	return filepath.EvalSymlinks(path)
 }
 
+// FollowSymlinkInScope will call symlink.FollowSymlinkInScope.
+func (RealOS) FollowSymlinkInScope(path, scope string) (string, error) {
+	return symlink.FollowSymlinkInScope(path, scope)
+}
+
 // CopyFile will copy src file to dest file
 func (RealOS) CopyFile(src, dest string, perm os.FileMode) error {
 	in, err := os.Open(src)
@@ -107,17 +114,21 @@ func (RealOS) Mount(source string, target string, fstype string, flags uintptr,
 	return unix.Mount(source, target, fstype, flags, data)
 }
 
-// Unmount will call unix.Unmount to unmount the file. The function doesn't
-// return error if target is not mounted.
+// Unmount will call Unmount to unmount the file.
 func (RealOS) Unmount(target string, flags int) error {
-	// TODO(random-liu): Follow symlink to make sure the result is correct.
-	if mounted, err := mount.Mounted(target); err != nil || !mounted {
-		return err
-	}
-	return unix.Unmount(target, flags)
+	return Unmount(target, flags)
 }
 
 // LookupMount gets mount info of a given path.
 func (RealOS) LookupMount(path string) (containerdmount.Info, error) {
 	return containerdmount.Lookup(path)
 }
+
+// Unmount will call unix.Unmount to unmount the file. The function doesn't
+// return error if target is not mounted.
+func Unmount(target string, flags int) error {
+	if mounted, err := mount.Mounted(target); err != nil || !mounted {
+		return err
+	}
+	return unix.Unmount(target, flags)
+}

pkg/os/testing/fake_os.go

@@ -40,18 +40,19 @@ type CalledDetail struct {
 // of the real call.
 type FakeOS struct {
 	sync.Mutex
-	MkdirAllFn            func(string, os.FileMode) error
-	RemoveAllFn           func(string) error
-	OpenFifoFn            func(context.Context, string, int, os.FileMode) (io.ReadWriteCloser, error)
-	StatFn                func(string) (os.FileInfo, error)
-	ResolveSymbolicLinkFn func(string) (string, error)
-	CopyFileFn            func(string, string, os.FileMode) error
-	WriteFileFn           func(string, []byte, os.FileMode) error
-	MountFn               func(source string, target string, fstype string, flags uintptr, data string) error
-	UnmountFn             func(target string, flags int) error
-	LookupMountFn         func(path string) (containerdmount.Info, error)
-	calls                 []CalledDetail
-	errors                map[string]error
+	MkdirAllFn             func(string, os.FileMode) error
+	RemoveAllFn            func(string) error
+	OpenFifoFn             func(context.Context, string, int, os.FileMode) (io.ReadWriteCloser, error)
+	StatFn                 func(string) (os.FileInfo, error)
+	ResolveSymbolicLinkFn  func(string) (string, error)
+	FollowSymlinkInScopeFn func(string, string) (string, error)
+	CopyFileFn             func(string, string, os.FileMode) error
+	WriteFileFn            func(string, []byte, os.FileMode) error
+	MountFn                func(source string, target string, fstype string, flags uintptr, data string) error
+	UnmountFn              func(target string, flags int) error
+	LookupMountFn          func(path string) (containerdmount.Info, error)
+	calls                  []CalledDetail
+	errors                 map[string]error
 }
 
 var _ osInterface.OS = &FakeOS{}
@@ -176,6 +177,19 @@ func (f *FakeOS) ResolveSymbolicLink(path string) (string, error) {
 	return path, nil
 }
 
+// FollowSymlinkInScope is a fake call that invokes FollowSymlinkInScope or returns its input
+func (f *FakeOS) FollowSymlinkInScope(path, scope string) (string, error) {
+	f.appendCalls("FollowSymlinkInScope", path, scope)
+	if err := f.getError("FollowSymlinkInScope"); err != nil {
+		return "", err
+	}
+
+	if f.FollowSymlinkInScopeFn != nil {
+		return f.FollowSymlinkInScopeFn(path, scope)
+	}
+	return path, nil
+}
+
 // CopyFile is a fake call that invokes CopyFileFn or just return nil.
 func (f *FakeOS) CopyFile(src, dest string, perm os.FileMode) error {
 	f.appendCalls("CopyFile", src, dest, perm)

pkg/server/container_create.go

@@ -134,7 +134,7 @@ func (c *criService) CreateContainer(ctx context.Context, r *runtime.CreateConta
 	logrus.Debugf("Use OCI %+v for container %q", ociRuntime, id)
 
 	// Create container root directory.
-	containerRootDir := getContainerRootDir(c.config.RootDir, id)
+	containerRootDir := c.getContainerRootDir(id)
 	if err = c.os.MkdirAll(containerRootDir, 0755); err != nil {
 		return nil, errors.Wrapf(err, "failed to create container root directory %q",
 			containerRootDir)
@@ -148,12 +148,26 @@ func (c *criService) CreateContainer(ctx context.Context, r *runtime.CreateConta
 			}
 		}
 	}()
+	volatileContainerRootDir := c.getVolatileContainerRootDir(id)
+	if err = c.os.MkdirAll(volatileContainerRootDir, 0755); err != nil {
+		return nil, errors.Wrapf(err, "failed to create volatile container root directory %q",
+			volatileContainerRootDir)
+	}
+	defer func() {
+		if retErr != nil {
+			// Cleanup the volatile container root directory.
+			if err = c.os.RemoveAll(volatileContainerRootDir); err != nil {
+				logrus.WithError(err).Errorf("Failed to remove volatile container root directory %q",
+					volatileContainerRootDir)
+			}
+		}
+	}()
 
 	// Create container volumes mounts.
 	volumeMounts := c.generateVolumeMounts(containerRootDir, config.GetMounts(), &image.ImageSpec.Config)
 
 	// Generate container runtime spec.
-	mounts := c.generateContainerMounts(getSandboxRootDir(c.config.RootDir, sandboxID), config)
+	mounts := c.generateContainerMounts(sandboxID, config)
 
 	spec, err := c.generateContainerSpec(id, sandboxID, sandboxPid, config, sandboxConfig, &image.ImageSpec.Config, append(mounts, volumeMounts...))
 	if err != nil {
@@ -188,7 +202,7 @@ func (c *criService) CreateContainer(ctx context.Context, r *runtime.CreateConta
 	}
 
 	containerIO, err := cio.NewContainerIO(id,
-		cio.WithNewFIFOs(containerRootDir, config.GetTty(), config.GetStdin()))
+		cio.WithNewFIFOs(volatileContainerRootDir, config.GetTty(), config.GetStdin()))
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to create container io")
 	}
@@ -416,13 +430,13 @@ func (c *criService) generateVolumeMounts(containerRootDir string, criMounts []*
 
 // generateContainerMounts sets up necessary container mounts including /dev/shm, /etc/hosts
 // and /etc/resolv.conf.
-func (c *criService) generateContainerMounts(sandboxRootDir string, config *runtime.ContainerConfig) []*runtime.Mount {
+func (c *criService) generateContainerMounts(sandboxID string, config *runtime.ContainerConfig) []*runtime.Mount {
 	var mounts []*runtime.Mount
 	securityContext := config.GetLinux().GetSecurityContext()
 	if !isInCRIMounts(etcHosts, config.GetMounts()) {
 		mounts = append(mounts, &runtime.Mount{
 			ContainerPath: etcHosts,
-			HostPath:      getSandboxHosts(sandboxRootDir),
+			HostPath:      c.getSandboxHosts(sandboxID),
 			Readonly:      securityContext.GetReadonlyRootfs(),
 		})
 	}
@@ -432,13 +446,13 @@ func (c *criService) generateContainerMounts(sandboxRootDir string, config *runt
 	if !isInCRIMounts(resolvConfPath, config.GetMounts()) {
 		mounts = append(mounts, &runtime.Mount{
 			ContainerPath: resolvConfPath,
-			HostPath:      getResolvPath(sandboxRootDir),
+			HostPath:      c.getResolvPath(sandboxID),
 			Readonly:      securityContext.GetReadonlyRootfs(),
 		})
 	}
 
 	if !isInCRIMounts(devShm, config.GetMounts()) {
-		sandboxDevShm := getSandboxDevShm(sandboxRootDir)
+		sandboxDevShm := c.getSandboxDevShm(sandboxID)
 		if securityContext.GetNamespaceOptions().GetIpc() == runtime.NamespaceMode_NODE {
 			sandboxDevShm = devShm
 		}

pkg/server/container_create_test.go

@@ -497,7 +497,7 @@ func TestGenerateVolumeMounts(t *testing.T) {
 }
 
 func TestGenerateContainerMounts(t *testing.T) {
-	testSandboxRootDir := "test-sandbox-root"
+	const testSandboxID = "test-id"
 	for desc, test := range map[string]struct {
 		criMounts       []*runtime.Mount
 		securityContext *runtime.LinuxContainerSecurityContext
@@ -510,17 +510,17 @@ func TestGenerateContainerMounts(t *testing.T) {
 			expectedMounts: []*runtime.Mount{
 				{
 					ContainerPath: "/etc/hosts",
-					HostPath:      testSandboxRootDir + "/hosts",
+					HostPath:      filepath.Join(testRootDir, sandboxesDir, testSandboxID, "hosts"),
 					Readonly:      true,
 				},
 				{
 					ContainerPath: resolvConfPath,
-					HostPath:      testSandboxRootDir + "/resolv.conf",
+					HostPath:      filepath.Join(testRootDir, sandboxesDir, testSandboxID, "resolv.conf"),
 					Readonly:      true,
 				},
 				{
 					ContainerPath: "/dev/shm",
-					HostPath:      testSandboxRootDir + "/shm",
+					HostPath:      filepath.Join(testStateDir, sandboxesDir, testSandboxID, "shm"),
 					Readonly:      false,
 				},
 			},
@@ -530,17 +530,17 @@ func TestGenerateContainerMounts(t *testing.T) {
 			expectedMounts: []*runtime.Mount{
 				{
 					ContainerPath: "/etc/hosts",
-					HostPath:      testSandboxRootDir + "/hosts",
+					HostPath:      filepath.Join(testRootDir, sandboxesDir, testSandboxID, "hosts"),
 					Readonly:      false,
 				},
 				{
 					ContainerPath: resolvConfPath,
-					HostPath:      testSandboxRootDir + "/resolv.conf",
+					HostPath:      filepath.Join(testRootDir, sandboxesDir, testSandboxID, "resolv.conf"),
 					Readonly:      false,
 				},
 				{
 					ContainerPath: "/dev/shm",
-					HostPath:      testSandboxRootDir + "/shm",
+					HostPath:      filepath.Join(testStateDir, sandboxesDir, testSandboxID, "shm"),
 					Readonly:      false,
 				},
 			},
@@ -552,12 +552,12 @@ func TestGenerateContainerMounts(t *testing.T) {
 			expectedMounts: []*runtime.Mount{
 				{
 					ContainerPath: "/etc/hosts",
-					HostPath:      testSandboxRootDir + "/hosts",
+					HostPath:      filepath.Join(testRootDir, sandboxesDir, testSandboxID, "hosts"),
 					Readonly:      false,
 				},
 				{
 					ContainerPath: resolvConfPath,
-					HostPath:      testSandboxRootDir + "/resolv.conf",
+					HostPath:      filepath.Join(testRootDir, sandboxesDir, testSandboxID, "resolv.conf"),
 					Readonly:      false,
 				},
 				{
@@ -597,7 +597,7 @@ func TestGenerateContainerMounts(t *testing.T) {
 			},
 		}
 		c := newTestCRIService()
-		mounts := c.generateContainerMounts(testSandboxRootDir, config)
+		mounts := c.generateContainerMounts(testSandboxID, config)
 		assert.Equal(t, test.expectedMounts, mounts, desc)
 	}
 }

pkg/server/container_execsync.go

@@ -116,12 +116,12 @@ func (c *criService) execInContainer(ctx context.Context, id string, opts execOp
 	}
 	execID := util.GenerateID()
 	logrus.Debugf("Generated exec id %q for container %q", execID, id)
-	rootDir := getContainerRootDir(c.config.RootDir, id)
+	volatileRootDir := c.getVolatileContainerRootDir(id)
 	var execIO *cio.ExecIO
 	process, err := task.Exec(ctx, execID, pspec,
 		func(id string) (containerdio.IO, error) {
 			var err error
-			execIO, err = cio.NewExecIO(id, rootDir, opts.tty, opts.stdin != nil)
+			execIO, err = cio.NewExecIO(id, volatileRootDir, opts.tty, opts.stdin != nil)
 			return execIO, err
 		},
 	)

pkg/server/container_remove.go

@@ -76,11 +76,16 @@ func (c *criService) RemoveContainer(ctx context.Context, r *runtime.RemoveConta
 		return nil, errors.Wrapf(err, "failed to delete container checkpoint for %q", id)
 	}
 
-	containerRootDir := getContainerRootDir(c.config.RootDir, id)
+	containerRootDir := c.getContainerRootDir(id)
 	if err := system.EnsureRemoveAll(containerRootDir); err != nil {
 		return nil, errors.Wrapf(err, "failed to remove container root directory %q",
 			containerRootDir)
 	}
+	volatileContainerRootDir := c.getVolatileContainerRootDir(id)
+	if err := system.EnsureRemoveAll(volatileContainerRootDir); err != nil {
+		return nil, errors.Wrapf(err, "failed to remove volatile container root directory %q",
+			volatileContainerRootDir)
+	}
 
 	c.containerStore.Delete(id)
 

pkg/server/helpers.go

@@ -156,29 +156,42 @@ func getCgroupsPath(cgroupsParent, id string, systemdCgroup bool) string {
 }
 
 // getSandboxRootDir returns the root directory for managing sandbox files,
+// e.g. hosts files.
+func (c *criService) getSandboxRootDir(id string) string {
+	return filepath.Join(c.config.RootDir, sandboxesDir, id)
+}
+
+// getVolatileSandboxRootDir returns the root directory for managing volatile sandbox files,
 // e.g. named pipes.
-func getSandboxRootDir(rootDir, id string) string {
-	return filepath.Join(rootDir, sandboxesDir, id)
+func (c *criService) getVolatileSandboxRootDir(id string) string {
+	return filepath.Join(c.config.StateDir, sandboxesDir, id)
 }
 
-// getContainerRootDir returns the root directory for managing container files.
-func getContainerRootDir(rootDir, id string) string {
-	return filepath.Join(rootDir, containersDir, id)
+// getContainerRootDir returns the root directory for managing container files,
+// e.g. state checkpoint.
+func (c *criService) getContainerRootDir(id string) string {
+	return filepath.Join(c.config.RootDir, containersDir, id)
+}
+
+// getVolatileContainerRootDir returns the root directory for managing volatile container files,
+// e.g. named pipes.
+func (c *criService) getVolatileContainerRootDir(id string) string {
+	return filepath.Join(c.config.StateDir, containersDir, id)
 }
 
 // getSandboxHosts returns the hosts file path inside the sandbox root directory.
-func getSandboxHosts(sandboxRootDir string) string {
-	return filepath.Join(sandboxRootDir, "hosts")
+func (c *criService) getSandboxHosts(id string) string {
+	return filepath.Join(c.getSandboxRootDir(id), "hosts")
 }
 
 // getResolvPath returns resolv.conf filepath for specified sandbox.
-func getResolvPath(sandboxRoot string) string {
-	return filepath.Join(sandboxRoot, "resolv.conf")
+func (c *criService) getResolvPath(id string) string {
+	return filepath.Join(c.getSandboxRootDir(id), "resolv.conf")
 }
 
 // getSandboxDevShm returns the shm file path inside the sandbox root directory.
-func getSandboxDevShm(sandboxRootDir string) string {
-	return filepath.Join(sandboxRootDir, "shm")
+func (c *criService) getSandboxDevShm(id string) string {
+	return filepath.Join(c.getVolatileSandboxRootDir(id), "shm")
 }
 
 // getNetworkNamespace returns the network namespace of a process.