build: RunStepError rename, store more info (#1936)

internal/build/error.go

@@ -23,25 +23,29 @@ func WrapContainerExecError(err error, cID container.ID, cmd model.Cmd) error {
 			return fmt.Errorf("executing %v on container %s: killed by container engine", cmd, cID.ShortStr())
 		}
 
-		return UserBuildFailure{ExitCode: exitErr.ExitCode}
+		return RunStepFailure{
+			Cmd:      cmd,
+			ExitCode: exitErr.ExitCode,
+		}
 	}
 
 	return errors.Wrapf(err, "executing %v on container %s", cmd, cID.ShortStr())
 }
 
-// Indicates that the build failed because the user script failed
-// (as opposed to an infrastructure issue).
-type UserBuildFailure struct {
+// Indicates that the update failed because one of the user's Runs failed
+// (i.e. exited non-zero) -- as opposed to an infrastructure issue.
+type RunStepFailure struct {
+	Cmd      model.Cmd
 	ExitCode int
 }
 
-func (e UserBuildFailure) Error() string {
-	return fmt.Sprintf("Command failed with exit code: %d", e.ExitCode)
+func (e RunStepFailure) Error() string {
+	return fmt.Sprintf("Run step %q failed with exit code: %d", e.Cmd.String(), e.ExitCode)
 }
 
-func IsUserBuildFailure(err error) bool {
-	_, ok := err.(UserBuildFailure)
+func IsRunStepFailure(err error) bool {
+	_, ok := err.(RunStepFailure)
 	return ok
 }
 
-var _ error = UserBuildFailure{}
+var _ error = RunStepFailure{}

internal/container/container.go

@@ -2,6 +2,7 @@ package container
 
 import (
 	"fmt"
+	"strings"
 
 	"github.com/docker/distribution/reference"
 	"github.com/pkg/errors"
@@ -19,6 +20,14 @@ func (id ID) ShortStr() string {
 	return string(id)
 }
 
+func ShortStrs(ids []ID) string {
+	shortStrs := make([]string, len(ids))
+	for i, id := range ids {
+		shortStrs[i] = id.ShortStr()
+	}
+	return strings.Join(shortStrs, ", ")
+}
+
 func (n Name) String() string { return string(n) }
 
 func ParseNamed(s string) (reference.Named, error) {

internal/containerupdate/synclet_updater.go

@@ -38,7 +38,7 @@ func (cu *SyncletUpdater) UpdateContainer(ctx context.Context, cInfo store.Conta
 	}
 
 	err = sCli.UpdateContainer(ctx, cInfo.ContainerID, archiveBytes, filesToDelete, cmds, hotReload)
-	if err != nil && build.IsUserBuildFailure(err) {
+	if err != nil && build.IsRunStepFailure(err) {
 		return err
 	}
 	return nil

internal/engine/build_and_deployer_test.go

@@ -372,7 +372,7 @@ func TestIncrementalBuildFailure(t *testing.T) {
 	manifest := NewSanchoFastBuildManifest(f)
 	targets := buildTargets(manifest)
 	_, err := f.bd.BuildAndDeploy(f.ctx, f.st, targets, bs)
-	msg := "Command failed with exit code: 1"
+	msg := "Run step \"go install github.com/windmilleng/sancho\" failed with exit code: 1"
 	if err == nil || !strings.Contains(err.Error(), msg) {
 		t.Fatalf("Expected error message %q, actual: %v", msg, err)
 	}

internal/engine/live_update_build_and_deployer.go

@@ -124,8 +124,9 @@ func (lubad *LiveUpdateBuildAndDeployer) BuildAndDeploy(ctx context.Context, st
 func (lubad *LiveUpdateBuildAndDeployer) buildAndDeploy(ctx context.Context, cu containerupdate.ContainerUpdater, iTarget model.ImageTarget, state store.BuildState, changedFiles []build.PathMapping, runs []model.Run, hotReload bool) error {
 	l := logger.Get(ctx)
 	l.Infof("  → Updating container…")
-
 	cInfo := state.OneContainerInfo()
+	cIDStr := cInfo.ContainerID.ShortStr()
+
 	filter := ignore.CreateBuildContextFilter(iTarget)
 	boiledSteps, err := build.BoilRuns(runs, changedFiles)
 	if err != nil {
@@ -139,7 +140,7 @@ func (lubad *LiveUpdateBuildAndDeployer) buildAndDeploy(ctx context.Context, cu
 	}
 
 	if len(toRemove) > 0 {
-		l.Infof("Will delete %d file(s) from container: %s", len(toRemove), cInfo.ContainerID.ShortStr())
+		l.Infof("Will delete %d file(s) from container(s): %s", len(toRemove), cIDStr)
 		for _, pm := range toRemove {
 			l.Infof("- '%s' (matched local path: '%s')", pm.ContainerPath, pm.LocalPath)
 		}
@@ -159,15 +160,15 @@ func (lubad *LiveUpdateBuildAndDeployer) buildAndDeploy(ctx context.Context, cu
 	}()
 
 	if len(toArchive) > 0 {
-		l.Infof("Will copy %d file(s) to container: %s", len(toArchive), cInfo.ContainerID.ShortStr())
+		l.Infof("Will copy %d file(s) to container(s): %s", len(toArchive), cIDStr)
 		for _, pm := range toArchive {
 			l.Infof("- %s", pm.PrettyStr())
 		}
 	}
 
 	err = cu.UpdateContainer(ctx, cInfo, pr, build.PathMappingsToContainerPaths(toRemove), boiledSteps, hotReload)
 	if err != nil {
-		if build.IsUserBuildFailure(err) {
+		if build.IsRunStepFailure(err) {
 			return WrapDontFallBackError(err)
 		}
 		return err

internal/engine/live_update_build_and_deployer_test.go

@@ -100,7 +100,7 @@ func TestDontFallBackOnUserError(t *testing.T) {
 	f := newFixture(t)
 	defer f.teardown()
 
-	f.cu.UpdateErr = build.UserBuildFailure{ExitCode: 12345}
+	f.cu.UpdateErr = build.RunStepFailure{ExitCode: 12345}
 
 	err := f.lubad.buildAndDeploy(f.ctx, f.cu, model.ImageTarget{}, TestBuildState, nil, nil, false)
 	if assert.NotNil(t, err) {

internal/store/build_result.go

@@ -212,6 +212,14 @@ func (c ContainerInfo) Empty() bool {
 	return c == ContainerInfo{}
 }
 
+func IDsForInfos(infos []ContainerInfo) []container.ID {
+	ids := make([]container.ID, len(infos))
+	for i, info := range infos {
+		ids[i] = info.ContainerID
+	}
+	return ids
+}
+
 // If all containers running the given image are ready, returns info for them.
 // (Currently only supports containers running on a single pod.)
 func RunningContainersForTarget(iTarget model.ImageTarget, deployID model.DeployID, podSet PodSet) []ContainerInfo {