Merge pull request kata-containers#292 from jshachm/remove-host-shared

virtcontainers : fix shared dir resource remaining

virtcontainers/agent.go

@@ -152,6 +152,9 @@ 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)
 

virtcontainers/container.go

@@ -510,11 +510,19 @@ func (c *Container) mountSharedDirMounts(hostSharedDir, guestSharedDir string) (
 func (c *Container) unmountHostMounts() error {
 	for _, m := range c.mounts {
 		if m.HostPath != "" {
+			logger := c.Logger().WithField("host-path", m.HostPath)
 			if err := syscall.Unmount(m.HostPath, 0); err != nil {
-				c.Logger().WithFields(logrus.Fields{
-					"host-path": m.HostPath,
-					"error":     err,
-				}).Warn("Could not umount")
+				// 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")
 				return err
 			}
 		}

virtcontainers/hyperstart_agent.go

@@ -854,3 +854,7 @@ func (h *hyper) resumeContainer(sandbox *Sandbox, c Container) error {
 	// hyperstart-agent does not support resume container
 	return nil
 }
+
+func (h *hyper) cleanupSandbox(sandbox *Sandbox) error {
+	return nil
+}

virtcontainers/kata_agent.go

@@ -549,6 +549,10 @@ func (k *kataAgent) stopSandbox(sandbox *Sandbox) error {
 	return k.proxy.stop(sandbox, k.state.ProxyPid)
 }
 
+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
 
@@ -951,7 +955,14 @@ func (k *kataAgent) stopContainer(sandbox *Sandbox, c Container) error {
 		return err
 	}
 
-	return bindUnmountContainerRootfs(kataHostSharedDir, sandbox.id, c.id)
+	if err := bindUnmountContainerRootfs(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)
 }
 
 func (k *kataAgent) signalProcess(c *Container, processID string, signal syscall.Signal, all bool) error {

virtcontainers/noop_agent.go

@@ -51,6 +51,11 @@ 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

virtcontainers/sandbox.go

@@ -1182,6 +1182,13 @@ 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)
 }