Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: missing image build errors #2651

Merged
merged 1 commit into from
Aug 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 11 additions & 12 deletions container_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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",
Expand All @@ -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",
},
}

Expand All @@ -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)
})
}
}
Expand Down
26 changes: 12 additions & 14 deletions docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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

Expand Down
23 changes: 23 additions & 0 deletions from_dockerfile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"io"
"log"
"path/filepath"
"strings"
"testing"
"time"
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion generic.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
4 changes: 4 additions & 0 deletions testdata/error.Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
FROM docker.io/alpine

RUN exit 1

Loading