From 480f673ff72c3e964fc1d19f00744e38110518fc Mon Sep 17 00:00:00 2001 From: qwerty287 <80460567+qwerty287@users.noreply.github.com> Date: Mon, 2 Mar 2026 15:35:43 +0100 Subject: [PATCH] Fix status merging with skipped pipelines (#6176) --- cmd/server/openapi/docs.go | 4 ++++ rpc/proto/woodpecker_grpc.pb.go | 2 +- server/forge/github/convert.go | 2 +- server/model/const.go | 3 ++- server/pipeline/cancel.go | 12 ++++++++++-- server/pipeline/items.go | 3 --- server/pipeline/pipeline_status.go | 4 ++-- server/pipeline/pipeline_status_test.go | 2 +- server/pipeline/status.go | 16 +++++++++++++++- server/pipeline/status_test.go | 17 ++++++++++++++++- server/pipeline/step_status.go | 4 ++-- server/pipeline/step_status_test.go | 4 ++-- server/pipeline/stepbuilder/step_builder.go | 14 -------------- server/rpc/rpc.go | 2 +- web/src/components/atomic/Icon.vue | 8 +++++++- .../components/repo/pipeline/PipelineLog.vue | 6 +++--- .../repo/pipeline/PipelineStatusIcon.vue | 5 ++++- .../components/repo/pipeline/pipeline-status.ts | 1 + web/src/lib/api/types/pipeline.ts | 3 ++- 19 files changed, 74 insertions(+), 38 deletions(-) diff --git a/cmd/server/openapi/docs.go b/cmd/server/openapi/docs.go index c5c78746bf..68ce140b63 100644 --- a/cmd/server/openapi/docs.go +++ b/cmd/server/openapi/docs.go @@ -5529,6 +5529,7 @@ const docTemplate = `{ "success", "failure", "killed", + "canceled", "error", "blocked", "declined", @@ -5536,6 +5537,7 @@ const docTemplate = `{ ], "x-enum-comments": { "StatusBlocked": "waiting for approval", + "StatusCanceled": "canceled but hasn't been started", "StatusCreated": "created / internal use only", "StatusDeclined": "blocked and declined", "StatusError": "error with the config / while parsing / some other system problem", @@ -5553,6 +5555,7 @@ const docTemplate = `{ "successfully finished", "failed to finish (exit code != 0)", "killed by user", + "canceled but hasn't been started", "error with the config / while parsing / some other system problem", "waiting for approval", "blocked and declined", @@ -5565,6 +5568,7 @@ const docTemplate = `{ "StatusSuccess", "StatusFailure", "StatusKilled", + "StatusCanceled", "StatusError", "StatusBlocked", "StatusDeclined", diff --git a/rpc/proto/woodpecker_grpc.pb.go b/rpc/proto/woodpecker_grpc.pb.go index 88ca6e7a96..4dfd78a0ef 100644 --- a/rpc/proto/woodpecker_grpc.pb.go +++ b/rpc/proto/woodpecker_grpc.pb.go @@ -15,7 +15,7 @@ // Code generated by protoc-gen-go-grpc. DO NOT EDIT. // versions: -// - protoc-gen-go-grpc v1.6.0 +// - protoc-gen-go-grpc v1.6.1 // - protoc v6.33.1 // source: woodpecker.proto diff --git a/server/forge/github/convert.go b/server/forge/github/convert.go index 7bec159eba..ad85889b23 100644 --- a/server/forge/github/convert.go +++ b/server/forge/github/convert.go @@ -49,7 +49,7 @@ const ( // GitHub commit status. func convertStatus(status model.StatusValue) string { switch status { - case model.StatusPending, model.StatusRunning, model.StatusBlocked, model.StatusSkipped: + case model.StatusPending, model.StatusRunning, model.StatusBlocked, model.StatusSkipped, model.StatusCanceled: return statusPending case model.StatusFailure, model.StatusDeclined: return statusFailure diff --git a/server/model/const.go b/server/model/const.go index 6b35782c04..124f07d7f9 100644 --- a/server/model/const.go +++ b/server/model/const.go @@ -61,6 +61,7 @@ const ( StatusSuccess StatusValue = "success" // successfully finished StatusFailure StatusValue = "failure" // failed to finish (exit code != 0) StatusKilled StatusValue = "killed" // killed by user + StatusCanceled StatusValue = "canceled" // canceled but hasn't been started StatusError StatusValue = "error" // error with the config / while parsing / some other system problem StatusBlocked StatusValue = "blocked" // waiting for approval StatusDeclined StatusValue = "declined" // blocked and declined @@ -71,7 +72,7 @@ var ErrInvalidStatusValue = errors.New("invalid status value") func (s StatusValue) Validate() error { switch s { - case StatusSkipped, StatusPending, StatusRunning, StatusSuccess, StatusFailure, StatusKilled, StatusError, StatusBlocked, StatusDeclined, StatusCreated: + case StatusSkipped, StatusPending, StatusRunning, StatusSuccess, StatusFailure, StatusKilled, StatusCanceled, StatusError, StatusBlocked, StatusDeclined, StatusCreated: return nil default: return fmt.Errorf("%w: %s", ErrInvalidStatusValue, s) diff --git a/server/pipeline/cancel.go b/server/pipeline/cancel.go index a55031f8e7..e82c2a2584 100644 --- a/server/pipeline/cancel.go +++ b/server/pipeline/cancel.go @@ -53,6 +53,8 @@ func Cancel(ctx context.Context, _forge forge.Forge, store store.Store, repo *mo } } + hasPendingOnly := true + // Then update the DB status for pending pipelines // Running ones will be set when the agents stop on the cancel signal for _, workflow := range workflows { @@ -60,17 +62,23 @@ func Cancel(ctx context.Context, _forge forge.Forge, store store.Store, repo *mo if _, err = UpdateWorkflowToStatusSkipped(store, *workflow); err != nil { log.Error().Err(err).Msgf("cannot update workflow with id %d state", workflow.ID) } + } else { + hasPendingOnly = false } for _, step := range workflow.Children { if step.State == model.StatusPending { - if _, err = UpdateStepToStatusSkipped(store, *step, 0); err != nil { + if _, err = UpdateStepToStatusSkipped(store, *step, 0, model.StatusCanceled); err != nil { log.Error().Err(err).Msgf("cannot update workflow with id %d state", workflow.ID) } } } } - killedPipeline, err := UpdateToStatusKilled(store, *pipeline, cancelInfo) + plState := model.StatusKilled + if hasPendingOnly { + plState = model.StatusCanceled + } + killedPipeline, err := UpdateToStatusKilled(store, *pipeline, cancelInfo, plState) if err != nil { log.Error().Err(err).Msgf("UpdateToStatusKilled: %v", pipeline) return err diff --git a/server/pipeline/items.go b/server/pipeline/items.go index 4cf3c2dd04..d0bf26b3b7 100644 --- a/server/pipeline/items.go +++ b/server/pipeline/items.go @@ -189,9 +189,6 @@ func setPipelineStepsOnPipeline(pipeline *model.Pipeline, pipelineItems []*stepb Failure: step.Failure, Type: model.StepType(step.Type), } - if item.Workflow.State == model.StatusSkipped { - step.State = model.StatusSkipped - } if pipeline.Status == model.StatusBlocked { step.State = model.StatusBlocked } diff --git a/server/pipeline/pipeline_status.go b/server/pipeline/pipeline_status.go index 86bf7183bb..2ab801b0db 100644 --- a/server/pipeline/pipeline_status.go +++ b/server/pipeline/pipeline_status.go @@ -70,8 +70,8 @@ func UpdateToStatusError(store store.Store, pipeline model.Pipeline, err error) return &pipeline, store.UpdatePipeline(&pipeline) } -func UpdateToStatusKilled(store store.Store, pipeline model.Pipeline, cancelInfo *model.CancelInfo) (*model.Pipeline, error) { - pipeline.Status = model.StatusKilled +func UpdateToStatusKilled(store store.Store, pipeline model.Pipeline, cancelInfo *model.CancelInfo, state model.StatusValue) (*model.Pipeline, error) { + pipeline.Status = state pipeline.Finished = time.Now().Unix() pipeline.CancelInfo = cancelInfo return &pipeline, store.UpdatePipeline(&pipeline) diff --git a/server/pipeline/pipeline_status_test.go b/server/pipeline/pipeline_status_test.go index 8964e007a6..e0a1b0f3a0 100644 --- a/server/pipeline/pipeline_status_test.go +++ b/server/pipeline/pipeline_status_test.go @@ -98,7 +98,7 @@ func TestUpdateToStatusKilled(t *testing.T) { SupersededBy: 2, } - pipeline, _ := UpdateToStatusKilled(mockStorePipeline(t), model.Pipeline{}, cancelInfo) + pipeline, _ := UpdateToStatusKilled(mockStorePipeline(t), model.Pipeline{}, cancelInfo, model.StatusKilled) assert.Equal(t, model.StatusKilled, pipeline.Status) assert.NotNil(t, pipeline.CancelInfo) diff --git a/server/pipeline/status.go b/server/pipeline/status.go index 4efdf1c135..8b06f9bd38 100644 --- a/server/pipeline/status.go +++ b/server/pipeline/status.go @@ -29,7 +29,7 @@ var statusPriorityOrder = []model.StatusValue{ // skipped and killed cannot appear together with running/pending. model.StatusKilled, - model.StatusSkipped, + model.StatusCanceled, // running states model.StatusRunning, @@ -38,6 +38,9 @@ var statusPriorityOrder = []model.StatusValue{ // finished states model.StatusFailure, model.StatusSuccess, + + // skipped due to status condition + model.StatusSkipped, } var priorityMap map[model.StatusValue]int = buildPriorityMap() @@ -51,5 +54,16 @@ func buildPriorityMap() map[model.StatusValue]int { } func MergeStatusValues(s, t model.StatusValue) model.StatusValue { + // both are skipped due to cancellation -> canceled + if s == model.StatusCanceled && t == model.StatusCanceled { + return model.StatusCanceled + } + // if only one was skipped -> use killed as the workflow/pipeline was running once already + if s == model.StatusCanceled { + s = model.StatusKilled + } + if t == model.StatusCanceled { + t = model.StatusKilled + } return statusPriorityOrder[min(priorityMap[s], priorityMap[t])] } diff --git a/server/pipeline/status_test.go b/server/pipeline/status_test.go index 18171d281c..1268da7c25 100644 --- a/server/pipeline/status_test.go +++ b/server/pipeline/status_test.go @@ -31,7 +31,7 @@ func TestStatusValueMerge(t *testing.T) { { s: model.StatusSuccess, t: model.StatusSkipped, - e: model.StatusSkipped, + e: model.StatusSuccess, }, { s: model.StatusSuccess, @@ -68,6 +68,21 @@ func TestStatusValueMerge(t *testing.T) { t: model.StatusSkipped, e: model.StatusSkipped, }, + { + s: model.StatusSkipped, + t: model.StatusCanceled, + e: model.StatusKilled, + }, + { + s: model.StatusSuccess, + t: model.StatusCanceled, + e: model.StatusKilled, + }, + { + s: model.StatusFailure, + t: model.StatusCanceled, + e: model.StatusKilled, + }, } for _, tt := range tests { assert.Equal(t, tt.e, MergeStatusValues(tt.s, tt.t)) diff --git a/server/pipeline/step_status.go b/server/pipeline/step_status.go index a45c136160..5d2b5ddf94 100644 --- a/server/pipeline/step_status.go +++ b/server/pipeline/step_status.go @@ -89,8 +89,8 @@ func UpdateStepStatus(store store.Store, step *model.Step, state rpc.StepState) return store.StepUpdate(step) } -func UpdateStepToStatusSkipped(store store.Store, step model.Step, finished int64) (*model.Step, error) { - step.State = model.StatusSkipped +func UpdateStepToStatusSkipped(store store.Store, step model.Step, finished int64, status model.StatusValue) (*model.Step, error) { + step.State = status if step.Started != 0 { step.State = model.StatusSuccess // for daemons that are killed step.Finished = finished diff --git a/server/pipeline/step_status_test.go b/server/pipeline/step_status_test.go index 2a00aef879..52db2eb8f5 100644 --- a/server/pipeline/step_status_test.go +++ b/server/pipeline/step_status_test.go @@ -237,7 +237,7 @@ func TestUpdateStepToStatusSkipped(t *testing.T) { t.Run("NotStarted", func(t *testing.T) { t.Parallel() - step, err := UpdateStepToStatusSkipped(mockStoreStep(t), model.Step{}, int64(1)) + step, err := UpdateStepToStatusSkipped(mockStoreStep(t), model.Step{}, int64(1), model.StatusSkipped) assert.NoError(t, err) assert.Equal(t, model.StatusSkipped, step.State) @@ -247,7 +247,7 @@ func TestUpdateStepToStatusSkipped(t *testing.T) { t.Run("AlreadyStarted", func(t *testing.T) { t.Parallel() - step, err := UpdateStepToStatusSkipped(mockStoreStep(t), model.Step{Started: 42}, int64(100)) + step, err := UpdateStepToStatusSkipped(mockStoreStep(t), model.Step{Started: 42}, int64(100), model.StatusSkipped) assert.NoError(t, err) assert.Equal(t, model.StatusSuccess, step.State) diff --git a/server/pipeline/stepbuilder/step_builder.go b/server/pipeline/stepbuilder/step_builder.go index f98ca4f749..42b0b3bc7d 100644 --- a/server/pipeline/stepbuilder/step_builder.go +++ b/server/pipeline/stepbuilder/step_builder.go @@ -108,11 +108,6 @@ func (b *StepBuilder) Build() (items []*Item, errorsAndWarnings error) { items = filterItemsWithMissingDependencies(items) - // check if at least one step can start if slice is not empty - if len(items) > 0 && !workflowListContainsItemsToRun(items) { - return nil, fmt.Errorf("pipeline has no steps to run") - } - return items, errorsAndWarnings } @@ -221,15 +216,6 @@ func (b *StepBuilder) genItemForWorkflow(workflow *model.Workflow, axis matrix.A return item, errorsAndWarnings } -func workflowListContainsItemsToRun(items []*Item) bool { - for i := range items { - if items[i].Workflow.State == model.StatusPending { - return true - } - } - return false -} - func filterItemsWithMissingDependencies(items []*Item) []*Item { itemsToRemove := make([]*Item, 0) diff --git a/server/rpc/rpc.go b/server/rpc/rpc.go index 6fa97735d7..6680404f2c 100644 --- a/server/rpc/rpc.go +++ b/server/rpc/rpc.go @@ -559,7 +559,7 @@ func (s *RPC) checkAgentPermissionByWorkflow(_ context.Context, agent *model.Age func (s *RPC) completeChildrenIfParentCompleted(completedWorkflow *model.Workflow) { for _, c := range completedWorkflow.Children { if c.Running() { - if _, err := pipeline.UpdateStepToStatusSkipped(s.store, *c, completedWorkflow.Finished); err != nil { + if _, err := pipeline.UpdateStepToStatusSkipped(s.store, *c, completedWorkflow.Finished, model.StatusSkipped); err != nil { log.Error().Err(err).Msgf("done: cannot update step_id %d child state", c.ID) } } diff --git a/web/src/components/atomic/Icon.vue b/web/src/components/atomic/Icon.vue index d04e1a51ba..9c3d68cd86 100644 --- a/web/src/components/atomic/Icon.vue +++ b/web/src/components/atomic/Icon.vue @@ -48,7 +48,12 @@ :path="mdiRadioboxIndeterminateVariant" size="1.3rem" /> - + @@ -216,6 +221,7 @@ export type IconNames = | 'status-skipped' | 'status-started' | 'status-success' + | 'status-canceled' | 'gitea' | 'gitlab' | 'bitbucket' diff --git a/web/src/components/repo/pipeline/PipelineLog.vue b/web/src/components/repo/pipeline/PipelineLog.vue index 7769509316..0b383297c4 100644 --- a/web/src/components/repo/pipeline/PipelineLog.vue +++ b/web/src/components/repo/pipeline/PipelineLog.vue @@ -101,7 +101,7 @@
- {{ $t('repo.pipeline.actions.canceled') }} + {{ $t('repo.pipeline.actions.canceled') }} {{ $t('repo.pipeline.step_not_started') }}
{{ $t('repo.pipeline.loading') }}
{{ $t('repo.pipeline.no_logs') }}
@@ -174,8 +174,8 @@ const fullscreen = ref(false); const loadedLogs = computed(() => !!log.value); const hasLogs = computed( () => - // we do not have logs for skipped steps - repo?.value && pipeline.value && step.value && step.value.state !== 'skipped', + // we do not have logs for skipped/canceled steps + repo?.value && pipeline.value && step.value && step.value.state !== 'skipped' && step.value.state !== 'canceled', ); const autoScroll = useStorage('woodpecker:log-auto-scroll', true); const showActions = ref(false); diff --git a/web/src/components/repo/pipeline/PipelineStatusIcon.vue b/web/src/components/repo/pipeline/PipelineStatusIcon.vue index e32ac798d1..106a6b27d8 100644 --- a/web/src/components/repo/pipeline/PipelineStatusIcon.vue +++ b/web/src/components/repo/pipeline/PipelineStatusIcon.vue @@ -44,6 +44,7 @@ const statusDescriptions = { pending: t('repo.pipeline.status.pending'), running: t('repo.pipeline.status.running'), skipped: t('repo.pipeline.status.skipped'), + canceled: t('repo.pipeline.status.skipped'), started: t('repo.pipeline.status.started'), success: t('repo.pipeline.status.success'), } satisfies { @@ -52,6 +53,8 @@ const statusDescriptions = { }; const shouldShowBgCircle = computed(() => { - return service ? false : ['blocked', 'declined', 'error', 'failure', 'killed', 'skipped', 'success'].includes(status); + return service + ? false + : ['blocked', 'declined', 'error', 'failure', 'killed', 'skipped', 'canceled', 'success'].includes(status); }); diff --git a/web/src/components/repo/pipeline/pipeline-status.ts b/web/src/components/repo/pipeline/pipeline-status.ts index b69c046344..aa4be57f29 100644 --- a/web/src/components/repo/pipeline/pipeline-status.ts +++ b/web/src/components/repo/pipeline/pipeline-status.ts @@ -8,6 +8,7 @@ export const pipelineStatusColors: Record