Allow depends_on for services and deprecate detached

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

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
 

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

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"`

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,

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{},
+					}},
+				}},
+			},
 		},
 	}
 

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,

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
 }
 

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"`

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
 	}