Logstream: ensure all failed targets are marked as such (#3951)

A missing func call was leading to some target statuses not being updated.
earthly · Mar 28, 2024 · 5eef69b · 5eef69b
1 parent 9822b73
commit 5eef69b
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 9 deletions.
diff --git a/earthfile2llb/converter.go b/earthfile2llb/converter.go
@@ -1862,12 +1862,11 @@ func (c *Converter) FinalizeStates(ctx context.Context) (*states.MultiTarget, er
 		if c.ftrs.ExecAfterParallel {
 			err = c.forceExecution(ctx, c.mts.Final.MainState, c.mts.Final.PlatformResolver)
 			if err != nil {
+				c.RecordTargetFailure(ctx, err)
 				return errors.Wrapf(err, "async force execution for %s", c.mts.FinalTarget().String())
 			}
 		}
-		now := time.Now()
-		st := logstream.RunStatus_RUN_STATUS_SUCCESS
-		c.logbusTarget.SetEnd(now, st, c.platr.Current().String())
+		c.logbusTarget.SetEnd(time.Now(), logstream.RunStatus_RUN_STATUS_SUCCESS, c.platr.Current().String())
 		return nil
 	})
 	return c.mts, nil

diff --git a/earthfile2llb/earthfile2llb.go b/earthfile2llb/earthfile2llb.go
@@ -342,12 +342,24 @@ func Earthfile2LLB(ctx context.Context, target domain.Target, opt ConvertOpt, in
 		}
 		opt.MainTargetDetailsFunc = nil
 	}
-	opt.Console.VerbosePrintf("earthfile2llb building %s with OverridingVars=%v", targetWithMetadata.StringCanonical(), opt.OverridingVars.Map())
+
+	opt.Console.VerbosePrintf("earthfile2llb building %s with OverridingVars=%v",
+		targetWithMetadata.StringCanonical(), opt.OverridingVars.Map())
+
 	converter, err := NewConverter(ctx, targetWithMetadata, bc, sts, opt)
 	if err != nil {
 		return nil, err
 	}
-	interpreter := newInterpreter(converter, targetWithMetadata, opt.AllowPrivileged, opt.ParallelConversion, opt.Console, opt.GitLookup)
+
+	interpreter := newInterpreter(
+		converter,
+		targetWithMetadata,
+		opt.AllowPrivileged,
+		opt.ParallelConversion,
+		opt.Console,
+		opt.GitLookup,
+	)
+
 	err = interpreter.Run(ctx, bc.Earthfile)
 	if err != nil {
 		return nil, err
@@ -372,5 +384,6 @@ func Earthfile2LLB(ctx context.Context, target domain.Target, opt ConvertOpt, in
 			return nil, err
 		}
 	}
+
 	return mts, nil
 }
diff --git a/earthfile2llb/interpreter.go b/earthfile2llb/interpreter.go
@@ -68,10 +68,10 @@ func newInterpreter(c *Converter, t domain.Target, allowPrivileged, parallelConv
 }
 
 // Run interprets the commands in the given Earthfile AST, for a specific target.
-func (i *Interpreter) Run(ctx context.Context, ef spec.Earthfile) (err error) {
+func (i *Interpreter) Run(ctx context.Context, ef spec.Earthfile) (retErr error) {
 	defer func() {
-		if err != nil {
-			i.converter.RecordTargetFailure(ctx, err)
+		if retErr != nil {
+			i.converter.RecordTargetFailure(ctx, retErr)
 		}
 	}()
 	if i.target.Target == "base" {
@@ -88,7 +88,7 @@ func (i *Interpreter) Run(ctx context.Context, ef spec.Earthfile) (err error) {
 	return i.errorf(ef.SourceLocation, "target %s not found", i.target.Target)
 }
 
-func (i *Interpreter) isPipelineTarget(ctx context.Context, t spec.Target) bool {
+func (i *Interpreter) isPipelineTarget(_ context.Context, t spec.Target) bool {
 	for _, stmt := range t.Recipe {
 		if stmt.Command != nil && stmt.Command.Name == "PIPELINE" {
 			return true