test: add missing integration test for PouchContainer

Signed-off-by: Allen Sun <[email protected]>

Author: allencloud

apis/server/exec_bridge.go

@@ -9,6 +9,7 @@ import (
 	"strconv"
 
 	"github.com/alibaba/pouch/apis/types"
+	"github.com/alibaba/pouch/pkg/errtypes"
 	"github.com/alibaba/pouch/pkg/httputils"
 	"github.com/alibaba/pouch/pkg/streams"
 
@@ -102,6 +103,9 @@ func (s *Server) startContainerExec(ctx context.Context, rw http.ResponseWriter,
 	}
 
 	if err := s.ContainerMgr.StartExec(ctx, name, attach); err != nil {
+		if errtypes.IsConflict(err) {
+			return err
+		}
 		if config.Detach {
 			return err
 		}

apis/server/router.go

@@ -241,7 +241,7 @@ func HandleErrorResponse(w http.ResponseWriter, err error) {
 		code = http.StatusNotFound
 	} else if errtypes.IsInvalidParam(err) {
 		code = http.StatusBadRequest
-	} else if errtypes.IsAlreadyExisted(err) {
+	} else if errtypes.IsAlreadyExisted(err) || errtypes.IsConflict(err) {
 		code = http.StatusConflict
 	} else if errtypes.IsNotModified(err) {
 		code = http.StatusNotModified

daemon/mgr/container.go

@@ -1656,7 +1656,7 @@ func (mgr *ContainerManager) attachCRILog(c *Container, logPath string) error {
 
 func (mgr *ContainerManager) initExecIO(id string, withStdin bool) (*containerio.IO, error) {
 	if io := mgr.IOs.Get(id); io != nil {
-		return nil, errors.Wrap(errtypes.ErrConflict, "failed to create containerIO")
+		return nil, errors.Wrap(errtypes.ErrConflict, "failed to create execIO")
 	}
 
 	cntrio := containerio.NewIO(id, withStdin)

pkg/errtypes/errors.go

@@ -102,6 +102,11 @@ func IsPreCheckFailed(err error) bool {
 	return checkError(err, codePreCheckFailed)
 }
 
+// IsConflict checks if the error is conflict.
+func IsConflict(err error) bool {
+	return checkError(err, codeConflict)
+}
+
 func checkError(err error, code int) bool {
 	err = causeError(err)
 

test/api_container_create_test.go

@@ -215,13 +215,29 @@ func testCreateContainerWithBadParam(c *check.C, cname string, obj map[string]in
 }
 
 // TestCreateWithBadStopTimeout using bad stopTimeout to create container.
-func (suite *APIContainerCreateSuite) TestCreateWithBadStopTimeout(c *check.C) {
+func (suite *APIContainerCreateSuite) testCreateWithBadParams(c *check.C) {
 	testCreateContainerWithBadParam(c,
 		"TestCreateWithBadStopTimeout",
 		map[string]interface{}{
 			"Image":       busyboxImage,
 			"StopTimeout": -1,
 		})
+
+	// too long length
+	testCreateContainerWithBadParam(c,
+		"1234567890-1234567890-1234567890-1234567890123456789",
+		map[string]interface{}{
+			"Image": busyboxImage,
+		})
+
+	// invalid characters
+	testCreateContainerWithBadParam(c,
+		"??????><!@#$%^&*",
+		map[string]interface{}{
+			"Image": busyboxImage,
+		})
+
+	// TODO: add more container creation option check
 }
 
 func (suite *APIContainerCreateSuite) TestCreateNvidiaConfig(c *check.C) {

test/api_container_exec_inspect_test.go

@@ -3,9 +3,7 @@ package main
 import (
 	"time"
 
-	"github.com/alibaba/pouch/apis/types"
 	"github.com/alibaba/pouch/test/environment"
-	"github.com/alibaba/pouch/test/request"
 
 	"github.com/go-check/check"
 )
@@ -26,70 +24,38 @@ func (suite *APIContainerExecInspectSuite) SetUpTest(c *check.C) {
 
 // TestContainerCreateExecOk tests execing containers is OK.
 func (suite *APIContainerExecInspectSuite) TestContainerExecInspectOk(c *check.C) {
-	c.Skip("skip flaky test due to issue#1372")
 	cname := "TestContainerExecInspectOk"
 
 	CreateBusyboxContainerOk(c, cname)
+	defer DelContainerForceMultyTime(c, cname)
 
 	StartContainerOk(c, cname)
 
-	obj := map[string]interface{}{
-		"Cmd":    []string{"sleep", "9"},
-		"Detach": true,
-	}
-	body := request.WithJSONBody(obj)
-	resp, err := request.Post("/containers/"+cname+"/exec", body)
-	c.Assert(err, check.IsNil)
-	CheckRespStatus(c, resp, 201)
-
-	var execCreateResp types.ExecCreateResp
-	err = request.DecodeBody(&execCreateResp, resp.Body)
-	c.Assert(err, check.IsNil)
-
-	execid := execCreateResp.ID
-
-	// inspect the exec before exec start
-	resp, err = request.Get("/exec/" + execid + "/json")
-	c.Assert(err, check.IsNil)
-	CheckRespStatus(c, resp, 200)
-
-	var execInspect01 types.ContainerExecInspect
-	request.DecodeBody(&execInspect01, resp.Body)
+	// create an exec and get the execID
+	// and inspect the exec before exec start
 
-	c.Assert(execInspect01.Running, check.Equals, false)
-	c.Assert(execInspect01.ExitCode, check.Equals, int64(0))
-
-	// start the exec
+	execid := CreateExecCmdOk(c, cname, "sleep", "9")
 	{
-		resp, conn, _, err := StartContainerExec(c, execid, false, false)
-		c.Assert(err, check.IsNil)
-		CheckRespStatus(c, resp, 101)
-		c.Assert(conn.Close(), check.IsNil)
+		execInspectResp := InspectExecOk(execid)
+		c.Assert(execInspectResp.Running, check.Equals, false)
+		c.Assert(execInspectResp.ExitCode, check.Equals, int64(0))
 	}
 
-	// inspect the exec after exec start
-	resp, err = request.Get("/exec/" + execid + "/json")
-	c.Assert(err, check.IsNil)
-	CheckRespStatus(c, resp, 200)
-
-	var execInspect02 types.ContainerExecInspect
-	request.DecodeBody(&execInspect02, resp.Body)
+	// start the exec
+	StartContainerExecOk(c, execid)
 
-	c.Assert(execInspect02.Running, check.Equals, true)
-	c.Assert(execInspect02.ExitCode, check.Equals, int64(0))
+	// inspect the exec after exec start
+	{
+		execInspectResp = InspectExecOk(execid)
+		c.Assert(execInspectResp.Running, check.Equals, true)
+		c.Assert(execInspectResp.ExitCode, check.Equals, int64(0))
+	}
 
 	// sleep 10s to wait the process exit
-	time.Sleep(10 * time.Second)
-
-	resp, err = request.Get("/exec/" + execid + "/json")
-	c.Assert(err, check.IsNil)
-	CheckRespStatus(c, resp, 200)
-
-	var execInspect03 types.ContainerExecInspect
-	request.DecodeBody(&execInspect03, resp.Body)
-
-	c.Assert(execInspect03.Running, check.Equals, false)
-	c.Assert(execInspect03.ExitCode, check.Equals, int64(0))
-
-	DelContainerForceMultyTime(c, cname)
+	{
+		time.Sleep(10 * time.Second)
+		execInspectResp = InspectExecOk(execid)
+		c.Assert(execInspectResp.Running, check.Equals, false)
+		c.Assert(execInspectResp.ExitCode, check.Equals, int64(0))
+	}
 }

test/api_container_exec_start_test.go

@@ -5,6 +5,7 @@ import (
 	"bytes"
 	"crypto/rand"
 	"encoding/hex"
+	"fmt"
 	"io"
 	"net"
 	"strings"
@@ -114,7 +115,7 @@ func (suite *APIContainerExecStartSuite) TestContainerExecStartWithoutUpgrade(c
 	defer DelContainerForceMultyTime(c, cname)
 
 	StartContainerOk(c, cname)
-	execid := CreateExecEchoOk(c, cname, content)
+	execid := CreateExecCmdOk(c, cname, "echo", content)
 
 	obj := map[string]interface{}{}
 	resp, conn, br, err := request.Hijack("/exec/"+execid+"/start", request.WithJSONBody(obj))
@@ -133,7 +134,7 @@ func (suite *APIContainerExecStartSuite) TestContainerExecStart(c *check.C) {
 
 	StartContainerOk(c, cname)
 
-	execid := CreateExecEchoOk(c, cname, content)
+	execid := CreateExecCmdOk(c, cname, "echo", content)
 
 	resp, conn, reader, err := StartContainerExec(c, execid, false, false)
 	c.Assert(err, check.IsNil)
@@ -151,20 +152,58 @@ func (suite *APIContainerExecStartSuite) TestContainerExecStartNotFound(c *check
 	CheckRespStatus(c, resp, 404)
 }
 
-// TestContainerExecStartStopped tests start a process in a stopped container return error.
-func (suite *APIContainerExecStartSuite) TestContainerExecStartStopped(c *check.C) {
-	// TODO: missing case
-	helpwantedForMissingCase(c, "container api exec start stoped case")
-}
+// TestContainerExecStartStopped tests start a process in a stopped and pause container return error.
+func (suite *APIContainerExecStartSuite) TestExecStartInStoppedAndPausedContainer(c *check.C) {
+	cname := "TestExecStartInStoppedAndPausedContainer"
+
+	// only create the container, the container is stopped/created.
+	CreateBusyboxContainerOk(c, cname)
+	defer DelContainerForceMultyTime(c, cname)
+	StartContainerOk(c, cname)
+
+	execid := CreateExecCmdOk(c, cname, "echo", "test")
+
+	// test exec start in stopped container
+
+	// stop the running container
+	StopContainerOk(c, cname)
+
+	obj := map[string]interface{}{}
+	body := request.WithJSONBody(obj)
+	resp, err := request.Post(fmt.Sprintf("/exec/%s/start", execid), body)
+
+	c.Assert(err, check.IsNil)
+	CheckRespStatus(c, resp, 409)
+
+	// start and pause the container
+	StartContainerOk(c, cname)
+	PauseContainerOk(c, cname)
 
-// TestContainerExecStartPaused tests start a process in a paused container return error.
-func (suite *APIContainerExecStartSuite) TestContainerExecStartPaused(c *check.C) {
-	// TODO: missing case
-	helpwantedForMissingCase(c, "container api exec start paused case")
+	// test exec start in paused container
+	resp, err = request.Post(fmt.Sprintf("/exec/%s/start", execid), body)
+
+	c.Assert(err, check.IsNil)
+	CheckRespStatus(c, resp, 409)
 }
 
 // TestContainerExecStartDup tests start a process twice return error.
 func (suite *APIContainerExecStartSuite) TestContainerExecStartDup(c *check.C) {
-	// TODO: missing case
-	helpwantedForMissingCase(c, "container api exec start twice case")
+	cname := "TestContainerExecStartDup"
+
+	CreateBusyboxContainerOk(c, cname)
+	defer DelContainerForceMultyTime(c, cname)
+
+	StartContainerOk(c, cname)
+
+	obj := map[string]interface{}{}
+	body := request.WithJSONBody(obj)
+
+	execid := CreateExecCmdOk(c, cname, "top")
+	resp, err := request.Post(fmt.Sprintf("/exec/%s/start", execid), body)
+	c.Assert(err, check.IsNil)
+	CheckRespStatus(c, resp, 200)
+
+	resp, err = request.Post(fmt.Sprintf("/exec/%s/start", execid), body)
+	c.Assert(err, check.IsNil)
+	CheckRespStatus(c, resp, 409)
 }

test/api_container_start_test.go

@@ -105,5 +105,4 @@ func (suite *APIContainerStartSuite) TestStartAlreadyRunningContainer(c *check.C
 	resp, err := request.Post("/containers/" + cname + "/start")
 	c.Assert(err, check.IsNil)
 	CheckRespStatus(c, resp, 304)
-
 }

test/util_api.go

@@ -198,10 +198,10 @@ func DelImageForceOk(c *check.C, iname string) {
 	CheckRespStatus(c, resp, 204)
 }
 
-// CreateExecEchoOk exec process's environment with "echo" CMD.
-func CreateExecEchoOk(c *check.C, cname string, echo string) string {
+// CreateExecCmdOk exec process's environment with specific CMD.
+func CreateExecCmdOk(c *check.C, cname string, cmd ...string) string {
 	obj := map[string]interface{}{
-		"Cmd":          []string{"echo", echo},
+		"Cmd":          cmd,
 		"Detach":       true,
 		"AttachStderr": true,
 		"AttachStdout": true,
@@ -221,8 +221,16 @@ func CreateExecEchoOk(c *check.C, cname string, echo string) string {
 	return got.ID
 }
 
+// StartContainerExecOk starts executing a process in container and check if it is successful.
+func StartContainerExecOk(c *check.C, execID string) {
+	resp, conn, _, err := startContainerExec(c, execID, false, false)
+	c.Assert(err, check.IsNil)
+	CheckRespStatus(c, resp, 101)
+	c.Assert(conn.Close(), check.IsNil)
+}
+
 // StartContainerExec starts executing a process in the container.
-func StartContainerExec(c *check.C, execid string, tty bool, detach bool) (*http.Response, net.Conn, *bufio.Reader, error) {
+func startContainerExec(c *check.C, execid string, tty bool, detach bool) (*http.Response, net.Conn, *bufio.Reader, error) {
 	obj := map[string]interface{}{
 		"Detach": detach,
 		"Tty":    tty,
@@ -234,6 +242,18 @@ func StartContainerExec(c *check.C, execid string, tty bool, detach bool) (*http
 		request.WithJSONBody(obj))
 }
 
+// InspectExecOk inspects an exec of container.
+func InspectExecOk(c *check.C, execID string) types.ContainerExecInspect {
+	resp, err := request.Get("/exec/" + execid + "/json")
+	c.Assert(err, check.IsNil)
+	CheckRespStatus(c, resp, 200)
+
+	var execInspectResp types.ContainerExecInspect
+	err = request.DecodeBody(&execInspectResp, resp.Body)
+	c.Assert(err, check.IsNil)
+	return execInspectResp
+}
+
 // CreateVolumeOK creates a volume in pouchd.
 func CreateVolumeOK(c *check.C, name, driver string, options map[string]string) {
 	obj := map[string]interface{}{