From 94df315de82212c28357652a083c24d2accc29f5 Mon Sep 17 00:00:00 2001 From: Mike Brown Date: Thu, 22 Mar 2018 14:49:21 -0500 Subject: [PATCH] adds volatile state directory to the fs plan for cntrs/pods/fifo Signed-off-by: Mike Brown --- cri.go | 6 +- docs/config.md | 2 +- docs/crictl.md | 3 +- pkg/config/config.go | 2 + pkg/server/container_create.go | 28 ++++++-- pkg/server/container_create_test.go | 20 +++--- pkg/server/container_execsync.go | 4 +- pkg/server/container_remove.go | 7 +- pkg/server/helpers.go | 35 +++++++--- pkg/server/restart.go | 103 +++++++++++++--------------- pkg/server/sandbox_remove.go | 9 ++- pkg/server/sandbox_run.go | 34 ++++++--- pkg/server/sandbox_run_test.go | 31 ++++++--- pkg/server/sandbox_stop.go | 5 +- pkg/server/service_test.go | 6 +- 15 files changed, 177 insertions(+), 118 deletions(-) diff --git a/cri.go b/cri.go index 57320c32a..b51b284de 100644 --- a/cri.go +++ b/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) diff --git a/docs/config.md b/docs/config.md index 9b377251c..91a8b910d 100644 --- a/docs/config.md +++ b/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", ] -``` \ No newline at end of file +``` diff --git a/docs/crictl.md b/docs/crictl.md index 8e7b7cf00..8d0578af2 100644 --- a/docs/crictl.md +++ b/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" } diff --git a/pkg/config/config.go b/pkg/config/config.go index c4de6ee91..5c24faf4d 100644 --- a/pkg/config/config.go +++ b/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. diff --git a/pkg/server/container_create.go b/pkg/server/container_create.go index 61037c2cb..be9a79890 100644 --- a/pkg/server/container_create.go +++ b/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 } diff --git a/pkg/server/container_create_test.go b/pkg/server/container_create_test.go index 73b346fb2..1baeb5d47 100644 --- a/pkg/server/container_create_test.go +++ b/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) } } diff --git a/pkg/server/container_execsync.go b/pkg/server/container_execsync.go index bf4efffd5..cf7f1e668 100644 --- a/pkg/server/container_execsync.go +++ b/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 }, ) diff --git a/pkg/server/container_remove.go b/pkg/server/container_remove.go index 83fec6522..e05d79618 100644 --- a/pkg/server/container_remove.go +++ b/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) diff --git a/pkg/server/helpers.go b/pkg/server/helpers.go index 06e43a257..b09f28151 100644 --- a/pkg/server/helpers.go +++ b/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. diff --git a/pkg/server/restart.go b/pkg/server/restart.go index 9912a5854..d37f8a287 100644 --- a/pkg/server/restart.go +++ b/pkg/server/restart.go @@ -78,8 +78,9 @@ func (c *criService) recover(ctx context.Context) error { return errors.Wrap(err, "failed to list containers") } for _, container := range containers { - containerDir := getContainerRootDir(c.config.RootDir, container.ID()) - cntr, err := loadContainer(ctx, container, containerDir) + containerDir := c.getContainerRootDir(container.ID()) + volatileContainerDir := c.getVolatileContainerRootDir(container.ID()) + cntr, err := loadContainer(ctx, container, containerDir, volatileContainerDir) if err != nil { logrus.WithError(err).Errorf("Failed to load container %q", container.ID()) continue @@ -113,21 +114,42 @@ func (c *criService) recover(ctx context.Context) error { // we can't even get metadata, we should cleanup orphaned sandbox/container directories // with best effort. - // Cleanup orphaned sandbox directories without corresponding containerd container. - if err := cleanupOrphanedSandboxDirs(sandboxes, filepath.Join(c.config.RootDir, "sandboxes")); err != nil { - return errors.Wrap(err, "failed to cleanup orphaned sandbox directories") - } - - // Cleanup orphaned container directories without corresponding containerd container. - if err := cleanupOrphanedContainerDirs(containers, filepath.Join(c.config.RootDir, "containers")); err != nil { - return errors.Wrap(err, "failed to cleanup orphaned container directories") + // Cleanup orphaned sandbox and container directories without corresponding containerd container. + for _, cleanup := range []struct { + cntrs []containerd.Container + base string + errMsg string + }{ + { + cntrs: sandboxes, + base: filepath.Join(c.config.RootDir, sandboxesDir), + errMsg: "failed to cleanup orphaned sandbox directories", + }, + { + cntrs: sandboxes, + base: filepath.Join(c.config.StateDir, sandboxesDir), + errMsg: "failed to cleanup orphaned volatile sandbox directories", + }, + { + cntrs: containers, + base: filepath.Join(c.config.RootDir, containersDir), + errMsg: "failed to cleanup orphaned container directories", + }, + { + cntrs: containers, + base: filepath.Join(c.config.StateDir, containersDir), + errMsg: "failed to cleanup orphaned volatile container directories", + }, + } { + if err := cleanupOrphanedIDDirs(cleanup.cntrs, cleanup.base); err != nil { + return errors.Wrap(err, cleanup.errMsg) + } } - return nil } // loadContainer loads container from containerd and status checkpoint. -func loadContainer(ctx context.Context, cntr containerd.Container, containerDir string) (containerstore.Container, error) { +func loadContainer(ctx context.Context, cntr containerd.Container, containerDir, volatileContainerDir string) (containerstore.Container, error) { id := cntr.ID() var container containerstore.Container // Load container metadata. @@ -197,7 +219,7 @@ func loadContainer(ctx context.Context, cntr containerd.Container, containerDir // containerd got restarted during that. In that case, we still // treat the container as `CREATED`. containerIO, err = cio.NewContainerIO(id, - cio.WithNewFIFOs(containerDir, meta.Config.GetTty(), meta.Config.GetStdin()), + cio.WithNewFIFOs(volatileContainerDir, meta.Config.GetTty(), meta.Config.GetStdin()), ) if err != nil { return container, errors.Wrap(err, "failed to create container io") @@ -448,59 +470,30 @@ func loadImages(ctx context.Context, cImages []containerd.Image, return images, nil } -func cleanupOrphanedSandboxDirs(cntrs []containerd.Container, sandboxesRoot string) error { - // Cleanup orphaned sandbox directories. - dirs, err := ioutil.ReadDir(sandboxesRoot) - if err != nil && !os.IsNotExist(err) { - return errors.Wrapf(err, "failed to read pod sandboxes directory %q", sandboxesRoot) - } - cntrsMap := make(map[string]containerd.Container) - for _, cntr := range cntrs { - cntrsMap[cntr.ID()] = cntr - } - for _, d := range dirs { - if !d.IsDir() { - logrus.Warnf("Invalid file %q found in pod sandboxes directory", d.Name()) - continue - } - if _, ok := cntrsMap[d.Name()]; ok { - // Do not remove sandbox directory if corresponding container is found. - continue - } - sandboxDir := filepath.Join(sandboxesRoot, d.Name()) - if err := system.EnsureRemoveAll(sandboxDir); err != nil { - logrus.WithError(err).Warnf("Failed to remove pod sandbox directory %q", sandboxDir) - } else { - logrus.Debugf("Cleanup orphaned pod sandbox directory %q", sandboxDir) - } - } - return nil -} - -func cleanupOrphanedContainerDirs(cntrs []containerd.Container, containersRoot string) error { - // Cleanup orphaned container directories. - dirs, err := ioutil.ReadDir(containersRoot) +func cleanupOrphanedIDDirs(cntrs []containerd.Container, base string) error { + // Cleanup orphaned id directories. + dirs, err := ioutil.ReadDir(base) if err != nil && !os.IsNotExist(err) { - return errors.Wrapf(err, "failed to read containers directory %q", containersRoot) + return errors.Wrap(err, "failed to read base directory") } - cntrsMap := make(map[string]containerd.Container) + idsMap := make(map[string]containerd.Container) for _, cntr := range cntrs { - cntrsMap[cntr.ID()] = cntr + idsMap[cntr.ID()] = cntr } for _, d := range dirs { if !d.IsDir() { - logrus.Warnf("Invalid file %q found in containers directory", d.Name()) + logrus.Warnf("Invalid file %q found in base directory %q", d.Name(), base) continue } - if _, ok := cntrsMap[d.Name()]; ok { - // Do not remove container directory if corresponding container is found. + if _, ok := idsMap[d.Name()]; ok { + // Do not remove id directory if corresponding container is found. continue } - containerDir := filepath.Join(containersRoot, d.Name()) - if err := system.EnsureRemoveAll(containerDir); err != nil { - logrus.WithError(err).Warnf("Failed to remove container directory %q", containerDir) + dir := filepath.Join(base, d.Name()) + if err := system.EnsureRemoveAll(dir); err != nil { + logrus.WithError(err).Warnf("Failed to remove id directory %q", dir) } else { - logrus.Debugf("Cleanup orphaned container directory %q", containerDir) + logrus.Debugf("Cleanup orphaned id directory %q", dir) } } return nil diff --git a/pkg/server/sandbox_remove.go b/pkg/server/sandbox_remove.go index 4f8e0a858..3d3849a8d 100644 --- a/pkg/server/sandbox_remove.go +++ b/pkg/server/sandbox_remove.go @@ -72,12 +72,17 @@ func (c *criService) RemovePodSandbox(ctx context.Context, r *runtime.RemovePodS } } - // Cleanup the sandbox root directory. - sandboxRootDir := getSandboxRootDir(c.config.RootDir, id) + // Cleanup the sandbox root directories. + sandboxRootDir := c.getSandboxRootDir(id) if err := system.EnsureRemoveAll(sandboxRootDir); err != nil { return nil, errors.Wrapf(err, "failed to remove sandbox root directory %q", sandboxRootDir) } + volatileSandboxRootDir := c.getVolatileSandboxRootDir(id) + if err := system.EnsureRemoveAll(volatileSandboxRootDir); err != nil { + return nil, errors.Wrapf(err, "failed to remove volatile sandbox root directory %q", + volatileSandboxRootDir) + } // Delete sandbox container. if err := sandbox.Container.Delete(ctx, containerd.WithSnapshotCleanup); err != nil { diff --git a/pkg/server/sandbox_run.go b/pkg/server/sandbox_run.go index 237290709..de0b42bee 100644 --- a/pkg/server/sandbox_run.go +++ b/pkg/server/sandbox_run.go @@ -189,8 +189,8 @@ func (c *criService) RunPodSandbox(ctx context.Context, r *runtime.RunPodSandbox } }() - // Create sandbox container root directory. - sandboxRootDir := getSandboxRootDir(c.config.RootDir, id) + // Create sandbox container root directories. + sandboxRootDir := c.getSandboxRootDir(id) if err := c.os.MkdirAll(sandboxRootDir, 0755); err != nil { return nil, errors.Wrapf(err, "failed to create sandbox root directory %q", sandboxRootDir) @@ -204,14 +204,28 @@ func (c *criService) RunPodSandbox(ctx context.Context, r *runtime.RunPodSandbox } } }() + volatileSandboxRootDir := c.getVolatileSandboxRootDir(id) + if err := c.os.MkdirAll(volatileSandboxRootDir, 0755); err != nil { + return nil, errors.Wrapf(err, "failed to create volatile sandbox root directory %q", + volatileSandboxRootDir) + } + defer func() { + if retErr != nil { + // Cleanup the volatile sandbox root directory. + if err := c.os.RemoveAll(volatileSandboxRootDir); err != nil { + logrus.WithError(err).Errorf("Failed to remove volatile sandbox root directory %q", + volatileSandboxRootDir) + } + } + }() // Setup sandbox /dev/shm, /etc/hosts and /etc/resolv.conf. - if err = c.setupSandboxFiles(sandboxRootDir, config); err != nil { + if err = c.setupSandboxFiles(id, config); err != nil { return nil, errors.Wrapf(err, "failed to setup sandbox files") } defer func() { if retErr != nil { - if err = c.unmountSandboxFiles(sandboxRootDir, config); err != nil { + if err = c.unmountSandboxFiles(id, config); err != nil { logrus.WithError(err).Errorf("Failed to unmount sandbox files in %q", sandboxRootDir) } @@ -398,9 +412,9 @@ func (c *criService) generateSandboxContainerSpec(id string, config *runtime.Pod // setupSandboxFiles sets up necessary sandbox files including /dev/shm, /etc/hosts // and /etc/resolv.conf. -func (c *criService) setupSandboxFiles(rootDir string, config *runtime.PodSandboxConfig) error { +func (c *criService) setupSandboxFiles(id string, config *runtime.PodSandboxConfig) error { // TODO(random-liu): Consider whether we should maintain /etc/hosts and /etc/resolv.conf in kubelet. - sandboxEtcHosts := getSandboxHosts(rootDir) + sandboxEtcHosts := c.getSandboxHosts(id) if err := c.os.CopyFile(etcHosts, sandboxEtcHosts, 0644); err != nil { return errors.Wrapf(err, "failed to generate sandbox hosts file %q", sandboxEtcHosts) } @@ -414,7 +428,7 @@ func (c *criService) setupSandboxFiles(rootDir string, config *runtime.PodSandbo return errors.Wrapf(err, "failed to parse sandbox DNSConfig %+v", dnsConfig) } } - resolvPath := getResolvPath(rootDir) + resolvPath := c.getResolvPath(id) if resolvContent == "" { // copy host's resolv.conf to resolvPath err = c.os.CopyFile(resolvConfPath, resolvPath, 0644) @@ -434,7 +448,7 @@ func (c *criService) setupSandboxFiles(rootDir string, config *runtime.PodSandbo return errors.Wrapf(err, "host %q is not available for host ipc", devShm) } } else { - sandboxDevShm := getSandboxDevShm(rootDir) + sandboxDevShm := c.getSandboxDevShm(id) if err := c.os.MkdirAll(sandboxDevShm, 0700); err != nil { return errors.Wrap(err, "failed to create sandbox shm") } @@ -475,9 +489,9 @@ func parseDNSOptions(servers, searches, options []string) (string, error) { // remove these files. Unmount should *NOT* return error when: // 1) The mount point is already unmounted. // 2) The mount point doesn't exist. -func (c *criService) unmountSandboxFiles(rootDir string, config *runtime.PodSandboxConfig) error { +func (c *criService) unmountSandboxFiles(id string, config *runtime.PodSandboxConfig) error { if config.GetLinux().GetSecurityContext().GetNamespaceOptions().GetIpc() != runtime.NamespaceMode_NODE { - if err := c.os.Unmount(getSandboxDevShm(rootDir), unix.MNT_DETACH); err != nil && !os.IsNotExist(err) { + if err := c.os.Unmount(c.getSandboxDevShm(id), unix.MNT_DETACH); err != nil && !os.IsNotExist(err) { return err } } diff --git a/pkg/server/sandbox_run_test.go b/pkg/server/sandbox_run_test.go index c5a355a6c..cac8637e9 100644 --- a/pkg/server/sandbox_run_test.go +++ b/pkg/server/sandbox_run_test.go @@ -18,6 +18,7 @@ package server import ( "os" + "path/filepath" "testing" cni "github.com/containerd/go-cni" @@ -177,7 +178,7 @@ func TestGenerateSandboxContainerSpec(t *testing.T) { } func TestSetupSandboxFiles(t *testing.T) { - testRootDir := "test-sandbox-root" + const testID = "test-id" for desc, test := range map[string]struct { dnsConfig *runtime.DNSConfig ipcMode runtime.NamespaceMode @@ -189,13 +190,17 @@ func TestSetupSandboxFiles(t *testing.T) { { Name: "CopyFile", Arguments: []interface{}{ - "/etc/hosts", testRootDir + "/hosts", os.FileMode(0644), + "/etc/hosts", + filepath.Join(testRootDir, sandboxesDir, testID, "hosts"), + os.FileMode(0644), }, }, { Name: "CopyFile", Arguments: []interface{}{ - "/etc/resolv.conf", testRootDir + "/resolv.conf", os.FileMode(0644), + "/etc/resolv.conf", + filepath.Join(testRootDir, sandboxesDir, testID, "resolv.conf"), + os.FileMode(0644), }, }, { @@ -215,13 +220,16 @@ func TestSetupSandboxFiles(t *testing.T) { { Name: "CopyFile", Arguments: []interface{}{ - "/etc/hosts", testRootDir + "/hosts", os.FileMode(0644), + "/etc/hosts", + filepath.Join(testRootDir, sandboxesDir, testID, "hosts"), + os.FileMode(0644), }, }, { Name: "WriteFile", Arguments: []interface{}{ - testRootDir + "/resolv.conf", []byte(`search 114.114.114.114 + filepath.Join(testRootDir, sandboxesDir, testID, "resolv.conf"), + []byte(`search 114.114.114.114 nameserver 8.8.8.8 options timeout:1 `), os.FileMode(0644), @@ -239,19 +247,24 @@ options timeout:1 { Name: "CopyFile", Arguments: []interface{}{ - "/etc/hosts", testRootDir + "/hosts", os.FileMode(0644), + "/etc/hosts", + filepath.Join(testRootDir, sandboxesDir, testID, "hosts"), + os.FileMode(0644), }, }, { Name: "CopyFile", Arguments: []interface{}{ - "/etc/resolv.conf", testRootDir + "/resolv.conf", os.FileMode(0644), + "/etc/resolv.conf", + filepath.Join(testRootDir, sandboxesDir, testID, "resolv.conf"), + os.FileMode(0644), }, }, { Name: "MkdirAll", Arguments: []interface{}{ - testRootDir + "/shm", os.FileMode(0700), + filepath.Join(testStateDir, sandboxesDir, testID, "shm"), + os.FileMode(0700), }, }, { @@ -273,7 +286,7 @@ options timeout:1 }, }, } - c.setupSandboxFiles(testRootDir, cfg) + c.setupSandboxFiles(testID, cfg) calls := c.os.(*ostesting.FakeOS).GetCalls() assert.Len(t, calls, len(test.expectedCalls)) for i, expected := range test.expectedCalls { diff --git a/pkg/server/sandbox_stop.go b/pkg/server/sandbox_stop.go index ce9dca26b..eb00c8d55 100644 --- a/pkg/server/sandbox_stop.go +++ b/pkg/server/sandbox_stop.go @@ -81,9 +81,8 @@ func (c *criService) StopPodSandbox(ctx context.Context, r *runtime.StopPodSandb logrus.Infof("TearDown network for sandbox %q successfully", id) - sandboxRoot := getSandboxRootDir(c.config.RootDir, id) - if err := c.unmountSandboxFiles(sandboxRoot, sandbox.Config); err != nil { - return nil, errors.Wrapf(err, "failed to unmount sandbox files in %q", sandboxRoot) + if err := c.unmountSandboxFiles(id, sandbox.Config); err != nil { + return nil, errors.Wrap(err, "failed to unmount sandbox files") } // Only stop sandbox container when it's running. diff --git a/pkg/server/service_test.go b/pkg/server/service_test.go index eb0d79eed..0bc81e670 100644 --- a/pkg/server/service_test.go +++ b/pkg/server/service_test.go @@ -28,7 +28,8 @@ import ( ) const ( - testRootDir = "/test/rootfs" + testRootDir = "/test/root" + testStateDir = "/test/state" // Use an image id as test sandbox image to avoid image name resolve. // TODO(random-liu): Change this to image name after we have complete image // management unit test framework. @@ -40,7 +41,8 @@ const ( func newTestCRIService() *criService { return &criService{ config: criconfig.Config{ - RootDir: testRootDir, + RootDir: testRootDir, + StateDir: testStateDir, PluginConfig: criconfig.PluginConfig{ SandboxImage: testSandboxImage, },