fix: enforce running all post executor

To make sure every post executor/step is executed, it is chained with it's own Finally executor.
xing · May 19, 2022 · 95bdb89 · 95bdb89
1 parent 28e739e
commit 95bdb89
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 8 deletions.
diff --git a/pkg/runner/action_composite.go b/pkg/runner/action_composite.go
@@ -123,7 +123,7 @@ type compositeSteps struct {
 func (rc *RunContext) compositeExecutor(action *model.Action) *compositeSteps {
 	steps := make([]common.Executor, 0)
 	preSteps := make([]common.Executor, 0)
-	postSteps := make([]common.Executor, 0)
+	var postExecutor common.Executor
 
 	sf := &stepFactoryImpl{}
 
@@ -157,7 +157,12 @@ func (rc *RunContext) compositeExecutor(action *model.Action) *compositeSteps {
 			return nil
 		})
 
-		postSteps = append([]common.Executor{step.post()}, postSteps...)
+		// run the post executor in reverse order
+		if postExecutor != nil {
+			postExecutor = step.post().Finally(postExecutor)
+		} else {
+			postExecutor = step.post()
+		}
 	}
 
 	steps = append(steps, common.JobError)
@@ -166,7 +171,7 @@ func (rc *RunContext) compositeExecutor(action *model.Action) *compositeSteps {
 		main: rc.newCompositeCommandExecutor(func(ctx context.Context) error {
 			return common.NewPipelineExecutor(steps...)(common.WithJobErrorContainer(ctx))
 		}),
-		post: rc.newCompositeCommandExecutor(common.NewPipelineExecutor(postSteps...)),
+		post: rc.newCompositeCommandExecutor(postExecutor),
 	}
 }
 

diff --git a/pkg/runner/job_executor.go b/pkg/runner/job_executor.go
@@ -21,7 +21,7 @@ type jobInfo interface {
 func newJobExecutor(info jobInfo, sf stepFactory, rc *RunContext) common.Executor {
 	steps := make([]common.Executor, 0)
 	preSteps := make([]common.Executor, 0)
-	postSteps := make([]common.Executor, 0)
+	var postExecutor common.Executor
 
 	steps = append(steps, func(ctx context.Context) error {
 		if len(info.matrix()) > 0 {
@@ -72,10 +72,15 @@ func newJobExecutor(info jobInfo, sf stepFactory, rc *RunContext) common.Executo
 			})(withStepLogger(ctx, stepName))
 		})
 
-		postSteps = append([]common.Executor{step.post()}, postSteps...)
+		// run the post exector in reverse order
+		if postExecutor != nil {
+			postExecutor = step.post().Finally(postExecutor)
+		} else {
+			postExecutor = step.post()
+		}
 	}
 
-	postSteps = append(postSteps, func(ctx context.Context) error {
+	postExecutor = postExecutor.Finally(func(ctx context.Context) error {
 		jobError := common.JobError(ctx)
 		if jobError != nil {
 			info.result("failure")
@@ -93,7 +98,9 @@ func newJobExecutor(info jobInfo, sf stepFactory, rc *RunContext) common.Executo
 	pipeline := make([]common.Executor, 0)
 	pipeline = append(pipeline, preSteps...)
 	pipeline = append(pipeline, steps...)
-	pipeline = append(pipeline, postSteps...)
 
-	return common.NewPipelineExecutor(pipeline...).Finally(info.interpolateOutputs()).Finally(info.closeContainer())
+	return common.NewPipelineExecutor(pipeline...).
+		Finally(postExecutor).
+		Finally(info.interpolateOutputs()).
+		Finally(info.closeContainer())
 }