Implement when syntax in .gitlab-ci.yml

10 files changed, 279 insertions, 124 deletions

diff --git a/CHANGELOG b/CHANGELOG
index aba823948a7..39926692147 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -22,6 +22,7 @@ v 8.1.0 (unreleased)
   - Add first and last to pagination (Zeger-Jan van de Weg)
   - Added Commit Status API
   - Added Builds View
+  - Added when to .gitlab-ci.yml
   - Show CI status on commit page
   - Added CI_BUILD_TAG, _STAGE, _NAME and _TRIGGERED to CI builds
   - Show CI status on Your projects page and Starred projects page
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 5f8d44148ca..b19e2ac1363 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -93,10 +93,7 @@ module Ci
           Ci::WebHookService.new.build_end(build)
         end
 
-        if build.commit.should_create_next_builds?(build)
-          build.commit.create_next_builds(build.ref, build.tag, build.user, build.trigger_request)
-        end
-
+        build.commit.create_next_builds(build)
         project.execute_services(build)
 
         if project.coverage_enabled?
diff --git a/app/models/ci/commit.rb b/app/models/ci/commit.rb
index cd45366b34e..13437b2483f 100644
--- a/app/models/ci/commit.rb
+++ b/app/models/ci/commit.rb
@@ -91,19 +91,28 @@ module Ci
     def create_builds(ref, tag, user, trigger_request = nil)
       return unless config_processor
       config_processor.stages.any? do |stage|
-        CreateBuildsService.new.execute(self, stage, ref, tag, user, trigger_request).present?
+        CreateBuildsService.new.execute(self, stage, ref, tag, user, trigger_request, 'success').present?
       end
     end
 
-    def create_next_builds(ref, tag, user, trigger_request)
+    def create_next_builds(build)
       return unless config_processor
 
-      stages = builds.where(ref: ref, tag: tag, trigger_request: trigger_request).group_by(&:stage)
+      # don't create other builds if this one is retried
+      latest_builds = builds.similar(build).latest
+      return unless latest_builds.exists?(build.id)
 
-      config_processor.stages.any? do |stage|
-        unless stages.include?(stage)
-          CreateBuildsService.new.execute(self, stage, ref, tag, user, trigger_request).present?
-        end
+      # get list of stages after this build
+      next_stages = config_processor.stages.drop_while { |stage| stage != build.stage }
+      next_stages.delete(build.stage)
+
+      # get status for all prior builds
+      prior_builds = latest_builds.reject { |other_build| next_stages.include?(other_build.stage) }
+      status = Ci::Status.get_status(prior_builds)
+
+      # create builds for next stages based
+      next_stages.any? do |stage|
+        CreateBuildsService.new.execute(self, stage, build.ref, build.tag, build.user, build.trigger_request, status).present?
       end
     end
 
@@ -132,24 +141,7 @@ module Ci
         return 'failed'
       end
 
-      @status ||= begin
-        latest = latest_statuses
-        latest.reject! { |status| status.try(&:allow_failure?) }
-
-        if latest.none?
-          'skipped'
-        elsif latest.all?(&:success?)
-          'success'
-        elsif latest.all?(&:pending?)
-          'pending'
-        elsif latest.any?(&:running?) || latest.any?(&:pending?)
-          'running'
-        elsif latest.all?(&:canceled?)
-          'canceled'
-        else
-          'failed'
-        end
-      end
+      @status ||= Ci::Status.get_status(latest_statuses)
     end
 
     def pending?
@@ -219,16 +211,6 @@ module Ci
       update!(committed_at: DateTime.now)
     end
 
-    def should_create_next_builds?(build)
-      # don't create other builds if this one is retried
-      other_builds = builds.similar(build).latest
-      return false unless other_builds.include?(build)
-
-      other_builds.all? do |build|
-        build.success? || build.ignored?
-      end
-    end
-
     private
 
     def save_yaml_error(error)
diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb
index 0b71838d515..8188ba3a28e 100644
--- a/app/models/commit_status.rb
+++ b/app/models/commit_status.rb
@@ -28,7 +28,7 @@ class CommitStatus < ActiveRecord::Base
     end
 
     event :drop do
-      transition running: :failed
+      transition [:pending, :running] => :failed
     end
 
     event :success do
diff --git a/app/services/ci/create_builds_service.rb b/app/services/ci/create_builds_service.rb
index c420f3268fd..912eb6258a4 100644
--- a/app/services/ci/create_builds_service.rb
+++ b/app/services/ci/create_builds_service.rb
@@ -1,8 +1,20 @@
 module Ci
   class CreateBuildsService
-    def execute(commit, stage, ref, tag, user, trigger_request)
+    def execute(commit, stage, ref, tag, user, trigger_request, status)
       builds_attrs = commit.config_processor.builds_for_stage_and_ref(stage, ref, tag)
 
+      # check when to create next build
+      builds_attrs = builds_attrs.select do |build_attrs|
+        case build_attrs[:when]
+        when 'on_success'
+          status == 'success'
+        when 'on_failure'
+          status == 'failed'
+        when 'always'
+          %w(success failed).include?(status)
+        end
+      end
+
       builds_attrs.map do |build_attrs|
         # don't create the same build twice
         unless commit.builds.find_by(ref: ref, tag: tag, trigger_request: trigger_request, name: build_attrs[:name])
diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md
index 4caeccacb7f..8507389f1ce 100644
--- a/doc/ci/yaml/README.md
+++ b/doc/ci/yaml/README.md
@@ -140,6 +140,7 @@ job_name:
 | except        | optional | Defines a list of git refs for which build is not created |
 | tags          | optional | Defines a list of tags which are used to select runner |
 | allow_failure | optional | Allow build to fail. Failed build doesn't contribute to commit status |
+| when          | optional | Define when to run build. Can be on_success, on_failure or always |
 
 ### script
 `script` is a shell script which is executed by runner. The shell script is prepended with `before_script`.
@@ -196,6 +197,54 @@ job:
 
 The above specification will make sure that `job` is built by a runner that have `ruby` AND `postgres` tags defined.
 
+### when
+`when` is used to implement jobs that are run in case of failure or despite the failure.
+
+The `when` can be set to one of the following values:
+1. `on_success` - execute build only when all builds from prior stages succeeded. This is default.
+1. `on_failure` - execute build only when at least one of the build from prior stages failed.
+1. `always` - execute build despite the status of builds from prior stages.
+
+```
+stages:
+- build
+- cleanup_build
+- test
+- deploy
+- cleanup
+
+build:
+  stage: build
+  script:
+  - make build
+
+cleanup_build:
+  stage: cleanup_build
+  script:
+  - cleanup build when failed
+  when: on_failure
+
+test:
+  stage: test
+  script:
+  - make test
+
+deploy:
+  stage: deploy
+  script:
+  - make deploy
+
+cleanup:
+  stage: cleanup
+  script:
+  - cleanup after builds
+  when: always
+```
+
+The above script will:
+1. Execute `cleanup_build` only when the `build` failed,
+2. Always execute `cleanup` as the last step in pipeline.
+
 ## Validate the .gitlab-ci.yml
 Each instance of GitLab CI has an embedded debug tool called Lint.
 You can find the link to the Lint in the project's settings page or use short url `/lint`.
diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index c47951bc5d1..58be188387f 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -5,7 +5,7 @@ module Ci
     DEFAULT_STAGES = %w(build test deploy)
     DEFAULT_STAGE = 'test'
     ALLOWED_YAML_KEYS = [:before_script, :image, :services, :types, :stages, :variables]
-    ALLOWED_JOB_KEYS = [:tags, :script, :only, :except, :type, :image, :services, :allow_failure, :type, :stage]
+    ALLOWED_JOB_KEYS = [:tags, :script, :only, :except, :type, :image, :services, :allow_failure, :type, :stage, :when]
 
     attr_reader :before_script, :image, :services, :variables
 
@@ -93,6 +93,7 @@ module Ci
         only: job[:only],
         except: job[:except],
         allow_failure: job[:allow_failure] || false,
+        when: job[:when] || 'on_success',
         options: {
           image: job[:image] || @image,
           services: job[:services] || @services
@@ -184,6 +185,10 @@ module Ci
       if job[:allow_failure] && !job[:allow_failure].in?([true, false])
         raise ValidationError, "#{name}: allow_failure parameter should be an boolean"
       end
+
+      if job[:when] && !job[:when].in?(%w(on_success on_failure always))
+        raise ValidationError, "#{name}: when should be on_success, on_failure or always"
+      end
     end
 
     private
diff --git a/lib/ci/status.rb b/lib/ci/status.rb
new file mode 100644
index 00000000000..94c94261d83
--- /dev/null
+++ b/lib/ci/status.rb
@@ -0,0 +1,21 @@
+module Ci
+  class Status
+    def self.get_status(statuses)
+      statuses.reject! { |status| status.try(&:allow_failure?) }
+
+      if statuses.none?
+        'skipped'
+      elsif statuses.all?(&:success?)
+        'success'
+      elsif statuses.all?(&:pending?)
+        'pending'
+      elsif statuses.any?(&:running?) || statuses.any?(&:pending?)
+        'running'
+      elsif statuses.all?(&:canceled?)
+        'canceled'
+      else
+        'failed'
+      end
+    end
+  end
+end
\ No newline at end of file
diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
index aba957da488..65696cb1ed3 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -125,7 +125,8 @@ module Ci
             image: "ruby:2.1",
             services: ["mysql"]
           },
-          allow_failure: false
+          allow_failure: false,
+          when: "on_success"
         })
       end
 
@@ -152,7 +153,8 @@ module Ci
             image: "ruby:2.5",
             services: ["postgresql"]
           },
-          allow_failure: false
+          allow_failure: false,
+          when: "on_success"
         })
       end
     end
@@ -174,6 +176,21 @@ module Ci
       end
     end
 
+    describe "When" do
+      %w(on_success on_failure always).each do |when_state|
+        it "returns #{when_state} when defined" do
+          config = YAML.dump({
+                               rspec: { script: "rspec", when: when_state }
+                             })
+
+          config_processor = GitlabCiYamlProcessor.new(config)
+          builds = config_processor.builds_for_stage_and_ref("test", "master")
+          expect(builds.size).to eq(1)
+          expect(builds.first[:when]).to eq(when_state)
+        end
+      end
+    end
+
     describe "Error handling" do
       it "indicates that object is invalid" do
         expect{GitlabCiYamlProcessor.new("invalid_yaml\n!ccdvlf%612334@@@@")}.to raise_error(GitlabCiYamlProcessor::ValidationError)
@@ -311,6 +328,13 @@ module Ci
           GitlabCiYamlProcessor.new(config)
         end.to raise_error(GitlabCiYamlProcessor::ValidationError, "variables should be a map of key-valued strings")
       end
+
+      it "returns errors if job when is not on_success, on_failure or always" do
+        config = YAML.dump({ rspec: { script: "test", when: false } })
+        expect do
+          GitlabCiYamlProcessor.new(config)
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: when parameter should be on_success, on_failure or always")
+      end
     end
   end
 end
diff --git a/spec/models/ci/commit_spec.rb b/spec/models/ci/commit_spec.rb
index d1cecce5a6d..9ad30407769 100644
--- a/spec/models/ci/commit_spec.rb
+++ b/spec/models/ci/commit_spec.rb
@@ -161,28 +161,28 @@ describe Ci::Commit do
   end
 
   describe :create_builds do
-    let(:commit) { FactoryGirl.create :ci_commit, gl_project: gl_project }
+    let!(:commit) { FactoryGirl.create :ci_commit, gl_project: gl_project }
 
     def create_builds(trigger_request = nil)
       commit.create_builds('master', false, nil, trigger_request)
     end
 
-    def create_next_builds(trigger_request = nil)
-      commit.create_next_builds('master', false, nil, trigger_request)
+    def create_next_builds
+      commit.create_next_builds(commit.builds.order(:id).last)
     end
 
     it 'creates builds' do
       expect(create_builds).to be_truthy
-      commit.builds.reload
-      expect(commit.builds.size).to eq(2)
+      commit.builds.update_all(status: "success")
+      expect(commit.builds.count(:all)).to eq(2)
 
       expect(create_next_builds).to be_truthy
-      commit.builds.reload
-      expect(commit.builds.size).to eq(4)
+      commit.builds.update_all(status: "success")
+      expect(commit.builds.count(:all)).to eq(4)
 
       expect(create_next_builds).to be_truthy
-      commit.builds.reload
-      expect(commit.builds.size).to eq(5)
+      commit.builds.update_all(status: "success")
+      expect(commit.builds.count(:all)).to eq(5)
 
       expect(create_next_builds).to be_falsey
     end
@@ -194,12 +194,12 @@ describe Ci::Commit do
 
       it 'creates builds' do
         expect(create_builds).to be_truthy
-        commit.builds.reload
-        expect(commit.builds.size).to eq(2)
+        commit.builds.update_all(status: "success")
+        expect(commit.builds.count(:all)).to eq(2)
 
         expect(create_develop_builds).to be_truthy
-        commit.builds.reload
-        expect(commit.builds.size).to eq(4)
+        commit.builds.update_all(status: "success")
+        expect(commit.builds.count(:all)).to eq(4)
         expect(commit.refs.size).to eq(2)
         expect(commit.builds.pluck(:name).uniq.size).to eq(2)
       end
@@ -211,28 +211,24 @@ describe Ci::Commit do
 
       it 'creates builds' do
         expect(create_builds(trigger_request)).to be_truthy
-        commit.builds.reload
-        expect(commit.builds.size).to eq(2)
+        expect(commit.builds.count(:all)).to eq(2)
       end
 
       it 'rebuilds commit' do
         expect(create_builds).to be_truthy
-        commit.builds.reload
-        expect(commit.builds.size).to eq(2)
+        expect(commit.builds.count(:all)).to eq(2)
 
         expect(create_builds(trigger_request)).to be_truthy
-        commit.builds.reload
-        expect(commit.builds.size).to eq(4)
+        expect(commit.builds.count(:all)).to eq(4)
       end
 
       it 'creates next builds' do
         expect(create_builds(trigger_request)).to be_truthy
-        commit.builds.reload
-        expect(commit.builds.size).to eq(2)
+        expect(commit.builds.count(:all)).to eq(2)
+        commit.builds.update_all(status: "success")
 
-        expect(create_next_builds(trigger_request)).to be_truthy
-        commit.builds.reload
-        expect(commit.builds.size).to eq(4)
+        expect(create_next_builds).to be_truthy
+        expect(commit.builds.count(:all)).to eq(4)
       end
 
       context 'for [ci skip]' do
@@ -242,7 +238,7 @@ describe Ci::Commit do
 
         it 'rebuilds commit' do
           expect(commit.status).to eq('skipped')
-          expect(create_builds(trigger_request)).to be_truthy
+          expect(create_builds).to be_truthy
 
           # since everything in Ci::Commit is cached we need to fetch a new object
           new_commit = Ci::Commit.find_by_id(commit.id)
@@ -250,6 +246,129 @@ describe Ci::Commit do
         end
       end
     end
+
+    context 'properly creates builds "when" is defined' do
+      let(:yaml) {
+        {
+          stages: ["build", "test", "test_failure", "deploy", "cleanup"],
+          build: {
+            stage: "build",
+            script: "BUILD",
+          },
+          test: {
+            stage: "test",
+            script: "TEST",
+          },
+          test_failure: {
+            stage: "test_failure",
+            script: "ON test failure",
+            when: "on_failure",
+          },
+          deploy: {
+            stage: "deploy",
+            script: "PUBLISH",
+          },
+          cleanup: {
+            stage: "cleanup",
+            script: "TIDY UP",
+            when: "always",
+          }
+        }
+      }
+
+      before do
+        stub_ci_commit_yaml_file(YAML.dump(yaml))
+      end
+
+      it 'properly creates builds' do
+        expect(create_builds).to be_truthy
+        expect(commit.builds.pluck(:name)).to contain_exactly('build')
+        expect(commit.builds.pluck(:status)).to contain_exactly('pending')
+        commit.builds.running_or_pending.each(&:success)
+
+        expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test')
+        expect(commit.builds.pluck(:status)).to contain_exactly('success', 'pending')
+        commit.builds.running_or_pending.each(&:success)
+
+        expect(commit.builds.pluck(:status)).to contain_exactly('success', 'success', 'pending')
+        expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy')
+        commit.builds.running_or_pending.each(&:success)
+
+        expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy', 'cleanup')
+        expect(commit.builds.pluck(:status)).to contain_exactly('success', 'success', 'success', 'pending')
+        commit.builds.running_or_pending.each(&:success)
+
+        expect(commit.builds.pluck(:status)).to contain_exactly('success', 'success', 'success', 'success')
+        expect(commit.status).to eq('success')
+      end
+
+      it 'properly creates builds when test fails' do
+        expect(create_builds).to be_truthy
+        expect(commit.builds.pluck(:name)).to contain_exactly('build')
+        expect(commit.builds.pluck(:status)).to contain_exactly('pending')
+        commit.builds.running_or_pending.each(&:success)
+
+        expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test')
+        expect(commit.builds.pluck(:status)).to contain_exactly('success', 'pending')
+        commit.builds.running_or_pending.each(&:drop)
+
+        expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure')
+        expect(commit.builds.pluck(:status)).to contain_exactly('success', 'failed', 'pending')
+        commit.builds.running_or_pending.each(&:success)
+
+        expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure', 'cleanup')
+        expect(commit.builds.pluck(:status)).to contain_exactly('success', 'failed', 'success', 'pending')
+        commit.builds.running_or_pending.each(&:success)
+
+        expect(commit.builds.pluck(:status)).to contain_exactly('success', 'failed', 'success', 'success')
+        expect(commit.status).to eq('failed')
+      end
+
+      it 'properly creates builds when test and test_failure fails' do
+        expect(create_builds).to be_truthy
+        expect(commit.builds.pluck(:name)).to contain_exactly('build')
+        expect(commit.builds.pluck(:status)).to contain_exactly('pending')
+        commit.builds.running_or_pending.each(&:success)
+
+        expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test')
+        expect(commit.builds.pluck(:status)).to contain_exactly('success', 'pending')
+        commit.builds.running_or_pending.each(&:drop)
+
+        expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure')
+        expect(commit.builds.pluck(:status)).to contain_exactly('success', 'failed', 'pending')
+        commit.builds.running_or_pending.each(&:drop)
+
+        expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure', 'cleanup')
+        expect(commit.builds.pluck(:status)).to contain_exactly('success', 'failed', 'failed', 'pending')
+        commit.builds.running_or_pending.each(&:success)
+
+        expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure', 'cleanup')
+        expect(commit.builds.pluck(:status)).to contain_exactly('success', 'failed', 'failed', 'success')
+        expect(commit.status).to eq('failed')
+      end
+
+      it 'properly creates builds when deploy fails' do
+        expect(create_builds).to be_truthy
+        expect(commit.builds.pluck(:name)).to contain_exactly('build')
+        expect(commit.builds.pluck(:status)).to contain_exactly('pending')
+        commit.builds.running_or_pending.each(&:success)
+
+        expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test')
+        expect(commit.builds.pluck(:status)).to contain_exactly('success', 'pending')
+        commit.builds.running_or_pending.each(&:success)
+
+        expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy')
+        expect(commit.builds.pluck(:status)).to contain_exactly('success', 'success', 'pending')
+        commit.builds.running_or_pending.each(&:drop)
+
+        expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy', 'cleanup')
+        expect(commit.builds.pluck(:status)).to contain_exactly('success', 'success', 'failed', 'pending')
+        commit.builds.running_or_pending.each(&:success)
+
+        expect(commit.builds.pluck(:status)).to contain_exactly('success', 'success', 'failed', 'success')
+        expect(commit.status).to eq('failed')
+      end
+    end
   end
 
   describe "#finished_at" do
@@ -299,59 +418,4 @@ describe Ci::Commit do
       expect(commit.coverage).to be_nil
     end
   end
-
-  describe :should_create_next_builds? do
-    before do
-      @build1 = FactoryGirl.create :ci_build, commit: commit, name: 'build1', ref: 'master', tag: false, status: 'success'
-      @build2 = FactoryGirl.create :ci_build, commit: commit, name: 'build1', ref: 'develop', tag: false, status: 'failed'
-      @build3 = FactoryGirl.create :ci_build, commit: commit, name: 'build1', ref: 'master', tag: true, status: 'failed'
-      @build4 = FactoryGirl.create :ci_build, commit: commit, name: 'build4', ref: 'master', tag: false, status: 'success'
-    end
-
-    context 'for success' do
-      it 'to create if all succeeded' do
-        expect(commit.should_create_next_builds?(@build4)).to be_truthy
-      end
-    end
-
-    context 'for failed' do
-      before do
-        @build4.update_attributes(status: 'failed')
-      end
-
-      it 'to not create' do
-        expect(commit.should_create_next_builds?(@build4)).to be_falsey
-      end
-
-      context 'and ignore failures for current' do
-        before do
-          @build4.update_attributes(allow_failure: true)
-        end
-
-        it 'to create' do
-          expect(commit.should_create_next_builds?(@build4)).to be_truthy
-        end
-      end
-    end
-
-    context 'for running' do
-      before do
-        @build4.update_attributes(status: 'running')
-      end
-
-      it 'to not create' do
-        expect(commit.should_create_next_builds?(@build4)).to be_falsey
-      end
-    end
-
-    context 'for retried' do
-      before do
-        @build5 = FactoryGirl.create :ci_build, commit: commit, name: 'build4', ref: 'master', tag: false, status: 'failed'
-      end
-
-      it 'to not create' do
-        expect(commit.should_create_next_builds?(@build4)).to be_falsey
-      end
-    end
-  end
 end