Skip to content

Commit

Permalink
build: support cancellation of custom image builds (#5971)
Browse files Browse the repository at this point in the history 
Custom image builds now create an explicit Cmd object
in the API server. But for now these Cmds are kept up
to date as part of the buildcontroller, rather than
by the runtime controller.

Fixes #5926

Signed-off-by: Nick Santos <[email protected]>

Signed-off-by: Nick Santos <[email protected]>
  • Loading branch information
@nicks
nicks authored Nov 14, 2022
1 parent 0e34b8b commit e2ad6ab
Show file tree
Hide file tree
Showing 20 changed files with 276 additions and 90 deletions.
32 changes: 16 additions & 16 deletions internal/build/custom_builder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"fmt"
"os"
"os/exec"
"strings"

"github.com/docker/distribution/reference"
Expand All @@ -14,6 +13,7 @@ import (

"github.com/tilt-dev/tilt/internal/container"
"github.com/tilt-dev/tilt/internal/controllers/apis/imagemap"
"github.com/tilt-dev/tilt/internal/controllers/core/cmd"
"github.com/tilt-dev/tilt/internal/docker"
"github.com/tilt-dev/tilt/pkg/apis/core/v1alpha1"
"github.com/tilt-dev/tilt/pkg/logger"
Expand All @@ -23,17 +23,20 @@ import (
type CustomBuilder struct {
dCli docker.Client
clock Clock
cmds *cmd.Controller
}

func NewCustomBuilder(dCli docker.Client, clock Clock) *CustomBuilder {
func NewCustomBuilder(dCli docker.Client, clock Clock, cmds *cmd.Controller) *CustomBuilder {
return &CustomBuilder{
dCli: dCli,
clock: clock,
cmds: cmds,
}
}

func (b *CustomBuilder) Build(ctx context.Context, refs container.RefSet,
spec v1alpha1.CmdImageSpec,
cmd *v1alpha1.Cmd,
imageMaps map[ktypes.NamespacedName]*v1alpha1.ImageMap) (container.TaggedRefs, error) {
expectedTag := spec.OutputTag
outputsImageRefTo := spec.OutputsImageRefTo
Expand Down Expand Up @@ -71,11 +74,7 @@ func (b *CustomBuilder) Build(ctx context.Context, refs container.RefSet,

expectedBuildResult := expectedBuildRefs.LocalRef

// TODO(nick): Use the Cmd API.
// TODO(nick): Inject the image maps into the command environment.
cmd := exec.CommandContext(ctx, spec.Args[0], spec.Args[1:]...)
cmd.Dir = spec.Dir
cmd.Env = logger.DefaultEnv(ctx)
cmd = cmd.DeepCopy()

l := logger.Get(ctx)

Expand Down Expand Up @@ -107,20 +106,21 @@ func (b *CustomBuilder) Build(ctx context.Context, refs container.RefSet,
l.Infof(" %s", v)
}
}
cmd.Env = append(os.Environ(), extraEnvVars...)
err = imagemap.InjectIntoLocalEnv(cmd, spec.ImageMaps, imageMaps)
cmd.Spec.Env = append(cmd.Spec.Env, extraEnvVars...)
cmd, err = imagemap.InjectIntoLocalEnv(cmd, spec.ImageMaps, imageMaps)
if err != nil {
return container.TaggedRefs{}, errors.Wrap(err, "custom_build")
}

w := l.Writer(logger.InfoLvl)
cmd.Stdout = w
cmd.Stderr = w

l.Infof("Running custom build cmd %q", model.Cmd{Argv: spec.Args}.String())
err = cmd.Run()
status, err := b.cmds.ForceRun(ctx, cmd)
if err != nil {
return container.TaggedRefs{}, errors.Wrap(err, "Custom build command failed")
return container.TaggedRefs{}, fmt.Errorf("Custom build %q failed: %v",
model.ArgListToString(cmd.Spec.Args), err)
} else if status.Terminated == nil {
return container.TaggedRefs{}, fmt.Errorf("Custom build didn't terminate")
} else if status.Terminated.ExitCode != 0 {
return container.TaggedRefs{}, fmt.Errorf("Custom build %q failed: %v",
model.ArgListToString(cmd.Spec.Args), status.Terminated.Reason)
}

if outputsImageRefTo != "" {
Expand Down
59 changes: 40 additions & 19 deletions internal/build/custom_builder_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,19 @@ import (
"time"

"github.com/docker/docker/api/types"
"github.com/jonboulle/clockwork"
"github.com/opencontainers/go-digest"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
ktypes "k8s.io/apimachinery/pkg/types"

"github.com/tilt-dev/tilt/internal/container"
"github.com/tilt-dev/tilt/internal/controllers/core/cmd"
"github.com/tilt-dev/tilt/internal/controllers/fake"
"github.com/tilt-dev/tilt/internal/docker"
"github.com/tilt-dev/tilt/internal/localexec"
"github.com/tilt-dev/tilt/internal/store"
"github.com/tilt-dev/tilt/internal/testutils"
"github.com/tilt-dev/tilt/internal/testutils/tempdir"
"github.com/tilt-dev/tilt/pkg/apis/core/v1alpha1"
Expand All @@ -38,7 +43,7 @@ func TestCustomBuildSuccess(t *testing.T) {
sha := digest.Digest("sha256:11cd0eb38bc3ceb958ffb2f9bd70be3fb317ce7d255c8a4c3f4af30e298aa1aab")
f.dCli.Images["gcr.io/foo/bar:tilt-build-1551202573"] = types.ImageInspect{ID: string(sha)}
cb := f.customBuild("exit 0")
refs, err := f.cb.Build(f.ctx, refSetFromString("gcr.io/foo/bar"), cb.CmdImageSpec, nil)
refs, err := f.Build(refSetFromString("gcr.io/foo/bar"), cb, nil)
require.NoError(t, err)

assert.Equal(f.t, container.MustParseNamed("gcr.io/foo/bar:tilt-11cd0eb38bc3ceb9"), refs.LocalRef)
Expand All @@ -51,7 +56,7 @@ func TestCustomBuildSuccessClusterRefTaggedWithDigest(t *testing.T) {
sha := digest.Digest("sha256:11cd0eb38bc3ceb958ffb2f9bd70be3fb317ce7d255c8a4c3f4af30e298aa1aab")
f.dCli.Images["localhost:1234/foo_bar:tilt-build-1551202573"] = types.ImageInspect{ID: string(sha)}
cb := f.customBuild("exit 0")
refs, err := f.cb.Build(f.ctx, refSetWithRegistryFromString("foo/bar", TwoURLRegistry), cb.CmdImageSpec, nil)
refs, err := f.Build(refSetWithRegistryFromString("foo/bar", TwoURLRegistry), cb, nil)
require.NoError(t, err)

assert.Equal(f.t, container.MustParseNamed("localhost:1234/foo_bar:tilt-11cd0eb38bc3ceb9"), refs.LocalRef)
Expand All @@ -65,7 +70,7 @@ func TestCustomBuildSuccessClusterRefWithCustomTag(t *testing.T) {
f.dCli.Images["gcr.io/foo/bar:my-tag"] = types.ImageInspect{ID: string(sha)}
cb := f.customBuild("exit 0")
cb.CmdImageSpec.OutputTag = "my-tag"
refs, err := f.cb.Build(f.ctx, refSetWithRegistryFromString("gcr.io/foo/bar", TwoURLRegistry), cb.CmdImageSpec, nil)
refs, err := f.Build(refSetWithRegistryFromString("gcr.io/foo/bar", TwoURLRegistry), cb, nil)
require.NoError(t, err)

assert.Equal(f.t, container.MustParseNamed("localhost:1234/gcr.io_foo_bar:tilt-11cd0eb38bc3ceb9"), refs.LocalRef)
Expand All @@ -77,7 +82,7 @@ func TestCustomBuildSuccessSkipsLocalDocker(t *testing.T) {

cb := f.customBuild("exit 0")
cb.CmdImageSpec.OutputMode = v1alpha1.CmdImageOutputRemote
refs, err := f.cb.Build(f.ctx, refSetFromString("gcr.io/foo/bar"), cb.CmdImageSpec, nil)
refs, err := f.Build(refSetFromString("gcr.io/foo/bar"), cb, nil)
require.NoError(f.t, err)

assert.Equal(f.t, container.MustParseNamed("gcr.io/foo/bar:tilt-build-1551202573"), refs.LocalRef)
Expand All @@ -89,7 +94,7 @@ func TestCustomBuildSuccessClusterRefTaggedIfSkipsLocalDocker(t *testing.T) {

cb := f.customBuild("exit 0")
cb.CmdImageSpec.OutputMode = v1alpha1.CmdImageOutputRemote
refs, err := f.cb.Build(f.ctx, refSetWithRegistryFromString("foo/bar", TwoURLRegistry), cb.CmdImageSpec, nil)
refs, err := f.Build(refSetWithRegistryFromString("foo/bar", TwoURLRegistry), cb, nil)
require.NoError(f.t, err)

assert.Equal(f.t, container.MustParseNamed("localhost:1234/foo_bar:tilt-build-1551202573"), refs.LocalRef)
Expand All @@ -100,16 +105,16 @@ func TestCustomBuildCmdFails(t *testing.T) {
f := newFakeCustomBuildFixture(t)

cb := f.customBuild("exit 1")
_, err := f.cb.Build(f.ctx, refSetFromString("gcr.io/foo/bar"), cb.CmdImageSpec, nil)
_, err := f.Build(refSetFromString("gcr.io/foo/bar"), cb, nil)
// TODO(dmiller) better error message
assert.EqualError(t, err, "Custom build command failed: exit status 1")
assert.EqualError(t, err, "Custom build \"exit 1\" failed: exit status 1")
}

func TestCustomBuildImgNotFound(t *testing.T) {
f := newFakeCustomBuildFixture(t)

cb := f.customBuild("exit 0")
_, err := f.cb.Build(f.ctx, refSetFromString("gcr.io/foo/bar"), cb.CmdImageSpec, nil)
_, err := f.Build(refSetFromString("gcr.io/foo/bar"), cb, nil)
assert.Contains(t, err.Error(), "fake docker client error: object not found")
}

Expand All @@ -121,7 +126,7 @@ func TestCustomBuildExpectedTag(t *testing.T) {

cb := f.customBuild("exit 0")
cb.CmdImageSpec.OutputTag = "the-tag"
refs, err := f.cb.Build(f.ctx, refSetFromString("gcr.io/foo/bar"), cb.CmdImageSpec, nil)
refs, err := f.Build(refSetFromString("gcr.io/foo/bar"), cb, nil)
require.NoError(t, err)

assert.Equal(f.t, container.MustParseNamed("gcr.io/foo/bar:tilt-11cd0eb38bc3ceb9"), refs.LocalRef)
Expand All @@ -140,7 +145,7 @@ func TestCustomBuilderExecsRelativeToTiltfile(t *testing.T) {
f.dCli.Images["gcr.io/foo/bar:tilt-build-1551202573"] = types.ImageInspect{ID: string(sha)}
cb := f.customBuild("./build.sh")
cb.CmdImageSpec.Dir = filepath.Join(f.Path(), "proj")
refs, err := f.cb.Build(f.ctx, refSetFromString("gcr.io/foo/bar"), cb.CmdImageSpec, nil)
refs, err := f.Build(refSetFromString("gcr.io/foo/bar"), cb, nil)
if err != nil {
f.t.Fatal(err)
}
Expand All @@ -156,7 +161,7 @@ func TestCustomBuildOutputsToImageRefSuccess(t *testing.T) {
f.dCli.Images[myTag] = types.ImageInspect{ID: string(sha)}
cb := f.customBuild("echo gcr.io/foo/bar:dev > ref.txt")
cb.CmdImageSpec.OutputsImageRefTo = f.JoinPath("ref.txt")
refs, err := f.cb.Build(f.ctx, refSetFromString("gcr.io/foo/bar"), cb.CmdImageSpec, nil)
refs, err := f.Build(refSetFromString("gcr.io/foo/bar"), cb, nil)
require.NoError(t, err)

assert.Equal(f.t, container.MustParseNamed(myTag), refs.LocalRef)
Expand All @@ -169,7 +174,7 @@ func TestCustomBuildOutputsToImageRefMissingImage(t *testing.T) {
myTag := "gcr.io/foo/bar:dev"
cb := f.customBuild(fmt.Sprintf("echo %s > ref.txt", myTag))
cb.CmdImageSpec.OutputsImageRefTo = f.JoinPath("ref.txt")
_, err := f.cb.Build(f.ctx, refSetFromString("gcr.io/foo/bar"), cb.CmdImageSpec, nil)
_, err := f.Build(refSetFromString("gcr.io/foo/bar"), cb, nil)
require.NotNil(t, err)
assert.Contains(t, err.Error(),
fmt.Sprintf("fake docker client error: object not found (fakeClient.Images key: %s)", myTag))
Expand All @@ -180,7 +185,7 @@ func TestCustomBuildOutputsToImageRefMalformedImage(t *testing.T) {

cb := f.customBuild("echo 999 > ref.txt")
cb.CmdImageSpec.OutputsImageRefTo = f.JoinPath("ref.txt")
_, err := f.cb.Build(f.ctx, refSetFromString("gcr.io/foo/bar"), cb.CmdImageSpec, nil)
_, err := f.Build(refSetFromString("gcr.io/foo/bar"), cb, nil)
require.NotNil(t, err)
assert.Contains(t, err.Error(),
fmt.Sprintf("Output image ref in file %s was invalid: Expected reference \"999\" to contain a tag",
Expand All @@ -194,7 +199,7 @@ func TestCustomBuildOutputsToImageRefSkipsLocalDocker(t *testing.T) {
cb := f.customBuild(fmt.Sprintf("echo %s > ref.txt", myTag))
cb.CmdImageSpec.OutputsImageRefTo = f.JoinPath("ref.txt")
cb.CmdImageSpec.OutputMode = v1alpha1.CmdImageOutputRemote
refs, err := f.cb.Build(f.ctx, refSetFromString("gcr.io/foo/bar"), cb.CmdImageSpec, nil)
refs, err := f.Build(refSetFromString("gcr.io/foo/bar"), cb, nil)
require.NoError(t, err)
assert.Equal(f.t, container.MustParseNamed(myTag), refs.LocalRef)
assert.Equal(f.t, container.MustParseNamed(myTag), refs.ClusterRef)
Expand All @@ -210,7 +215,7 @@ func TestCustomBuildOutputsToImageRef_DifferentClusterHost(t *testing.T) {
cb := f.customBuild(fmt.Sprintf("echo %s > ref.txt", myTag))
cb.CmdImageSpec.OutputsImageRefTo = f.JoinPath("ref.txt")
reg := &v1alpha1.RegistryHosting{Host: "localhost:5000", HostFromContainerRuntime: "registry:5000"}
refs, err := f.cb.Build(f.ctx, refSetWithRegistryFromString("localhost:5000/foo/bar", reg), cb.CmdImageSpec, nil)
refs, err := f.Build(refSetWithRegistryFromString("localhost:5000/foo/bar", reg), cb, nil)
require.NoError(t, err)
assert.Equal(f.t, container.MustParseNamed(myTag), refs.LocalRef)
assert.Equal(f.t, container.MustParseNamed(myClusterTag), refs.ClusterRef)
Expand All @@ -236,7 +241,7 @@ func TestCustomBuildImageDep(t *testing.T) {
},
}

_, err := f.cb.Build(f.ctx, refSetFromString("gcr.io/foo/bar"), cb.CmdImageSpec, imageMaps)
_, err := f.Build(refSetFromString("gcr.io/foo/bar"), cb, imageMaps)
require.NoError(t, err)

assert.Equal(f.t, "base:tilt-12345", strings.TrimSpace(f.ReadFile("image-0.txt")))
Expand Down Expand Up @@ -265,7 +270,7 @@ func TestEnvVars(t *testing.T) {
sha := digest.Digest("sha256:11cd0eb38bc3ceb958ffb2f9bd70be3fb317ce7d255c8a4c3f4af30e298aa1aab")
f.dCli.Images["localhost:1234/foo_bar:tilt-build-1551202573"] = types.ImageInspect{ID: string(sha)}
cb := f.customBuild(strings.Join(script, "\n"))
_, err := f.cb.Build(f.ctx, refSetWithRegistryFromString("foo/bar", TwoURLRegistry), cb.CmdImageSpec, nil)
_, err := f.Build(refSetWithRegistryFromString("foo/bar", TwoURLRegistry), cb, nil)
require.NoError(t, err)
}

Expand Down Expand Up @@ -298,7 +303,7 @@ func TestEnvVars_ConfigRefWithLocalRegistry(t *testing.T) {
sha := digest.Digest("sha256:11cd0eb38bc3ceb958ffb2f9bd70be3fb317ce7d255c8a4c3f4af30e298aa1aab")
f.dCli.Images["localhost:1234/foo/bar:tilt-build-1551202573"] = types.ImageInspect{ID: string(sha)}
cb := f.customBuild(strings.Join(script, "\n"))
_, err := f.cb.Build(f.ctx, refSetWithRegistryFromString("localhost:1234/foo/bar", TwoURLRegistry), cb.CmdImageSpec, nil)
_, err := f.Build(refSetWithRegistryFromString("localhost:1234/foo/bar", TwoURLRegistry), cb, nil)
require.NoError(t, err)
}

Expand All @@ -318,7 +323,13 @@ func newFakeCustomBuildFixture(t *testing.T) *fakeCustomBuildFixture {
now: time.Unix(1551202573, 0),
}

cb := NewCustomBuilder(dCli, clock)
ctrlClient := fake.NewFakeTiltClient()
fe := cmd.NewProcessExecer(localexec.EmptyEnv())
fpm := cmd.NewFakeProberManager()
cclock := clockwork.NewFakeClock()
st := store.NewTestingStore()
cmds := cmd.NewController(ctx, fe, fpm, ctrlClient, st, cclock, v1alpha1.NewScheme())
cb := NewCustomBuilder(dCli, clock, cmds)

return &fakeCustomBuildFixture{
TempDirFixture: tempdir.NewTempDirFixture(t),
Expand All @@ -338,6 +349,16 @@ func (f *fakeCustomBuildFixture) customBuild(args string) model.CustomBuild {
}
}

func (f *fakeCustomBuildFixture) Build(refs container.RefSet, cb model.CustomBuild, imageMaps map[ktypes.NamespacedName]*v1alpha1.ImageMap) (container.TaggedRefs, error) {
return f.cb.Build(f.ctx, refs, cb.CmdImageSpec, &v1alpha1.Cmd{
ObjectMeta: metav1.ObjectMeta{Name: "img"},
Spec: v1alpha1.CmdSpec{
Args: cb.CmdImageSpec.Args,
Dir: cb.CmdImageSpec.Dir,
},
}, imageMaps)
}

func refSetFromString(s string) container.RefSet {
sel := container.MustParseSelector(s)
return container.MustSimpleRefSet(sel)
Expand Down
6 changes: 4 additions & 2 deletions internal/build/image_builder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,11 @@ func (ib *ImageBuilder) CanReuseRef(ctx context.Context, iTarget model.ImageTarg
// The error is simply the "main" build failure reason.
func (ib *ImageBuilder) Build(ctx context.Context,
iTarget model.ImageTarget,
customBuildCmd *v1alpha1.Cmd,
cluster *v1alpha1.Cluster,
imageMaps map[types.NamespacedName]*v1alpha1.ImageMap,
ps *PipelineState) (container.TaggedRefs, []v1alpha1.DockerImageStageStatus, error) {
refs, stages, err := ib.buildOnly(ctx, iTarget, cluster, imageMaps, ps)
refs, stages, err := ib.buildOnly(ctx, iTarget, customBuildCmd, cluster, imageMaps, ps)
if err != nil {
return refs, stages, err
}
Expand All @@ -74,6 +75,7 @@ func (ib *ImageBuilder) Build(ctx context.Context,
// Build the image, but don't do any push.
func (ib *ImageBuilder) buildOnly(ctx context.Context,
iTarget model.ImageTarget,
customBuildCmd *v1alpha1.Cmd,
cluster *v1alpha1.Cluster,
imageMaps map[types.NamespacedName]*v1alpha1.ImageMap,
ps *PipelineState,
Expand All @@ -99,7 +101,7 @@ func (ib *ImageBuilder) buildOnly(ctx context.Context,
case model.CustomBuild:
ps.StartPipelineStep(ctx, "Building Custom Build: [%s]", userFacingRefName)
defer ps.EndPipelineStep(ctx)
refs, err := ib.custb.Build(ctx, refs, bd.CmdImageSpec, imageMaps)
refs, err := ib.custb.Build(ctx, refs, bd.CmdImageSpec, customBuildCmd, imageMaps)
return refs, nil, err
}

Expand Down
18 changes: 9 additions & 9 deletions internal/cli/wire_gen.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit e2ad6ab

Please sign in to comment.