From 9e606b3da83a8e2eeb41cb7da9edfc570567139d Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Mon, 24 Sep 2018 11:43:56 -0500 Subject: [PATCH] virtcontainers: revert "fix shared dir resource remaining" This reverts commit 8a6d383715f6cd8e0777ab5d14a421129bdff8c0. Don't remove all directories in the shared directory because `docker cp` re-mounts all the mount points specified in the config.json causing serious problems in the host. fixes #777 Signed-off-by: Julio Montes --- virtcontainers/agent.go | 3 --- virtcontainers/container.go | 16 ++++------------ virtcontainers/hyperstart_agent.go | 4 ---- virtcontainers/kata_agent.go | 13 +------------ virtcontainers/noop_agent.go | 5 ----- virtcontainers/sandbox.go | 7 ------- 6 files changed, 5 insertions(+), 43 deletions(-) diff --git a/virtcontainers/agent.go b/virtcontainers/agent.go index 01291b994..9848ead4b 100644 --- a/virtcontainers/agent.go +++ b/virtcontainers/agent.go @@ -163,9 +163,6 @@ type agent interface { // stopSandbox will tell the agent to stop all containers related to the Sandbox. stopSandbox(sandbox *Sandbox) error - // cleanup will clean the resources for sandbox - cleanupSandbox(sandbox *Sandbox) error - // createContainer will tell the agent to create a container related to a Sandbox. createContainer(sandbox *Sandbox, c *Container) (*Process, error) diff --git a/virtcontainers/container.go b/virtcontainers/container.go index 2dff28925..bf063c0fd 100644 --- a/virtcontainers/container.go +++ b/virtcontainers/container.go @@ -507,19 +507,11 @@ func (c *Container) unmountHostMounts() error { span, _ := c.trace("unmount") span.SetTag("host-path", m.HostPath) - logger := c.Logger().WithField("host-path", m.HostPath) if err := syscall.Unmount(m.HostPath, 0); err != nil { - // Unable to unmount paths could be a really big problem here - // we need to make sure cause 'less damage' if things are - // really broken. For further, we need to give admins more of - // a chance to diagnose the problem. As the rules of `fail fast`, - // here we return an error as soon as we get it. - logger.WithError(err).Warn("Could not umount") - return err - } else if err := os.RemoveAll(m.HostPath); err != nil { - // since the mounts related to the shared dir is umounted - // we need to remove the host path to avoid resource remaining - logger.WithError(err).Warn("Could not be removed") + c.Logger().WithFields(logrus.Fields{ + "host-path": m.HostPath, + "error": err, + }).Warn("Could not umount") return err } diff --git a/virtcontainers/hyperstart_agent.go b/virtcontainers/hyperstart_agent.go index 1b8a8cd49..43f6489ce 100644 --- a/virtcontainers/hyperstart_agent.go +++ b/virtcontainers/hyperstart_agent.go @@ -953,10 +953,6 @@ func (h *hyper) resumeContainer(sandbox *Sandbox, c Container) error { return nil } -func (h *hyper) cleanupSandbox(sandbox *Sandbox) error { - return nil -} - func (h *hyper) reseedRNG(data []byte) error { // hyperstart-agent does not support reseeding return nil diff --git a/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go index 823cc1031..261166f59 100644 --- a/virtcontainers/kata_agent.go +++ b/virtcontainers/kata_agent.go @@ -670,10 +670,6 @@ func (k *kataAgent) stopSandbox(sandbox *Sandbox) error { return nil } -func (k *kataAgent) cleanupSandbox(sandbox *Sandbox) error { - return os.RemoveAll(filepath.Join(kataHostSharedDir, sandbox.id)) -} - func (k *kataAgent) replaceOCIMountSource(spec *specs.Spec, guestMounts []Mount) error { ociMounts := spec.Mounts @@ -1167,14 +1163,7 @@ func (k *kataAgent) stopContainer(sandbox *Sandbox, c Container) error { return err } - if err := bindUnmountContainerRootfs(k.ctx, kataHostSharedDir, sandbox.id, c.id); err != nil { - return err - } - - // since rootfs is umounted it's safe to remove the dir now - rootPathParent := filepath.Join(kataHostSharedDir, sandbox.id, c.id) - - return os.RemoveAll(rootPathParent) + return bindUnmountContainerRootfs(k.ctx, kataHostSharedDir, sandbox.id, c.id) } func (k *kataAgent) signalProcess(c *Container, processID string, signal syscall.Signal, all bool) error { diff --git a/virtcontainers/noop_agent.go b/virtcontainers/noop_agent.go index 5649d81f0..51764fbb6 100644 --- a/virtcontainers/noop_agent.go +++ b/virtcontainers/noop_agent.go @@ -58,11 +58,6 @@ func (n *noopAgent) stopSandbox(sandbox *Sandbox) error { return nil } -// cleanup is the Noop agent clean up resource implementation. It does nothing. -func (n *noopAgent) cleanupSandbox(sandbox *Sandbox) error { - return nil -} - // createContainer is the Noop agent Container creation implementation. It does nothing. func (n *noopAgent) createContainer(sandbox *Sandbox, c *Container) (*Process, error) { return &Process{}, nil diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index f46d778df..2f6a33184 100644 --- a/virtcontainers/sandbox.go +++ b/virtcontainers/sandbox.go @@ -1455,13 +1455,6 @@ func (s *Sandbox) stop() error { return err } - // vm is stopped remove the sandbox shared dir - if err := s.agent.cleanupSandbox(s); err != nil { - // cleanup resource failed shouldn't block destroy sandbox - // just raise a warning - s.Logger().WithError(err).Warnf("cleanup sandbox failed") - } - return s.setSandboxState(StateStopped) }