diff --git a/virtcontainers/agent.go b/virtcontainers/agent.go index 9327786e49..0f32e9d03c 100644 --- a/virtcontainers/agent.go +++ b/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) diff --git a/virtcontainers/container.go b/virtcontainers/container.go index b3a10adc8b..941b7d3968 100644 --- a/virtcontainers/container.go +++ b/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 } } diff --git a/virtcontainers/hyperstart_agent.go b/virtcontainers/hyperstart_agent.go index 230f2393e4..4669930a2c 100644 --- a/virtcontainers/hyperstart_agent.go +++ b/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 +} diff --git a/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go index b49efae9dc..c96c05273e 100644 --- a/virtcontainers/kata_agent.go +++ b/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 { diff --git a/virtcontainers/noop_agent.go b/virtcontainers/noop_agent.go index 016eda65a7..57dc0e6da1 100644 --- a/virtcontainers/noop_agent.go +++ b/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 diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index ba70fadb94..2d0abbfff9 100644 --- a/virtcontainers/sandbox.go +++ b/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) }