From 513011d1bfaa0819fb08f2950d350ccba91fb76e Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sun, 22 Mar 2026 18:20:44 +0100 Subject: [PATCH] Forward skipped step state asap it is known (#6295) --- agent/tracer.go | 39 +++--- pipeline/backend/types/state.go | 2 + pipeline/runtime/executor.go | 28 ++--- pipeline/runtime/runtime_test.go | 179 +++++++++++++--------------- pipeline/state/state.go | 31 +++-- pipeline/tracing/tracer.go | 8 +- rpc/proto/version.go | 2 +- rpc/types.go | 1 + server/pipeline/step_status.go | 24 ++-- server/pipeline/step_status_test.go | 23 ++++ 10 files changed, 179 insertions(+), 158 deletions(-) diff --git a/agent/tracer.go b/agent/tracer.go index df4b0c814..836badfbf 100644 --- a/agent/tracer.go +++ b/agent/tracer.go @@ -36,24 +36,25 @@ func (r *Runner) createTracer(ctxMeta context.Context, uploads *sync.WaitGroup, defer uploads.Done() stepLogger := logger.With(). - Str("image", state.Pipeline.Step.Image). + Str("image", state.CurrStep.Image). Str("workflow_id", workflow.ID). - Err(state.Process.Error). - Int("exit_code", state.Process.ExitCode). - Bool("exited", state.Process.Exited). + Err(state.CurrStepState.Error). + Int("exit_code", state.CurrStepState.ExitCode). + Bool("exited", state.CurrStepState.Exited). Logger() stepState := rpc.StepState{ - StepUUID: state.Pipeline.Step.UUID, - Exited: state.Process.Exited, - ExitCode: state.Process.ExitCode, - Started: state.Process.Started, - Canceled: errors.Is(state.Process.Error, pipeline_errors.ErrCancel), + StepUUID: state.CurrStep.UUID, + Exited: state.CurrStepState.Exited, + ExitCode: state.CurrStepState.ExitCode, + Started: state.CurrStepState.Started, + Canceled: errors.Is(state.CurrStepState.Error, pipeline_errors.ErrCancel), + Skipped: state.CurrStepState.Skipped, } - if state.Process.Error != nil { - stepState.Error = state.Process.Error.Error() + if state.CurrStepState.Error != nil { + stepState.Error = state.CurrStepState.Error.Error() } - if state.Process.Exited { + if state.CurrStepState.Exited { stepState.Finished = time.Now().Unix() } @@ -68,21 +69,21 @@ func (r *Runner) createTracer(ctxMeta context.Context, uploads *sync.WaitGroup, stepLogger.Debug().Msg("update step status complete") }() - if state.Process.Exited { + if state.CurrStepState.Exited { return nil } - if state.Pipeline.Step.Environment == nil { - state.Pipeline.Step.Environment = map[string]string{} + if state.CurrStep.Environment == nil { + state.CurrStep.Environment = map[string]string{} } // TODO: find better way to update this state and move it to pipeline to have the same env in cli-exec - state.Pipeline.Step.Environment["CI_MACHINE"] = r.hostname + state.CurrStep.Environment["CI_MACHINE"] = r.hostname - state.Pipeline.Step.Environment["CI_PIPELINE_STARTED"] = strconv.FormatInt(state.Pipeline.Started, 10) + state.CurrStep.Environment["CI_PIPELINE_STARTED"] = strconv.FormatInt(state.Workflow.Started, 10) - state.Pipeline.Step.Environment["CI_STEP_STARTED"] = strconv.FormatInt(state.Pipeline.Started, 10) + state.CurrStep.Environment["CI_STEP_STARTED"] = strconv.FormatInt(state.Workflow.Started, 10) - state.Pipeline.Step.Environment["CI_SYSTEM_PLATFORM"] = runtime.GOOS + "/" + runtime.GOARCH + state.CurrStep.Environment["CI_SYSTEM_PLATFORM"] = runtime.GOOS + "/" + runtime.GOARCH return nil } diff --git a/pipeline/backend/types/state.go b/pipeline/backend/types/state.go index 9f4ff1b33..a1c2c1db1 100644 --- a/pipeline/backend/types/state.go +++ b/pipeline/backend/types/state.go @@ -22,6 +22,8 @@ type State struct { ExitCode int `json:"exit_code"` // Container exited, true or false Exited bool `json:"exited"` + // Step was skipped by the runtime (OnSuccess/OnFailure filter) + Skipped bool `json:"skipped"` // Container is oom killed, true or false // TODO (6024): well known errors as string enum into ./errors.go OOMKilled bool `json:"oom_killed"` diff --git a/pipeline/runtime/executor.go b/pipeline/runtime/executor.go index 8a363d531..80cc57ff9 100644 --- a/pipeline/runtime/executor.go +++ b/pipeline/runtime/executor.go @@ -60,9 +60,9 @@ func (r *Runtime) Run(runnerCtx context.Context) error { var stepErr *pipeline_errors.ErrInvalidWorkflowSetup if errors.As(err, &stepErr) { state := new(state.State) - state.Pipeline.Step = stepErr.Step - state.Pipeline.Error = stepErr.Err - state.Process = backend.State{ + state.CurrStep = stepErr.Step + state.Workflow.Error = stepErr.Err + state.CurrStepState = backend.State{ Error: stepErr.Err, Exited: true, ExitCode: 1, @@ -103,19 +103,19 @@ func (r *Runtime) traceStep(processState *backend.State, err error, step *backen } state := new(state.State) - state.Pipeline.Started = r.started - state.Pipeline.Step = step - state.Pipeline.Error = r.err.Get() + state.Workflow.Started = r.started + state.CurrStep = step + state.Workflow.Error = r.err.Get() // We have an error while starting the step if processState == nil && err != nil { - state.Process = backend.State{ + state.CurrStepState = backend.State{ Error: err, Exited: true, OOMKilled: false, } } else if processState != nil { - state.Process = *processState + state.CurrStepState = *processState } if traceErr := r.tracer.Trace(state); traceErr != nil { @@ -141,18 +141,18 @@ func (r *Runtime) execAll(runnerCtx context.Context, steps []*backend.Step) <-ch Str("step", step.Name). Msg("prepare") - switch rErr := r.err.Get(); { - case rErr != nil && !step.OnFailure: + rErr := r.err.Get() + if rErr != nil && !step.OnFailure { logger.Debug(). Str("step", step.Name). - Err(rErr). Msgf("skipped due to OnFailure=%t", step.OnFailure) - return nil - case rErr == nil && !step.OnSuccess: + return r.traceStep(&backend.State{Skipped: true}, nil, step) + } + if rErr == nil && !step.OnSuccess { logger.Debug(). Str("step", step.Name). Msgf("skipped due to OnSuccess=%t", step.OnSuccess) - return nil + return r.traceStep(&backend.State{Skipped: true}, nil, step) } // Trace started. diff --git a/pipeline/runtime/runtime_test.go b/pipeline/runtime/runtime_test.go index 67ef08a47..8778f61e8 100644 --- a/pipeline/runtime/runtime_test.go +++ b/pipeline/runtime/runtime_test.go @@ -17,11 +17,13 @@ package runtime import ( + "context" "errors" "fmt" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "go.woodpecker-ci.org/woodpecker/v3/pipeline/backend/dummy" @@ -29,6 +31,7 @@ import ( pipeline_errors "go.woodpecker-ci.org/woodpecker/v3/pipeline/errors" "go.woodpecker-ci.org/woodpecker/v3/pipeline/frontend/metadata" "go.woodpecker-ci.org/woodpecker/v3/pipeline/state" + tracer_mocks "go.woodpecker-ci.org/woodpecker/v3/pipeline/tracing/mocks" ) // @@ -106,7 +109,7 @@ func withStartFail() func(*backend.Step) { func findFirstTraceByName(traces []state.State, name string) *state.State { for i := range traces { - if traces[i].Pipeline.Step != nil && traces[i].Pipeline.Step.Name == name { + if traces[i].CurrStep != nil && traces[i].CurrStep.Name == name { return &traces[i] } } @@ -115,7 +118,7 @@ func findFirstTraceByName(traces []state.State, name string) *state.State { func findLastTraceByName(traces []state.State, name string) *state.State { for i := len(traces) - 1; i >= 0; i-- { - if traces[i].Pipeline.Step != nil && traces[i].Pipeline.Step.Name == name { + if traces[i].CurrStep != nil && traces[i].CurrStep.Name == name { return &traces[i] } } @@ -124,7 +127,7 @@ func findLastTraceByName(traces []state.State, name string) *state.State { func findStartedTrace(traces []state.State, name string) *state.State { for i := range traces { - if traces[i].Pipeline.Step != nil && traces[i].Pipeline.Step.Name == name && !traces[i].Process.Exited { + if traces[i].CurrStep != nil && traces[i].CurrStep.Name == name && !traces[i].CurrStepState.Exited { return &traces[i] } } @@ -157,17 +160,17 @@ func TestWorkflowCloneBuildDeploy(t *testing.T) { traces := getTracerStates(tracer) assert.Len(t, traces, 6) for i := 0; i < 6; i += 2 { - assert.False(t, traces[i].Process.Exited, "trace %d should be step-started", i) - assert.True(t, traces[i+1].Process.Exited, "trace %d should be step-completed", i+1) - assert.Equal(t, 0, traces[i+1].Process.ExitCode) + assert.False(t, traces[i].CurrStepState.Exited, "trace %d should be step-started", i) + assert.True(t, traces[i+1].CurrStepState.Exited, "trace %d should be step-completed", i+1) + assert.Equal(t, 0, traces[i+1].CurrStepState.ExitCode) } for _, name := range []string{"clone", "build", "deploy"} { last := findLastTraceByName(traces, name) require.NotNil(t, last, "%s should have a final trace", name) - assert.True(t, last.Process.Exited, "%s last trace should be exited", name) - assert.Equal(t, 0, last.Process.ExitCode, "%s should exit with code 0", name) - assert.False(t, last.Process.OOMKilled, "%s should not be OOM killed", name) + assert.True(t, last.CurrStepState.Exited, "%s last trace should be exited", name) + assert.Equal(t, 0, last.CurrStepState.ExitCode, "%s should exit with code 0", name) + assert.False(t, last.CurrStepState.OOMKilled, "%s should not be OOM killed", name) } } @@ -192,43 +195,38 @@ func TestWorkflowWithServiceStep(t *testing.T) { assert.NoError(t, r.Run(t.Context())) traces := getTracerStates(tracer) if assert.Len(t, traces, 5) { - assert.EqualValues(t, backend.State{}, traces[0].Process) - assert.Greater(t, traces[2].Process.Started, int64(0)) - assert.EqualValues(t, backend.State{Started: traces[2].Process.Started, Exited: true}, traces[2].Process) - assert.EqualValues(t, backend.State{}, traces[3].Process) - assert.Greater(t, traces[4].Process.Started, int64(0)) - assert.EqualValues(t, backend.State{Started: traces[4].Process.Started, Exited: true}, traces[4].Process) + assert.EqualValues(t, backend.State{}, traces[0].CurrStepState) + assert.Greater(t, traces[2].CurrStepState.Started, int64(0)) + assert.EqualValues(t, backend.State{Started: traces[2].CurrStepState.Started, Exited: true}, traces[2].CurrStepState) + assert.EqualValues(t, backend.State{}, traces[3].CurrStepState) + assert.Greater(t, traces[4].CurrStepState.Started, int64(0)) + assert.EqualValues(t, backend.State{Started: traces[4].CurrStepState.Started, Exited: true}, traces[4].CurrStepState) - assert.Greater(t, traces[4].Pipeline.Started, int64(0)) - assert.EqualValues(t, traces[4], state.State{ - Pipeline: struct { - Started int64 `json:"time"` - Step *backend.Step `json:"step"` - Error error `json:"error"` + assert.Greater(t, traces[4].Workflow.Started, int64(0)) + assert.EqualValues(t, state.State{ + Workflow: struct { + Started int64 `json:"time"` + Error error `json:"error"` }{ - Started: traces[4].Pipeline.Started, - Step: &backend.Step{ - Name: "test", - UUID: "test-uuid", - Type: "commands", - OnSuccess: true, - Environment: map[string]string{ - "DRONE_BUILD_STATUS": "success", - "DRONE_REPO_SCM": "git", - "PULLREQUEST_DRONE_PULL_REQUEST": "0", - }, - Commands: []string{"echo test"}, - }, + Started: traces[4].Workflow.Started, }, - Process: backend.State{ - Started: traces[4].Process.Started, + CurrStep: &backend.Step{ + Name: "test", + UUID: "test-uuid", + Type: "commands", + OnSuccess: true, + Environment: map[string]string{}, + Commands: []string{"echo test"}, + }, + CurrStepState: backend.State{ + Started: traces[4].CurrStepState.Started, Exited: true, }, - }) + }, traces[4]) } } -func TestWorkflowDetachedStepDoesNotBlockPipeline(t *testing.T) { +func TestWorkflowDetachedStepDoesNotBlockWorkflow(t *testing.T) { t.Parallel() r := New( &backend.Config{ @@ -271,17 +269,15 @@ func TestWorkflowBuildFailSkipsSubsequentStages(t *testing.T) { require.True(t, errors.As(err, &exitErr)) assert.Equal(t, 1, exitErr.Code) - // traces := getTracerStates(tracer) + traces := getTracerStates(tracer) - // TODO: signal failed back (https://github.com/woodpecker-ci/woodpecker/pull/6166) - // deployTrace := findFirstTraceByName(calls, "build") - // require.NotNil(t, deployTrace, "build step should fail") - // assert.EqualValues(t, 1, deployTrace.Process.ExitCode) + buildTrace := findLastTraceByName(traces, "build") + require.NotNil(t, buildTrace, "build step should fail") + assert.EqualValues(t, 1, buildTrace.CurrStepState.ExitCode) - // TODO: signal skipped back (https://github.com/woodpecker-ci/woodpecker/pull/6166) - // deployTrace := findFirstTraceByName(calls, "deploy") - // require.NotNil(t, deployTrace, "deploy step should still be traced") - // assert.True(t, deployTrace.Process.Skipped) + deployTrace := findLastTraceByName(traces, "deploy") + require.NotNil(t, deployTrace, "deploy step should still be traced") + assert.True(t, deployTrace.CurrStepState.Skipped) } func TestWorkflowOnFailureStepRuns(t *testing.T) { @@ -300,14 +296,15 @@ func TestWorkflowOnFailureStepRuns(t *testing.T) { ) err := r.Run(t.Context()) + traces := getTracerStates(tracer) assert.Error(t, err) - assert.NotNil(t, findStartedTrace(getTracerStates(tracer), "notify-failure"), "OnFailure step should have started") + assert.NotNil(t, findStartedTrace(traces, "notify-failure"), "OnFailure step should have started") - last := findLastTraceByName(getTracerStates(tracer), "notify-failure") + last := findLastTraceByName(traces, "notify-failure") require.NotNil(t, last) - assert.True(t, last.Process.Exited, "notify-failure should have exited") - assert.Equal(t, 0, last.Process.ExitCode, "notify-failure step itself should succeed") + assert.Greater(t, last.CurrStepState.Started, int64(0), "step should have started") + assert.EqualValues(t, backend.State{Started: last.CurrStepState.Started, Exited: true}, last.CurrStepState) } func TestWorkflowOnFailureStepSkippedOnSuccess(t *testing.T) { @@ -326,12 +323,13 @@ func TestWorkflowOnFailureStepSkippedOnSuccess(t *testing.T) { ) err := r.Run(t.Context()) + require.NoError(t, err) + traces := getTracerStates(tracer) - assert.NoError(t, err) - // TODO: signal skipped back (https://github.com/woodpecker-ci/woodpecker/pull/6166) - // cleanupTrace := findFirstTraceByName(getTracerStates(tracer), "cleanup-on-fail") - // require.NotNil(t, cleanupTrace, "cleanup step should be traced even when skipped") - // assert.True(t, cleanupTrace.Process.Skipped) + firstCleanupTrace := findFirstTraceByName(traces, "cleanup-on-fail") + lastCleanupTrace := findLastTraceByName(traces, "cleanup-on-fail") + assert.Equal(t, firstCleanupTrace, lastCleanupTrace, "we expect on skipped steps to only have one trace") + assert.True(t, lastCleanupTrace.CurrStepState.Skipped, "cleanup-on-fail should be skipped after no failure happened") } func TestWorkflowFailureIgnore(t *testing.T) { @@ -358,11 +356,11 @@ func TestWorkflowFailureIgnore(t *testing.T) { last := findLastTraceByName(getTracerStates(tracer), "build") require.NotNil(t, last) - assert.True(t, last.Process.Exited) - assert.Equal(t, 0, last.Process.ExitCode) + assert.True(t, last.CurrStepState.Exited) + assert.Equal(t, 0, last.CurrStepState.ExitCode) } -func TestWorkflowFailureIgnoreDoesNotSetPipelineError(t *testing.T) { +func TestWorkflowFailureIgnoreDoesNotSetWorkflowError(t *testing.T) { t.Parallel() tracer := newTestTracer(t) r := New( @@ -382,13 +380,11 @@ func TestWorkflowFailureIgnoreDoesNotSetPipelineError(t *testing.T) { err := r.Run(t.Context()) assert.NoError(t, err) - // TODO: signal skipped back (https://github.com/woodpecker-ci/woodpecker/pull/6166) - // traces := getTracerStates(tracer) - // for _, c := range traces { - // if c.Pipeline.Step != nil && c.Pipeline.Step.Name == "deploy" { - // assert.False(t, c.Process.Skipped, "deploy should not be skipped after failure=ignore step") - // } - // } + traces := getTracerStates(tracer) + firstDeployTrace := findFirstTraceByName(traces, "deploy") + lastDeployTrace := findLastTraceByName(traces, "deploy") + assert.NotEqualValues(t, firstDeployTrace, lastDeployTrace, "we expect two traces") + assert.False(t, lastDeployTrace.CurrStepState.Skipped, "deploy should not be skipped after failure=ignore step") } func TestWorkflowPluginStep(t *testing.T) { @@ -429,9 +425,9 @@ func TestWorkflowOOMKilledStep(t *testing.T) { last := findLastTraceByName(getTracerStates(tracer), "build") require.NotNil(t, last) - assert.True(t, last.Process.Exited) - assert.True(t, last.Process.OOMKilled) - assert.Equal(t, 137, last.Process.ExitCode) + assert.True(t, last.CurrStepState.Exited) + assert.True(t, last.CurrStepState.OOMKilled) + assert.Equal(t, 137, last.CurrStepState.ExitCode) } func TestWorkflowParallelStepsInStage(t *testing.T) { @@ -484,13 +480,13 @@ func TestWorkflowParallelStepOneFailsOthersComplete(t *testing.T) { lastFast := findLastTraceByName(getTracerStates(tracer), "test-fast") require.NotNil(t, lastFast) - assert.True(t, lastFast.Process.Exited) - assert.Equal(t, 0, lastFast.Process.ExitCode, "test-fast should succeed") + assert.True(t, lastFast.CurrStepState.Exited) + assert.Equal(t, 0, lastFast.CurrStepState.ExitCode, "test-fast should succeed") lastSlow := findLastTraceByName(getTracerStates(tracer), "test-slow") require.NotNil(t, lastSlow) - assert.True(t, lastSlow.Process.Exited) - assert.Equal(t, 1, lastSlow.Process.ExitCode, "test-slow should fail with code 1") + assert.True(t, lastSlow.CurrStepState.Exited) + assert.Equal(t, 1, lastSlow.CurrStepState.ExitCode, "test-slow should fail with code 1") } func TestWorkflowStepStartFailure(t *testing.T) { @@ -513,12 +509,9 @@ func TestWorkflowStepStartFailure(t *testing.T) { assert.Error(t, err) deployTrace := findFirstTraceByName(getTracerStates(tracer), "build") require.NotNil(t, deployTrace) - // TODO: signal skipped back (https://github.com/woodpecker-ci/woodpecker/pull/6166) - // assert.True(t, deployTrace.Process.Skipped) + assert.EqualValues(t, backend.State{}, deployTrace.CurrStepState) } -// TODO: signal skipped back (https://github.com/woodpecker-ci/woodpecker/pull/6166) -/* func TestWorkflowContextCancelDuringExecution(t *testing.T) { t.Parallel() ctx, cancel := context.WithCancelCause(t.Context()) @@ -527,7 +520,7 @@ func TestWorkflowContextCancelDuringExecution(t *testing.T) { tracer := tracer_mocks.NewMockTracer(t) tracer.On("Trace", mock.Anything).Run(func(args mock.Arguments) { s, _ := args.Get(0).(*state.State) - if s.Process.Exited && !s.Process.Skipped { + if s.CurrStepState.Exited && !s.CurrStepState.Skipped { stageCount++ if stageCount >= 1 { cancel(nil) @@ -551,8 +544,7 @@ func TestWorkflowContextCancelDuringExecution(t *testing.T) { err := r.Run(t.Context()) assert.ErrorIs(t, err, pipeline_errors.ErrCancel) -}. -*/ +} func TestWorkflowSetupFailure(t *testing.T) { t.Parallel() @@ -602,20 +594,19 @@ func TestWorkflowServiceWithParallelBuildAndOnFailure(t *testing.T) { assert.Error(t, err) traces := getTracerStates(tracer) - deployTrace := findLastTraceByName(traces, "notify") - require.NotNil(t, deployTrace) - assert.True(t, deployTrace.Process.Exited, "notify should exited") - assert.EqualValues(t, 0, deployTrace.Process.ExitCode, "notify should be successful") + notifyTrace := findLastTraceByName(traces, "notify") + require.NotNil(t, notifyTrace) + assert.True(t, notifyTrace.CurrStepState.Exited, "notify should exited") + assert.EqualValues(t, 0, notifyTrace.CurrStepState.ExitCode, "notify should be successful") lastBuild := findLastTraceByName(traces, "lint") require.NotNil(t, lastBuild) - assert.True(t, lastBuild.Process.Exited) - assert.Equal(t, 1, lastBuild.Process.ExitCode, "lint should have failed") + assert.True(t, lastBuild.CurrStepState.Exited) + assert.Equal(t, 1, lastBuild.CurrStepState.ExitCode, "lint should have failed") - // TODO: signal skipped back (https://github.com/woodpecker-ci/woodpecker/pull/6166) - // deployTrace := findFirstTraceByName(traces, "deploy") - // require.NotNil(t, deployTrace) - // assert.True(t, deployTrace.Process.Skipped, "deploy should be skipped after lint failure") + deployTrace := findFirstTraceByName(traces, "deploy") + require.NotNil(t, deployTrace) + assert.True(t, deployTrace.CurrStepState.Skipped, "deploy should be skipped after lint failure") assert.NotNil(t, findStartedTrace(traces, "notify"), "notify (OnFailure) should have started") @@ -644,13 +635,11 @@ func TestWorkflowIgnoredFailureFollowedByOnFailureStep(t *testing.T) { assert.NoError(t, err) traces := getTracerStates(tracer) - notifyTrace := findFirstTraceByName(traces, "build") + notifyTrace := findFirstTraceByName(traces, "error-notify") require.NotNil(t, notifyTrace) - // TODO: signal skipped back (https://github.com/woodpecker-ci/woodpecker/pull/6166) - // assert.True(t, notifyTrace.Process.Skipped, "OnFailure step should be skipped when prior failure was ignored") + assert.True(t, notifyTrace.CurrStepState.Skipped, "OnFailure step should be skipped when prior failure was ignored") - assert.NotNil(t, findStartedTrace(traces, "build"), - "build should run after ignored failure") + assert.NotNil(t, findStartedTrace(traces, "build"), "build should run after ignored failure") } func TestWorkflowEmptyStages(t *testing.T) { diff --git a/pipeline/state/state.go b/pipeline/state/state.go index 6a496700e..c27876d2a 100644 --- a/pipeline/state/state.go +++ b/pipeline/state/state.go @@ -18,20 +18,19 @@ import ( backend "go.woodpecker-ci.org/woodpecker/v3/pipeline/backend/types" ) -type ( - // State defines the pipeline and process state. - State struct { - // Global state of the pipeline. - Pipeline struct { - // Pipeline time started - Started int64 `json:"time"` - // Current pipeline step - Step *backend.Step `json:"step"` - // Current pipeline error state - Error error `json:"error"` - } - - // Current process state. - Process backend.State +// State is used to signal the current workflow and step state. +// Only steps using the trace func report back what's going on. +// And the workflow is updated alongside it. +type State struct { + // Global state of the currently running Workflow. + Workflow struct { + // Workflow start time + Started int64 `json:"time"` + // Current workflow error state + Error error `json:"error"` } -) + // Current step that updates the step and workflow state + CurrStep *backend.Step `json:"step"` + // Current step state + CurrStepState backend.State +} diff --git a/pipeline/tracing/tracer.go b/pipeline/tracing/tracer.go index 77b9f3a7d..800cb9e3c 100644 --- a/pipeline/tracing/tracer.go +++ b/pipeline/tracing/tracer.go @@ -38,15 +38,15 @@ func (f TraceFunc) Trace(state *state.State) error { // variables to include the correct timestamp and status. // TODO: find either a new home or better name for this. var DefaultTracer = TraceFunc(func(state *state.State) error { - if state.Process.Exited { + if state.CurrStepState.Exited { return nil } - if state.Pipeline.Step.Environment == nil { + if state.CurrStep.Environment == nil { return nil } - state.Pipeline.Step.Environment["CI_PIPELINE_STARTED"] = strconv.FormatInt(state.Pipeline.Started, 10) + state.CurrStep.Environment["CI_PIPELINE_STARTED"] = strconv.FormatInt(state.Workflow.Started, 10) - state.Pipeline.Step.Environment["CI_STEP_STARTED"] = strconv.FormatInt(state.Pipeline.Started, 10) + state.CurrStep.Environment["CI_STEP_STARTED"] = strconv.FormatInt(state.Workflow.Started, 10) return nil }) diff --git a/rpc/proto/version.go b/rpc/proto/version.go index ee63ab7cd..a0ececc67 100644 --- a/rpc/proto/version.go +++ b/rpc/proto/version.go @@ -16,4 +16,4 @@ package proto // Version is the version of the woodpecker.proto file, // IMPORTANT: increased by 1 each time it get changed. -const Version int32 = 15 +const Version int32 = 16 diff --git a/rpc/types.go b/rpc/types.go index 18dc6ca23..e26a28222 100644 --- a/rpc/types.go +++ b/rpc/types.go @@ -33,6 +33,7 @@ type ( ExitCode int `json:"exit_code"` Error string `json:"error"` Canceled bool `json:"canceled"` + Skipped bool `json:"Skipped"` } // WorkflowState defines the workflow state. diff --git a/server/pipeline/step_status.go b/server/pipeline/step_status.go index 6b3ee0c96..3ba0531aa 100644 --- a/server/pipeline/step_status.go +++ b/server/pipeline/step_status.go @@ -43,6 +43,10 @@ func UpdateStepStatus(ctx context.Context, store store.Store, step *model.Step, step.Started = time.Now().Unix() } + if state.Skipped { + step.State = model.StatusSkipped + } + // Handle direct transition to finished if step setup error happened if state.Exited || state.Error != "" { step.Finished = state.Finished @@ -52,16 +56,18 @@ func UpdateStepStatus(ctx context.Context, store store.Store, step *model.Step, step.ExitCode = state.ExitCode step.Error = state.Error - if state.ExitCode == 0 && state.Error == "" { - step.State = model.StatusSuccess - } else { - step.State = model.StatusFailure + if !state.Skipped { + if state.ExitCode == 0 && state.Error == "" { + step.State = model.StatusSuccess + } else { + step.State = model.StatusFailure - if step.Failure == model.FailureCancel { - // cancel the pipeline - err := cancelPipelineFromStep(ctx, store, step) - if err != nil { - return err + if step.Failure == model.FailureCancel { + // cancel the pipeline + err := cancelPipelineFromStep(ctx, store, step) + if err != nil { + return err + } } } } diff --git a/server/pipeline/step_status_test.go b/server/pipeline/step_status_test.go index 72304b9ae..11625aaa3 100644 --- a/server/pipeline/step_status_test.go +++ b/server/pipeline/step_status_test.go @@ -216,6 +216,29 @@ func TestUpdateStepStatus(t *testing.T) { assert.Equal(t, 1, step.ExitCode) assert.Equal(t, "canceled", step.Error) }) + + t.Run("PendingToSkipped", func(t *testing.T) { + t.Parallel() + step := &model.Step{State: model.StatusPending} + state := rpc.StepState{Skipped: true} + + err := UpdateStepStatus(t.Context(), mockStoreStep(t), step, state) + + assert.NoError(t, err) + assert.Equal(t, model.StatusSkipped, step.State) + }) + + t.Run("PendingToSkippedWithFinishTime", func(t *testing.T) { + t.Parallel() + step := &model.Step{State: model.StatusPending} + state := rpc.StepState{Skipped: true, Exited: true, Finished: 50} + + err := UpdateStepStatus(t.Context(), mockStoreStep(t), step, state) + + assert.NoError(t, err) + assert.Equal(t, model.StatusSkipped, step.State) + assert.Equal(t, int64(50), step.Finished) + }) }) t.Run("TerminalState", func(t *testing.T) {