From edb1f310022e48268867572431fb2a467c85c0b5 Mon Sep 17 00:00:00 2001 From: Gianluca Arbezzano Date: Wed, 7 Aug 2019 11:45:09 +0200 Subject: [PATCH 1/2] Fixed regression after SkipRepaer Fixed #78 Signed-off-by: Gianluca Arbezzano --- docker.go | 15 +++-- docker_test.go | 150 +++++++++++++++++++++++++++++++++++++++++++++++++ go.sum | 2 - reaper.go | 7 ++- 4 files changed, 163 insertions(+), 11 deletions(-) diff --git a/docker.go b/docker.go index 1953ca280b..0748633b8c 100644 --- a/docker.go +++ b/docker.go @@ -36,6 +36,7 @@ type DockerContainer struct { provider *DockerProvider sessionID uuid.UUID terminationSignal chan bool + skipReaper bool } func (c *DockerContainer) GetContainerID() string { @@ -143,11 +144,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 +224,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") @@ -300,6 +302,7 @@ func (p *DockerProvider) CreateContainer(ctx context.Context, req ContainerReque sessionID: sessionID, provider: p, terminationSignal: termSignal, + skipReaper: req.SkipReaper, } return c, nil diff --git a/docker_test.go b/docker_test.go index 9e8b150ff1..a0051fedea 100644 --- a/docker_test.go +++ b/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{ diff --git a/go.sum b/go.sum index ca18973f24..ac3148ff13 100644 --- a/go.sum +++ b/go.sum @@ -83,6 +83,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= diff --git a/reaper.go b/reaper.go index cb9b21b889..39e43c81ce 100644 --- a/reaper.go +++ b/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) From c7eb4ce82795dfb3ab79dba62da0f87942f5ab11 Mon Sep 17 00:00:00 2001 From: Gianluca Arbezzano Date: Wed, 7 Aug 2019 12:13:54 +0200 Subject: [PATCH 2/2] Added exponential backoff during image pull Signed-off-by: Gianluca Arbezzano --- docker.go | 8 +++++++- go.mod | 1 + go.sum | 2 ++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/docker.go b/docker.go index 0748633b8c..f8e1c2dc87 100644 --- a/docker.go +++ b/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" @@ -258,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 } diff --git a/go.mod b/go.mod index f5730933e8..bb0b37ae2e 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index ac3148ff13..1e74868c26 100644 --- a/go.sum +++ b/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=