diff --git a/virtcontainers/agent.go b/virtcontainers/agent.go index c1e685c168..050de36aa9 100644 --- a/virtcontainers/agent.go +++ b/virtcontainers/agent.go @@ -254,5 +254,5 @@ type agent interface { markDead() // cleanup removes all on disk information generated by the agent - cleanup(id string) + cleanup(s *Sandbox) } diff --git a/virtcontainers/api_test.go b/virtcontainers/api_test.go index 68a2bc8803..dff982d534 100644 --- a/virtcontainers/api_test.go +++ b/virtcontainers/api_test.go @@ -322,6 +322,9 @@ func TestStartSandboxFailing(t *testing.T) { } func TestStopSandboxNoopAgentSuccessful(t *testing.T) { + if tc.NotValid(ktu.NeedRoot()) { + t.Skip(testDisabledAsNonRoot) + } defer cleanUp() assert := assert.New(t) @@ -910,6 +913,9 @@ func TestStartContainerFailingSandboxNotStarted(t *testing.T) { } func TestStopContainerNoopAgentSuccessful(t *testing.T) { + if tc.NotValid(ktu.NeedRoot()) { + t.Skip(testDisabledAsNonRoot) + } defer cleanUp() assert := assert.New(t) @@ -942,6 +948,9 @@ func TestStopContainerNoopAgentSuccessful(t *testing.T) { } func TestStopContainerFailingNoSandbox(t *testing.T) { + if tc.NotValid(ktu.NeedRoot()) { + t.Skip(testDisabledAsNonRoot) + } defer cleanUp() contID := "100" @@ -951,6 +960,9 @@ func TestStopContainerFailingNoSandbox(t *testing.T) { } func TestStopContainerFailingNoContainer(t *testing.T) { + if tc.NotValid(ktu.NeedRoot()) { + t.Skip(testDisabledAsNonRoot) + } defer cleanUp() assert := assert.New(t) diff --git a/virtcontainers/container.go b/virtcontainers/container.go index 4b25a0a472..fd52493fb2 100644 --- a/virtcontainers/container.go +++ b/virtcontainers/container.go @@ -1049,6 +1049,14 @@ func (c *Container) stop(force bool) error { return err } + if err := c.unmountHostMounts(); err != nil && !force { + return err + } + + if err := bindUnmountContainerRootfs(c.ctx, kataHostSharedDir, c.sandbox.id, c.id); err != nil && !force { + return err + } + if err := c.detachDevices(); err != nil && !force { return err } diff --git a/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go index d90ca12f1a..6e1579ee24 100644 --- a/virtcontainers/kata_agent.go +++ b/virtcontainers/kata_agent.go @@ -1419,19 +1419,8 @@ func (k *kataAgent) stopContainer(sandbox *Sandbox, c Container) error { span, _ := k.trace("stopContainer") defer span.Finish() - req := &grpc.RemoveContainerRequest{ - ContainerId: c.id, - } - - if _, err := k.sendReq(req); err != nil { - return err - } - - if err := c.unmountHostMounts(); err != nil { - return err - } - - return bindUnmountContainerRootfs(k.ctx, kataHostSharedDir, sandbox.id, c.id) + _, err := k.sendReq(&grpc.RemoveContainerRequest{ContainerId: c.id}) + return err } func (k *kataAgent) signalProcess(c *Container, processID string, signal syscall.Signal, all bool) error { @@ -2075,9 +2064,12 @@ func (k *kataAgent) markDead() { k.disconnect() } -func (k *kataAgent) cleanup(id string) { - path := k.getSharePath(id) +func (k *kataAgent) cleanup(s *Sandbox) { + path := k.getSharePath(s.id) k.Logger().WithField("path", path).Infof("cleanup agent") + if err := bindUnmountAllRootfs(k.ctx, path, s); err != nil { + k.Logger().WithError(err).Errorf("failed to unmount container share path %s", path) + } if err := os.RemoveAll(path); err != nil { k.Logger().WithError(err).Errorf("failed to cleanup vm share path %s", path) } diff --git a/virtcontainers/kata_agent_test.go b/virtcontainers/kata_agent_test.go index e33524f879..3c4576ad0c 100644 --- a/virtcontainers/kata_agent_test.go +++ b/virtcontainers/kata_agent_test.go @@ -933,8 +933,8 @@ func TestKataCleanupSandbox(t *testing.T) { err := os.MkdirAll(dir, 0777) assert.Nil(err) - k := &kataAgent{} - k.cleanup(s.id) + k := &kataAgent{ctx: context.Background()} + k.cleanup(&s) _, err = os.Stat(dir) assert.False(os.IsExist(err)) diff --git a/virtcontainers/noop_agent.go b/virtcontainers/noop_agent.go index 5ebfe5faee..307caa7f2a 100644 --- a/virtcontainers/noop_agent.go +++ b/virtcontainers/noop_agent.go @@ -231,5 +231,5 @@ func (n *noopAgent) copyFile(src, dst string) error { func (n *noopAgent) markDead() { } -func (n *noopAgent) cleanup(id string) { +func (n *noopAgent) cleanup(s *Sandbox) { } diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index 1356bfcc82..3095891a92 100644 --- a/virtcontainers/sandbox.go +++ b/virtcontainers/sandbox.go @@ -764,7 +764,7 @@ func (s *Sandbox) Delete() error { s.Logger().WithError(err).Error("failed to cleanup hypervisor") } - s.agent.cleanup(s.id) + s.agent.cleanup(s) return s.store.Delete() }