Merge pull request #328 from devimc/fix/pauseRemove

Fix pause-remove container

cli/kill.go

@@ -103,9 +103,9 @@ func kill(containerID, signal string, all bool) error {
 		return err
 	}
 
-	// container MUST be created or running
-	if status.State.State != vc.StateReady && status.State.State != vc.StateRunning {
-		return fmt.Errorf("Container %s not ready or running, cannot send a signal", containerID)
+	// container MUST be created, running or paused
+	if status.State.State != vc.StateReady && status.State.State != vc.StateRunning && status.State.State != vc.StatePaused {
+		return fmt.Errorf("Container %s not ready, running or paused, cannot send a signal", containerID)
 	}
 
 	if err := vci.KillContainer(sandboxID, containerID, signum, all); err != nil {

cli/kill_test.go

@@ -312,7 +312,7 @@ func TestKillCLIFunctionInvalidSignalFailure(t *testing.T) {
 	execCLICommandFunc(assert, killCLICommand, set, true)
 }
 
-func TestKillCLIFunctionInvalidStatePausedFailure(t *testing.T) {
+func TestKillCLIFunctionStatePausedSuccessful(t *testing.T) {
 	assert := assert.New(t)
 
 	state := vc.State{
@@ -320,24 +320,27 @@ func TestKillCLIFunctionInvalidStatePausedFailure(t *testing.T) {
 	}
 
 	testingImpl.KillContainerFunc = testKillContainerFuncReturnNil
+	testingImpl.StopContainerFunc = testStopContainerFuncReturnNil
 
 	path, err := createTempContainerIDMapping(testContainerID, testSandboxID)
 	assert.NoError(err)
 	defer os.RemoveAll(path)
 
 	testingImpl.StatusContainerFunc = func(sandboxID, containerID string) (vc.ContainerStatus, error) {
-		return newSingleContainerStatus(testContainerID, state, map[string]string{}), nil
+		return newSingleContainerStatus(testContainerID, state,
+			map[string]string{string(vcAnnotations.ContainerTypeKey): string(vc.PodContainer)}), nil
 	}
 
 	defer func() {
 		testingImpl.KillContainerFunc = nil
 		testingImpl.StatusContainerFunc = nil
+		testingImpl.StopContainerFunc = nil
 	}()
 
 	set := flag.NewFlagSet("", 0)
 	set.Parse([]string{testContainerID})
 
-	execCLICommandFunc(assert, killCLICommand, set, true)
+	execCLICommandFunc(assert, killCLICommand, set, false)
 }
 
 func TestKillCLIFunctionInvalidStateStoppedFailure(t *testing.T) {

cli/pause.go

@@ -42,15 +42,17 @@ Where "<container-id>" is the container name to be resumed.`,
 
 func toggleContainerPause(containerID string, pause bool) (err error) {
 	// Checks the MUST and MUST NOT from OCI runtime specification
-	_, sandboxID, err := getExistingContainerInfo(containerID)
+	status, sandboxID, err := getExistingContainerInfo(containerID)
 	if err != nil {
 		return err
 	}
 
+	containerID = status.ID
+
 	if pause {
-		_, err = vci.PauseSandbox(sandboxID)
+		err = vci.PauseContainer(sandboxID, containerID)
 	} else {
-		_, err = vci.ResumeSandbox(sandboxID)
+		err = vci.ResumeContainer(sandboxID, containerID)
 	}
 
 	return err

cli/pause_test.go

@@ -12,17 +12,16 @@ import (
 	"testing"
 
 	vc "github.com/kata-containers/runtime/virtcontainers"
-	"github.com/kata-containers/runtime/virtcontainers/pkg/vcmock"
 	"github.com/stretchr/testify/assert"
 )
 
 var (
-	testPauseSandboxFuncReturnNil = func(sandboxID string) (vc.VCSandbox, error) {
-		return &vcmock.Sandbox{}, nil
+	testPauseContainerFuncReturnNil = func(sandboxID, containerID string) error {
+		return nil
 	}
 
-	testResumeSandboxFuncReturnNil = func(sandboxID string) (vc.VCSandbox, error) {
-		return &vcmock.Sandbox{}, nil
+	testResumeContainerFuncReturnNil = func(sandboxID, containerID string) error {
+		return nil
 	}
 )
 
@@ -33,7 +32,7 @@ func TestPauseCLIFunctionSuccessful(t *testing.T) {
 		State: vc.StateRunning,
 	}
 
-	testingImpl.PauseSandboxFunc = testPauseSandboxFuncReturnNil
+	testingImpl.PauseContainerFunc = testPauseContainerFuncReturnNil
 
 	path, err := createTempContainerIDMapping(testContainerID, testSandboxID)
 	assert.NoError(err)
@@ -44,7 +43,7 @@ func TestPauseCLIFunctionSuccessful(t *testing.T) {
 	}
 
 	defer func() {
-		testingImpl.PauseSandboxFunc = nil
+		testingImpl.PauseContainerFunc = nil
 		testingImpl.StatusContainerFunc = nil
 	}()
 
@@ -57,7 +56,7 @@ func TestPauseCLIFunctionSuccessful(t *testing.T) {
 func TestPauseCLIFunctionContainerNotExistFailure(t *testing.T) {
 	assert := assert.New(t)
 
-	testingImpl.PauseSandboxFunc = testPauseSandboxFuncReturnNil
+	testingImpl.PauseContainerFunc = testPauseContainerFuncReturnNil
 
 	path, err := ioutil.TempDir("", "containers-mapping")
 	assert.NoError(err)
@@ -65,7 +64,7 @@ func TestPauseCLIFunctionContainerNotExistFailure(t *testing.T) {
 	ctrsMapTreePath = path
 
 	defer func() {
-		testingImpl.PauseSandboxFunc = nil
+		testingImpl.PauseContainerFunc = nil
 	}()
 
 	set := flag.NewFlagSet("", 0)
@@ -74,7 +73,7 @@ func TestPauseCLIFunctionContainerNotExistFailure(t *testing.T) {
 	execCLICommandFunc(assert, pauseCLICommand, set, true)
 }
 
-func TestPauseCLIFunctionPauseSandboxFailure(t *testing.T) {
+func TestPauseCLIFunctionPauseContainerFailure(t *testing.T) {
 	assert := assert.New(t)
 
 	state := vc.State{
@@ -106,7 +105,7 @@ func TestResumeCLIFunctionSuccessful(t *testing.T) {
 		State: vc.StateRunning,
 	}
 
-	testingImpl.ResumeSandboxFunc = testResumeSandboxFuncReturnNil
+	testingImpl.ResumeContainerFunc = testResumeContainerFuncReturnNil
 
 	path, err := createTempContainerIDMapping(testContainerID, testSandboxID)
 	assert.NoError(err)
@@ -117,7 +116,7 @@ func TestResumeCLIFunctionSuccessful(t *testing.T) {
 	}
 
 	defer func() {
-		testingImpl.ResumeSandboxFunc = nil
+		testingImpl.ResumeContainerFunc = nil
 		testingImpl.StatusContainerFunc = nil
 	}()
 
@@ -130,14 +129,14 @@ func TestResumeCLIFunctionSuccessful(t *testing.T) {
 func TestResumeCLIFunctionContainerNotExistFailure(t *testing.T) {
 	assert := assert.New(t)
 
-	testingImpl.ResumeSandboxFunc = testResumeSandboxFuncReturnNil
+	testingImpl.ResumeContainerFunc = testResumeContainerFuncReturnNil
 
 	path, err := createTempContainerIDMapping(testContainerID, testSandboxID)
 	assert.NoError(err)
 	defer os.RemoveAll(path)
 
 	defer func() {
-		testingImpl.ResumeSandboxFunc = nil
+		testingImpl.ResumeContainerFunc = nil
 	}()
 
 	set := flag.NewFlagSet("", 0)
@@ -146,7 +145,7 @@ func TestResumeCLIFunctionContainerNotExistFailure(t *testing.T) {
 	execCLICommandFunc(assert, resumeCLICommand, set, true)
 }
 
-func TestResumeCLIFunctionPauseSandboxFailure(t *testing.T) {
+func TestResumeCLIFunctionPauseContainerFailure(t *testing.T) {
 	assert := assert.New(t)
 
 	state := vc.State{

virtcontainers/agent.go

@@ -197,4 +197,10 @@ type agent interface {
 
 	// statsContainer will tell the agent to get stats from a container related to a Sandbox
 	statsContainer(sandbox *Sandbox, c Container) (*ContainerStats, error)
+
+	// pauseContainer will pause a container
+	pauseContainer(sandbox *Sandbox, c Container) error
+
+	// resumeContainer will resume a paused container
+	resumeContainer(sandbox *Sandbox, c Container) error
 }

virtcontainers/api.go

@@ -653,3 +653,46 @@ func StatsContainer(sandboxID, containerID string) (ContainerStats, error) {
 
 	return s.StatsContainer(containerID)
 }
+
+func togglePauseContainer(sandboxID, containerID string, pause bool) error {
+	if sandboxID == "" {
+		return errNeedSandboxID
+	}
+
+	if containerID == "" {
+		return errNeedContainerID
+	}
+
+	lockFile, err := rwLockSandbox(sandboxID)
+	if err != nil {
+		return err
+	}
+	defer unlockSandbox(lockFile)
+
+	s, err := fetchSandbox(sandboxID)
+	if err != nil {
+		return err
+	}
+
+	// Fetch the container.
+	c, err := s.findContainer(containerID)
+	if err != nil {
+		return err
+	}
+
+	if pause {
+		return c.pause()
+	}
+
+	return c.resume()
+}
+
+// PauseContainer is the virtcontainers container pause entry point.
+func PauseContainer(sandboxID, containerID string) error {
+	return togglePauseContainer(sandboxID, containerID, true)
+}
+
+// ResumeContainer is the virtcontainers container resume entry point.
+func ResumeContainer(sandboxID, containerID string) error {
+	return togglePauseContainer(sandboxID, containerID, false)
+}

virtcontainers/api_test.go

@@ -2400,3 +2400,43 @@ func TestUpdateContainer(t *testing.T) {
 	err = UpdateContainer(s.ID(), contID, resources)
 	assert.NoError(err)
 }
+
+func TestPauseResumeContainer(t *testing.T) {
+	if os.Geteuid() != 0 {
+		t.Skip(testDisabledAsNonRoot)
+	}
+
+	cleanUp()
+
+	assert := assert.New(t)
+	err := PauseContainer("", "")
+	assert.Error(err)
+
+	err = PauseContainer("abc", "")
+	assert.Error(err)
+
+	contID := "100"
+	config := newTestSandboxConfigNoop()
+
+	s, sandboxDir, err := createAndStartSandbox(config)
+	assert.NoError(err)
+	assert.NotNil(s)
+
+	contConfig := newTestContainerConfigNoop(contID)
+	_, c, err := CreateContainer(s.ID(), contConfig)
+	assert.NoError(err)
+	assert.NotNil(c)
+
+	contDir := filepath.Join(sandboxDir, contID)
+	_, err = os.Stat(contDir)
+	assert.NoError(err)
+
+	_, err = StartContainer(s.ID(), contID)
+	assert.NoError(err)
+
+	err = PauseContainer(s.ID(), contID)
+	assert.NoError(err)
+
+	err = ResumeContainer(s.ID(), contID)
+	assert.NoError(err)
+}

virtcontainers/container.go

@@ -868,8 +868,8 @@ func (c *Container) signalProcess(processID string, signal syscall.Signal, all b
 		return fmt.Errorf("Sandbox not ready or running, impossible to signal the container")
 	}
 
-	if c.state.State != StateReady && c.state.State != StateRunning {
-		return fmt.Errorf("Container not ready or running, impossible to signal the container")
+	if c.state.State != StateReady && c.state.State != StateRunning && c.state.State != StatePaused {
+		return fmt.Errorf("Container not ready, running or paused, impossible to signal the container")
 	}
 
 	return c.sandbox.agent.signalProcess(c, processID, signal, all)
@@ -938,6 +938,38 @@ func (c *Container) update(resources specs.LinuxResources) error {
 	return c.sandbox.agent.updateContainer(c.sandbox, *c, resources)
 }
 
+func (c *Container) pause() error {
+	if err := c.checkSandboxRunning("pause"); err != nil {
+		return err
+	}
+
+	if c.state.State != StateRunning && c.state.State != StateReady {
+		return fmt.Errorf("Container not running or ready, impossible to pause")
+	}
+
+	if err := c.sandbox.agent.pauseContainer(c.sandbox, *c); err != nil {
+		return err
+	}
+
+	return c.setContainerState(StatePaused)
+}
+
+func (c *Container) resume() error {
+	if err := c.checkSandboxRunning("resume"); err != nil {
+		return err
+	}
+
+	if c.state.State != StatePaused {
+		return fmt.Errorf("Container not paused, impossible to resume")
+	}
+
+	if err := c.sandbox.agent.resumeContainer(c.sandbox, *c); err != nil {
+		return err
+	}
+
+	return c.setContainerState(StateRunning)
+}
+
 func (c *Container) hotplugDrive() error {
 	dev, err := getDeviceForPath(c.rootFs)
 

virtcontainers/container_test.go

@@ -418,11 +418,6 @@ func TestKillContainerErrorState(t *testing.T) {
 	err := c.kill(syscall.SIGKILL, true)
 	assert.Error(err)
 
-	// Container paused
-	c.state.State = StatePaused
-	err = c.kill(syscall.SIGKILL, false)
-	assert.Error(err)
-
 	// Container stopped
 	c.state.State = StateStopped
 	err = c.kill(syscall.SIGKILL, true)

virtcontainers/hyperstart_agent.go

@@ -844,3 +844,13 @@ func (h *hyper) readProcessStderr(c *Container, processID string, data []byte) (
 	// hyperstart-agent does not support stderr read request
 	return 0, nil
 }
+
+func (h *hyper) pauseContainer(sandbox *Sandbox, c Container) error {
+	// hyperstart-agent does not support pause container
+	return nil
+}
+
+func (h *hyper) resumeContainer(sandbox *Sandbox, c Container) error {
+	// hyperstart-agent does not support resume container
+	return nil
+}

virtcontainers/implementation.go

@@ -125,3 +125,13 @@ func (impl *VCImpl) ProcessListContainer(sandboxID, containerID string, options
 func (impl *VCImpl) UpdateContainer(sandboxID, containerID string, resources specs.LinuxResources) error {
 	return UpdateContainer(sandboxID, containerID, resources)
 }
+
+// PauseContainer implements the VC function of the same name.
+func (impl *VCImpl) PauseContainer(sandboxID, containerID string) error {
+	return PauseContainer(sandboxID, containerID)
+}
+
+// ResumeContainer implements the VC function of the same name.
+func (impl *VCImpl) ResumeContainer(sandboxID, containerID string) error {
+	return ResumeContainer(sandboxID, containerID)
+}

virtcontainers/interfaces.go

@@ -38,6 +38,8 @@ type VC interface {
 	StopContainer(sandboxID, containerID string) (VCContainer, error)
 	ProcessListContainer(sandboxID, containerID string, options ProcessListOptions) (ProcessList, error)
 	UpdateContainer(sandboxID, containerID string, resources specs.LinuxResources) error
+	PauseContainer(sandboxID, containerID string) error
+	ResumeContainer(sandboxID, containerID string) error
 }
 
 // VCSandbox is the Sandbox interface

virtcontainers/kata_agent.go

@@ -982,6 +982,24 @@ func (k *kataAgent) updateContainer(sandbox *Sandbox, c Container, resources spe
 	return err
 }
 
+func (k *kataAgent) pauseContainer(sandbox *Sandbox, c Container) error {
+	req := &grpc.PauseContainerRequest{
+		ContainerId: c.id,
+	}
+
+	_, err := k.sendReq(req)
+	return err
+}
+
+func (k *kataAgent) resumeContainer(sandbox *Sandbox, c Container) error {
+	req := &grpc.ResumeContainerRequest{
+		ContainerId: c.id,
+	}
+
+	_, err := k.sendReq(req)
+	return err
+}
+
 func (k *kataAgent) onlineCPUMem(cpus uint32) error {
 	req := &grpc.OnlineCPUMemRequest{
 		Wait:   false,
@@ -1155,6 +1173,12 @@ func (k *kataAgent) installReqFunc(c *kataclient.AgentClient) {
 	k.reqHandlers["grpc.StatsContainerRequest"] = func(ctx context.Context, req interface{}, opts ...golangGrpc.CallOption) (interface{}, error) {
 		return k.client.StatsContainer(ctx, req.(*grpc.StatsContainerRequest), opts...)
 	}
+	k.reqHandlers["grpc.PauseContainerRequest"] = func(ctx context.Context, req interface{}, opts ...golangGrpc.CallOption) (interface{}, error) {
+		return k.client.PauseContainer(ctx, req.(*grpc.PauseContainerRequest), opts...)
+	}
+	k.reqHandlers["grpc.ResumeContainerRequest"] = func(ctx context.Context, req interface{}, opts ...golangGrpc.CallOption) (interface{}, error) {
+		return k.client.ResumeContainer(ctx, req.(*grpc.ResumeContainerRequest), opts...)
+	}
 }
 
 func (k *kataAgent) sendReq(request interface{}) (interface{}, error) {

virtcontainers/noop_agent.go

@@ -125,3 +125,13 @@ func (n *noopAgent) readProcessStdout(c *Container, processID string, data []byt
 func (n *noopAgent) readProcessStderr(c *Container, processID string, data []byte) (int, error) {
 	return 0, nil
 }
+
+// pauseContainer is the Noop agent Container pause implementation. It does nothing.
+func (n *noopAgent) pauseContainer(sandbox *Sandbox, c Container) error {
+	return nil
+}
+
+// resumeContainer is the Noop agent Container resume implementation. It does nothing.
+func (n *noopAgent) resumeContainer(sandbox *Sandbox, c Container) error {
+	return nil
+}

virtcontainers/noop_agent_test.go

@@ -130,3 +130,29 @@ func TestNoopAgentStatsContainer(t *testing.T) {
 		t.Fatal(err)
 	}
 }
+
+func TestNoopAgentPauseContainer(t *testing.T) {
+	n := &noopAgent{}
+	sandbox, container, err := testCreateNoopContainer()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer cleanUp()
+	err = n.pauseContainer(sandbox, *container)
+	if err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestNoopAgentResumeContainer(t *testing.T) {
+	n := &noopAgent{}
+	sandbox, container, err := testCreateNoopContainer()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer cleanUp()
+	err = n.resumeContainer(sandbox, *container)
+	if err != nil {
+		t.Fatal(err)
+	}
+}

virtcontainers/pkg/oci/utils.go

@@ -65,6 +65,9 @@ const (
 
 	// StateStopped represents a container that has been stopped.
 	StateStopped = "stopped"
+
+	// StatePaused represents a container that has been paused.
+	StatePaused = "paused"
 )
 
 // CompatOCIProcess is a structure inheriting from spec.Process defined
@@ -612,6 +615,8 @@ func StateToOCIState(state vc.State) string {
 		return StateRunning
 	case vc.StateStopped:
 		return StateStopped
+	case vc.StatePaused:
+		return StatePaused
 	default:
 		return ""
 	}

virtcontainers/pkg/oci/utils_test.go

@@ -437,6 +437,11 @@ func TestStateToOCIState(t *testing.T) {
 	if ociState := StateToOCIState(state); ociState != "stopped" {
 		t.Fatalf("Expecting \"created\" state, got \"%s\"", ociState)
 	}
+
+	state.State = vc.StatePaused
+	if ociState := StateToOCIState(state); ociState != "paused" {
+		t.Fatalf("Expecting \"paused\" state, got \"%s\"", ociState)
+	}
 }
 
 func TestEnvVars(t *testing.T) {

virtcontainers/pkg/vcmock/mock.go

@@ -214,3 +214,21 @@ func (m *VCMock) UpdateContainer(sandboxID, containerID string, resources specs.
 
 	return fmt.Errorf("%s: %s (%+v): sandboxID: %v, containerID: %v", mockErrorPrefix, getSelf(), m, sandboxID, containerID)
 }
+
+// PauseContainer implements the VC function of the same name.
+func (m *VCMock) PauseContainer(sandboxID, containerID string) error {
+	if m.PauseContainerFunc != nil {
+		return m.PauseContainerFunc(sandboxID, containerID)
+	}
+
+	return fmt.Errorf("%s: %s (%+v): sandboxID: %v, containerID: %v", mockErrorPrefix, getSelf(), m, sandboxID, containerID)
+}
+
+// ResumeContainer implements the VC function of the same name.
+func (m *VCMock) ResumeContainer(sandboxID, containerID string) error {
+	if m.ResumeContainerFunc != nil {
+		return m.ResumeContainerFunc(sandboxID, containerID)
+	}
+
+	return fmt.Errorf("%s: %s (%+v): sandboxID: %v, containerID: %v", mockErrorPrefix, getSelf(), m, sandboxID, containerID)
+}

virtcontainers/pkg/vcmock/mock_test.go

@@ -623,3 +623,55 @@ func TestVCMockFetchSandbox(t *testing.T) {
 	assert.True(IsMockError(err))
 
 }
+
+func TestVCMockPauseContainer(t *testing.T) {
+	assert := assert.New(t)
+
+	m := &VCMock{}
+	config := &vc.SandboxConfig{}
+	assert.Nil(m.PauseContainerFunc)
+
+	err := m.PauseContainer(config.ID, config.ID)
+	assert.Error(err)
+	assert.True(IsMockError(err))
+
+	m.PauseContainerFunc = func(sid, cid string) error {
+		return nil
+	}
+
+	err = m.PauseContainer(config.ID, config.ID)
+	assert.NoError(err)
+
+	// reset
+	m.PauseContainerFunc = nil
+
+	err = m.PauseContainer(config.ID, config.ID)
+	assert.Error(err)
+	assert.True(IsMockError(err))
+}
+
+func TestVCMockResumeContainer(t *testing.T) {
+	assert := assert.New(t)
+
+	m := &VCMock{}
+	config := &vc.SandboxConfig{}
+	assert.Nil(m.ResumeContainerFunc)
+
+	err := m.ResumeContainer(config.ID, config.ID)
+	assert.Error(err)
+	assert.True(IsMockError(err))
+
+	m.ResumeContainerFunc = func(sid, cid string) error {
+		return nil
+	}
+
+	err = m.ResumeContainer(config.ID, config.ID)
+	assert.NoError(err)
+
+	// reset
+	m.ResumeContainerFunc = nil
+
+	err = m.ResumeContainer(config.ID, config.ID)
+	assert.Error(err)
+	assert.True(IsMockError(err))
+}

virtcontainers/pkg/vcmock/types.go

@@ -58,4 +58,6 @@ type VCMock struct {
 	StopContainerFunc        func(sandboxID, containerID string) (vc.VCContainer, error)
 	ProcessListContainerFunc func(sandboxID, containerID string, options vc.ProcessListOptions) (vc.ProcessList, error)
 	UpdateContainerFunc      func(sandboxID, containerID string, resources specs.LinuxResources) error
+	PauseContainerFunc       func(sandboxID, containerID string) error
+	ResumeContainerFunc      func(sandboxID, containerID string) error
 }