Use step uuid instead of name in GRPC status calls

agent/rpc/client_grpc.go

@@ -170,7 +170,7 @@ func (c *client) Init(ctx context.Context, id string, state rpc.State) (err erro
 	req.State.Exited = state.Exited
 	req.State.Finished = state.Finished
 	req.State.Started = state.Started
-	req.State.Name = state.Step
+	req.State.StepUuid = state.StepUUID
 	for {
 		_, err = c.client.Init(ctx, req)
 		if err == nil {
@@ -211,7 +211,7 @@ func (c *client) Done(ctx context.Context, id string, state rpc.State) (err erro
 	req.State.Exited = state.Exited
 	req.State.Finished = state.Finished
 	req.State.Started = state.Started
-	req.State.Name = state.Step
+	req.State.StepUuid = state.StepUUID
 	for {
 		_, err = c.client.Done(ctx, req)
 		if err == nil {
@@ -286,7 +286,7 @@ func (c *client) Update(ctx context.Context, id string, state rpc.State) (err er
 	req.State.Exited = state.Exited
 	req.State.Finished = state.Finished
 	req.State.Started = state.Started
-	req.State.Name = state.Step
+	req.State.StepUuid = state.StepUUID
 	for {
 		_, err = c.client.Update(ctx, req)
 		if err == nil {

agent/tracer.go

@@ -37,7 +37,7 @@ func (r *Runner) createTracer(ctxmeta context.Context, logger zerolog.Logger, wo
 			Logger()
 
 		stepState := rpc.State{
-			Step:     state.Pipeline.Step.Alias,
+			StepUUID: state.Pipeline.Step.UUID,
 			Exited:   state.Process.Exited,
 			ExitCode: state.Process.ExitCode,
 			Started:  time.Now().Unix(), // TODO do not do this

pipeline/rpc/peer.go

@@ -27,9 +27,9 @@ type (
 		Labels map[string]string `json:"labels"`
 	}
 
-	// State defines the workflow state.
+	// State defines the step state.
 	State struct {
-		Step     string `json:"step"`
+		StepUUID string `json:"step_uuid"`
 		Exited   bool   `json:"exited"`
 		ExitCode int    `json:"exit_code"`
 		Started  int64  `json:"started"`
@@ -61,16 +61,16 @@ type Peer interface {
 	// Wait blocks until the workflow is complete
 	Wait(c context.Context, id string) error
 
-	// Init signals the workflow is initialized
+	// Init signals the step is initialized
 	Init(c context.Context, id string, state State) error
 
-	// Done signals the workflow is complete
+	// Done signals the step is complete
 	Done(c context.Context, id string, state State) error
 
 	// Extend extends the workflow deadline
 	Extend(c context.Context, id string) error
 
-	// Update updates the workflow state
+	// Update updates the step state
 	Update(c context.Context, id string, state State) error
 
 	// Log writes the workflow log entry

pipeline/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 !IMPORTANT!
-const Version int32 = 6
+const Version int32 = 7

pipeline/rpc/proto/woodpecker.proto

@@ -42,7 +42,7 @@ service Woodpecker {
 //
 
 message State {
-  string name = 1;
+  string step_uuid = 1;
   bool   exited = 2;
   int32  exit_code = 3;
   int64  started = 4;

server/grpc/rpc.go

@@ -106,29 +106,38 @@ func (s *RPC) Update(_ context.Context, id string, state rpc.State) error {
 
 	workflow, err := s.store.WorkflowLoad(workflowID)
 	if err != nil {
-		log.Error().Msgf("error: rpc.update: cannot find workflow with id %d: %s", workflowID, err)
+		log.Error().Err(err).Msgf("rpc.update: cannot find workflow with id %d", workflowID)
 		return err
 	}
 
 	currentPipeline, err := s.store.GetPipeline(workflow.PipelineID)
 	if err != nil {
-		log.Error().Msgf("error: cannot find pipeline with id %d: %s", workflow.PipelineID, err)
+		log.Error().Err(err).Msgf("cannot find pipeline with id %d", workflow.PipelineID)
 		return err
 	}
 
-	step, err := s.store.StepChild(currentPipeline, workflow.PID, state.Step)
+	step, err := s.store.StepByUUID(state.StepUUID)
 	if err != nil {
-		log.Error().Msgf("error: cannot find step with name %s: %s", state.Step, err)
+		log.Error().Err(err).Msgf("cannot find step with uuid %s", state.StepUUID)
 		return err
 	}
 
+	if step.PipelineID != currentPipeline.ID {
+		msg := fmt.Sprintf("agent returned status with step uuid '%s' which does not belong to current pipeline", state.StepUUID)
+		log.Error().
+			Int64("stepPipelineID", step.PipelineID).
+			Int64("currentPipelineID", currentPipeline.ID).
+			Msg(msg)
+		return fmt.Errorf(msg)
+	}
+
 	repo, err := s.store.GetRepo(currentPipeline.RepoID)
 	if err != nil {
-		log.Error().Msgf("error: cannot find repo with id %d: %s", currentPipeline.RepoID, err)
+		log.Error().Err(err).Msgf("cannot find repo with id %d", currentPipeline.RepoID)
 		return err
 	}
 
-	if err := pipeline.UpdateStepStatus(s.store, step, state, currentPipeline.Started); err != nil {
+	if err := pipeline.UpdateStepStatus(s.store, step, state); err != nil {
 		log.Error().Err(err).Msg("rpc.update: cannot update step")
 	}
 

server/grpc/server.go

@@ -95,7 +95,7 @@ func (s *WoodpeckerServer) Init(c context.Context, req *proto.InitRequest) (*pro
 		ExitCode: int(req.GetState().GetExitCode()),
 		Finished: req.GetState().GetFinished(),
 		Started:  req.GetState().GetStarted(),
-		Step:     req.GetState().GetName(),
+		StepUUID: req.GetState().GetStepUuid(),
 		Exited:   req.GetState().GetExited(),
 	}
 	res := new(proto.Empty)
@@ -109,7 +109,7 @@ func (s *WoodpeckerServer) Update(c context.Context, req *proto.UpdateRequest) (
 		ExitCode: int(req.GetState().GetExitCode()),
 		Finished: req.GetState().GetFinished(),
 		Started:  req.GetState().GetStarted(),
-		Step:     req.GetState().GetName(),
+		StepUUID: req.GetState().GetStepUuid(),
 		Exited:   req.GetState().GetExited(),
 	}
 	res := new(proto.Empty)
@@ -123,7 +123,7 @@ func (s *WoodpeckerServer) Done(c context.Context, req *proto.DoneRequest) (*pro
 		ExitCode: int(req.GetState().GetExitCode()),
 		Finished: req.GetState().GetFinished(),
 		Started:  req.GetState().GetStarted(),
-		Step:     req.GetState().GetName(),
+		StepUUID: req.GetState().GetStepUuid(),
 		Exited:   req.GetState().GetExited(),
 	}
 	res := new(proto.Empty)

server/pipeline/stepStatus.go

@@ -22,7 +22,7 @@ import (
 	"go.woodpecker-ci.org/woodpecker/v2/server/model"
 )
 
-func UpdateStepStatus(store model.UpdateStepStore, step *model.Step, state rpc.State, started int64) error {
+func UpdateStepStatus(store model.UpdateStepStore, step *model.Step, state rpc.State) error {
 	if state.Exited {
 		step.Stopped = state.Finished
 		step.ExitCode = state.ExitCode
@@ -34,14 +34,10 @@ func UpdateStepStatus(store model.UpdateStepStore, step *model.Step, state rpc.S
 		if state.ExitCode == 137 {
 			step.State = model.StatusKilled
 		}
-	} else {
+	} else if step.Stopped == 0 {
 		step.Started = state.Started
 		step.State = model.StatusRunning
 	}
-
-	if step.Started == 0 && step.Stopped != 0 {
-		step.Started = started
-	}
 	return store.StepUpdate(step)
 }
 

server/pipeline/stepStatus_test.go

@@ -33,7 +33,10 @@ func (m *mockUpdateStepStore) StepUpdate(_ *model.Step) error {
 
 func TestUpdateStepStatusNotExited(t *testing.T) {
 	t.Parallel()
+	// step in db before update
+	step := &model.Step{}
 
+	// advertised step status
 	state := rpc.State{
 		Started: int64(42),
 		Exited:  false,
@@ -42,54 +45,47 @@ func TestUpdateStepStatusNotExited(t *testing.T) {
 		ExitCode: 137,
 		Error:    "not an error",
 	}
-	step := &model.Step{}
-	err := UpdateStepStatus(&mockUpdateStepStore{}, step, state, int64(1))
-	assert.NoError(t, err)
 
-	if step.State != model.StatusRunning {
-		t.Errorf("Step status not equals '%s' != '%s'", model.StatusRunning, step.State)
-	} else if step.Started != int64(42) {
-		t.Errorf("Step started not equals 42 != %d", step.Started)
-	} else if step.Stopped != int64(0) {
-		t.Errorf("Step stopped not equals 0 != %d", step.Stopped)
-	} else if step.ExitCode != 0 {
-		t.Errorf("Step exit code not equals 0 != %d", step.ExitCode)
-	} else if step.Error != "" {
-		t.Errorf("Step error not equals '' != '%s'", step.Error)
-	}
+	err := UpdateStepStatus(&mockUpdateStepStore{}, step, state)
+	assert.NoError(t, err)
+	assert.EqualValues(t, model.StatusRunning, step.State)
+	assert.EqualValues(t, 42, step.Started)
+	assert.EqualValues(t, 0, step.Stopped)
+	assert.EqualValues(t, 0, step.ExitCode)
+	assert.EqualValues(t, "", step.Error)
 }
 
 func TestUpdateStepStatusNotExitedButStopped(t *testing.T) {
 	t.Parallel()
 
-	step := &model.Step{Stopped: int64(64)}
+	// step in db before update
+	step := &model.Step{Started: 42, Stopped: 64, State: model.StatusKilled}
 
+	// advertised step status
 	state := rpc.State{
 		Exited: false,
 		// Dummy data
 		Finished: int64(1),
 		ExitCode: 137,
 		Error:    "not an error",
 	}
-	err := UpdateStepStatus(&mockUpdateStepStore{}, step, state, int64(42))
-	assert.NoError(t, err)
 
-	if step.State != model.StatusRunning {
-		t.Errorf("Step status not equals '%s' != '%s'", model.StatusRunning, step.State)
-	} else if step.Started != int64(42) {
-		t.Errorf("Step started not equals 42 != %d", step.Started)
-	} else if step.Stopped != int64(64) {
-		t.Errorf("Step stopped not equals 64 != %d", step.Stopped)
-	} else if step.ExitCode != 0 {
-		t.Errorf("Step exit code not equals 0 != %d", step.ExitCode)
-	} else if step.Error != "" {
-		t.Errorf("Step error not equals '' != '%s'", step.Error)
-	}
+	err := UpdateStepStatus(&mockUpdateStepStore{}, step, state)
+	assert.NoError(t, err)
+	assert.EqualValues(t, model.StatusKilled, step.State)
+	assert.EqualValues(t, 42, step.Started)
+	assert.EqualValues(t, 64, step.Stopped)
+	assert.EqualValues(t, 0, step.ExitCode)
+	assert.EqualValues(t, "", step.Error)
 }
 
 func TestUpdateStepStatusExited(t *testing.T) {
 	t.Parallel()
 
+	// step in db before update
+	step := &model.Step{Started: 42}
+
+	// advertised step status
 	state := rpc.State{
 		Started:  int64(42),
 		Exited:   true,
@@ -98,52 +94,42 @@ func TestUpdateStepStatusExited(t *testing.T) {
 		Error:    "an error",
 	}
 
-	step := &model.Step{}
-	err := UpdateStepStatus(&mockUpdateStepStore{}, step, state, int64(42))
+	err := UpdateStepStatus(&mockUpdateStepStore{}, step, state)
 	assert.NoError(t, err)
-
-	if step.State != model.StatusKilled {
-		t.Errorf("Step status not equals '%s' != '%s'", model.StatusKilled, step.State)
-	} else if step.Started != int64(42) {
-		t.Errorf("Step started not equals 42 != %d", step.Started)
-	} else if step.Stopped != int64(34) {
-		t.Errorf("Step stopped not equals 34 != %d", step.Stopped)
-	} else if step.ExitCode != 137 {
-		t.Errorf("Step exit code not equals 137 != %d", step.ExitCode)
-	} else if step.Error != "an error" {
-		t.Errorf("Step error not equals 'an error' != '%s'", step.Error)
-	}
+	assert.EqualValues(t, model.StatusKilled, step.State)
+	assert.EqualValues(t, 42, step.Started)
+	assert.EqualValues(t, 34, step.Stopped)
+	assert.EqualValues(t, 137, step.ExitCode)
+	assert.EqualValues(t, "an error", step.Error)
 }
 
 func TestUpdateStepStatusExitedButNot137(t *testing.T) {
 	t.Parallel()
 
+	// step in db before update
+	step := &model.Step{Started: 42}
+
+	// advertised step status
 	state := rpc.State{
 		Started:  int64(42),
 		Exited:   true,
 		Finished: int64(34),
 		Error:    "an error",
 	}
-	step := &model.Step{}
-	err := UpdateStepStatus(&mockUpdateStepStore{}, step, state, int64(42))
-	assert.NoError(t, err)
 
-	if step.State != model.StatusFailure {
-		t.Errorf("Step status not equals '%s' != '%s'", model.StatusFailure, step.State)
-	} else if step.Started != int64(42) {
-		t.Errorf("Step started not equals 42 != %d", step.Started)
-	} else if step.Stopped != int64(34) {
-		t.Errorf("Step stopped not equals 34 != %d", step.Stopped)
-	} else if step.ExitCode != 0 {
-		t.Errorf("Step exit code not equals 0 != %d", step.ExitCode)
-	} else if step.Error != "an error" {
-		t.Errorf("Step error not equals 'an error' != '%s'", step.Error)
-	}
+	err := UpdateStepStatus(&mockUpdateStepStore{}, step, state)
+	assert.NoError(t, err)
+	assert.EqualValues(t, model.StatusFailure, step.State)
+	assert.EqualValues(t, 42, step.Started)
+	assert.EqualValues(t, 34, step.Stopped)
+	assert.EqualValues(t, 0, step.ExitCode)
+	assert.EqualValues(t, "an error", step.Error)
 }
 
 func TestUpdateStepStatusExitedWithCode(t *testing.T) {
 	t.Parallel()
 
+	// advertised step status
 	state := rpc.State{
 		Started:  int64(42),
 		Exited:   true,
@@ -152,7 +138,7 @@ func TestUpdateStepStatusExitedWithCode(t *testing.T) {
 		Error:    "an error",
 	}
 	step := &model.Step{}
-	err := UpdateStepStatus(&mockUpdateStepStore{}, step, state, int64(42))
+	err := UpdateStepStatus(&mockUpdateStepStore{}, step, state)
 	assert.NoError(t, err)
 
 	if step.State != model.StatusFailure {