diff --git a/cli/delete.go b/cli/delete.go index 19dc4fb7a2..c2ce52a468 100644 --- a/cli/delete.go +++ b/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 } } diff --git a/cli/delete_test.go b/cli/delete_test.go index 97768e1b44..77ded07f5e 100644 --- a/cli/delete_test.go +++ b/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 } diff --git a/cli/kill.go b/cli/kill.go index 4c93ecf02f..60fa41e09e 100644 --- a/cli/kill.go +++ b/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: diff --git a/cli/kill_test.go b/cli/kill_test.go index cd8910cf99..a80bc2673d 100644 --- a/cli/kill_test.go +++ b/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 } ) diff --git a/containerd-shim-v2/service.go b/containerd-shim-v2/service.go index 9103885080..af3dd47da5 100644 --- a/containerd-shim-v2/service.go +++ b/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() diff --git a/containerd-shim-v2/start.go b/containerd-shim-v2/start.go index 71a90ed131..173ca7c769 100644 --- a/containerd-shim-v2/start.go +++ b/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 { diff --git a/containerd-shim-v2/utils.go b/containerd-shim-v2/utils.go index b704b17454..c9c7a15b5e 100644 --- a/containerd-shim-v2/utils.go +++ b/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 diff --git a/containerd-shim-v2/wait.go b/containerd-shim-v2/wait.go index d36408dd5b..f99bca47f2 100644 --- a/containerd-shim-v2/wait.go +++ b/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. +} diff --git a/virtcontainers/agent.go b/virtcontainers/agent.go index bd4dd90676..050de36aa9 100644 --- a/virtcontainers/agent.go +++ b/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) } diff --git a/virtcontainers/api.go b/virtcontainers/api.go index 8605b978f4..76c2a96190 100644 --- a/virtcontainers/api.go +++ b/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 } } diff --git a/virtcontainers/api_test.go b/virtcontainers/api_test.go index 371900800d..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) @@ -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) } diff --git a/virtcontainers/container.go b/virtcontainers/container.go index 4ae6754333..fd52493fb2 100644 --- a/virtcontainers/container.go +++ b/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 } diff --git a/virtcontainers/documentation/api/1.0/api.md b/virtcontainers/documentation/api/1.0/api.md index c0051e56a9..4753f5c9d1 100644 --- a/virtcontainers/documentation/api/1.0/api.md +++ b/virtcontainers/documentation/api/1.0/api.md @@ -773,7 +773,6 @@ type VCContainer interface { ID() string Sandbox() VCSandbox Process() Process - SetPid(pid int) error } ``` diff --git a/virtcontainers/implementation.go b/virtcontainers/implementation.go index 865a84271d..fd91535444 100644 --- a/virtcontainers/implementation.go +++ b/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. diff --git a/virtcontainers/interfaces.go b/virtcontainers/interfaces.go index d62389d7f0..38dfc5f4ed 100644 --- a/virtcontainers/interfaces.go +++ b/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 } diff --git a/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go index d11f65c217..6e1579ee24 100644 --- a/virtcontainers/kata_agent.go +++ b/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) } 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/kata_builtin_proxy.go b/virtcontainers/kata_builtin_proxy.go index 8ac90d84de..a3717dfd32 100644 --- a/virtcontainers/kata_builtin_proxy.go +++ b/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. diff --git a/virtcontainers/monitor.go b/virtcontainers/monitor.go index 8e2a879746..25c55f4250 100644 --- a/virtcontainers/monitor.go +++ b/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 +} diff --git a/virtcontainers/no_proxy.go b/virtcontainers/no_proxy.go index a97270c3d2..8fb54d9f2d 100644 --- a/virtcontainers/no_proxy.go +++ b/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. diff --git a/virtcontainers/noop_agent.go b/virtcontainers/noop_agent.go index 592634a70e..307caa7f2a 100644 --- a/virtcontainers/noop_agent.go +++ b/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) { } diff --git a/virtcontainers/noop_proxy.go b/virtcontainers/noop_proxy.go index 5a80f7b60d..d72f71e574 100644 --- a/virtcontainers/noop_proxy.go +++ b/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. diff --git a/virtcontainers/pkg/vcmock/container.go b/virtcontainers/pkg/vcmock/container.go index d93c271811..63fcc8a02b 100644 --- a/virtcontainers/pkg/vcmock/container.go +++ b/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 diff --git a/virtcontainers/pkg/vcmock/mock.go b/virtcontainers/pkg/vcmock/mock.go index b2455850fb..4b60575b9e 100644 --- a/virtcontainers/pkg/vcmock/mock.go +++ b/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) diff --git a/virtcontainers/pkg/vcmock/mock_test.go b/virtcontainers/pkg/vcmock/mock_test.go index d369e6574a..dc94d740a3 100644 --- a/virtcontainers/pkg/vcmock/mock_test.go +++ b/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)) } diff --git a/virtcontainers/pkg/vcmock/sandbox.go b/virtcontainers/pkg/vcmock/sandbox.go index 2c00d23f82..3d770db8fd 100644 --- a/virtcontainers/pkg/vcmock/sandbox.go +++ b/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 } diff --git a/virtcontainers/pkg/vcmock/types.go b/virtcontainers/pkg/vcmock/types.go index 6f7df5a3d8..9a65389cd3 100644 --- a/virtcontainers/pkg/vcmock/types.go +++ b/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) diff --git a/virtcontainers/proxy.go b/virtcontainers/proxy.go index 46c266d30a..8c0ec4a49c 100644 --- a/virtcontainers/proxy.go +++ b/virtcontainers/proxy.go @@ -28,6 +28,7 @@ type proxyParams struct { agentURL string consoleURL string logger *logrus.Entry + hid int debug bool } diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index 0d43a279d7..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() } @@ -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 } diff --git a/virtcontainers/sandbox_test.go b/virtcontainers/sandbox_test.go index df731dcc9b..2450900023 100644 --- a/virtcontainers/sandbox_test.go +++ b/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) }