[testutil] Stop container using docker rm -f #124

- kill the docker run process does not work
- add labels to created container so they can be cleaned in batch

.travis.yml

@@ -1,8 +1,12 @@
 language: go
 sudo: false
 
+services:
+  - docker
+
 go:
   - "1.13"
+  - "1.14"
   - tip
 
 env:
@@ -18,5 +22,9 @@ script:
   - make vet
   - make test-cover
 
+after_script:
+  - docker rm -f $(docker ps --filter="label=gommon-container" -q)
+
 after_success:
-  - bash <(curl -s https://codecov.io/bash)
+  - bash <(curl -s https://codecov.io/bash)
+

util/testutil/docker.go

@@ -1,9 +1,11 @@
 package testutil
 
 import (
+	"bytes"
 	"errors"
 	"fmt"
 	"os/exec"
+	"time"
 )
 
 // docker.go starts and stop container for testing by shell out to docker cli
@@ -21,53 +23,119 @@ type ContainerConfig struct {
 	// Image contains image name with tag, it is what you put after docker run, e.g. dyweb/go-dev:1.14
 	Image string
 	Ports []PortMapping
+
+	now time.Time
 }
 
 type Container struct {
-	cfg ContainerConfig
-	cmd *exec.Cmd
+	cfg        ContainerConfig
+	createTime time.Time
+
+	// TODO: do we really need to save the docker run process? we are not doing anything with it after switch to docker rm
+	cmd    *exec.Cmd
+	id     string
+	labels []string
 }
 
-// NewContainer calls docker run to start a container.
+// NewContainer shell out to docker cli and runs a container in foreground.
 func NewContainer(cfg ContainerConfig) (*Container, error) {
-	c := &Container{cfg: cfg}
+	now := time.Now()
+	if !cfg.now.IsZero() {
+		now = cfg.now
+	}
+	c := &Container{
+		cfg:        cfg,
+		createTime: now,
+	}
+	c.labels = []string{"gommon-container=1", fmt.Sprintf("gommon-test-start-time=%d", testStart.UnixNano()),
+		fmt.Sprintf("gommon-create-time=%d", now.UnixNano())}
 	return c, c.run()
 }
 
-// ToDockerArgs validates and converts a container config to docker run arguments.
-func (cfg *ContainerConfig) ToDockerArgs() ([]string, error) {
+// NewContainerWithoutRun validate config, generate labels and does not execute any docker command.
+func NewContainerWithoutRun(cfg ContainerConfig) (*Container, error) {
+	if cfg.Image == "" {
+		return nil, errors.New("image is empty")
+	}
+
+	now := time.Now()
+	if !cfg.now.IsZero() {
+		now = cfg.now
+	}
+	c := &Container{
+		cfg:        cfg,
+		createTime: now,
+	}
+	c.labels = []string{"gommon-container=1", fmt.Sprintf("gommon-test-start-time=%d", testStart.UnixNano()),
+		fmt.Sprintf("gommon-create-time=%d", now.UnixNano())}
+	return c, nil
+}
+
+// DockerRunArgs converts a container config to docker run arguments.
+func (c *Container) DockerRunArgs() []string {
 	args := []string{"run"}
-	for _, port := range cfg.Ports {
+	for _, port := range c.cfg.Ports {
 		// https://docs.docker.com/config/containers/container-networking/#published-ports
 		// -p 8080:80 Map TCP port 80 in the container to port 8080 on the Docker host
 		args = append(args, "-p", fmt.Sprintf("%d:%d", port.Host, port.Container))
 	}
-	if cfg.Image == "" {
-		return nil, errors.New("image is empty")
+	for _, l := range c.labels {
+		args = append(args, "-l", l)
 	}
-	args = append(args, cfg.Image)
-	return args, nil
+	args = append(args, c.cfg.Image)
+	return args
 }
 
-func (c *Container) run() error {
-	args, err := c.cfg.ToDockerArgs()
-	if err != nil {
-		return err
+// DockerPsArgs is used to filter out container by all of its labels
+func (c *Container) DockerPsArgs() []string {
+	args := []string{"ps"}
+	for _, l := range c.labels {
+		args = append(args, "--filter", "label="+l)
 	}
-	cmd := exec.Command("docker", args...)
-	if err := cmd.Start(); err != nil {
+	args = append(args, "-q")
+	return args
+}
+
+// run calls docker run in foreground and collect its id.
+func (c *Container) run() error {
+	// TODO(at15): we do a docker pull first to return message for image not exist error
+
+	// docker run
+	runCmd := exec.Command("docker", c.DockerRunArgs()...)
+	if err := runCmd.Start(); err != nil {
 		return fmt.Errorf("error start docker command %w", err)
 	}
-	c.cmd = cmd
-	return nil
+	c.cmd = runCmd
+
+	// TODO(#126): manual retry until we have a retry package
+	// The drawback of shell out is we don't know when the container will be ready especially when pulling is needed.
+	var (
+		out []byte
+		err error
+	)
+	for i := 0; i < 5; i++ {
+		// docker ps to get id
+		psCmd := exec.Command("docker", c.DockerPsArgs()...)
+		out, err = psCmd.CombinedOutput()
+		id := string(bytes.TrimSpace(out))
+		if err != nil || id == "" {
+			time.Sleep(1 * time.Duration(i) * time.Second)
+			continue
+		}
+		c.id = id
+		return nil
+	}
+	return fmt.Errorf("error get conatienr id : %w %s", err, string(out))
 }
 
+// Stop removes the (running) container by id
 func (c *Container) Stop() error {
-	// TODO: kill the process only works if we run container in foreground
-	// FIXME: it seems the container is still running after sending kill, might need to call docker rm
-	// might check https://github.com/docker/cli/blob/master/cli/command/container/run.go
-	if err := c.cmd.Process.Kill(); err != nil {
-		return fmt.Errorf("error kill docker run %w", err)
+	// TODO: kill the foreground docker run process? though it will exit once the container is removed
+
+	delCmd := exec.Command("docker", "rm", "-f", c.id)
+	out, err := delCmd.CombinedOutput()
+	if err != nil {
+		return fmt.Errorf("error remove container %s: %w %s", c.id, err, string(out))
 	}
 	return nil
 }

util/testutil/docker_test.go

@@ -10,9 +10,13 @@ import (
 
 	"github.com/dyweb/gommon/util/netutil"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
-func TestContainerConfig_ToDockerArgs(t *testing.T) {
+func TestContainer_DockerRunArgs(t *testing.T) {
+	tStart := time.Unix(1583459680, 0)
+	SetTestStart(tStart)
+	tCreate := time.Unix(1583459690, 0)
 	cfg := ContainerConfig{
 		Image: "nginx",
 		Ports: []PortMapping{
@@ -21,10 +25,12 @@ func TestContainerConfig_ToDockerArgs(t *testing.T) {
 				Container: 80,
 			},
 		},
+		now: tCreate,
 	}
-	args, err := cfg.ToDockerArgs()
-	assert.Nil(t, err)
-	assert.Equal(t, "run -p 8093:80 nginx", strings.Join(args, " "))
+	c, err := NewContainerWithoutRun(cfg)
+	require.Nil(t, err)
+	args := c.DockerRunArgs()
+	assert.Equal(t, "run -p 8093:80 -l gommon-container=1 -l gommon-test-start-time=1583459680000000000 -l gommon-create-time=1583459690000000000 nginx", strings.Join(args, " "))
 }
 
 func TestContainer_Stop(t *testing.T) {
@@ -44,7 +50,7 @@ func TestContainer_Stop(t *testing.T) {
 	c, err := NewContainer(cfg)
 	assert.Nil(t, err)
 	// TODO: wait until container is ready, this needs to be provided by the client ...
-	time.Sleep(2 * time.Second)
+	time.Sleep(1 * time.Second)
 	res, err := http.Get(fmt.Sprintf("http://localhost:%d", port))
 	assert.Nil(t, err)
 	b, err := ioutil.ReadAll(res.Body)

util/testutil/pkg.go

@@ -1,2 +1,17 @@
 // Package testutil defines helper functions like condition, golden file, docker container etc.
 package testutil
+
+import "time"
+
+var testStart time.Time
+
+// SetTestStart allows you to override the global test start time.
+// Which is used as a simple identifier to distinguish different tests.
+// For instance, it is used as label in container test.
+func SetTestStart(t time.Time) {
+	testStart = t
+}
+
+func init() {
+	testStart = time.Now()
+}