summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKamil Trzciński <ayufan@ayufan.eu>2019-08-13 19:08:44 +0200
committerKamil Trzciński <ayufan@ayufan.eu>2019-08-14 15:16:37 +0200
commit24f93807ccca38597db37e8e60216f97e46cc5e0 (patch)
tree3dbaaddef618a912beae50f2cf0db013f080957d
parentebb1314878c4456070547c16f418b03685c46c8b (diff)
downloadgitlab-ce-24f93807ccca38597db37e8e60216f97e46cc5e0.tar.gz
Implement Composite Status in Ruby
-rw-r--r--app/models/ci/pipeline.rb23
-rw-r--r--app/models/commit_status.rb8
-rw-r--r--app/models/concerns/has_status.rb41
-rw-r--r--app/services/ci/process_pipeline_service.rb12
-rw-r--r--lib/gitlab/ci/status/composite_status.rb75
-rw-r--r--lib/gitlab/ci/status/grouped_statuses.rb75
-rw-r--r--spec/models/commit_status_spec.rb23
7 files changed, 171 insertions, 86 deletions
diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 3b28eb246db..ab1f0641eb6 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -369,20 +369,15 @@ module Ci
def legacy_stages
# TODO, this needs refactoring, see gitlab-ce#26481.
- stages_query = statuses
- .group('stage').select(:stage).order('max(stage_idx)')
-
- status_sql = statuses.latest.where('stage=sg.stage').status_sql
-
- warnings_sql = statuses.latest.select('COUNT(*)')
- .where('stage=sg.stage').failed_but_allowed.to_sql
-
- stages_with_statuses = CommitStatus.from(stages_query, :sg)
- .pluck('sg.stage', status_sql, "(#{warnings_sql})")
-
- stages_with_statuses.map do |stage|
- Ci::LegacyStage.new(self, Hash[%i[name status warnings].zip(stage)])
- end
+ Gitlab::Ci::Status::GroupedStatuses
+ .new(statuses.latest, :stage, :stage_idx)
+ .group(:stage, :stage_idx)
+ .map do |stage|
+ Ci::LegacyStage.new(self,
+ name: stage[:stage],
+ status: stage[:status],
+ warnings: stage[:warnings])
+ end
end
def valid_commit_sha
diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb
index a88cac6b8e6..633d2d0bb51 100644
--- a/app/models/commit_status.rb
+++ b/app/models/commit_status.rb
@@ -27,14 +27,6 @@ class CommitStatus < ApplicationRecord
where(allow_failure: true, status: [:failed, :canceled])
end
- scope :exclude_ignored, -> do
- # We want to ignore failed but allowed to fail jobs.
- #
- # TODO, we also skip ignored optional manual actions.
- where("allow_failure = ? OR status IN (?)",
- false, all_state_names - [:failed, :canceled, :manual])
- end
-
scope :latest, -> { where(retried: [false, nil]) }
scope :retried, -> { where(retried: true) }
scope :ordered, -> { order(:name) }
diff --git a/app/models/concerns/has_status.rb b/app/models/concerns/has_status.rb
index 27a5c3d5286..bc6aed53343 100644
--- a/app/models/concerns/has_status.rb
+++ b/app/models/concerns/has_status.rb
@@ -10,6 +10,7 @@ module HasStatus
ACTIVE_STATUSES = %w[preparing pending running].freeze
COMPLETED_STATUSES = %w[success failed canceled skipped].freeze
ORDERED_STATUSES = %w[failed preparing pending running manual scheduled canceled success skipped created].freeze
+ WARNING_STATUSES = %w[manual failed canceled].to_set.freeze
STATUSES_ENUM = { created: 0, pending: 1, running: 2, success: 3,
failed: 4, canceled: 5, skipped: 6, manual: 7,
scheduled: 8, preparing: 9 }.freeze
@@ -17,44 +18,10 @@ module HasStatus
UnknownStatusError = Class.new(StandardError)
class_methods do
- def status_sql
- scope_relevant = respond_to?(:exclude_ignored) ? exclude_ignored : all
- scope_warnings = respond_to?(:failed_but_allowed) ? failed_but_allowed : none
-
- builds = scope_relevant.select('count(*)').to_sql
- created = scope_relevant.created.select('count(*)').to_sql
- success = scope_relevant.success.select('count(*)').to_sql
- manual = scope_relevant.manual.select('count(*)').to_sql
- scheduled = scope_relevant.scheduled.select('count(*)').to_sql
- preparing = scope_relevant.preparing.select('count(*)').to_sql
- pending = scope_relevant.pending.select('count(*)').to_sql
- running = scope_relevant.running.select('count(*)').to_sql
- skipped = scope_relevant.skipped.select('count(*)').to_sql
- canceled = scope_relevant.canceled.select('count(*)').to_sql
- warnings = scope_warnings.select('count(*) > 0').to_sql.presence || 'false'
-
- Arel.sql(
- "(CASE
- WHEN (#{builds})=(#{skipped}) AND (#{warnings}) THEN 'success'
- WHEN (#{builds})=(#{skipped}) THEN 'skipped'
- WHEN (#{builds})=(#{success}) THEN 'success'
- WHEN (#{builds})=(#{created}) THEN 'created'
- WHEN (#{builds})=(#{preparing}) THEN 'preparing'
- WHEN (#{builds})=(#{success})+(#{skipped}) THEN 'success'
- WHEN (#{builds})=(#{success})+(#{skipped})+(#{canceled}) THEN 'canceled'
- WHEN (#{builds})=(#{created})+(#{skipped})+(#{pending}) THEN 'pending'
- WHEN (#{running})+(#{pending})>0 THEN 'running'
- WHEN (#{manual})>0 THEN 'manual'
- WHEN (#{scheduled})>0 THEN 'scheduled'
- WHEN (#{preparing})>0 THEN 'preparing'
- WHEN (#{created})>0 THEN 'running'
- ELSE 'failed'
- END)"
- )
- end
-
def status
- all.pluck(status_sql).first
+ Gitlab::Ci::Status::GroupedStatuses
+ .new(all)
+ .one&.dig(:status)
end
def started_at
diff --git a/app/services/ci/process_pipeline_service.rb b/app/services/ci/process_pipeline_service.rb
index 99d4ff9ecd1..bc40b698ba6 100644
--- a/app/services/ci/process_pipeline_service.rb
+++ b/app/services/ci/process_pipeline_service.rb
@@ -33,9 +33,9 @@ module Ci
return unless HasStatus::COMPLETED_STATUSES.include?(current_status)
- created_processables_in_stage_without_needs(index).select do |build|
+ created_processables_in_stage_without_needs(index).find_each.select do |build|
process_build(build, current_status)
- end
+ end.any?
end
def process_builds_with_needs(trigger_build_ids)
@@ -72,13 +72,13 @@ module Ci
# rubocop: disable CodeReuse/ActiveRecord
def status_for_prior_stages(index)
- pipeline.builds.where('stage_idx < ?', index).latest.status || 'success'
+ pipeline.processables.where('stage_idx < ?', index).latest.status || 'success'
end
# rubocop: enable CodeReuse/ActiveRecord
# rubocop: disable CodeReuse/ActiveRecord
def status_for_build_needs(needs)
- pipeline.builds.where(name: needs).latest.status || 'success'
+ pipeline.processables.where(name: needs).latest.status || 'success'
end
# rubocop: enable CodeReuse/ActiveRecord
@@ -92,6 +92,10 @@ module Ci
# rubocop: disable CodeReuse/ActiveRecord
def created_processables_in_stage_without_needs(index)
created_processables_without_needs
+ .preload(:project)
+ .preload(:taggings)
+ .preload(:deployment)
+ .preload(:user)
.where(stage_idx: index)
end
# rubocop: enable CodeReuse/ActiveRecord
diff --git a/lib/gitlab/ci/status/composite_status.rb b/lib/gitlab/ci/status/composite_status.rb
new file mode 100644
index 00000000000..227c32bb18a
--- /dev/null
+++ b/lib/gitlab/ci/status/composite_status.rb
@@ -0,0 +1,75 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module Ci
+ module Status
+ class CompositeStatus
+ def initialize(all_statuses)
+ @status_set = build_status_set(all_statuses)
+ end
+
+ def status
+ case
+ when only?(:skipped, :warning)
+ :success
+ when only?(:skipped)
+ :skipped
+ when only?(:success)
+ :skipped
+ when only?(:created)
+ :created
+ when only?(:preparing)
+ :preparing
+ when only?(:success, :skipped)
+ :success
+ when only?(:success, :skipped, :canceled)
+ :canceled
+ when only?(:created, :skipped, :pending)
+ :pending
+ when include?(:running, :pending)
+ :running
+ when include?(:manual)
+ :manual
+ when include?(:scheduled)
+ :scheduled
+ when include?(:preparing)
+ :preparing
+ when include?(:created)
+ :created
+ when include?
+ :failed
+ end
+ end
+
+ def warnings?
+ include?(:warning)
+ end
+
+ private
+
+ def include?(*names)
+ names.all? { |name| @status_set.include?(name) }
+ end
+
+ def only?(*names)
+ @status_set.size == names.size &&
+ names.all? { |name| @status_set.include?(name) }
+ end
+
+ def build_status_set(all_statuses)
+ status_set = Set.new
+
+ all_statuses.each do |status|
+ if status[:allow_failure] && HasStatus::WARNING_STATUSES.include?(status[:status])
+ status_set.add(:warning)
+ else
+ status_set.add(status[:status].to_sym)
+ end
+ end
+
+ status_set
+ end
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/ci/status/grouped_statuses.rb b/lib/gitlab/ci/status/grouped_statuses.rb
new file mode 100644
index 00000000000..4b5f2e13160
--- /dev/null
+++ b/lib/gitlab/ci/status/grouped_statuses.rb
@@ -0,0 +1,75 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module Ci
+ module Status
+ class GroupedStatuses
+ def initialize(subject, *keys)
+ @subject = subject
+ @keys = keys
+ end
+
+ def one(**query)
+ validate_keys!(query.keys)
+
+ item_hash = find_one(data_hash, query)
+ status_for_key(query, item_hash) if item_hash
+ end
+
+ def group(*keys)
+ validate_keys!(keys)
+
+ grouped_statuses(data_hash, keys)
+ end
+
+ private
+
+ def validate_keys!(keys)
+ missing_keys = @keys - keys
+ if missing_keys.present?
+ raise ArgumentError, "the keys '#{missing_keys.join(',')} are not subset of #{@keys.join(',')}"
+ end
+ end
+
+ def data_hash
+ @data_hash ||= hash_from_relation(@subject, @keys)
+ end
+
+ def hash_from_relation(subject, keys)
+ columns = keys.dup
+ columns << :status
+
+ # we request allow_failure when
+ # we don't have column_names, or such column does exist
+ columns << :allow_failure if !subject.respond_to?(:column_names) || subject.column_names.include?('allow_failure')
+
+ subject
+ .pluck(*columns)
+ .map { |attrs| columns.zip(attrs).to_h }
+ end
+
+ def find_one(subject, query)
+ subject.select do |attrs|
+ query.all? do |key, value|
+ attrs[key] == value
+ end
+ end.presence
+ end
+
+ def grouped_statuses(subject, keys)
+ subject
+ .group_by { |attrs| attrs.slice(*keys) }
+ .map { |key, all_attrs| status_for_key(key, all_attrs) }
+ end
+
+ def status_for_key(key, all_attrs)
+ composite_status = Gitlab::Ci::Status::CompositeStatus.new(all_attrs)
+
+ key.merge(
+ status: composite_status.status.to_s,
+ warnings: composite_status.warnings?)
+ end
+ end
+ end
+ end
+end
diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb
index 017cca0541e..f4fcd9d05e0 100644
--- a/spec/models/commit_status_spec.rb
+++ b/spec/models/commit_status_spec.rb
@@ -270,29 +270,6 @@ describe CommitStatus do
end
end
- describe '.exclude_ignored' do
- subject { described_class.exclude_ignored.order(:id) }
-
- let(:statuses) do
- [create_status(when: 'manual', status: 'skipped'),
- create_status(when: 'manual', status: 'success'),
- create_status(when: 'manual', status: 'failed'),
- create_status(when: 'on_failure', status: 'skipped'),
- create_status(when: 'on_failure', status: 'success'),
- create_status(when: 'on_failure', status: 'failed'),
- create_status(allow_failure: true, status: 'success'),
- create_status(allow_failure: true, status: 'failed'),
- create_status(allow_failure: false, status: 'success'),
- create_status(allow_failure: false, status: 'failed'),
- create_status(allow_failure: true, status: 'manual'),
- create_status(allow_failure: false, status: 'manual')]
- end
-
- it 'returns statuses without what we want to ignore' do
- is_expected.to eq(statuses.values_at(0, 1, 2, 3, 4, 5, 6, 8, 9, 11))
- end
- end
-
describe '.failed_but_allowed' do
subject { described_class.failed_but_allowed.order(:id) }