Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use step uuid instead of name in GRPC status calls #3143

Merged
merged 12 commits into from
Jan 9, 2024
6 changes: 3 additions & 3 deletions agent/rpc/client_grpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion agent/tracer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions pipeline/rpc/peer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down Expand Up @@ -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 step state
6543 marked this conversation as resolved.
Show resolved Hide resolved
Update(c context.Context, id string, state State) error

// Log writes the workflow log entry
Expand Down
2 changes: 1 addition & 1 deletion pipeline/rpc/proto/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
276 changes: 138 additions & 138 deletions pipeline/rpc/proto/woodpecker.pb.go

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pipeline/rpc/proto/woodpecker.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
21 changes: 15 additions & 6 deletions server/grpc/rpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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().Msgf("rpc.update: cannot find workflow with id %d: %s", workflowID, err)
6543 marked this conversation as resolved.
Show resolved Hide resolved
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().Msgf("cannot find pipeline with id %d: %s", workflow.PipelineID, err)
6543 marked this conversation as resolved.
Show resolved Hide resolved
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().Msgf("cannot find step with uuid %s: %s", state.StepUUID, err)
6543 marked this conversation as resolved.
Show resolved Hide resolved
return err
}

if step.PipelineID != currentPipeline.ID {
msg := fmt.Sprintf("agent return status with step uuid '%s' of step, witch does not belonging to current pipeline", state.StepUUID)
6543 marked this conversation as resolved.
Show resolved Hide resolved
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().Msgf("cannot find repo with id %d: %s", currentPipeline.RepoID, err)
6543 marked this conversation as resolved.
Show resolved Hide resolved
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")
}

Expand Down
6 changes: 3 additions & 3 deletions server/grpc/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions server/pipeline/stepStatus.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -40,7 +40,7 @@ func UpdateStepStatus(store model.UpdateStepStore, step *model.Step, state rpc.S
}

if step.Started == 0 && step.Stopped != 0 {
step.Started = started
step.Started = time.Now().Unix()
}
return store.StepUpdate(step)
}
Expand Down
10 changes: 5 additions & 5 deletions server/pipeline/stepStatus_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func TestUpdateStepStatusNotExited(t *testing.T) {
Error: "not an error",
}
step := &model.Step{}
err := UpdateStepStatus(&mockUpdateStepStore{}, step, state, int64(1))
err := UpdateStepStatus(&mockUpdateStepStore{}, step, state)
assert.NoError(t, err)

if step.State != model.StatusRunning {
Expand Down Expand Up @@ -71,7 +71,7 @@ func TestUpdateStepStatusNotExitedButStopped(t *testing.T) {
ExitCode: 137,
Error: "not an error",
}
err := UpdateStepStatus(&mockUpdateStepStore{}, step, state, int64(42))
err := UpdateStepStatus(&mockUpdateStepStore{}, step, state)
assert.NoError(t, err)

if step.State != model.StatusRunning {
Expand Down Expand Up @@ -99,7 +99,7 @@ func TestUpdateStepStatusExited(t *testing.T) {
}

step := &model.Step{}
err := UpdateStepStatus(&mockUpdateStepStore{}, step, state, int64(42))
err := UpdateStepStatus(&mockUpdateStepStore{}, step, state)
assert.NoError(t, err)

if step.State != model.StatusKilled {
Expand All @@ -125,7 +125,7 @@ func TestUpdateStepStatusExitedButNot137(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 {
Expand All @@ -152,7 +152,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 {
Expand Down