Merge pull request #82 from testcontainers/hotfix/78

Fixed regression after SkipRepaer

docker.go

@@ -10,6 +10,7 @@ import (
 	"os/exec"
 	"strings"
 
+	"github.com/cenkalti/backoff"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/api/types/mount"
@@ -36,6 +37,7 @@ type DockerContainer struct {
 	provider          *DockerProvider
 	sessionID         uuid.UUID
 	terminationSignal chan bool
+	skipReaper        bool
 }
 
 func (c *DockerContainer) GetContainerID() string {
@@ -143,11 +145,12 @@ func (c *DockerContainer) Start(ctx context.Context) error {
 }
 
 // Terminate is used to kill the container. It is usally triggered by as defer function.
-func (c *DockerContainer) Terminate(_ context.Context) error {
-	if c.terminationSignal != nil {
-		c.terminationSignal <- true
-	}
-	return nil
+func (c *DockerContainer) Terminate(ctx context.Context) error {
+	err := c.provider.client.ContainerRemove(ctx, c.GetContainerID(), types.ContainerRemoveOptions{
+		RemoveVolumes: true,
+		Force:         true,
+	})
+	return err
 }
 
 func (c *DockerContainer) inspectContainer(ctx context.Context) (*types.ContainerJSON, error) {
@@ -222,7 +225,7 @@ func (p *DockerProvider) CreateContainer(ctx context.Context, req ContainerReque
 	sessionID := uuid.NewV4()
 
 	var termSignal chan bool
-	if req.SkipReaper {
+	if !req.SkipReaper {
 		r, err := NewReaper(ctx, sessionID.String(), p)
 		if err != nil {
 			return nil, errors.Wrap(err, "creating reaper failed")
@@ -256,7 +259,12 @@ func (p *DockerProvider) CreateContainer(ctx context.Context, req ContainerReque
 			if req.RegistryCred != "" {
 				pullOpt.RegistryAuth = req.RegistryCred
 			}
-			pull, err := p.client.ImagePull(ctx, req.Image, pullOpt)
+			var pull io.ReadCloser
+			err := backoff.Retry(func() error {
+				var err error
+				pull, err = p.client.ImagePull(ctx, req.Image, pullOpt)
+				return err
+			}, backoff.NewExponentialBackOff())
 			if err != nil {
 				return nil, err
 			}
@@ -300,6 +308,7 @@ func (p *DockerProvider) CreateContainer(ctx context.Context, req ContainerReque
 		sessionID:         sessionID,
 		provider:          p,
 		terminationSignal: termSignal,
+		skipReaper:        req.SkipReaper,
 	}
 
 	return c, nil

docker_test.go

@@ -11,6 +11,9 @@ import (
 	// Import mysql into the scope of this package (required)
 	_ "github.com/go-sql-driver/mysql"
 
+	"github.com/docker/docker/api/types"
+	"github.com/docker/docker/api/types/filters"
+	"github.com/docker/docker/client"
 	"github.com/docker/go-connections/nat"
 	"github.com/testcontainers/testcontainers-go/wait"
 )
@@ -34,6 +37,153 @@ func TestContainerReturnItsContainerID(t *testing.T) {
 	}
 }
 
+func TestContainerStartsWithoutTheReaper(t *testing.T) {
+	t.Skip("need to use the sessionID")
+	ctx := context.Background()
+	client, err := client.NewEnvClient()
+	if err != nil {
+		t.Fatal(err)
+	}
+	client.NegotiateAPIVersion(ctx)
+	_, err = GenericContainer(ctx, GenericContainerRequest{
+		ContainerRequest: ContainerRequest{
+			Image: "nginx",
+			ExposedPorts: []string{
+				"80/tcp",
+			},
+			SkipReaper: true,
+		},
+		Started: true,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	filtersJSON := fmt.Sprintf(`{"label":{"%s":true}}`, TestcontainerLabelIsReaper)
+	f, err := filters.FromJSON(filtersJSON)
+	if err != nil {
+		t.Fatal(err)
+	}
+	resp, err := client.ContainerList(ctx, types.ContainerListOptions{
+		Filters: f,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(resp) != 0 {
+		t.Fatal("expected zero reaper running.")
+	}
+}
+
+func TestContainerStartsWithTheReaper(t *testing.T) {
+	ctx := context.Background()
+	client, err := client.NewEnvClient()
+	if err != nil {
+		t.Fatal(err)
+	}
+	client.NegotiateAPIVersion(ctx)
+	_, err = GenericContainer(ctx, GenericContainerRequest{
+		ContainerRequest: ContainerRequest{
+			Image: "nginx",
+			ExposedPorts: []string{
+				"80/tcp",
+			},
+		},
+		Started: true,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	filtersJSON := fmt.Sprintf(`{"label":{"%s":true}}`, TestcontainerLabelIsReaper)
+	f, err := filters.FromJSON(filtersJSON)
+	if err != nil {
+		t.Fatal(err)
+	}
+	resp, err := client.ContainerList(ctx, types.ContainerListOptions{
+		Filters: f,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(resp) == 0 {
+		t.Fatal("expected at least one reaper to be running.")
+	}
+}
+
+func TestContainerTerminationWithReaper(t *testing.T) {
+	ctx := context.Background()
+	client, err := client.NewEnvClient()
+	if err != nil {
+		t.Fatal(err)
+	}
+	client.NegotiateAPIVersion(ctx)
+	nginxA, err := GenericContainer(ctx, GenericContainerRequest{
+		ContainerRequest: ContainerRequest{
+			Image: "nginx",
+			ExposedPorts: []string{
+				"80/tcp",
+			},
+		},
+		Started: true,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	containerID := nginxA.GetContainerID()
+	resp, err := client.ContainerInspect(ctx, containerID)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if resp.State.Running != true {
+		t.Fatal("The container shoud be in running state")
+	}
+	err = nginxA.Terminate(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
+	_, err = client.ContainerInspect(ctx, containerID)
+	if err == nil {
+		t.Fatal("expected error from container inspect.")
+	}
+}
+
+func TestContainerTerminationWithoutReaper(t *testing.T) {
+	ctx := context.Background()
+	client, err := client.NewEnvClient()
+	if err != nil {
+		t.Fatal(err)
+	}
+	client.NegotiateAPIVersion(ctx)
+	nginxA, err := GenericContainer(ctx, GenericContainerRequest{
+		ContainerRequest: ContainerRequest{
+			Image: "nginx",
+			ExposedPorts: []string{
+				"80/tcp",
+			},
+			SkipReaper: true,
+		},
+		Started: true,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	containerID := nginxA.GetContainerID()
+	resp, err := client.ContainerInspect(ctx, containerID)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if resp.State.Running != true {
+		t.Fatal("The container shoud be in running state")
+	}
+	err = nginxA.Terminate(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
+	_, err = client.ContainerInspect(ctx, containerID)
+	if err == nil {
+		t.Fatal("expected error from container inspect.")
+	}
+}
+
 func TestTwoContainersExposingTheSamePort(t *testing.T) {
 	ctx := context.Background()
 	nginxA, err := GenericContainer(ctx, GenericContainerRequest{

go.mod

@@ -5,6 +5,7 @@ replace github.com/docker/docker => github.com/docker/engine v0.0.0-201907171610
 require (
 	github.com/Azure/go-ansiterm v0.0.0-20170929234023-d6e3b3328b78 // indirect
 	github.com/Microsoft/go-winio v0.4.11 // indirect
+	github.com/cenkalti/backoff v2.2.1+incompatible
 	github.com/docker/distribution v2.7.1-0.20190205005809-0d3efadf0154+incompatible // indirect
 	github.com/docker/docker v0.7.3-0.20190506211059-b20a14b54661
 	github.com/docker/go-connections v0.4.0

go.sum

@@ -3,6 +3,8 @@ github.com/Azure/go-ansiterm v0.0.0-20170929234023-d6e3b3328b78 h1:w+iIsaOQNcT7O
 github.com/Azure/go-ansiterm v0.0.0-20170929234023-d6e3b3328b78/go.mod h1:LmzpDX56iTiv29bbRTIsUNlaFfuhWRQBWjQdVyAevI8=
 github.com/Microsoft/go-winio v0.4.11 h1:zoIOcVf0xPN1tnMVbTtEdI+P8OofVk3NObnwOQ6nK2Q=
 github.com/Microsoft/go-winio v0.4.11/go.mod h1:VhR8bwka0BXejwEJY73c50VrPtXAaKcyvVC4A4RozmA=
+github.com/cenkalti/backoff v2.2.1+incompatible h1:tNowT99t7UNflLxfYYSlKYsBpXdEet03Pg2g16Swow4=
+github.com/cenkalti/backoff v2.2.1+incompatible/go.mod h1:90ReRw6GdpyfrHakVjL/QHaoyV4aDUVVkXQJJJ3NXXM=
 github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
@@ -83,6 +85,4 @@ gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 h1:qIbj1fsPNlZgppZ+VLlY7N33
 gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gotest.tools v0.0.0-20181223230014-1083505acf35 h1:zpdCK+REwbk+rqjJmHhiCN6iBIigrZ39glqSF0P3KF0=
 gotest.tools v0.0.0-20181223230014-1083505acf35/go.mod h1:R//lfYlUuTOTfblYI3lGoAAAebUdzjvbmQsuB7Ykd90=
-gotest.tools v0.0.0-20190719212235-400e4d0d57c4 h1:IufeX23XqH4hHp+aGbtU57mGf+xSzMD820TexKcjUqg=
-gotest.tools v0.0.0-20190719212235-400e4d0d57c4/go.mod h1:R//lfYlUuTOTfblYI3lGoAAAebUdzjvbmQsuB7Ykd90=
 honnef.co/go/tools v0.0.0-20180728063816-88497007e858/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=

reaper.go

@@ -15,6 +15,7 @@ import (
 const (
 	TestcontainerLabel          = "org.testcontainers.golang"
 	TestcontainerLabelSessionID = TestcontainerLabel + ".sessionId"
+	TestcontainerLabelIsReaper  = TestcontainerLabel + ".reaper"
 	ReaperDefaultImage          = "quay.io/testcontainers/ryuk:0.2.2"
 )
 
@@ -44,13 +45,13 @@ func NewReaper(ctx context.Context, sessionID string, provider ReaperProvider) (
 		Image:        ReaperDefaultImage,
 		ExposedPorts: []string{"8080"},
 		Labels: map[string]string{
-			TestcontainerLabel:             "true",
-			TestcontainerLabel + ".reaper": "true",
+			TestcontainerLabel:         "true",
+			TestcontainerLabelIsReaper: "true",
 		},
+		SkipReaper: true,
 		BindMounts: map[string]string{
 			"/var/run/docker.sock": "/var/run/docker.sock",
 		},
-		SkipReaper: false,
 	}
 
 	c, err := provider.RunContainer(ctx, req)