From 855eae14385bdfe42d91af82347c605dd61a4620 Mon Sep 17 00:00:00 2001 From: Sam Oliver Date: Mon, 30 Mar 2026 15:27:01 +0200 Subject: [PATCH] Set workflow services stuck in running state to finished (#6337) --- server/pipeline/workflow_status_test.go | 1 + server/rpc/rpc.go | 13 +++++++-- server/rpc/rpc_test.go | 38 +++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 3 deletions(-) diff --git a/server/pipeline/workflow_status_test.go b/server/pipeline/workflow_status_test.go index 09b21b1ed..89b921bbc 100644 --- a/server/pipeline/workflow_status_test.go +++ b/server/pipeline/workflow_status_test.go @@ -294,6 +294,7 @@ func TestUpdateWorkflowStatusToDone(t *testing.T) { result, err := UpdateWorkflowStatusToDone(mockStore, workflow, state) assert.NoError(t, err) + assert.Equal(t, model.StatusSuccess, result.State) assert.Equal(t, int64(1234567900), result.Finished) }) } diff --git a/server/rpc/rpc.go b/server/rpc/rpc.go index 1fccfa150..72372589f 100644 --- a/server/rpc/rpc.go +++ b/server/rpc/rpc.go @@ -346,6 +346,10 @@ func (s *RPC) Done(c context.Context, strWorkflowID string, state rpc.WorkflowSt logger.Debug().Msgf("workflow state in store: %#v", workflow) logger.Debug().Msgf("gRPC Done with state: %#v", state) + // Complete any still-running children (e.g. service containers) before + // computing the workflow status, so their final state is reflected. + s.completeChildrenIfParentCompleted(workflow, state.Finished) + if workflow, err = pipeline.UpdateWorkflowStatusToDone(s.store, *workflow, state); err != nil { logger.Error().Err(err).Msgf("pipeline.UpdateWorkflowStatusToDone: cannot update workflow state: %s", err) } @@ -372,7 +376,6 @@ func (s *RPC) Done(c context.Context, strWorkflowID string, state rpc.WorkflowSt if err != nil { return err } - s.completeChildrenIfParentCompleted(workflow) if !model.IsThereRunningStage(currentPipeline.Workflows) { if currentPipeline, err = pipeline.UpdateStatusToDone(s.store, *currentPipeline, pipeline.PipelineStatus(currentPipeline.Workflows), workflow.Finished); err != nil { @@ -562,11 +565,15 @@ func (s *RPC) checkAgentPermissionByWorkflow(_ context.Context, agent *model.Age return errors.New(msg) } -func (s *RPC) completeChildrenIfParentCompleted(completedWorkflow *model.Workflow) { +func (s *RPC) completeChildrenIfParentCompleted(completedWorkflow *model.Workflow, finished int64) { for _, c := range completedWorkflow.Children { if c.Running() { - if _, err := pipeline.UpdateStepToStatusSkipped(s.store, *c, completedWorkflow.Finished, model.StatusSkipped); err != nil { + if updated, err := pipeline.UpdateStepToStatusSkipped(s.store, *c, finished, model.StatusSkipped); err != nil { log.Error().Err(err).Msgf("done: cannot update step_id %d child state", c.ID) + } else { + // Update in-memory state so WorkflowStatus sees the final state + c.State = updated.State + c.Finished = updated.Finished } } } diff --git a/server/rpc/rpc_test.go b/server/rpc/rpc_test.go index 04a6a6f60..ef1eb4df7 100644 --- a/server/rpc/rpc_test.go +++ b/server/rpc/rpc_test.go @@ -25,6 +25,7 @@ import ( "go.woodpecker-ci.org/woodpecker/v3/rpc" "go.woodpecker-ci.org/woodpecker/v3/server/model" + "go.woodpecker-ci.org/woodpecker/v3/server/pipeline" store_mocks "go.woodpecker-ci.org/woodpecker/v3/server/store/mocks" ) @@ -109,6 +110,43 @@ func TestRegisterAgent(t *testing.T) { }) } +func TestCompleteChildrenIfParentCompleted(t *testing.T) { + t.Run("When a service step is still running it should update state so workflow finishes as success", func(t *testing.T) { + successStep := &model.Step{ + ID: 1, + State: model.StatusSuccess, + Started: 1234567800, + } + runningService := &model.Step{ + ID: 2, + State: model.StatusRunning, + Started: 1234567800, + } + workflow := model.Workflow{ + ID: 7, + State: model.StatusRunning, + Children: []*model.Step{successStep, runningService}, + } + + mockStore := store_mocks.NewMockStore(t) + mockStore.On("StepUpdate", mock.Anything).Return(nil) + mockStore.On("WorkflowUpdate", mock.Anything).Return(nil) + + s := RPC{store: mockStore} + s.completeChildrenIfParentCompleted(&workflow, 1234567900) + + assert.Equal(t, model.StatusSuccess, runningService.State) + assert.Equal(t, int64(1234567900), runningService.Finished) + + result, err := pipeline.UpdateWorkflowStatusToDone(mockStore, workflow, rpc.WorkflowState{ + Started: 1234567800, + Finished: 1234567900, + }) + require.NoError(t, err) + assert.Equal(t, model.StatusSuccess, result.State) + }) +} + func TestUpdateAgentLastWork(t *testing.T) { t.Run("When last work was never updated it should update last work timestamp", func(t *testing.T) { agent := model.Agent{