From 22bfc2a1c318f0af8b55cb99f607ec9606b40fb2 Mon Sep 17 00:00:00 2001 From: Maia McCormick Date: Tue, 30 Jul 2019 13:57:50 -0400 Subject: [PATCH] build: RunStepError rename, store more info (#1936) --- internal/build/error.go | 22 +++++++++++-------- internal/container/container.go | 9 ++++++++ internal/containerupdate/synclet_updater.go | 2 +- internal/engine/build_and_deployer_test.go | 2 +- .../engine/live_update_build_and_deployer.go | 9 ++++---- .../live_update_build_and_deployer_test.go | 2 +- internal/store/build_result.go | 8 +++++++ 7 files changed, 38 insertions(+), 16 deletions(-) diff --git a/internal/build/error.go b/internal/build/error.go index d1e3f8679c..6f7bd01a25 100644 --- a/internal/build/error.go +++ b/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{} diff --git a/internal/container/container.go b/internal/container/container.go index 74f8e4892b..744fa43cca 100644 --- a/internal/container/container.go +++ b/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) { diff --git a/internal/containerupdate/synclet_updater.go b/internal/containerupdate/synclet_updater.go index 06c4ad0106..21f9436942 100644 --- a/internal/containerupdate/synclet_updater.go +++ b/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 diff --git a/internal/engine/build_and_deployer_test.go b/internal/engine/build_and_deployer_test.go index 388bf8ba17..fe3d088736 100644 --- a/internal/engine/build_and_deployer_test.go +++ b/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) } diff --git a/internal/engine/live_update_build_and_deployer.go b/internal/engine/live_update_build_and_deployer.go index f57ddb1d37..2b02b3162d 100644 --- a/internal/engine/live_update_build_and_deployer.go +++ b/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,7 +160,7 @@ 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()) } @@ -167,7 +168,7 @@ func (lubad *LiveUpdateBuildAndDeployer) buildAndDeploy(ctx context.Context, cu 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 diff --git a/internal/engine/live_update_build_and_deployer_test.go b/internal/engine/live_update_build_and_deployer_test.go index a2b9000314..5dbeb888af 100644 --- a/internal/engine/live_update_build_and_deployer_test.go +++ b/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) { diff --git a/internal/store/build_result.go b/internal/store/build_result.go index 1b5dfa490d..e89584fc4a 100644 --- a/internal/store/build_result.go +++ b/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 {