Merge pull request #1897 from bergwolf/monitor

improve robustness w.r.t. dead hypervisor

cli/delete.go

@@ -112,7 +112,7 @@ func delete(ctx context.Context, containerID string, force bool) error {
 
 	switch containerType {
 	case vc.PodSandbox:
-		if err := deleteSandbox(ctx, sandboxID); err != nil {
+		if err := deleteSandbox(ctx, sandboxID, force); err != nil {
 			return err
 		}
 	case vc.PodContainer:
@@ -131,7 +131,7 @@ func delete(ctx context.Context, containerID string, force bool) error {
 	return katautils.DelContainerIDMapping(ctx, containerID)
 }
 
-func deleteSandbox(ctx context.Context, sandboxID string) error {
+func deleteSandbox(ctx context.Context, sandboxID string, force bool) error {
 	span, _ := katautils.Trace(ctx, "deleteSandbox")
 	defer span.Finish()
 
@@ -141,7 +141,7 @@ func deleteSandbox(ctx context.Context, sandboxID string) error {
 	}
 
 	if oci.StateToOCIState(status.State.State) != oci.StateStopped {
-		if _, err := vci.StopSandbox(ctx, sandboxID); err != nil {
+		if _, err := vci.StopSandbox(ctx, sandboxID, force); err != nil {
 			return err
 		}
 	}

cli/delete_test.go

@@ -200,7 +200,7 @@ func TestDeleteSandbox(t *testing.T) {
 	assert.Error(err)
 	assert.True(vcmock.IsMockError(err))
 
-	testingImpl.StopSandboxFunc = func(ctx context.Context, sandboxID string) (vc.VCSandbox, error) {
+	testingImpl.StopSandboxFunc = func(ctx context.Context, sandboxID string, force bool) (vc.VCSandbox, error) {
 		return sandbox, nil
 	}
 
@@ -310,7 +310,7 @@ func TestDeleteSandboxRunning(t *testing.T) {
 		}, nil
 	}
 
-	testingImpl.StopSandboxFunc = func(ctx context.Context, sandboxID string) (vc.VCSandbox, error) {
+	testingImpl.StopSandboxFunc = func(ctx context.Context, sandboxID string, force bool) (vc.VCSandbox, error) {
 		return sandbox, nil
 	}
 
@@ -564,7 +564,7 @@ func TestDeleteCLIFunctionSuccess(t *testing.T) {
 		}, nil
 	}
 
-	testingImpl.StopSandboxFunc = func(ctx context.Context, sandboxID string) (vc.VCSandbox, error) {
+	testingImpl.StopSandboxFunc = func(ctx context.Context, sandboxID string, force bool) (vc.VCSandbox, error) {
 		return sandbox, nil
 	}
 

cli/kill.go

@@ -152,7 +152,7 @@ func kill(ctx context.Context, containerID, signal string, all bool) error {
 
 	switch containerType {
 	case vc.PodSandbox:
-		_, err = vci.StopSandbox(ctx, sandboxID)
+		_, err = vci.StopSandbox(ctx, sandboxID, signum == syscall.SIGKILL)
 	case vc.PodContainer:
 		_, err = vci.StopContainer(ctx, sandboxID, containerID)
 	default:

cli/kill_test.go

@@ -29,7 +29,7 @@ var (
 		return &vcmock.Container{}, nil
 	}
 
-	testStopSandboxFuncReturnNil = func(ctx context.Context, sandboxID string) (vc.VCSandbox, error) {
+	testStopSandboxFuncReturnNil = func(ctx context.Context, sandboxID string, force bool) (vc.VCSandbox, error) {
 		return &vcmock.Sandbox{}, nil
 	}
 )

containerd-shim-v2/service.go

@@ -114,6 +114,7 @@ type service struct {
 	containers map[string]*container
 	config     *oci.RuntimeConfig
 	events     chan interface{}
+	monitor    chan error
 
 	cancel func()
 

containerd-shim-v2/start.go

@@ -30,6 +30,12 @@ func startContainer(ctx context.Context, s *service, c *container) error {
 		if err != nil {
 			return err
 		}
+		// Start monitor after starting sandbox
+		s.monitor, err = s.sandbox.Monitor()
+		if err != nil {
+			return err
+		}
+		go watchSandbox(s)
 	} else {
 		_, err := s.sandbox.StartContainer(c.id)
 		if err != nil {

containerd-shim-v2/utils.go

@@ -70,7 +70,7 @@ func cleanupContainer(ctx context.Context, sid, cid, bundlePath string) error {
 	}
 
 	if len(sandbox.GetAllContainers()) == 0 {
-		err = sandbox.Stop()
+		err = sandbox.Stop(true)
 		if err != nil {
 			logrus.WithError(err).WithField("sandbox", sid).Warn("failed to stop sandbox")
 			return err

containerd-shim-v2/wait.go

@@ -6,9 +6,11 @@
 package containerdshim
 
 import (
+	"path"
 	"time"
 
 	"github.com/containerd/containerd/api/types/task"
+	"github.com/containerd/containerd/mount"
 	"github.com/sirupsen/logrus"
 )
 
@@ -51,7 +53,7 @@ func wait(s *service, c *container, execID string) (int32, error) {
 		// sandbox.
 
 		if c.cType.IsSandbox() {
-			if err = s.sandbox.Stop(); err != nil {
+			if err = s.sandbox.Stop(true); err != nil {
 				logrus.WithField("sandbox", s.sandbox.ID()).Error("failed to stop sandbox")
 			}
 
@@ -82,3 +84,41 @@ func wait(s *service, c *container, execID string) (int32, error) {
 
 	return ret, nil
 }
+
+func watchSandbox(s *service) {
+	if s.monitor == nil {
+		return
+	}
+	err := <-s.monitor
+	if err == nil {
+		return
+	}
+	s.monitor = nil
+
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	// sandbox malfunctioning, cleanup as much as we can
+	logrus.WithError(err).Warn("sandbox stopped unexpectedly")
+	err = s.sandbox.Stop(true)
+	if err != nil {
+		logrus.WithError(err).Warn("stop sandbox failed")
+	}
+	err = s.sandbox.Delete()
+	if err != nil {
+		logrus.WithError(err).Warn("delete sandbox failed")
+	}
+
+	if s.mount {
+		for _, c := range s.containers {
+			rootfs := path.Join(c.bundle, "rootfs")
+			logrus.WithField("rootfs", rootfs).WithField("id", c.id).Debug("container umount rootfs")
+			if err := mount.UnmountAll(rootfs, 0); err != nil {
+				logrus.WithError(err).Warn("failed to cleanup rootfs mount")
+			}
+		}
+	}
+	s.containers = make(map[string]*container)
+
+	// Existing container/exec will be cleaned up by its waiters.
+	// No need to send async events here.
+}

virtcontainers/agent.go

@@ -250,6 +250,9 @@ type agent interface {
 	// copyFile copies file from host to container's rootfs
 	copyFile(src, dst string) error
 
+	// markDead tell agent that the guest is dead
+	markDead()
+
 	// cleanup removes all on disk information generated by the agent
-	cleanup(id string)
+	cleanup(s *Sandbox)
 }

virtcontainers/api.go

@@ -235,7 +235,7 @@ func StartSandbox(ctx context.Context, sandboxID string) (VCSandbox, error) {
 
 // StopSandbox is the virtcontainers sandbox stopping entry point.
 // StopSandbox will talk to the given agent to stop an existing sandbox and destroy all containers within that sandbox.
-func StopSandbox(ctx context.Context, sandboxID string) (VCSandbox, error) {
+func StopSandbox(ctx context.Context, sandboxID string, force bool) (VCSandbox, error) {
 	span, ctx := trace(ctx, "StopSandbox")
 	defer span.Finish()
 
@@ -257,7 +257,7 @@ func StopSandbox(ctx context.Context, sandboxID string) (VCSandbox, error) {
 	defer s.releaseStatelessSandbox()
 
 	// Stop it.
-	err = s.Stop()
+	err = s.Stop(force)
 	if err != nil {
 		return nil, err
 	}
@@ -596,7 +596,7 @@ func statusContainer(sandbox *Sandbox, containerID string) (ContainerStatus, err
 						"state": container.state.State,
 						"pid":   container.process.Pid}).
 						Info("container isn't running")
-					if err := container.stop(); err != nil {
+					if err := container.stop(true); err != nil {
 						return ContainerStatus{}, err
 					}
 				}

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)
 
@@ -332,7 +335,7 @@ func TestStopSandboxNoopAgentSuccessful(t *testing.T) {
 	assert.NoError(err)
 	assert.NotNil(p)
 
-	vp, err := StopSandbox(ctx, p.ID())
+	vp, err := StopSandbox(ctx, p.ID(), false)
 	assert.NoError(err)
 	assert.NotNil(vp)
 }
@@ -427,7 +430,7 @@ func TestStopSandboxKataAgentSuccessful(t *testing.T) {
 	assert.NoError(err)
 	assert.NotNil(p)
 
-	p, err = StopSandbox(ctx, p.ID())
+	p, err = StopSandbox(ctx, p.ID(), false)
 	assert.NoError(err)
 	assert.NotNil(p)
 }
@@ -438,7 +441,7 @@ func TestStopSandboxFailing(t *testing.T) {
 	sandboxDir := store.SandboxConfigurationRootPath(testSandboxID)
 	os.Remove(sandboxDir)
 
-	p, err := StopSandbox(context.Background(), testSandboxID)
+	p, err := StopSandbox(context.Background(), testSandboxID, false)
 	assert.Error(t, err)
 	assert.Nil(t, p)
 }
@@ -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)
 
@@ -1461,7 +1473,7 @@ func createStartStopDeleteSandbox(b *testing.B, sandboxConfig SandboxConfig) {
 	}
 
 	// Stop sandbox
-	_, err = StopSandbox(ctx, p.ID())
+	_, err = StopSandbox(ctx, p.ID(), false)
 	if err != nil {
 		b.Fatalf("Could not stop sandbox: %s", err)
 	}

virtcontainers/container.go

@@ -368,16 +368,6 @@ func (c *Container) GetPid() int {
 	return c.process.Pid
 }
 
-// SetPid sets and stores the given pid as the pid of container's process.
-func (c *Container) SetPid(pid int) error {
-	c.process.Pid = pid
-
-	if !c.sandbox.supportNewStore() {
-		return c.storeProcess()
-	}
-	return nil
-}
-
 func (c *Container) setStateFstype(fstype string) error {
 	c.state.Fstype = fstype
 
@@ -956,7 +946,7 @@ func (c *Container) start() error {
 	if err := c.sandbox.agent.startContainer(c.sandbox, c); err != nil {
 		c.Logger().WithError(err).Error("Failed to start container")
 
-		if err := c.stop(); err != nil {
+		if err := c.stop(true); err != nil {
 			c.Logger().WithError(err).Warn("Failed to stop container")
 		}
 		return err
@@ -965,7 +955,7 @@ func (c *Container) start() error {
 	return c.setContainerState(types.StateRunning)
 }
 
-func (c *Container) stop() error {
+func (c *Container) stop(force bool) error {
 	span, _ := c.trace("stop")
 	defer span.Finish()
 
@@ -981,10 +971,6 @@ func (c *Container) stop() error {
 		return nil
 	}
 
-	if c.sandbox.state.State != types.StateReady && c.sandbox.state.State != types.StateRunning {
-		return fmt.Errorf("Sandbox not ready or running, impossible to stop the container")
-	}
-
 	if err := c.state.ValidTransition(c.state.State, types.StateStopped); err != nil {
 		return err
 	}
@@ -1013,7 +999,7 @@ func (c *Container) stop() error {
 	// return an error, but instead try to kill it forcefully.
 	if err := waitForShim(c.process.Pid); err != nil {
 		// Force the container to be killed.
-		if err := c.kill(syscall.SIGKILL, true); err != nil {
+		if err := c.kill(syscall.SIGKILL, true); err != nil && !force {
 			return err
 		}
 
@@ -1021,7 +1007,7 @@ func (c *Container) stop() error {
 		// to succeed. Indeed, we have already given a second chance
 		// to the container by trying to kill it with SIGKILL, there
 		// is no reason to try to go further if we got an error.
-		if err := waitForShim(c.process.Pid); err != nil {
+		if err := waitForShim(c.process.Pid); err != nil && !force {
 			return err
 		}
 	}
@@ -1059,15 +1045,23 @@ func (c *Container) stop() error {
 		}
 	}()
 
-	if err := c.sandbox.agent.stopContainer(c.sandbox, *c); err != nil {
+	if err := c.sandbox.agent.stopContainer(c.sandbox, *c); err != nil && !force {
 		return err
 	}
 
-	if err := c.detachDevices(); err != nil {
+	if err := c.unmountHostMounts(); err != nil && !force {
 		return err
 	}
 
-	if err := c.removeDrive(); err != nil {
+	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
+	}
+
+	if err := c.removeDrive(); err != nil && !force {
 		return err
 	}
 

virtcontainers/documentation/api/1.0/api.md

@@ -773,7 +773,6 @@ type VCContainer interface {
 	ID() string
 	Sandbox() VCSandbox
 	Process() Process
-	SetPid(pid int) error
 }
 ```
 

virtcontainers/implementation.go

@@ -52,8 +52,8 @@ func (impl *VCImpl) StartSandbox(ctx context.Context, sandboxID string) (VCSandb
 }
 
 // StopSandbox implements the VC function of the same name.
-func (impl *VCImpl) StopSandbox(ctx context.Context, sandboxID string) (VCSandbox, error) {
-	return StopSandbox(ctx, sandboxID)
+func (impl *VCImpl) StopSandbox(ctx context.Context, sandboxID string, force bool) (VCSandbox, error) {
+	return StopSandbox(ctx, sandboxID, force)
 }
 
 // RunSandbox implements the VC function of the same name.

virtcontainers/interfaces.go

@@ -32,7 +32,7 @@ type VC interface {
 	RunSandbox(ctx context.Context, sandboxConfig SandboxConfig) (VCSandbox, error)
 	StartSandbox(ctx context.Context, sandboxID string) (VCSandbox, error)
 	StatusSandbox(ctx context.Context, sandboxID string) (SandboxStatus, error)
-	StopSandbox(ctx context.Context, sandboxID string) (VCSandbox, error)
+	StopSandbox(ctx context.Context, sandboxID string, force bool) (VCSandbox, error)
 
 	CreateContainer(ctx context.Context, sandboxID string, containerConfig ContainerConfig) (VCSandbox, VCContainer, error)
 	DeleteContainer(ctx context.Context, sandboxID, containerID string) (VCContainer, error)
@@ -68,7 +68,7 @@ type VCSandbox interface {
 	SetAnnotations(annotations map[string]string) error
 
 	Start() error
-	Stop() error
+	Stop(force bool) error
 	Pause() error
 	Resume() error
 	Release() error
@@ -110,5 +110,4 @@ type VCContainer interface {
 	ID() string
 	Sandbox() VCSandbox
 	Process() Process
-	SetPid(pid int) error
 }

virtcontainers/kata_agent.go

@@ -136,6 +136,7 @@ type kataAgent struct {
 	keepConn       bool
 	proxyBuiltIn   bool
 	dynamicTracing bool
+	dead           bool
 
 	vmSocket interface{}
 	ctx      context.Context
@@ -615,6 +616,7 @@ func (k *kataAgent) startProxy(sandbox *Sandbox) error {
 
 	proxyParams := proxyParams{
 		id:         sandbox.id,
+		hid:        sandbox.hypervisor.pid(),
 		path:       sandbox.config.ProxyConfig.Path,
 		agentURL:   agentURL,
 		consoleURL: consoleURL,
@@ -1417,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 {
@@ -1581,6 +1572,9 @@ func (k *kataAgent) statsContainer(sandbox *Sandbox, c Container) (*ContainerSta
 }
 
 func (k *kataAgent) connect() error {
+	if k.dead {
+		return errors.New("Dead agent")
+	}
 	// lockless quick pass
 	if k.client != nil {
 		return nil
@@ -1596,9 +1590,17 @@ func (k *kataAgent) connect() error {
 		return nil
 	}
 
-	k.Logger().WithField("url", k.state.URL).Info("New client")
+	if k.state.ProxyPid > 0 {
+		// check that proxy is running before talk with it avoiding long timeouts
+		if err := syscall.Kill(k.state.ProxyPid, syscall.Signal(0)); err != nil {
+			return errors.New("Proxy is not running")
+		}
+	}
+
+	k.Logger().WithField("url", k.state.URL).WithField("proxy", k.state.ProxyPid).Info("New client")
 	client, err := kataclient.NewAgentClient(k.ctx, k.state.URL, k.proxyBuiltIn)
 	if err != nil {
+		k.dead = true
 		return err
 	}
 
@@ -1787,13 +1789,6 @@ func (k *kataAgent) sendReq(request interface{}) (interface{}, error) {
 	span.SetTag("request", request)
 	defer span.Finish()
 
-	if k.state.ProxyPid > 0 {
-		// check that proxy is running before talk with it avoiding long timeouts
-		if err := syscall.Kill(k.state.ProxyPid, syscall.Signal(0)); err != nil {
-			return nil, fmt.Errorf("Proxy is not running: %v", err)
-		}
-	}
-
 	if err := k.connect(); err != nil {
 		return nil, err
 	}
@@ -2063,9 +2058,18 @@ func (k *kataAgent) copyFile(src, dst string) error {
 	return nil
 }
 
-func (k *kataAgent) cleanup(id string) {
-	path := k.getSharePath(id)
+func (k *kataAgent) markDead() {
+	k.Logger().Infof("mark agent dead")
+	k.dead = true
+	k.disconnect()
+}
+
+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)
 	}

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))

virtcontainers/kata_builtin_proxy.go

@@ -64,7 +64,7 @@ func (p *kataBuiltInProxy) start(params proxyParams) (int, string, error) {
 		}
 	}
 
-	return -1, params.agentURL, nil
+	return params.hid, params.agentURL, nil
 }
 
 // stop is the proxy stop implementation for kata builtin proxy.

virtcontainers/monitor.go

@@ -7,10 +7,13 @@ package virtcontainers
 
 import (
 	"sync"
+	"syscall"
 	"time"
+
+	"github.com/pkg/errors"
 )
 
-const defaultCheckInterval = 10 * time.Second
+const defaultCheckInterval = 1 * time.Second
 
 type monitor struct {
 	sync.Mutex
@@ -52,6 +55,7 @@ func (m *monitor) newWatcher() (chan error, error) {
 					m.wg.Done()
 					return
 				case <-tick.C:
+					m.watchHypervisor()
 					m.watchAgent()
 				}
 			}
@@ -62,6 +66,8 @@ func (m *monitor) newWatcher() (chan error, error) {
 }
 
 func (m *monitor) notify(err error) {
+	m.sandbox.agent.markDead()
+
 	m.Lock()
 	defer m.Unlock()
 
@@ -115,6 +121,15 @@ func (m *monitor) stop() {
 func (m *monitor) watchAgent() {
 	err := m.sandbox.agent.check()
 	if err != nil {
-		m.notify(err)
+		// TODO: define and export error types
+		m.notify(errors.Wrapf(err, "failed to ping agent"))
 	}
 }
+
+func (m *monitor) watchHypervisor() error {
+	if err := syscall.Kill(m.sandbox.hypervisor.pid(), syscall.Signal(0)); err != nil {
+		m.notify(errors.Wrapf(err, "failed to ping hypervisor process"))
+		return err
+	}
+	return nil
+}

virtcontainers/no_proxy.go

@@ -34,7 +34,7 @@ func (p *noProxy) start(params proxyParams) (int, string, error) {
 		return -1, "", fmt.Errorf("AgentURL cannot be empty")
 	}
 
-	return 0, params.agentURL, nil
+	return params.hid, params.agentURL, nil
 }
 
 // stop is noProxy stop implementation for proxy interface.

virtcontainers/noop_agent.go

@@ -228,5 +228,8 @@ func (n *noopAgent) copyFile(src, dst string) error {
 	return nil
 }
 
-func (n *noopAgent) cleanup(id string) {
+func (n *noopAgent) markDead() {
+}
+
+func (n *noopAgent) cleanup(s *Sandbox) {
 }

virtcontainers/noop_proxy.go

@@ -14,7 +14,7 @@ var noopProxyURL = "noopProxyURL"
 // register is the proxy start implementation for testing purpose.
 // It does nothing.
 func (p *noopProxy) start(params proxyParams) (int, string, error) {
-	return 0, noopProxyURL, nil
+	return params.hid, noopProxyURL, nil
 }
 
 // stop is the proxy stop implementation for testing purpose.

virtcontainers/pkg/vcmock/container.go

@@ -38,12 +38,6 @@ func (c *Container) GetPid() int {
 	return c.MockPid
 }
 
-// SetPid implements the VCContainer function of the same name.
-func (c *Container) SetPid(pid int) error {
-	c.MockPid = pid
-	return nil
-}
-
 // GetAnnotations implements the VCContainer function of the same name.
 func (c *Container) GetAnnotations() map[string]string {
 	return c.MockAnnotations

virtcontainers/pkg/vcmock/mock.go

@@ -84,9 +84,9 @@ func (m *VCMock) StartSandbox(ctx context.Context, sandboxID string) (vc.VCSandb
 }
 
 // StopSandbox implements the VC function of the same name.
-func (m *VCMock) StopSandbox(ctx context.Context, sandboxID string) (vc.VCSandbox, error) {
+func (m *VCMock) StopSandbox(ctx context.Context, sandboxID string, force bool) (vc.VCSandbox, error) {
 	if m.StopSandboxFunc != nil {
-		return m.StopSandboxFunc(ctx, sandboxID)
+		return m.StopSandboxFunc(ctx, sandboxID, force)
 	}
 
 	return nil, fmt.Errorf("%s: %s (%+v): sandboxID: %v", mockErrorPrefix, getSelf(), m, sandboxID)

virtcontainers/pkg/vcmock/mock_test.go

@@ -339,22 +339,22 @@ func TestVCMockStopSandbox(t *testing.T) {
 	assert.Nil(m.StopSandboxFunc)
 
 	ctx := context.Background()
-	_, err := m.StopSandbox(ctx, testSandboxID)
+	_, err := m.StopSandbox(ctx, testSandboxID, false)
 	assert.Error(err)
 	assert.True(IsMockError(err))
 
-	m.StopSandboxFunc = func(ctx context.Context, sandboxID string) (vc.VCSandbox, error) {
+	m.StopSandboxFunc = func(ctx context.Context, sandboxID string, force bool) (vc.VCSandbox, error) {
 		return &Sandbox{}, nil
 	}
 
-	sandbox, err := m.StopSandbox(ctx, testSandboxID)
+	sandbox, err := m.StopSandbox(ctx, testSandboxID, false)
 	assert.NoError(err)
 	assert.Equal(sandbox, &Sandbox{})
 
 	// reset
 	m.StopSandboxFunc = nil
 
-	_, err = m.StopSandbox(ctx, testSandboxID)
+	_, err = m.StopSandbox(ctx, testSandboxID, false)
 	assert.Error(err)
 	assert.True(IsMockError(err))
 }

virtcontainers/pkg/vcmock/sandbox.go

@@ -74,7 +74,7 @@ func (s *Sandbox) Start() error {
 }
 
 // Stop implements the VCSandbox function of the same name.
-func (s *Sandbox) Stop() error {
+func (s *Sandbox) Stop(force bool) error {
 	return nil
 }
 

virtcontainers/pkg/vcmock/types.go

@@ -54,7 +54,7 @@ type VCMock struct {
 	StartSandboxFunc   func(ctx context.Context, sandboxID string) (vc.VCSandbox, error)
 	StatusSandboxFunc  func(ctx context.Context, sandboxID string) (vc.SandboxStatus, error)
 	StatsContainerFunc func(ctx context.Context, sandboxID, containerID string) (vc.ContainerStats, error)
-	StopSandboxFunc    func(ctx context.Context, sandboxID string) (vc.VCSandbox, error)
+	StopSandboxFunc    func(ctx context.Context, sandboxID string, force bool) (vc.VCSandbox, error)
 
 	CreateContainerFunc      func(ctx context.Context, sandboxID string, containerConfig vc.ContainerConfig) (vc.VCSandbox, vc.VCContainer, error)
 	DeleteContainerFunc      func(ctx context.Context, sandboxID, containerID string) (vc.VCContainer, error)

virtcontainers/proxy.go

@@ -28,6 +28,7 @@ type proxyParams struct {
 	agentURL   string
 	consoleURL string
 	logger     *logrus.Entry
+	hid        int
 	debug      bool
 }
 

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()
 }
@@ -1161,7 +1161,7 @@ func (s *Sandbox) StopContainer(containerID string) (VCContainer, error) {
 	}
 
 	// Stop it.
-	if err := c.stop(); err != nil {
+	if err := c.stop(false); err != nil {
 		return nil, err
 	}
 
@@ -1180,14 +1180,15 @@ func (s *Sandbox) KillContainer(containerID string, signal syscall.Signal, all b
 	}
 
 	// Send a signal to the process.
-	if err := c.kill(signal, all); err != nil {
-		return err
+	err = c.kill(signal, all)
+
+	// SIGKILL should never fail otherwise it is
+	// impossible to clean things up.
+	if signal == syscall.SIGKILL {
+		return nil
 	}
 
-	if err = s.storeSandbox(); err != nil {
-		return err
-	}
-	return nil
+	return err
 }
 
 // DeleteContainer deletes a container from the sandbox
@@ -1427,7 +1428,8 @@ func (s *Sandbox) Start() error {
 
 // Stop stops a sandbox. The containers that are making the sandbox
 // will be destroyed.
-func (s *Sandbox) Stop() error {
+// When force is true, ignore guest related stop failures.
+func (s *Sandbox) Stop(force bool) error {
 	span, _ := s.trace("stop")
 	defer span.Finish()
 
@@ -1441,12 +1443,12 @@ func (s *Sandbox) Stop() error {
 	}
 
 	for _, c := range s.containers {
-		if err := c.stop(); err != nil {
+		if err := c.stop(force); err != nil {
 			return err
 		}
 	}
 
-	if err := s.stopVM(); err != nil {
+	if err := s.stopVM(); err != nil && !force {
 		return err
 	}
 
@@ -1455,7 +1457,7 @@ func (s *Sandbox) Stop() error {
 	}
 
 	// Remove the network.
-	if err := s.removeNetwork(); err != nil {
+	if err := s.removeNetwork(); err != nil && !force {
 		return err
 	}
 

virtcontainers/sandbox_test.go

@@ -1350,7 +1350,7 @@ func TestSandboxStopStopped(t *testing.T) {
 		ctx:   context.Background(),
 		state: types.SandboxState{State: types.StateStopped},
 	}
-	err := s.Stop()
+	err := s.Stop(false)
 
 	assert.Nil(t, err)
 }