diff options
author | Kamil Trzciński <ayufan@ayufan.eu> | 2019-08-13 19:08:44 +0200 |
---|---|---|
committer | Kamil Trzciński <ayufan@ayufan.eu> | 2019-08-14 15:16:37 +0200 |
commit | 24f93807ccca38597db37e8e60216f97e46cc5e0 (patch) | |
tree | 3dbaaddef618a912beae50f2cf0db013f080957d | |
parent | ebb1314878c4456070547c16f418b03685c46c8b (diff) | |
download | gitlab-ce-24f93807ccca38597db37e8e60216f97e46cc5e0.tar.gz |
Implement Composite Status in Ruby
-rw-r--r-- | app/models/ci/pipeline.rb | 23 | ||||
-rw-r--r-- | app/models/commit_status.rb | 8 | ||||
-rw-r--r-- | app/models/concerns/has_status.rb | 41 | ||||
-rw-r--r-- | app/services/ci/process_pipeline_service.rb | 12 | ||||
-rw-r--r-- | lib/gitlab/ci/status/composite_status.rb | 75 | ||||
-rw-r--r-- | lib/gitlab/ci/status/grouped_statuses.rb | 75 | ||||
-rw-r--r-- | spec/models/commit_status_spec.rb | 23 |
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) } |