Commit 36e780e3 authored by Furkan Ayhan's avatar Furkan Ayhan

Fix skipped status of DAG pipelines

In this commit, I also moved the DAG hack from status-collection to
status-composite class.
parent 1a4cdc25
...@@ -77,15 +77,8 @@ module Ci ...@@ -77,15 +77,8 @@ module Ci
private private
def status_for_array(statuses, dag:) def status_for_array(statuses, dag:)
# TODO: This is hack to support
# the same exact behaviour for Atomic and Legacy processing
# that DAG is blocked from executing if dependent is not "complete"
if dag && statuses.any? { |status| Ci::HasStatus::COMPLETED_STATUSES.exclude?(status[:status]) }
return 'pending'
end
result = Gitlab::Ci::Status::Composite result = Gitlab::Ci::Status::Composite
.new(statuses) .new(statuses, dag: dag)
.status .status
result || 'success' result || 'success'
end end
......
---
title: Fix skipped status of DAG pipelines
merge_request: 39205
author:
type: fixed
...@@ -7,7 +7,7 @@ module Gitlab ...@@ -7,7 +7,7 @@ module Gitlab
include Gitlab::Utils::StrongMemoize include Gitlab::Utils::StrongMemoize
# This class accepts an array of arrays/hashes/or objects # This class accepts an array of arrays/hashes/or objects
def initialize(all_statuses, with_allow_failure: true) def initialize(all_statuses, with_allow_failure: true, dag: false)
unless all_statuses.respond_to?(:pluck) unless all_statuses.respond_to?(:pluck)
raise ArgumentError, "all_statuses needs to respond to `.pluck`" raise ArgumentError, "all_statuses needs to respond to `.pluck`"
end end
...@@ -15,6 +15,7 @@ module Gitlab ...@@ -15,6 +15,7 @@ module Gitlab
@status_set = Set.new @status_set = Set.new
@status_key = 0 @status_key = 0
@allow_failure_key = 1 if with_allow_failure @allow_failure_key = 1 if with_allow_failure
@dag = dag
consume_all_statuses(all_statuses) consume_all_statuses(all_statuses)
end end
...@@ -31,7 +32,13 @@ module Gitlab ...@@ -31,7 +32,13 @@ module Gitlab
return if none? return if none?
strong_memoize(:status) do strong_memoize(:status) do
if only_of?(:skipped, :ignored) if @dag && any_of?(:skipped)
# The DAG job is skipped if one of the needs does not run at all.
'skipped'
elsif @dag && !only_of?(:success, :failed, :canceled, :skipped, :success_with_warnings)
# DAG is blocked from executing if a dependent is not "complete"
'pending'
elsif only_of?(:skipped, :ignored)
'skipped' 'skipped'
elsif only_of?(:success, :skipped, :success_with_warnings, :ignored) elsif only_of?(:success, :skipped, :success_with_warnings, :ignored)
'success' 'success'
......
...@@ -20,7 +20,7 @@ RSpec.describe Gitlab::Ci::Status::Composite do ...@@ -20,7 +20,7 @@ RSpec.describe Gitlab::Ci::Status::Composite do
shared_examples 'compares status and warnings' do shared_examples 'compares status and warnings' do
let(:composite_status) do let(:composite_status) do
described_class.new(all_statuses) described_class.new(all_statuses, dag: dag)
end end
it 'returns status and warnings?' do it 'returns status and warnings?' do
...@@ -30,21 +30,29 @@ RSpec.describe Gitlab::Ci::Status::Composite do ...@@ -30,21 +30,29 @@ RSpec.describe Gitlab::Ci::Status::Composite do
end end
context 'allow_failure: false' do context 'allow_failure: false' do
where(:build_statuses, :result, :has_warnings) do where(:build_statuses, :dag, :result, :has_warnings) do
%i(skipped) | 'skipped' | false %i(skipped) | false | 'skipped' | false
%i(skipped success) | 'success' | false %i(skipped success) | false | 'success' | false
%i(created) | 'created' | false %i(skipped success) | true | 'skipped' | false
%i(preparing) | 'preparing' | false %i(created) | false | 'created' | false
%i(canceled success skipped) | 'canceled' | false %i(preparing) | false | 'preparing' | false
%i(pending created skipped) | 'pending' | false %i(canceled success skipped) | false | 'canceled' | false
%i(pending created skipped success) | 'running' | false %i(canceled success skipped) | true | 'skipped' | false
%i(running created skipped success) | 'running' | false %i(pending created skipped) | false | 'pending' | false
%i(success waiting_for_resource) | 'waiting_for_resource' | false %i(pending created skipped success) | false | 'running' | false
%i(success manual) | 'manual' | false %i(running created skipped success) | false | 'running' | false
%i(success scheduled) | 'scheduled' | false %i(pending created skipped) | true | 'skipped' | false
%i(created preparing) | 'preparing' | false %i(pending created skipped success) | true | 'skipped' | false
%i(created success pending) | 'running' | false %i(running created skipped success) | true | 'skipped' | false
%i(skipped success failed) | 'failed' | false %i(success waiting_for_resource) | false | 'waiting_for_resource' | false
%i(success manual) | false | 'manual' | false
%i(success scheduled) | false | 'scheduled' | false
%i(created preparing) | false | 'preparing' | false
%i(created success pending) | false | 'running' | false
%i(skipped success failed) | false | 'failed' | false
%i(skipped success failed) | true | 'skipped' | false
%i(success manual) | true | 'pending' | false
%i(success failed created) | true | 'pending' | false
end end
with_them do with_them do
...@@ -57,11 +65,12 @@ RSpec.describe Gitlab::Ci::Status::Composite do ...@@ -57,11 +65,12 @@ RSpec.describe Gitlab::Ci::Status::Composite do
end end
context 'allow_failure: true' do context 'allow_failure: true' do
where(:build_statuses, :result, :has_warnings) do where(:build_statuses, :dag, :result, :has_warnings) do
%i(manual) | 'skipped' | false %i(manual) | false | 'skipped' | false
%i(skipped failed) | 'success' | true %i(skipped failed) | false | 'success' | true
%i(created failed) | 'created' | true %i(skipped failed) | true | 'skipped' | true
%i(preparing manual) | 'preparing' | false %i(created failed) | false | 'created' | true
%i(preparing manual) | false | 'preparing' | false
end end
with_them do with_them do
......
...@@ -47,16 +47,13 @@ transitions: ...@@ -47,16 +47,13 @@ transitions:
- event: drop - event: drop
jobs: [build_2] jobs: [build_2]
expect: expect:
pipeline: running pipeline: failed
stages: stages:
build: failed build: failed
test: skipped test: skipped
deploy: pending deploy: skipped
jobs: jobs:
build_1: success build_1: success
build_2: failed build_2: failed
test: skipped test: skipped
deploy: pending deploy: skipped
# TODO: should we run deploy?
# Further discussions: https://gitlab.com/gitlab-org/gitlab/-/issues/213080
...@@ -33,31 +33,14 @@ init: ...@@ -33,31 +33,14 @@ init:
transitions: transitions:
- event: success - event: success
jobs: [build_1, build_2] jobs: [build_1, build_2]
expect:
pipeline: running
stages:
build: success
test: skipped
deploy: pending
jobs:
build_1: success
build_2: success
test: skipped
deploy: pending
- event: success
jobs: [deploy]
expect: expect:
pipeline: success pipeline: success
stages: stages:
build: success build: success
test: skipped test: skipped
deploy: success deploy: skipped
jobs: jobs:
build_1: success build_1: success
build_2: success build_2: success
test: skipped test: skipped
deploy: success deploy: skipped
# TODO: should we run deploy?
# Further discussions: https://gitlab.com/gitlab-org/gitlab/-/issues/213080
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment