From 517c3ac4675468417eeeecb7bf4c86f4bef373fb Mon Sep 17 00:00:00 2001 From: ChristopherHX Date: Sat, 29 Mar 2025 17:41:42 +0100 Subject: [PATCH] fix: reporting fetch failure as job error and log the error (#2715) E.g. if GoGitAction Cache had a fetch failure this error did not trigger report jobResult Failure. Also the error has been not printed until the last message before exit of act. * adds tests for both corner cases --- pkg/runner/job_executor.go | 28 ++-- pkg/runner/runner_test.go | 133 ++++++++++++++++++ .../push.yml | 8 ++ 3 files changed, 156 insertions(+), 13 deletions(-) create mode 100644 pkg/runner/testdata/action-cache-v2-fetch-failure-is-job-error/push.yml diff --git a/pkg/runner/job_executor.go b/pkg/runner/job_executor.go index e3ae2be..ed6e43f 100644 --- a/pkg/runner/job_executor.go +++ b/pkg/runner/job_executor.go @@ -53,6 +53,16 @@ func newJobExecutor(info jobInfo, sf stepFactory, rc *RunContext) common.Executo return nil }) + var setJobError = func(ctx context.Context, err error) error { + if err == nil { + return nil + } + logger := common.Logger(ctx) + logger.Errorf("%v", err) + common.SetJobError(ctx, err) + return err + } + for i, stepModel := range infoSteps { if stepModel == nil { return func(_ context.Context) error { @@ -69,18 +79,15 @@ func newJobExecutor(info jobInfo, sf stepFactory, rc *RunContext) common.Executo return common.NewErrorExecutor(err) } - preSteps = append(preSteps, useStepLogger(rc, stepModel, stepStagePre, step.pre())) + preSteps = append(preSteps, useStepLogger(rc, stepModel, stepStagePre, step.pre().ThenError(setJobError))) stepExec := step.main() steps = append(steps, useStepLogger(rc, stepModel, stepStageMain, func(ctx context.Context) error { - logger := common.Logger(ctx) err := stepExec(ctx) if err != nil { - logger.Errorf("%v", err) - common.SetJobError(ctx, err) + _ = setJobError(ctx, err) } else if ctx.Err() != nil { - logger.Errorf("%v", ctx.Err()) - common.SetJobError(ctx, ctx.Err()) + _ = setJobError(ctx, ctx.Err()) } return nil })) @@ -118,11 +125,6 @@ func newJobExecutor(info jobInfo, sf stepFactory, rc *RunContext) common.Executo return nil } - var setJobError = func(ctx context.Context, err error) error { - common.SetJobError(ctx, err) - return nil - } - pipeline := make([]common.Executor, 0) pipeline = append(pipeline, preSteps...) pipeline = append(pipeline, steps...) @@ -131,7 +133,7 @@ func newJobExecutor(info jobInfo, sf stepFactory, rc *RunContext) common.Executo common.NewFieldExecutor("step", "Set up job", common.NewFieldExecutor("stepid", []string{"--setup-job"}, common.NewPipelineExecutor(common.NewInfoExecutor("\u2B50 Run Set up job"), info.startContainer(), rc.InitializeNodeTool()). Then(common.NewFieldExecutor("stepResult", model.StepStatusSuccess, common.NewInfoExecutor(" \u2705 Success - Set up job"))). - OnError(common.NewFieldExecutor("stepResult", model.StepStatusFailure, common.NewInfoExecutor(" \u274C Failure - Set up job")).ThenError(setJobError)))), + ThenError(setJobError).OnError(common.NewFieldExecutor("stepResult", model.StepStatusFailure, common.NewInfoExecutor(" \u274C Failure - Set up job"))))), common.NewPipelineExecutor(pipeline...). Finally(func(ctx context.Context) error { //nolint:contextcheck var cancel context.CancelFunc @@ -149,7 +151,7 @@ func newJobExecutor(info jobInfo, sf stepFactory, rc *RunContext) common.Executo Finally( info.interpolateOutputs().Finally(info.closeContainer()).Then(common.NewFieldExecutor("stepResult", model.StepStatusSuccess, common.NewInfoExecutor(" \u2705 Success - Complete job"))). OnError(common.NewFieldExecutor("stepResult", model.StepStatusFailure, common.NewInfoExecutor(" \u274C Failure - Complete job"))), - )))).Finally(setJobResultExecutor)) + ))))).Finally(setJobResultExecutor) } func setJobResult(ctx context.Context, info jobInfo, rc *RunContext, success bool) { diff --git a/pkg/runner/runner_test.go b/pkg/runner/runner_test.go index cac1b83..a6eb46b 100644 --- a/pkg/runner/runner_test.go +++ b/pkg/runner/runner_test.go @@ -1,8 +1,10 @@ package runner import ( + "bufio" "bytes" "context" + "encoding/json" "fmt" "io" "os" @@ -13,6 +15,7 @@ import ( "testing" "github.com/joho/godotenv" + "github.com/sirupsen/logrus" log "github.com/sirupsen/logrus" assert "github.com/stretchr/testify/assert" "gopkg.in/yaml.v3" @@ -359,6 +362,136 @@ func TestRunEvent(t *testing.T) { } } +type captureJobLoggerFactory struct { + buffer bytes.Buffer +} + +func (factory *captureJobLoggerFactory) WithJobLogger() *logrus.Logger { + logger := logrus.New() + logger.SetOutput(&factory.buffer) + logger.SetLevel(log.TraceLevel) + logger.SetFormatter(&log.JSONFormatter{}) + return logger +} + +func TestPullFailureIsJobFailure(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test") + } + + tables := []TestJobFileInfo{ + {workdir, "checkout", "push", "pull failure", map[string]string{"ubuntu-latest": "localhost:0000/missing:latest"}, secrets}, + } + + for _, table := range tables { + t.Run(table.workflowPath, func(t *testing.T) { + factory := &captureJobLoggerFactory{} + + config := &Config{ + Secrets: table.secrets, + } + + eventFile := filepath.Join(workdir, table.workflowPath, "event.json") + if _, err := os.Stat(eventFile); err == nil { + config.EventPath = eventFile + } + config.ActionCache = &GoGitActionCache{ + path.Clean(path.Join(workdir, "cache")), + } + + logger := logrus.New() + logger.SetOutput(&factory.buffer) + logger.SetLevel(log.TraceLevel) + logger.SetFormatter(&log.JSONFormatter{}) + + table.runTest(common.WithLogger(WithJobLoggerFactory(t.Context(), factory), logger), t, config) + scan := bufio.NewScanner(&factory.buffer) + var hasJobResult, hasStepResult bool + for scan.Scan() { + t.Log(scan.Text()) + entry := map[string]interface{}{} + if json.Unmarshal(scan.Bytes(), &entry) == nil { + if val, ok := entry["jobResult"]; ok { + assert.Equal(t, "failure", val) + hasJobResult = true + } + if val, ok := entry["stepResult"]; ok && !hasStepResult { + assert.Equal(t, "failure", val) + hasStepResult = true + } + } + } + assert.True(t, hasStepResult, "stepResult not found") + assert.True(t, hasJobResult, "jobResult not found") + }) + } +} + +type mockCache struct { +} + +func (c mockCache) Fetch(ctx context.Context, cacheDir string, url string, ref string, token string) (string, error) { + _ = ctx + _ = cacheDir + _ = url + _ = ref + _ = token + return "", fmt.Errorf("fetch failure") +} +func (c mockCache) GetTarArchive(ctx context.Context, cacheDir string, sha string, includePrefix string) (io.ReadCloser, error) { + _ = ctx + _ = cacheDir + _ = sha + _ = includePrefix + return nil, fmt.Errorf("fetch failure") +} + +func TestFetchFailureIsJobFailure(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test") + } + + tables := []TestJobFileInfo{ + {workdir, "action-cache-v2-fetch-failure-is-job-error", "push", "fetch failure", map[string]string{"ubuntu-latest": "-self-hosted"}, secrets}, + } + + for _, table := range tables { + t.Run(table.workflowPath, func(t *testing.T) { + factory := &captureJobLoggerFactory{} + + config := &Config{ + Secrets: table.secrets, + } + + eventFile := filepath.Join(workdir, table.workflowPath, "event.json") + if _, err := os.Stat(eventFile); err == nil { + config.EventPath = eventFile + } + config.ActionCache = &mockCache{} + + logger := logrus.New() + logger.SetOutput(&factory.buffer) + logger.SetLevel(log.TraceLevel) + logger.SetFormatter(&log.JSONFormatter{}) + + table.runTest(common.WithLogger(WithJobLoggerFactory(t.Context(), factory), logger), t, config) + scan := bufio.NewScanner(&factory.buffer) + var hasJobResult bool + for scan.Scan() { + t.Log(scan.Text()) + entry := map[string]interface{}{} + if json.Unmarshal(scan.Bytes(), &entry) == nil { + if val, ok := entry["jobResult"]; ok { + assert.Equal(t, "failure", val) + hasJobResult = true + } + } + } + assert.True(t, hasJobResult, "jobResult not found") + }) + } +} + func TestRunEventHostEnvironment(t *testing.T) { if testing.Short() { t.Skip("skipping integration test") diff --git a/pkg/runner/testdata/action-cache-v2-fetch-failure-is-job-error/push.yml b/pkg/runner/testdata/action-cache-v2-fetch-failure-is-job-error/push.yml new file mode 100644 index 0000000..a5c93db --- /dev/null +++ b/pkg/runner/testdata/action-cache-v2-fetch-failure-is-job-error/push.yml @@ -0,0 +1,8 @@ +name: basic +on: push + +jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: nektos/test-override@a