Skip to content

Commit

Permalink
build: handle missing build contexts (#6141)
Browse files Browse the repository at this point in the history
Before this change, if you specified a directory that doesn't
exist as your docker_build context, you would get an empty build context.

After this change, if you specify a directory that doesn't exist,
you get a build-time error. You can use "-" to specify an empty build context.

this is a breaking change, but i think makes us more consistent with
how the docker cli works.

fixes #3897
fixes #6125

Signed-off-by: Nick Santos <nick.santos@docker.com>
  • Loading branch information
nicks authored Jun 12, 2023
1 parent 36001a6 commit 76c887e
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 3 deletions.
40 changes: 40 additions & 0 deletions internal/build/container_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/tilt-dev/tilt/internal/container"
"github.com/tilt-dev/tilt/internal/docker"
Expand Down Expand Up @@ -145,3 +146,42 @@ RUN echo 'failed to create LLB definition: failed commit on ref "unknown-sha256:
assert.Contains(t, out.String(), "[1/2] FROM docker.io/library/alpine") // buildkit-style output
assert.True(t, regexp.MustCompile("Step 1/[0-9]+ : FROM alpine").MatchString(out.String())) // Legacy output
}

func TestDockerBuildEmptyContext(t *testing.T) {
f := newDockerBuildFixture(t)

df := dockerfile.Dockerfile(`
FROM alpine
`)

spec := v1alpha1.DockerImageSpec{
DockerfileContents: df.String(),
Context: "-",
}
refs, _, err := f.b.BuildImage(f.ctx, f.ps, f.getNameFromTest(), spec,
defaultCluster,
nil,
model.EmptyMatcher)
require.NoError(t, err)
f.assertImageHasLabels(refs.LocalRef, docker.BuiltLabelSet)
}

func TestDockerBuildMissingContext(t *testing.T) {
f := newDockerBuildFixture(t)

df := dockerfile.Dockerfile(`
FROM alpine
`)

spec := v1alpha1.DockerImageSpec{
DockerfileContents: df.String(),
Context: "unknown-dir",
}
_, _, err := f.b.BuildImage(f.ctx, f.ps, f.getNameFromTest(), spec,
defaultCluster,
nil,
model.EmptyMatcher)
if assert.Error(t, err) {
assert.Contains(t, err.Error(), "reading build context: stat unknown-dir: no such file or directory")
}
}
25 changes: 23 additions & 2 deletions internal/build/docker_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,27 @@ func (d *DockerBuilder) BuildImage(ctx context.Context, ps *PipelineState, refs
func (d *DockerBuilder) buildToDigest(ctx context.Context, spec v1alpha1.DockerImageSpec, filter model.PathMatcher, allowBuildkit bool) (digest.Digest, []v1alpha1.DockerImageStageStatus, error) {
var contextReader io.Reader

buildContext := spec.Context

// Treat context: "-" as an empty context.
if buildContext == "-" {
emptyContextDir, err := os.MkdirTemp("", "tilt-dockercontext-")
if err != nil {
return "", nil, fmt.Errorf("creating context directory: %v", err)
}

defer func() {
_ = os.RemoveAll(emptyContextDir)
}()

buildContext = emptyContextDir
}

_, err := os.Stat(buildContext)
if err != nil {
return "", nil, fmt.Errorf("reading build context: %v", err)
}

// Buildkit allows us to use a fs sync server instead of uploading up-front.
useFSSync := allowBuildkit && d.dCli.BuilderVersion() == types.BuilderBuildKit
if !useFSSync {
Expand All @@ -217,7 +238,7 @@ func (d *DockerBuilder) buildToDigest(ctx context.Context, spec v1alpha1.DockerI
go func(ctx context.Context) {
paths := []PathMapping{
{
LocalPath: spec.Context,
LocalPath: buildContext,
ContainerPath: "/",
},
}
Expand All @@ -242,7 +263,7 @@ func (d *DockerBuilder) buildToDigest(ctx context.Context, spec v1alpha1.DockerI
if err != nil {
return "", nil, err
}
options.SyncedDirs = toSyncedDirs(spec.Context, dockerfileDir, filter)
options.SyncedDirs = toSyncedDirs(buildContext, dockerfileDir, filter)
options.Dockerfile = DockerfileName

defer func() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"archive/tar"
"context"
"fmt"
"os"
"testing"

"github.com/jonboulle/clockwork"
Expand Down Expand Up @@ -88,7 +89,7 @@ func TestTiltBuildsImageWithTag(t *testing.T) {

refWithTag := "gcr.io/foo:bar"
iTarget := model.MustNewImageTarget(container.MustParseSelector(refWithTag)).
WithBuildDetails(model.DockerBuild{})
WithBuildDetails(model.DockerBuild{DockerImageSpec: v1alpha1.DockerImageSpec{Context: "-"}})
manifest := manifestbuilder.New(f, "fe").
WithDockerCompose().
WithImageTarget(iTarget).
Expand Down Expand Up @@ -200,6 +201,10 @@ func newDCBDFixture(t *testing.T) *dcbdFixture {

f := tempdir.NewTempDirFixture(t)

// empty dirs for build contexts
_ = os.Mkdir(f.JoinPath("sancho"), 0777)
_ = os.Mkdir(f.JoinPath("sancho-base"), 0777)

dir := dirs.NewTiltDevDirAt(f.Path())
dcCli := dockercompose.NewFakeDockerComposeClient(t, ctx)
dCli := docker.NewFakeClient()
Expand Down
6 changes: 6 additions & 0 deletions internal/engine/buildcontrol/image_build_and_deployer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"archive/tar"
"context"
"fmt"
"os"
"sort"
"strings"
"testing"
Expand Down Expand Up @@ -1090,6 +1091,11 @@ type ibdFixture struct {

func newIBDFixture(t *testing.T, env clusterid.Product) *ibdFixture {
f := tempdir.NewTempDirFixture(t)

// empty dirs for build contexts
_ = os.Mkdir(f.JoinPath("sancho"), 0777)
_ = os.Mkdir(f.JoinPath("sancho-base"), 0777)

dir := dirs.NewTiltDevDirAt(f.Path())

dockerClient := docker.NewFakeClient()
Expand Down

0 comments on commit 76c887e

Please sign in to comment.