From cd59a852307b599c94bfcdc7a5d48e139187e809 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Tue, 9 Jan 2024 15:22:59 +0100 Subject: [PATCH] Use name in backend types instead of alias (#3142) --- agent/logger.go | 2 +- agent/tracer.go | 4 +-- cli/exec/exec.go | 2 +- pipeline/backend/types/stage.go | 2 -- pipeline/backend/types/step.go | 1 - pipeline/frontend/yaml/compiler/compiler.go | 36 +++++-------------- .../frontend/yaml/compiler/compiler_test.go | 36 +++++-------------- pipeline/frontend/yaml/compiler/convert.go | 5 ++- pipeline/frontend/yaml/compiler/dag.go | 20 ++++------- pipeline/frontend/yaml/compiler/dag_test.go | 36 +++++++------------ pipeline/pipeline.go | 10 +++--- server/pipeline/items.go | 2 +- 12 files changed, 48 insertions(+), 108 deletions(-) diff --git a/agent/logger.go b/agent/logger.go index 9d1c92ecc..bfa4baeb8 100644 --- a/agent/logger.go +++ b/agent/logger.go @@ -31,7 +31,7 @@ func (r *Runner) createLogger(logger zerolog.Logger, uploads *sync.WaitGroup, wo return func(step *backend.Step, rc multipart.Reader) error { loglogger := logger.With(). Str("image", step.Image). - Str("stage", step.Alias). + Str("workflowID", workflow.ID). Logger() part, rerr := rc.NextPart() diff --git a/agent/tracer.go b/agent/tracer.go index 36d210b72..773aa8bf0 100644 --- a/agent/tracer.go +++ b/agent/tracer.go @@ -30,14 +30,14 @@ func (r *Runner) createTracer(ctxmeta context.Context, logger zerolog.Logger, wo return func(state *pipeline.State) error { steplogger := logger.With(). Str("image", state.Pipeline.Step.Image). - Str("stage", state.Pipeline.Step.Alias). + Str("workflowID", workflow.ID). Err(state.Process.Error). Int("exit_code", state.Process.ExitCode). Bool("exited", state.Process.Exited). Logger() stepState := rpc.State{ - Step: state.Pipeline.Step.Alias, + Step: state.Pipeline.Step.Name, Exited: state.Process.Exited, ExitCode: state.Process.ExitCode, Started: time.Now().Unix(), // TODO do not do this diff --git a/cli/exec/exec.go b/cli/exec/exec.go index 352753034..956831691 100644 --- a/cli/exec/exec.go +++ b/cli/exec/exec.go @@ -258,7 +258,7 @@ var defaultLogger = pipeline.LogFunc(func(step *backendTypes.Step, rc multipart. return err } - logStream := NewLineWriter(step.Alias, step.UUID) + logStream := NewLineWriter(step.Name, step.UUID) _, err = io.Copy(logStream, part) return err }) diff --git a/pipeline/backend/types/stage.go b/pipeline/backend/types/stage.go index f5ea1f59d..e24e54644 100644 --- a/pipeline/backend/types/stage.go +++ b/pipeline/backend/types/stage.go @@ -16,7 +16,5 @@ package types // Stage denotes a collection of one or more steps. type Stage struct { - Name string `json:"name,omitempty"` - Alias string `json:"alias,omitempty"` Steps []*Step `json:"steps,omitempty"` } diff --git a/pipeline/backend/types/step.go b/pipeline/backend/types/step.go index 0d1b600bf..f24fabed8 100644 --- a/pipeline/backend/types/step.go +++ b/pipeline/backend/types/step.go @@ -19,7 +19,6 @@ type Step struct { Name string `json:"name"` UUID string `json:"uuid"` Type StepType `json:"type,omitempty"` - Alias string `json:"alias,omitempty"` Image string `json:"image,omitempty"` Pull bool `json:"pull,omitempty"` Detached bool `json:"detach,omitempty"` diff --git a/pipeline/frontend/yaml/compiler/compiler.go b/pipeline/frontend/yaml/compiler/compiler.go index 4b6ae9158..3e82fcaaa 100644 --- a/pipeline/frontend/yaml/compiler/compiler.go +++ b/pipeline/frontend/yaml/compiler/compiler.go @@ -27,8 +27,6 @@ import ( const ( defaultCloneName = "clone" - - nameServices = "services" ) // Registry represents registry credentials @@ -161,20 +159,17 @@ func (c *Compiler) Compile(conf *yaml_types.Workflow) (*backend_types.Config, er Settings: cloneSettings, Environment: c.cloneEnv, } - name := fmt.Sprintf("%s_clone", c.prefix) - step, err := c.createProcess(name, container, backend_types.StepTypeClone) + step, err := c.createProcess(container, backend_types.StepTypeClone) if err != nil { return nil, err } stage := new(backend_types.Stage) - stage.Name = name - stage.Alias = defaultCloneName stage.Steps = append(stage.Steps, step) config.Stages = append(config.Stages, stage) } else if !c.local && !conf.SkipClone { - for i, container := range conf.Clone.ContainerList { + for _, container := range conf.Clone.ContainerList { if match, err := container.When.Match(c.metadata, false, c.env); !match && err == nil { continue } else if err != nil { @@ -182,11 +177,8 @@ func (c *Compiler) Compile(conf *yaml_types.Workflow) (*backend_types.Config, er } stage := new(backend_types.Stage) - stage.Name = fmt.Sprintf("%s_clone_%v", c.prefix, i) - stage.Alias = container.Name - name := fmt.Sprintf("%s_clone_%d", c.prefix, i) - step, err := c.createProcess(name, container, backend_types.StepTypeClone) + step, err := c.createProcess(container, backend_types.StepTypeClone) if err != nil { return nil, err } @@ -212,18 +204,15 @@ func (c *Compiler) Compile(conf *yaml_types.Workflow) (*backend_types.Config, er // add services steps if len(conf.Services.ContainerList) != 0 { stage := new(backend_types.Stage) - stage.Name = fmt.Sprintf("%s_%s", c.prefix, nameServices) - stage.Alias = nameServices - for i, container := range conf.Services.ContainerList { + for _, container := range conf.Services.ContainerList { if match, err := container.When.Match(c.metadata, false, c.env); !match && err == nil { continue } else if err != nil { return nil, err } - name := fmt.Sprintf("%s_%s_%d", c.prefix, nameServices, i) - step, err := c.createProcess(name, container, backend_types.StepTypeService) + step, err := c.createProcess(container, backend_types.StepTypeService) if err != nil { return nil, err } @@ -247,12 +236,11 @@ func (c *Compiler) Compile(conf *yaml_types.Workflow) (*backend_types.Config, er return nil, err } - name := fmt.Sprintf("%s_step_%d", c.prefix, pos) stepType := backend_types.StepTypeCommands if container.IsPlugin() { stepType = backend_types.StepTypePlugin } - step, err := c.createProcess(name, container, stepType) + step, err := c.createProcess(container, stepType) if err != nil { return nil, err } @@ -274,7 +262,7 @@ func (c *Compiler) Compile(conf *yaml_types.Workflow) (*backend_types.Config, er } // generate stages out of steps - stepStages, err := newDAGCompiler(steps, c.prefix).compile() + stepStages, err := newDAGCompiler(steps).compile() if err != nil { return nil, err } @@ -295,15 +283,12 @@ func (c *Compiler) setupCache(conf *yaml_types.Workflow, ir *backend_types.Confi } container := c.cacher.Restore(path.Join(c.metadata.Repo.Owner, c.metadata.Repo.Name), c.metadata.Curr.Commit.Branch, conf.Cache) - name := fmt.Sprintf("%s_restore_cache", c.prefix) - step, err := c.createProcess(name, container, backend_types.StepTypeCache) + step, err := c.createProcess(container, backend_types.StepTypeCache) if err != nil { return err } stage := new(backend_types.Stage) - stage.Name = name - stage.Alias = "restore_cache" stage.Steps = append(stage.Steps, step) ir.Stages = append(ir.Stages, stage) @@ -317,15 +302,12 @@ func (c *Compiler) setupCacheRebuild(conf *yaml_types.Workflow, ir *backend_type } container := c.cacher.Rebuild(path.Join(c.metadata.Repo.Owner, c.metadata.Repo.Name), c.metadata.Curr.Commit.Branch, conf.Cache) - name := fmt.Sprintf("%s_rebuild_cache", c.prefix) - step, err := c.createProcess(name, container, backend_types.StepTypeCache) + step, err := c.createProcess(container, backend_types.StepTypeCache) if err != nil { return err } stage := new(backend_types.Stage) - stage.Name = name - stage.Alias = "rebuild_cache" stage.Steps = append(stage.Steps, step) ir.Stages = append(ir.Stages, stage) diff --git a/pipeline/frontend/yaml/compiler/compiler_test.go b/pipeline/frontend/yaml/compiler/compiler_test.go index 2f8b800f2..329235a17 100644 --- a/pipeline/frontend/yaml/compiler/compiler_test.go +++ b/pipeline/frontend/yaml/compiler/compiler_test.go @@ -79,11 +79,8 @@ func TestCompilerCompile(t *testing.T) { }} defaultCloneStage := &backend_types.Stage{ - Name: "test_clone", - Alias: "clone", Steps: []*backend_types.Step{{ - Name: "test_clone", - Alias: "clone", + Name: "clone", Type: backend_types.StepTypeClone, Image: constant.DefaultCloneImage, OnSuccess: true, @@ -127,11 +124,8 @@ func TestCompilerCompile(t *testing.T) { Networks: defaultNetworks, Volumes: defaultVolumes, Stages: []*backend_types.Stage{defaultCloneStage, { - Name: "test_stage_0", - Alias: "dummy", Steps: []*backend_types.Step{{ - Name: "test_step_0", - Alias: "dummy", + Name: "dummy", Type: backend_types.StepTypePlugin, Image: "dummy_img", OnSuccess: true, @@ -164,11 +158,8 @@ func TestCompilerCompile(t *testing.T) { Networks: defaultNetworks, Volumes: defaultVolumes, Stages: []*backend_types.Stage{defaultCloneStage, { - Name: "test_stage_0", - Alias: "echo env", Steps: []*backend_types.Step{{ - Name: "test_step_0", - Alias: "echo env", + Name: "echo env", Type: backend_types.StepTypeCommands, Image: "bash", Commands: []string{"env"}, @@ -179,11 +170,8 @@ func TestCompilerCompile(t *testing.T) { ExtraHosts: []backend_types.HostAlias{}, }}, }, { - Name: "test_stage_1", - Alias: "parallel echo 1", Steps: []*backend_types.Step{{ - Name: "test_step_1", - Alias: "parallel echo 1", + Name: "parallel echo 1", Type: backend_types.StepTypeCommands, Image: "bash", Commands: []string{"echo 1"}, @@ -193,8 +181,7 @@ func TestCompilerCompile(t *testing.T) { Networks: []backend_types.Conn{{Name: "test_default", Aliases: []string{"parallel echo 1"}}}, ExtraHosts: []backend_types.HostAlias{}, }, { - Name: "test_step_2", - Alias: "parallel echo 2", + Name: "parallel echo 2", Type: backend_types.StepTypeCommands, Image: "bash", Commands: []string{"echo 2"}, @@ -227,11 +214,8 @@ func TestCompilerCompile(t *testing.T) { Networks: defaultNetworks, Volumes: defaultVolumes, Stages: []*backend_types.Stage{defaultCloneStage, { - Name: "test_stage_0", - Alias: "test_stage_0", Steps: []*backend_types.Step{{ - Name: "test_step_0", - Alias: "echo env", + Name: "echo env", Type: backend_types.StepTypeCommands, Image: "bash", Commands: []string{"env"}, @@ -241,8 +225,7 @@ func TestCompilerCompile(t *testing.T) { Networks: []backend_types.Conn{{Name: "test_default", Aliases: []string{"echo env"}}}, ExtraHosts: []backend_types.HostAlias{}, }, { - Name: "test_step_2", - Alias: "echo 2", + Name: "echo 2", Type: backend_types.StepTypeCommands, Image: "bash", Commands: []string{"echo 2"}, @@ -253,11 +236,8 @@ func TestCompilerCompile(t *testing.T) { ExtraHosts: []backend_types.HostAlias{}, }}, }, { - Name: "test_stage_1", - Alias: "test_stage_1", Steps: []*backend_types.Step{{ - Name: "test_step_1", - Alias: "echo 1", + Name: "echo 1", Type: backend_types.StepTypeCommands, Image: "bash", Commands: []string{"echo 1"}, diff --git a/pipeline/frontend/yaml/compiler/convert.go b/pipeline/frontend/yaml/compiler/convert.go index 2ff8701f8..1656930d2 100644 --- a/pipeline/frontend/yaml/compiler/convert.go +++ b/pipeline/frontend/yaml/compiler/convert.go @@ -29,7 +29,7 @@ import ( "go.woodpecker-ci.org/woodpecker/v2/pipeline/frontend/yaml/utils" ) -func (c *Compiler) createProcess(name string, container *yaml_types.Container, stepType backend_types.StepType) (*backend_types.Step, error) { +func (c *Compiler) createProcess(container *yaml_types.Container, stepType backend_types.StepType) (*backend_types.Step, error) { var ( uuid = ulid.Make() @@ -171,10 +171,9 @@ func (c *Compiler) createProcess(name string, container *yaml_types.Container, s } return &backend_types.Step{ - Name: name, + Name: container.Name, UUID: uuid.String(), Type: stepType, - Alias: container.Name, Image: container.Image, Pull: container.Pull, Detached: detached, diff --git a/pipeline/frontend/yaml/compiler/dag.go b/pipeline/frontend/yaml/compiler/dag.go index 45d4292e8..461807259 100644 --- a/pipeline/frontend/yaml/compiler/dag.go +++ b/pipeline/frontend/yaml/compiler/dag.go @@ -15,7 +15,6 @@ package compiler import ( - "fmt" "sort" backend_types "go.woodpecker-ci.org/woodpecker/v2/pipeline/backend/types" @@ -30,14 +29,12 @@ type dagCompilerStep struct { } type dagCompiler struct { - steps []*dagCompilerStep - prefix string + steps []*dagCompilerStep } -func newDAGCompiler(steps []*dagCompilerStep, prefix string) dagCompiler { +func newDAGCompiler(steps []*dagCompilerStep) dagCompiler { return dagCompiler{ - steps: steps, - prefix: prefix, + steps: steps, } } @@ -68,8 +65,6 @@ func (c dagCompiler) compileByGroup() ([]*backend_types.Stage, error) { currentGroup = s.group currentStage = new(backend_types.Stage) - currentStage.Name = fmt.Sprintf("%s_stage_%v", c.prefix, s.position) - currentStage.Alias = s.name stages = append(stages, currentStage) } @@ -85,7 +80,7 @@ func (c dagCompiler) compileByDependsOn() ([]*backend_types.Stage, error) { for _, s := range c.steps { stepMap[s.name] = s } - return convertDAGToStages(stepMap, c.prefix) + return convertDAGToStages(stepMap) } func dfsVisit(steps map[string]*dagCompilerStep, name string, visited map[string]struct{}, path []string) error { @@ -107,7 +102,7 @@ func dfsVisit(steps map[string]*dagCompilerStep, name string, visited map[string return nil } -func convertDAGToStages(steps map[string]*dagCompilerStep, prefix string) ([]*backend_types.Stage, error) { +func convertDAGToStages(steps map[string]*dagCompilerStep) ([]*backend_types.Stage, error) { addedSteps := make(map[string]struct{}) stages := make([]*backend_types.Stage, 0) @@ -128,10 +123,7 @@ func convertDAGToStages(steps map[string]*dagCompilerStep, prefix string) ([]*ba for len(steps) > 0 { addedNodesThisLevel := make(map[string]struct{}) - stage := &backend_types.Stage{ - Name: fmt.Sprintf("%s_stage_%d", prefix, len(stages)), - Alias: fmt.Sprintf("%s_stage_%d", prefix, len(stages)), - } + stage := new(backend_types.Stage) var stepsToAdd []*dagCompilerStep for name, step := range steps { diff --git a/pipeline/frontend/yaml/compiler/dag_test.go b/pipeline/frontend/yaml/compiler/dag_test.go index ae774617c..2ab5cd729 100644 --- a/pipeline/frontend/yaml/compiler/dag_test.go +++ b/pipeline/frontend/yaml/compiler/dag_test.go @@ -36,7 +36,7 @@ func TestConvertDAGToStages(t *testing.T) { dependsOn: []string{"step2"}, }, } - _, err := convertDAGToStages(steps, "") + _, err := convertDAGToStages(steps) assert.ErrorIs(t, err, &ErrStepDependencyCycle{}) steps = map[string]*dagCompilerStep{ @@ -48,7 +48,7 @@ func TestConvertDAGToStages(t *testing.T) { step: &backend_types.Step{}, }, } - _, err = convertDAGToStages(steps, "") + _, err = convertDAGToStages(steps) assert.NoError(t, err) steps = map[string]*dagCompilerStep{ @@ -68,7 +68,7 @@ func TestConvertDAGToStages(t *testing.T) { dependsOn: []string{"b", "c"}, }, } - _, err = convertDAGToStages(steps, "") + _, err = convertDAGToStages(steps) assert.NoError(t, err) steps = map[string]*dagCompilerStep{ @@ -77,7 +77,7 @@ func TestConvertDAGToStages(t *testing.T) { dependsOn: []string{"not-existing-step"}, }, } - _, err = convertDAGToStages(steps, "") + _, err = convertDAGToStages(steps) assert.ErrorIs(t, err, &ErrStepMissingDependency{}) steps = map[string]*dagCompilerStep{ @@ -86,10 +86,9 @@ func TestConvertDAGToStages(t *testing.T) { name: "echo env", group: "", step: &backend_types.Step{ - Name: "test_step_0", UUID: "01HJDPEW6R7J0JBE3F1T7Q0TYX", Type: "commands", - Alias: "echo env", + Name: "echo env", Image: "bash", }, }, @@ -99,10 +98,9 @@ func TestConvertDAGToStages(t *testing.T) { group: "", dependsOn: []string{"echo env", "echo 2"}, step: &backend_types.Step{ - Name: "test_step_1", UUID: "01HJDPF770QGRZER8RF79XVS4M", Type: "commands", - Alias: "echo 1", + Name: "echo 1", Image: "bash", }, }, @@ -111,40 +109,32 @@ func TestConvertDAGToStages(t *testing.T) { name: "echo 2", group: "", step: &backend_types.Step{ - Name: "test_step_2", UUID: "01HJDPFF5RMEYZW0YTGR1Y1ZR0", Type: "commands", - Alias: "echo 2", + Name: "echo 2", Image: "bash", }, }, } - stages, err := convertDAGToStages(steps, "test") + stages, err := convertDAGToStages(steps) assert.NoError(t, err) assert.EqualValues(t, []*backend_types.Stage{{ - Name: "test_stage_0", - Alias: "test_stage_0", Steps: []*backend_types.Step{{ - Name: "test_step_0", UUID: "01HJDPEW6R7J0JBE3F1T7Q0TYX", Type: "commands", - Alias: "echo env", + Name: "echo env", Image: "bash", }, { - Name: "test_step_2", UUID: "01HJDPFF5RMEYZW0YTGR1Y1ZR0", Type: "commands", - Alias: "echo 2", + Name: "echo 2", Image: "bash", }}, }, { - Name: "test_stage_1", - Alias: "test_stage_1", Steps: []*backend_types.Step{{ - Name: "test_step_1", UUID: "01HJDPF770QGRZER8RF79XVS4M", Type: "commands", - Alias: "echo 1", + Name: "echo 1", Image: "bash", }}, }}, stages) @@ -156,7 +146,7 @@ func TestIsDag(t *testing.T) { step: &backend_types.Step{}, }, } - c := newDAGCompiler(steps, "") + c := newDAGCompiler(steps) assert.False(t, c.isDAG()) steps = []*dagCompilerStep{ @@ -165,6 +155,6 @@ func TestIsDag(t *testing.T) { dependsOn: []string{}, }, } - c = newDAGCompiler(steps, "") + c = newDAGCompiler(steps) assert.True(t, c.isDAG()) } diff --git a/pipeline/pipeline.go b/pipeline/pipeline.go index 2d560c879..48694562e 100644 --- a/pipeline/pipeline.go +++ b/pipeline/pipeline.go @@ -93,15 +93,15 @@ func (r *Runtime) MakeLogger() zerolog.Logger { func (r *Runtime) Run(runnerCtx context.Context) error { logger := r.MakeLogger() logger.Debug().Msgf("Executing %d stages, in order of:", len(r.spec.Stages)) - for _, stage := range r.spec.Stages { - steps := []string{} + for stagePos, stage := range r.spec.Stages { + stepNames := []string{} for _, step := range stage.Steps { - steps = append(steps, step.Name) + stepNames = append(stepNames, step.Name) } logger.Debug(). - Str("Stage", stage.Name). - Str("Steps", strings.Join(steps, ",")). + Int("StagePos", stagePos). + Str("Steps", strings.Join(stepNames, ",")). Msg("stage") } diff --git a/server/pipeline/items.go b/server/pipeline/items.go index d28b56338..e669625b9 100644 --- a/server/pipeline/items.go +++ b/server/pipeline/items.go @@ -130,7 +130,7 @@ func setPipelineStepsOnPipeline(pipeline *model.Pipeline, pipelineItems []*stepb gid = pidSequence } step := &model.Step{ - Name: step.Alias, + Name: step.Name, UUID: step.UUID, PipelineID: pipeline.ID, PID: pidSequence,