Merge branch 'all-skipped-equals-success' into 'master'

Skipped jobs should be considered successful

## What does this MR do?

If all jobs in previous stage are all skipped, the next stage should consider previous stage succeeded.

## Why was this MR needed?

Since for now we consider all manual jobs if skipped, should not block the next stage from running.

Closes #22598 

See also #20342 (because this merge request conflicts with it)

See merge request !6604

3 files changed, 34 insertions, 15 deletions

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 663c5b1e231..97df74b0cfe 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -196,7 +196,7 @@ module Ci
     end
 
     def has_warnings?
-      builds.latest.ignored.any?
+      builds.latest.failed_but_allowed.any?
     end
 
     def config_processor
diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb
index 736db1ab0f6..ee3396abe04 100644
--- a/app/models/commit_status.rb
+++ b/app/models/commit_status.rb
@@ -24,7 +24,22 @@ class CommitStatus < ActiveRecord::Base
 
   scope :retried, -> { where.not(id: latest) }
   scope :ordered, -> { order(:name) }
-  scope :ignored, -> { where(allow_failure: true, status: [:failed, :canceled]) }
+
+  scope :failed_but_allowed, -> do
+    where(allow_failure: true, status: [:failed, :canceled])
+  end
+
+  scope :exclude_ignored, -> do
+    quoted_when = connection.quote_column_name('when')
+    # We want to ignore failed_but_allowed jobs
+    where("allow_failure = ? OR status IN (?)",
+      false, all_state_names - [:failed, :canceled]).
+      # We want to ignore skipped manual jobs
+      where("#{quoted_when} <> ? OR status <> ?", 'manual', 'skipped').
+      # We want to ignore skipped on_failure
+      where("#{quoted_when} <> ? OR status <> ?", 'on_failure', 'skipped')
+  end
+
   scope :latest_ci_stages, -> { latest.ordered.includes(project: :namespace) }
   scope :retried_ci_stages, -> { retried.ordered.includes(project: :namespace) }
 
@@ -111,7 +126,7 @@ class CommitStatus < ActiveRecord::Base
     end
   end
 
-  def ignored?
+  def failed_but_allowed?
     allow_failure? && (failed? || canceled?)
   end
 
diff --git a/app/models/concerns/has_status.rb b/app/models/concerns/has_status.rb
index 0fa4df0fb56..9f64f76721d 100644
--- a/app/models/concerns/has_status.rb
+++ b/app/models/concerns/has_status.rb
@@ -8,32 +8,32 @@ module HasStatus
 
   class_methods do
     def status_sql
-      scope = all
+      scope = if respond_to?(:exclude_ignored)
+                exclude_ignored
+              else
+                all
+              end
       builds = scope.select('count(*)').to_sql
       created = scope.created.select('count(*)').to_sql
       success = scope.success.select('count(*)').to_sql
-      ignored = scope.ignored.select('count(*)').to_sql if scope.respond_to?(:ignored)
-      ignored ||= '0'
       pending = scope.pending.select('count(*)').to_sql
       running = scope.running.select('count(*)').to_sql
-      canceled = scope.canceled.select('count(*)').to_sql
       skipped = scope.skipped.select('count(*)').to_sql
+      canceled = scope.canceled.select('count(*)').to_sql
 
-      deduce_status = "(CASE
+      "(CASE
+        WHEN (#{builds})=(#{success}) THEN 'success'
         WHEN (#{builds})=(#{created}) THEN 'created'
-        WHEN (#{builds})=(#{skipped}) THEN 'skipped'
-        WHEN (#{builds})=(#{success})+(#{ignored})+(#{skipped}) THEN 'success'
-        WHEN (#{builds})=(#{created})+(#{pending})+(#{skipped}) THEN 'pending'
-        WHEN (#{builds})=(#{canceled})+(#{success})+(#{ignored})+(#{skipped}) THEN 'canceled'
+        WHEN (#{builds})=(#{success})+(#{skipped}) THEN 'skipped'
+        WHEN (#{builds})=(#{success})+(#{skipped})+(#{canceled}) THEN 'canceled'
+        WHEN (#{builds})=(#{created})+(#{skipped})+(#{pending}) THEN 'pending'
         WHEN (#{running})+(#{pending})+(#{created})>0 THEN 'running'
         ELSE 'failed'
       END)"
-
-      deduce_status
     end
 
     def status
-      all.pluck(self.status_sql).first
+      all.pluck(status_sql).first
     end
 
     def started_at
@@ -43,6 +43,10 @@ module HasStatus
     def finished_at
       all.maximum(:finished_at)
     end
+
+    def all_state_names
+      state_machines.values.flat_map(&:states).flat_map { |s| s.map(&:name) }
+    end
   end
 
   included do