From 34d9ea03729ced06573c57603606bdd03ed18f64 Mon Sep 17 00:00:00 2001 From: qwerty287 Date: Thu, 13 Jun 2024 11:56:46 +0200 Subject: [PATCH] Allow `depends_on` for services and deprecate `detached` --- docs/docs/20-usage/60-services.md | 22 ------ docs/docs/91-migrations.md | 1 + pipeline/backend/docker/convert_test.go | 1 - pipeline/backend/types/step.go | 1 - pipeline/frontend/yaml/compiler/compiler.go | 45 +++++++----- .../frontend/yaml/compiler/compiler_test.go | 73 +++++++++++++++++++ pipeline/frontend/yaml/compiler/convert.go | 12 ++- pipeline/frontend/yaml/linter/linter.go | 15 ++++ pipeline/frontend/yaml/types/container.go | 1 + pipeline/pipeline.go | 2 +- 10 files changed, 122 insertions(+), 51 deletions(-) diff --git a/docs/docs/20-usage/60-services.md b/docs/docs/20-usage/60-services.md index 4992e5dc2..983c01de7 100644 --- a/docs/docs/20-usage/60-services.md +++ b/docs/docs/20-usage/60-services.md @@ -53,28 +53,6 @@ Service containers generally expose environment variables to customize service s image: redis ``` -## Detachment - -Service and long running containers can also be included in the pipeline section of the configuration using the detach parameter without blocking other steps. This should be used when explicit control over startup order is required. - -```diff - steps: - - name: build - image: golang - commands: - - go build - - go test - - - name: database - image: redis -+ detach: true - - - name: test - image: golang - commands: - - go test -``` - Containers from detached steps will terminate when the pipeline ends. ## Initialization diff --git a/docs/docs/91-migrations.md b/docs/docs/91-migrations.md index 841dee3d0..e32bffa58 100644 --- a/docs/docs/91-migrations.md +++ b/docs/docs/91-migrations.md @@ -21,6 +21,7 @@ Some versions need some changes to the server configuration or the pipeline conf - Deprecated `environment` filter, use `when.evaluate` - Use `WOODPECKER_EXPERT_FORGE_OAUTH_HOST` instead of `WOODPECKER_DEV_GITEA_OAUTH_URL` or `WOODPECKER_DEV_OAUTH_HOST` - Deprecated `WOODPECKER_WEBHOOK_HOST` in favor of `WOODPECKER_EXPERT_WEBHOOK_HOST` +- Deprecated `detached` in favor of services ## 2.0.0 diff --git a/pipeline/backend/docker/convert_test.go b/pipeline/backend/docker/convert_test.go index b114398f6..ef2fe416c 100644 --- a/pipeline/backend/docker/convert_test.go +++ b/pipeline/backend/docker/convert_test.go @@ -130,7 +130,6 @@ func TestToConfigFull(t *testing.T) { Type: backend.StepTypeCommands, Image: "golang:1.2.3", Pull: true, - Detached: true, Privileged: true, WorkingDir: "/src/abc", Environment: map[string]string{"TAGS": "sqlite"}, diff --git a/pipeline/backend/types/step.go b/pipeline/backend/types/step.go index 6eb3471b9..db643dbb8 100644 --- a/pipeline/backend/types/step.go +++ b/pipeline/backend/types/step.go @@ -21,7 +21,6 @@ type Step struct { Type StepType `json:"type,omitempty"` Image string `json:"image,omitempty"` Pull bool `json:"pull,omitempty"` - Detached bool `json:"detach,omitempty"` Privileged bool `json:"privileged,omitempty"` WorkingDir string `json:"working_dir,omitempty"` Environment map[string]string `json:"environment,omitempty"` diff --git a/pipeline/frontend/yaml/compiler/compiler.go b/pipeline/frontend/yaml/compiler/compiler.go index 5db566513..0f2508afe 100644 --- a/pipeline/frontend/yaml/compiler/compiler.go +++ b/pipeline/frontend/yaml/compiler/compiler.go @@ -219,29 +219,36 @@ func (c *Compiler) Compile(conf *yaml_types.Workflow) (*backend_types.Config, er } } + steps := make([]*dagCompilerStep, 0, len(conf.Steps.ContainerList)+len(conf.Services.ContainerList)) + // add services steps - if len(conf.Services.ContainerList) != 0 { - stage := new(backend_types.Stage) - - 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 - } - - step, err := c.createProcess(container, backend_types.StepTypeService) - if err != nil { - return nil, err - } - - stage.Steps = append(stage.Steps, step) + for pos, container := range conf.Services.ContainerList { + // Skip if local and should not run local + if c.local && !container.When.IsLocal() { + continue } - config.Stages = append(config.Stages, stage) + + if match, err := container.When.Match(c.metadata, false, c.env); !match && err == nil { + continue + } else if err != nil { + return nil, err + } + + step, err := c.createProcess(container, backend_types.StepTypeService) + if err != nil { + return nil, err + } + + steps = append(steps, &dagCompilerStep{ + step: step, + position: pos, + name: container.Name, + dependsOn: container.DependsOn, + }) } // add pipeline steps - steps := make([]*dagCompilerStep, 0, len(conf.Steps.ContainerList)) + posServices := len(conf.Services.ContainerList) for pos, container := range conf.Steps.ContainerList { // Skip if local and should not run local if c.local && !container.When.IsLocal() { @@ -272,7 +279,7 @@ func (c *Compiler) Compile(conf *yaml_types.Workflow) (*backend_types.Config, er steps = append(steps, &dagCompilerStep{ step: step, - position: pos, + position: pos+posServices, // services should be shown on top name: container.Name, group: container.Group, dependsOn: container.DependsOn, diff --git a/pipeline/frontend/yaml/compiler/compiler_test.go b/pipeline/frontend/yaml/compiler/compiler_test.go index 41e055b36..391860eba 100644 --- a/pipeline/frontend/yaml/compiler/compiler_test.go +++ b/pipeline/frontend/yaml/compiler/compiler_test.go @@ -277,6 +277,79 @@ func TestCompilerCompile(t *testing.T) { }}}}, backConf: nil, expectedErr: "step 'dummy' depends on unknown step 'not exist'", + }, + { + name: "workflow with steps, services and depends_on", + fronConf: &yaml_types.Workflow{Steps: yaml_types.ContainerList{ContainerList: []*yaml_types.Container{{ + Name: "echo env", + Image: "bash", + Commands: []string{"env"}, + }}}, Services: yaml_types.ContainerList{ContainerList: []*yaml_types.Container{{ + Name: "service", + Image: "bash", + Commands: []string{"env"}, + }, { + Name: "service-depend", + Image: "bash", + Commands: []string{"echo 1"}, + DependsOn: []string{"echo env"}, + }, { + Name: "service-depend-on-service", + Image: "bash", + Commands: []string{"echo 1"}, + DependsOn: []string{"service-depend"}, + }}}}, + backConf: &backend_types.Config{ + Networks: defaultNetworks, + Volumes: defaultVolumes, + Stages: []*backend_types.Stage{defaultCloneStage, { + Steps: []*backend_types.Step{{ + Name: "service", + Type: backend_types.StepTypeService, + Image: "bash", + Commands: []string{"env"}, + OnSuccess: true, + Failure: "fail", + Volumes: []string{defaultVolumes[0].Name + ":"}, + Networks: []backend_types.Conn{{Name: "test_default", Aliases: []string{"service"}}}, + ExtraHosts: []backend_types.HostAlias{}, + }, { + Name: "echo env", + Type: backend_types.StepTypeCommands, + Image: "bash", + Commands: []string{"env"}, + OnSuccess: true, + Failure: "fail", + Volumes: []string{defaultVolumes[0].Name + ":"}, + Networks: []backend_types.Conn{{Name: "test_default", Aliases: []string{"echo env"}}}, + ExtraHosts: []backend_types.HostAlias{}, + }}, + }, { + Steps: []*backend_types.Step{{ + Name: "service-depend", + Type: backend_types.StepTypeService, + Image: "bash", + Commands: []string{"echo 1"}, + OnSuccess: true, + Failure: "fail", + Volumes: []string{defaultVolumes[0].Name + ":"}, + Networks: []backend_types.Conn{{Name: "test_default", Aliases: []string{"service-depend"}}}, + ExtraHosts: []backend_types.HostAlias{}, + }}, + }, { + Steps: []*backend_types.Step{{ + Name: "service-depend-on-service", + Type: backend_types.StepTypeService, + Image: "bash", + Commands: []string{"echo 1"}, + OnSuccess: true, + Failure: "fail", + Volumes: []string{defaultVolumes[0].Name + ":"}, + Networks: []backend_types.Conn{{Name: "test_default", Aliases: []string{"service-depend-on-service"}}}, + ExtraHosts: []backend_types.HostAlias{}, + }}, + }}, + }, }, } diff --git a/pipeline/frontend/yaml/compiler/convert.go b/pipeline/frontend/yaml/compiler/convert.go index 92d68fe45..ffd2e3213 100644 --- a/pipeline/frontend/yaml/compiler/convert.go +++ b/pipeline/frontend/yaml/compiler/convert.go @@ -34,7 +34,6 @@ func (c *Compiler) createProcess(container *yaml_types.Container, stepType backe var ( uuid = ulid.Make() - detached bool workingDir string workspace = fmt.Sprintf("%s_default:%s", c.prefix, c.base) @@ -80,11 +79,11 @@ func (c *Compiler) createProcess(container *yaml_types.Container, stepType backe environment["CI_WORKSPACE"] = path.Join(c.base, c.path) - if stepType == backend_types.StepTypeService || container.Detached { - detached = true + if container.Detached { + stepType = backend_types.StepTypeService } - if !detached || len(container.Commands) != 0 { + if stepType != backend_types.StepTypeService || len(container.Commands) != 0 { workingDir = c.stepWorkingDir(container) } @@ -104,8 +103,8 @@ func (c *Compiler) createProcess(container *yaml_types.Container, stepType backe return secret.Value, nil } - // TODO: why don't we pass secrets to detached steps? - if !detached { + // TODO: why don't we pass settings to services? + if stepType != backend_types.StepTypeService { if err := settings.ParamsToEnv(container.Settings, environment, "PLUGIN_", true, getSecretValue); err != nil { return nil, err } @@ -189,7 +188,6 @@ func (c *Compiler) createProcess(container *yaml_types.Container, stepType backe Type: stepType, Image: container.Image, Pull: container.Pull, - Detached: detached, Privileged: privileged, WorkingDir: workingDir, Environment: environment, diff --git a/pipeline/frontend/yaml/linter/linter.go b/pipeline/frontend/yaml/linter/linter.go index 7120d2ad8..dce17cd2f 100644 --- a/pipeline/frontend/yaml/linter/linter.go +++ b/pipeline/frontend/yaml/linter/linter.go @@ -351,6 +351,21 @@ func (l *Linter) lintDeprecations(config *WorkflowConfig) (err error) { } } + for _, step := range parsed.Steps.ContainerList { + if step.Detached { + err = multierr.Append(err, &errorTypes.PipelineError{ + Type: errorTypes.PipelineErrorTypeDeprecation, + Message: "Detached is deprecated, use services", + Data: errors.DeprecationErrorData{ + File: config.File, + Field: fmt.Sprintf("steps.%s.detached", step.Name), + Docs: "https://woodpecker-ci.org/docs/usage/services", + }, + IsWarning: true, + }) + } + } + return err } diff --git a/pipeline/frontend/yaml/types/container.go b/pipeline/frontend/yaml/types/container.go index 6de8ab9e7..654945882 100644 --- a/pipeline/frontend/yaml/types/container.go +++ b/pipeline/frontend/yaml/types/container.go @@ -36,6 +36,7 @@ type ( BackendOptions map[string]any `yaml:"backend_options,omitempty"` Commands base.StringOrSlice `yaml:"commands,omitempty"` Entrypoint base.StringOrSlice `yaml:"entrypoint,omitempty"` + // TODO deprecated remove in 3.0 Detached bool `yaml:"detach,omitempty"` Directory string `yaml:"directory,omitempty"` Failure string `yaml:"failure,omitempty"` diff --git a/pipeline/pipeline.go b/pipeline/pipeline.go index 7267c6a57..87a3d0da3 100644 --- a/pipeline/pipeline.go +++ b/pipeline/pipeline.go @@ -250,7 +250,7 @@ func (r *Runtime) exec(step *backend.Step) (*backend.State, error) { } // nothing else to do, this is a detached process. - if step.Detached { + if step.Type == backend.StepTypeService { return nil, nil }