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

Add depends_on support for steps #2771

Merged
merged 35 commits into from
Dec 24, 2023
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
b14e5cf
Add depends_on steps support
anbraten Nov 6, 2023
84a2976
update schema
anbraten Nov 6, 2023
788abbc
fix wrong log
anbraten Nov 6, 2023
bb787ac
fix infinity loop
anbraten Nov 7, 2023
c77062b
Merge remote-tracking branch 'upstream/main' into dag
anbraten Nov 24, 2023
889e0ad
update docs
anbraten Nov 24, 2023
5b383ad
add test
anbraten Nov 24, 2023
5a7edf3
add linter deprecation for group
anbraten Nov 24, 2023
fa4a5eb
add loop prevention error
anbraten Nov 24, 2023
1af9fde
fix cli lint throwing error on warnings
anbraten Nov 24, 2023
3bff611
fix tests
anbraten Nov 24, 2023
6c253f1
Apply suggestions from code review
anbraten Dec 14, 2023
4210779
Merge remote-tracking branch 'upstream/main' into dag
anbraten Dec 14, 2023
fa72e48
Merge branch 'dag' of github.com:anbraten/woodpecker into dag
anbraten Dec 14, 2023
d822161
use stringorslice
anbraten Dec 14, 2023
f3ab31f
Merge branch 'main' into dag
6543 Dec 19, 2023
3573bca
Merge branch 'main' into dag
6543 Dec 22, 2023
960cf73
update&add comment + alocate mem
6543 Dec 22, 2023
e8db845
move step dag generation into own file
6543 Dec 22, 2023
19aacf5
prepare to make dagCompiler more portable
6543 Dec 22, 2023
cfa9f62
move test and adjust
6543 Dec 22, 2023
3c9edd5
fix test :)
6543 Dec 22, 2023
cce7f2b
more tests and a fix
6543 Dec 22, 2023
bf1ecce
Merge branch 'main' into dag
6543 Dec 22, 2023
92a59b2
Merge branch 'dag' into dag_suggestions
6543 Dec 22, 2023
a0fe800
Merge branch 'main' into dag
6543 Dec 22, 2023
50ad30b
Merge branch 'dag' into dag_suggestions
6543 Dec 22, 2023
0c613e1
rename to steps
6543 Dec 24, 2023
5f8a415
curr to current
6543 Dec 24, 2023
05f2b1b
Merge branch 'dag_suggestions' into dag
6543 Dec 24, 2023
c3c73da
Merge branch 'main' into dag
6543 Dec 24, 2023
ddb05ea
please linter and move errors to dedicated file
6543 Dec 24, 2023
87010cd
ehh
6543 Dec 24, 2023
eca14ac
unify receiver names
anbraten Dec 24, 2023
ffde42f
nit
anbraten Dec 24, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cli/lint/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,9 @@ func lintFile(_ *cli.Context, file string) error {
// TODO: lint multiple files at once to allow checks for sth like "depends_on" to work
err = linter.New(linter.WithTrusted(true)).Lint([]*linter.WorkflowConfig{config})
if err != nil {
fmt.Printf("🔥 %s has errors:\n", output.String(config.File).Underline())
fmt.Printf("🔥 %s has warning / errors:\n", output.String(config.File).Underline())
anbraten marked this conversation as resolved.
Show resolved Hide resolved

hasErrors := true
hasErrors := false
6543 marked this conversation as resolved.
Show resolved Hide resolved
for _, err := range pipeline_errors.GetPipelineErrors(err) {
line := " "

Expand Down
39 changes: 17 additions & 22 deletions docs/docs/20-usage/20-workflow-syntax.md
Original file line number Diff line number Diff line change
Expand Up @@ -444,33 +444,28 @@ when:
- evaluate: 'SKIP != "true"'
```

### `group` - Parallel execution
### `depends_on`

Woodpecker supports parallel step execution for same-machine fan-in and fan-out. Parallel steps are configured using the `group` attribute. This instructs the agent to execute the named group in parallel.

Example parallel configuration:
Normally steps of a workflow are executed serially in the order in which they are defined. As soon as you set `depends_on` for a step a [directed acyclic graph](https://en.wikipedia.org/wiki/Directed_acyclic_graph), all steps of the workflow will be executed in parallel besides the steps that have a dependency set to another step using `depends_on`:
anbraten marked this conversation as resolved.
Show resolved Hide resolved

```diff
steps:
backend:
+ group: build
image: golang
commands:
- go build
- go test
frontend:
+ group: build
image: node
commands:
- npm install
- npm run test
- npm run build
publish:
image: plugins/docker
repo: octocat/hello-world
```
build: # build will be executed immediately
image: golang
commands:
- go build

deploy:
image: plugins/docker
settings:
repo: foo/bar
+ depends_on: [build, test] # deploy will be executed after build and test finished

In the above example, the `frontend` and `backend` steps are executed in parallel. The agent will not execute the `publish` step until the group completes.
test: # test will be executed immediately as no dependencies are set
image: golang
commands:
- go test
```

### `volumes`

Expand Down
2 changes: 1 addition & 1 deletion docs/docs/91-migrations.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ Some versions need some changes to the server configuration or the pipeline conf

## `next`

No breaking changes yet
- Deprecated `steps.[name].group` in favor of `steps.[name].depends_on` (see [workflow syntax](./20-usage/20-workflow-syntax.md#depends_on) to learn how to set dependencies)

## 2.0.0

Expand Down
4 changes: 2 additions & 2 deletions pipeline/backend/docker/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -318,10 +318,10 @@
for _, step := range stage.Steps {
containerName := toContainerName(step)
if err := e.client.ContainerKill(noContext, containerName, "9"); err != nil && !isErrContainerNotFoundOrNotRunning(err) {
log.Error().Err(err).Msgf("could not kill container '%s'", stage.Name)
log.Error().Err(err).Msgf("could not kill container '%s'", step.Name)

Check warning on line 321 in pipeline/backend/docker/docker.go

View check run for this annotation

Codecov / codecov/patch

pipeline/backend/docker/docker.go#L321

Added line #L321 was not covered by tests
6543 marked this conversation as resolved.
Show resolved Hide resolved
}
if err := e.client.ContainerRemove(noContext, containerName, removeOpts); err != nil && !isErrContainerNotFoundOrNotRunning(err) {
log.Error().Err(err).Msgf("could not remove container '%s'", stage.Name)
log.Error().Err(err).Msgf("could not remove container '%s'", step.Name)

Check warning on line 324 in pipeline/backend/docker/docker.go

View check run for this annotation

Codecov / codecov/patch

pipeline/backend/docker/docker.go#L324

Added line #L324 was not covered by tests
}
}
}
Expand Down
101 changes: 100 additions & 1 deletion pipeline/frontend/yaml/compiler/compiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,11 @@
netrcOnlyTrusted bool
}

type stepWithDependsOn struct {
step *backend_types.Step
dependsOn []string
}

// New creates a new Compiler with options.
func New(opts ...Option) *Compiler {
compiler := &Compiler{
Expand Down Expand Up @@ -234,8 +239,11 @@
}

// add pipeline steps. 1 pipeline step per stage, at the moment
stepStages := make([]*backend_types.Stage, 0)
6543 marked this conversation as resolved.
Show resolved Hide resolved
var stage *backend_types.Stage
var group string
steps := make(map[string]*stepWithDependsOn)
useDag := false
for i, container := range conf.Steps.ContainerList {
// Skip if local and should not run local
if c.local && !container.When.IsLocal() {
Expand All @@ -254,7 +262,11 @@
stage = new(backend_types.Stage)
stage.Name = fmt.Sprintf("%s_stage_%v", c.prefix, i)
stage.Alias = container.Name
config.Stages = append(config.Stages, stage)
stepStages = append(stepStages, stage)
}

if len(container.DependsOn) > 0 {
useDag = true
}

name := fmt.Sprintf("%s_step_%d", c.prefix, i)
Expand All @@ -274,9 +286,24 @@
}
}

steps[container.Name] = &stepWithDependsOn{
step: step,
dependsOn: container.DependsOn,
}
stage.Steps = append(stage.Steps, step)
}

// dag is used if one or more steps have a depends_on
if useDag {
stages, err := convertDAGToStages(steps)
6543 marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, err
}

Check warning on line 301 in pipeline/frontend/yaml/compiler/compiler.go

View check run for this annotation

Codecov / codecov/patch

pipeline/frontend/yaml/compiler/compiler.go#L300-L301

Added lines #L300 - L301 were not covered by tests
config.Stages = append(config.Stages, stages...)
} else {
config.Stages = append(config.Stages, stepStages...)
}

err = c.setupCacheRebuild(conf, config)
if err != nil {
return nil, err
Expand Down Expand Up @@ -328,3 +355,75 @@

return nil
}

func dfsVisit(steps map[string]*stepWithDependsOn, name string, visited map[string]struct{}, path []string) error {
6543 marked this conversation as resolved.
Show resolved Hide resolved
if _, ok := visited[name]; ok {
return fmt.Errorf("cycle detected: %v", path)
}

visited[name] = struct{}{}
path = append(path, name)

for _, dep := range steps[name].dependsOn {
if err := dfsVisit(steps, dep, visited, path); err != nil {
return err
}
}

return nil
}

func convertDAGToStages(steps map[string]*stepWithDependsOn) ([]*backend_types.Stage, error) {
6543 marked this conversation as resolved.
Show resolved Hide resolved
// Initialize the levels map
anbraten marked this conversation as resolved.
Show resolved Hide resolved
addedSteps := make(map[string]struct{})
stages := make([]*backend_types.Stage, 0)

for name, step := range steps {
// check if all depends_on are valid
for _, dep := range step.dependsOn {
if _, ok := steps[dep]; !ok {
return nil, fmt.Errorf("step %s depends on unknown step %s", name, dep)
}

Check warning on line 386 in pipeline/frontend/yaml/compiler/compiler.go

View check run for this annotation

Codecov / codecov/patch

pipeline/frontend/yaml/compiler/compiler.go#L385-L386

Added lines #L385 - L386 were not covered by tests
}

// check if there are cycles
visited := make(map[string]struct{})
if err := dfsVisit(steps, name, visited, []string{}); err != nil {
return nil, err
}
}

for len(steps) > 0 {
addedNodesThisLevel := make(map[string]struct{})
stage := &backend_types.Stage{
Name: fmt.Sprintf("stage_%d", len(stages)),
Alias: fmt.Sprintf("stage_%d", len(stages)),
}

for name, step := range steps {
if allDependenciesSatisfied(step, addedSteps) {
stage.Steps = append(stage.Steps, step.step)
addedNodesThisLevel[name] = struct{}{}
delete(steps, name)
}
}

for name := range addedNodesThisLevel {
addedSteps[name] = struct{}{}
}

stages = append(stages, stage)
}

return stages, nil
}

func allDependenciesSatisfied(step *stepWithDependsOn, addedSteps map[string]struct{}) bool {
for _, childName := range step.dependsOn {
_, ok := addedSteps[childName]
if !ok {
return false
}
}
return true
}
Loading