Set workflow services stuck in running state to finished (#6337)

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)
 	})
 }

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
 			}
 		}
 	}

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{