diff --git a/container_test.go b/container_test.go index 1d25c4d661..0da1b967f8 100644 --- a/container_test.go +++ b/container_test.go @@ -151,7 +151,7 @@ func Test_BuildImageWithContexts(t *testing.T) { ContextArchive func() (io.Reader, error) ExpectedEchoOutput string Dockerfile string - ExpectedError error + ExpectedError string } testCases := []TestCase{ @@ -252,7 +252,7 @@ func Test_BuildImageWithContexts(t *testing.T) { ExpectedEchoOutput: "hi this is from the say_hi.sh file!", }, { - Name: "test buildling from a context on the filesystem", + Name: "test building from a context on the filesystem", ContextPath: "./testdata", Dockerfile: "echo.Dockerfile", ExpectedEchoOutput: "this is from the echo test Dockerfile", @@ -266,7 +266,7 @@ func Test_BuildImageWithContexts(t *testing.T) { ContextArchive: func() (io.Reader, error) { return nil, nil }, - ExpectedError: errors.New("you must specify either a build context or an image: failed to create container"), + ExpectedError: "create container: you must specify either a build context or an image", }, } @@ -292,16 +292,15 @@ func Test_BuildImageWithContexts(t *testing.T) { ContainerRequest: req, Started: true, }) - switch { - case testCase.ExpectedError != nil && err != nil: - if testCase.ExpectedError.Error() != err.Error() { - t.Fatalf("unexpected error: %s, was expecting %s", err.Error(), testCase.ExpectedError.Error()) - } - case err != nil: - t.Fatal(err) - default: - terminateContainerOnEnd(t, ctx, c) + + defer terminateContainerOnEnd(t, ctx, c) + + if testCase.ExpectedError != "" { + require.EqualError(t, err, testCase.ExpectedError) + return } + + require.NoError(t, err) }) } } diff --git a/docker.go b/docker.go index e89b9f4649..c3dfcb8da2 100644 --- a/docker.go +++ b/docker.go @@ -887,14 +887,14 @@ func (p *DockerProvider) BuildImage(ctx context.Context, img ImageBuildInfo) (st var err error buildOptions, err = img.BuildOptions() if err != nil { - return types.ImageBuildResponse{}, backoff.Permanent(err) + return types.ImageBuildResponse{}, backoff.Permanent(fmt.Errorf("build options: %w", err)) } defer tryClose(buildOptions.Context) // release resources in any case resp, err := p.client.ImageBuild(ctx, buildOptions.Context, buildOptions) if err != nil { if isPermanentClientError(err) { - return types.ImageBuildResponse{}, backoff.Permanent(err) + return types.ImageBuildResponse{}, backoff.Permanent(fmt.Errorf("build image: %w", err)) } return types.ImageBuildResponse{}, err } @@ -908,30 +908,28 @@ func (p *DockerProvider) BuildImage(ctx context.Context, img ImageBuildInfo) (st }, ) if err != nil { - return "", err + return "", err // Error is already wrapped. } defer resp.Body.Close() + output := io.Discard if img.ShouldPrintBuildLog() { - termFd, isTerm := term.GetFdInfo(os.Stderr) - err = jsonmessage.DisplayJSONMessagesStream(resp.Body, os.Stderr, termFd, isTerm, nil) - if err != nil { - return "", err - } + output = os.Stderr } - // need to read the response from Docker, I think otherwise the image - // might not finish building before continuing to execute here - _, err = io.Copy(io.Discard, resp.Body) - if err != nil { - return "", err + // Always process the output, even if it is not printed + // to ensure that errors during the build process are + // correctly handled. + termFd, isTerm := term.GetFdInfo(output) + if err = jsonmessage.DisplayJSONMessagesStream(resp.Body, output, termFd, isTerm, nil); err != nil { + return "", fmt.Errorf("build image: %w", err) } // the first tag is the one we want return buildOptions.Tags[0], nil } -// CreateContainer fulfills a request for a container without starting it +// CreateContainer fulfils a request for a container without starting it func (p *DockerProvider) CreateContainer(ctx context.Context, req ContainerRequest) (Container, error) { var err error diff --git a/from_dockerfile_test.go b/from_dockerfile_test.go index ab25ae6dee..fc1d4052ea 100644 --- a/from_dockerfile_test.go +++ b/from_dockerfile_test.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "log" + "path/filepath" "strings" "testing" "time" @@ -88,6 +89,28 @@ func TestBuildImageFromDockerfile_NoRepo(t *testing.T) { }) } +func TestBuildImageFromDockerfile_BuildError(t *testing.T) { + ctx := context.Background() + dockerClient, err := NewDockerClientWithOpts(ctx) + require.NoError(t, err) + + defer dockerClient.Close() + + req := ContainerRequest{ + FromDockerfile: FromDockerfile{ + Dockerfile: "error.Dockerfile", + Context: filepath.Join(".", "testdata"), + }, + } + _, err = GenericContainer(ctx, GenericContainerRequest{ + ProviderType: providerType, + ContainerRequest: req, + Started: true, + }) + + require.EqualError(t, err, `create container: build image: The command '/bin/sh -c exit 1' returned a non-zero code: 1`) +} + func TestBuildImageFromDockerfile_NoTag(t *testing.T) { provider, err := NewDockerProvider() if err != nil { diff --git a/generic.go b/generic.go index 65fec35c88..f0cda13407 100644 --- a/generic.go +++ b/generic.go @@ -74,7 +74,7 @@ func GenericContainer(ctx context.Context, req GenericContainerRequest) (Contain } if err != nil { // At this point `c` might not be nil. Give the caller an opportunity to call Destroy on the container. - return c, fmt.Errorf("%w: failed to create container", err) + return c, fmt.Errorf("create container: %w", err) } if req.Started && !c.IsRunning() { diff --git a/testdata/error.Dockerfile b/testdata/error.Dockerfile new file mode 100644 index 0000000000..1284e7285f --- /dev/null +++ b/testdata/error.Dockerfile @@ -0,0 +1,4 @@ +FROM docker.io/alpine + +RUN exit 1 +