Skip to content

Inconsistent behavior when using needs with skipped jobs

Background info

This issue is very similar to #31264 (closed), so we can migrate these two issues into this one.

And we can work on solving these DAG "status" problems at once.

Summary

In issue #31526 (closed), we tried to fix a wrong behavior. Basic example for that (it was fixed by !23405 (merged)):

build:
  stage: build
  script: exit 1
test:
  stage: test
  script: exit 0
deploy:
  stage: deploy
  script: exit 0
  needs: [test]
  • build: failed
  • test: skipped, because build failed
  • deploy: run

We fixed it by not considering skipped status as a success status for DAGs: !23405 (diffs)

Later, there is a new use case for this bug that we did not cover: #31526 (comment 307179057)

Steps to reproduce

Introduction info: current_status of Ci::ProcessBuildService is calculated by Gitlab::Ci::Status::Composite. Composite returns success when the dependent statuses consist of only :success, :skipped, :success_with_warnings, :ignored.

Example 1

build_1:
  stage: build
  script:
    - exit 0

build_2:
  stage: build
  script:
    - exit 1

test:
  stage: test
  script:
    - exit 0

deploy:
  stage: deploy
  script:
    - exit 0
  needs: [build_1, test]

https://gitlab.com/furkanayhan/test-project-for-gitlab-213080/pipelines/131731985

Screen_Shot_2020-03-20_at_13.40.01

  • test:
    • "Composite" calculates with [success, failed], and returns failed.
    • "ProcessBuildService" decides this job to be skipped.
  • deploy:
    • "Composite" calculates with [success, skipped], and returns success.
    • "ProcessBuildService" decides this job to be run.
  • pipeline:
    • "Composite" calculates with [success, skipped, failed], and returns failed.

Example 2

build_1:
  stage: build
  script:
    - exit 0

build_2:
  stage: build
  script:
    - exit 0

test:
  stage: test
  when: on_failure
  script:
    - exit 0

deploy:
  stage: deploy
  script:
    - exit 0
  needs: [build_1, test]

https://gitlab.com/furkanayhan/test-project-for-gitlab-213080/pipelines/131732227

Screen_Shot_2020-03-24_at_17.54.48

  • test:
    • "Composite" calculates with [success], and returns success.
    • "ProcessBuildService" decides this job to be skipped.
  • deploy:
    • "Composite" calculates with [success, skipped], and returns success.
    • "ProcessBuildService" decides this job to be run.
  • pipeline:
    • "Composite" calculates with [success, skipped], and returns success.

Conclusion

If we say, needs is a keyword that makes "deploy" strictly be depended on "build_1" and "test", we should not allow "deploy" to be run when one of them is not "success".

  • For the 1st example, do we want deploy to be skipped?
  • For the 2nd example, do we want deploy to be skipped?

Example Project

What is the current bug behavior?

We allow jobs to run when some of its dependencies/needs are skipped.

What is the expected correct behavior?

OPEN TO DISCUSSION ❗

Edited by Furkan Ayhan