Merge branch 'feature/runner-lock-on-project' into 'master'

Make it possible to lock runner on a specific project

Make it possible to lock runner on a specific project.

![Screen_Shot_2016-06-20_at_4.03.08_PM](/uploads/186378643a20106ff0b67b6fd8bd7f28/Screen_Shot_2016-06-20_at_4.03.08_PM.png)

----

![Screen_Shot_2016-06-20_at_9.54.52_PM](/uploads/c479abdffaf19f383bb6b5a42bdd6cc3/Screen_Shot_2016-06-20_at_9.54.52_PM.png)

----

![Screen_Shot_2016-06-20_at_9.56.26_PM](/uploads/6ad838679b0c28a1fe2e20e9224387ea/Screen_Shot_2016-06-20_at_9.56.26_PM.png)

Closes #3407

See merge request !4093

5 files changed, 326 insertions, 87 deletions

diff --git a/spec/factories/ci/commits.rb b/spec/factories/ci/pipelines.rb
index a039bef6f3c..a039bef6f3c 100644
--- a/spec/factories/ci/commits.rb
+++ b/spec/factories/ci/pipelines.rb
diff --git a/spec/features/admin/admin_runners_spec.rb b/spec/features/admin/admin_runners_spec.rb
index 9499cd4e025..2d297776cb0 100644
--- a/spec/features/admin/admin_runners_spec.rb
+++ b/spec/features/admin/admin_runners_spec.rb
@@ -60,6 +60,40 @@ describe "Admin Runners" do
       it { expect(page).to have_content(@project1.name_with_namespace) }
       it { expect(page).not_to have_content(@project2.name_with_namespace) }
     end
+
+    describe 'enable/create' do
+      before do
+        @project1.runners << runner
+        visit admin_runner_path(runner)
+      end
+
+      it 'enables specific runner for project' do
+        within '.unassigned-projects' do
+          click_on 'Enable'
+        end
+
+        assigned_project = page.find('.assigned-projects')
+
+        expect(assigned_project).to have_content(@project2.path)
+      end
+    end
+
+    describe 'disable/destroy' do
+      before do
+        @project1.runners << runner
+        visit admin_runner_path(runner)
+      end
+
+      it 'enables specific runner for project' do
+        within '.assigned-projects' do
+          click_on 'Disable'
+        end
+
+        new_runner_project = page.find('.unassigned-projects')
+
+        expect(new_runner_project).to have_content(@project1.path)
+      end
+    end
   end
 
   describe 'runners registration token' do
diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb
index 5d1fa8226e5..479b7309579 100644
--- a/spec/models/build_spec.rb
+++ b/spec/models/build_spec.rb
@@ -36,32 +36,44 @@ describe Ci::Build, models: true do
     subject { build.ignored? }
 
     context 'if build is not allowed to fail' do
-      before { build.allow_failure = false }
+      before do
+        build.allow_failure = false
+      end
 
       context 'and build.status is success' do
-        before { build.status = 'success' }
+        before do
+          build.status = 'success'
+        end
 
         it { is_expected.to be_falsey }
       end
 
       context 'and build.status is failed' do
-        before { build.status = 'failed' }
+        before do
+          build.status = 'failed'
+        end
 
         it { is_expected.to be_falsey }
       end
     end
 
     context 'if build is allowed to fail' do
-      before { build.allow_failure = true }
+      before do
+        build.allow_failure = true
+      end
 
       context 'and build.status is success' do
-        before { build.status = 'success' }
+        before do
+          build.status = 'success'
+        end
 
         it { is_expected.to be_falsey }
       end
 
       context 'and build.status is failed' do
-        before { build.status = 'failed' }
+        before do
+          build.status = 'failed'
+        end
 
         it { is_expected.to be_truthy }
       end
@@ -75,7 +87,9 @@ describe Ci::Build, models: true do
 
     context 'if build.trace contains text' do
       let(:text) { 'example output' }
-      before { build.trace = text }
+      before do
+        build.trace = text
+      end
 
       it { is_expected.to include(text) }
       it { expect(subject.length).to be >= text.length }
@@ -188,7 +202,9 @@ describe Ci::Build, models: true do
         ]
       end
 
-      before { build.update_attributes(stage: 'stage') }
+      before do
+        build.update_attributes(stage: 'stage')
+      end
 
       it { is_expected.to eq(predefined_variables + yaml_variables) }
 
@@ -199,7 +215,9 @@ describe Ci::Build, models: true do
           ]
         end
 
-        before { build.update_attributes(tag: true) }
+        before do
+          build.update_attributes(tag: true)
+        end
 
         it { is_expected.to eq(tag_variable + predefined_variables + yaml_variables) }
       end
@@ -257,57 +275,6 @@ describe Ci::Build, models: true do
     end
   end
 
-  describe '#can_be_served?' do
-    let(:runner) { create(:ci_runner) }
-
-    before { build.project.runners << runner }
-
-    context 'when runner does not have tags' do
-      it 'can handle builds without tags' do
-        expect(build.can_be_served?(runner)).to be_truthy
-      end
-
-      it 'cannot handle build with tags' do
-        build.tag_list = ['aa']
-        expect(build.can_be_served?(runner)).to be_falsey
-      end
-    end
-
-    context 'when runner has tags' do
-      before { runner.tag_list = ['bb', 'cc'] }
-
-      shared_examples 'tagged build picker' do
-        it 'can handle build with matching tags' do
-          build.tag_list = ['bb']
-          expect(build.can_be_served?(runner)).to be_truthy
-        end
-
-        it 'cannot handle build without matching tags' do
-          build.tag_list = ['aa']
-          expect(build.can_be_served?(runner)).to be_falsey
-        end
-      end
-
-      context 'when runner can pick untagged jobs' do
-        it 'can handle builds without tags' do
-          expect(build.can_be_served?(runner)).to be_truthy
-        end
-
-        it_behaves_like 'tagged build picker'
-      end
-
-      context 'when runner can not pick untagged jobs' do
-        before { runner.run_untagged = false }
-
-        it 'can not handle builds without tags' do
-          expect(build.can_be_served?(runner)).to be_falsey
-        end
-
-        it_behaves_like 'tagged build picker'
-      end
-    end
-  end
-
   describe '#has_tags?' do
     context 'when build has tags' do
       subject { create(:ci_build, tag_list: ['tag']) }
@@ -348,7 +315,7 @@ describe Ci::Build, models: true do
       end
 
       it 'that cannot handle build' do
-        expect_any_instance_of(Ci::Build).to receive(:can_be_served?).and_return(false)
+        expect_any_instance_of(Ci::Runner).to receive(:can_pick?).and_return(false)
         is_expected.to be_falsey
       end
 
@@ -360,7 +327,9 @@ describe Ci::Build, models: true do
 
     %w(pending).each do |state|
       context "if commit_status.status is #{state}" do
-        before { build.status = state }
+        before do
+          build.status = state
+        end
 
         it { is_expected.to be_truthy }
 
@@ -379,7 +348,9 @@ describe Ci::Build, models: true do
 
     %w(success failed canceled running).each do |state|
       context "if commit_status.status is #{state}" do
-        before { build.status = state }
+        before do
+          build.status = state
+        end
 
         it { is_expected.to be_falsey }
       end
@@ -390,7 +361,10 @@ describe Ci::Build, models: true do
     subject { build.artifacts? }
 
     context 'artifacts archive does not exist' do
-      before { build.update_attributes(artifacts_file: nil) }
+      before do
+        build.update_attributes(artifacts_file: nil)
+      end
+
       it { is_expected.to be_falsy }
     end
 
@@ -623,7 +597,9 @@ describe Ci::Build, models: true do
       let!(:build) { create(:ci_build, :trace, :success, :artifacts) }
 
       describe '#erase' do
-        before { build.erase(erased_by: user) }
+        before do
+          build.erase(erased_by: user)
+        end
 
         context 'erased by user' do
           let!(:user) { create(:user, username: 'eraser') }
@@ -660,7 +636,9 @@ describe Ci::Build, models: true do
         end
 
         context 'build has been erased' do
-          before { build.erase }
+          before do
+            build.erase
+          end
 
           it { is_expected.to be true }
         end
@@ -668,7 +646,9 @@ describe Ci::Build, models: true do
 
       context 'metadata and build trace are not available' do
         let!(:build) { create(:ci_build, :success, :artifacts) }
-        before { build.remove_artifacts_metadata! }
+        before do
+          build.remove_artifacts_metadata!
+        end
 
         describe '#erase' do
           it 'should not raise error' do
diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb
index 5d04d8ffcff..ef65eb99328 100644
--- a/spec/models/ci/runner_spec.rb
+++ b/spec/models/ci/runner_spec.rb
@@ -20,34 +20,36 @@ describe Ci::Runner, models: true do
   end
 
   describe '#display_name' do
-    it 'should return the description if it has a value' do
+    it 'returns the description if it has a value' do
       runner = FactoryGirl.build(:ci_runner, description: 'Linux/Ruby-1.9.3-p448')
       expect(runner.display_name).to eq 'Linux/Ruby-1.9.3-p448'
     end
 
-    it 'should return the token if it does not have a description' do
+    it 'returns the token if it does not have a description' do
       runner = FactoryGirl.create(:ci_runner)
       expect(runner.display_name).to eq runner.description
     end
 
-    it 'should return the token if the description is an empty string' do
+    it 'returns the token if the description is an empty string' do
       runner = FactoryGirl.build(:ci_runner, description: '', token: 'token')
       expect(runner.display_name).to eq runner.token
     end
   end
 
-  describe :assign_to do
+  describe '#assign_to' do
     let!(:project) { FactoryGirl.create :empty_project }
     let!(:shared_runner) { FactoryGirl.create(:ci_runner, :shared) }
 
-    before { shared_runner.assign_to(project) }
+    before do
+      shared_runner.assign_to(project)
+    end
 
     it { expect(shared_runner).to be_specific }
     it { expect(shared_runner.projects).to eq([project]) }
     it { expect(shared_runner.only_for?(project)).to be_truthy }
   end
 
-  describe :online do
+  describe '.online' do
     subject { Ci::Runner.online }
 
     before do
@@ -58,60 +60,269 @@ describe Ci::Runner, models: true do
     it { is_expected.to eq([@runner2])}
   end
 
-  describe :online? do
+  describe '#online?' do
     let(:runner) { FactoryGirl.create(:ci_runner, :shared) }
 
     subject { runner.online? }
 
     context 'never contacted' do
-      before { runner.contacted_at = nil }
+      before do
+        runner.contacted_at = nil
+      end
 
       it { is_expected.to be_falsey }
     end
 
     context 'contacted long time ago time' do
-      before { runner.contacted_at = 1.year.ago }
+      before do
+        runner.contacted_at = 1.year.ago
+      end
 
       it { is_expected.to be_falsey }
     end
 
     context 'contacted 1s ago' do
-      before { runner.contacted_at = 1.second.ago }
+      before do
+        runner.contacted_at = 1.second.ago
+      end
 
       it { is_expected.to be_truthy }
     end
   end
 
-  describe :status do
+  describe '#can_pick?' do
+    let(:project) { create(:project) }
+    let(:pipeline) { create(:ci_pipeline, project: project) }
+    let(:build) { create(:ci_build, pipeline: pipeline) }
+    let(:runner) { create(:ci_runner) }
+
+    before do
+      build.project.runners << runner
+    end
+
+    context 'when runner does not have tags' do
+      it 'can handle builds without tags' do
+        expect(runner.can_pick?(build)).to be_truthy
+      end
+
+      it 'cannot handle build with tags' do
+        build.tag_list = ['aa']
+
+        expect(runner.can_pick?(build)).to be_falsey
+      end
+    end
+
+    context 'when runner has tags' do
+      before do
+        runner.tag_list = ['bb', 'cc']
+      end
+
+      shared_examples 'tagged build picker' do
+        it 'can handle build with matching tags' do
+          build.tag_list = ['bb']
+
+          expect(runner.can_pick?(build)).to be_truthy
+        end
+
+        it 'cannot handle build without matching tags' do
+          build.tag_list = ['aa']
+
+          expect(runner.can_pick?(build)).to be_falsey
+        end
+      end
+
+      context 'when runner can pick untagged jobs' do
+        it 'can handle builds without tags' do
+          expect(runner.can_pick?(build)).to be_truthy
+        end
+
+        it_behaves_like 'tagged build picker'
+      end
+
+      context 'when runner cannot pick untagged jobs' do
+        before do
+          runner.run_untagged = false
+        end
+
+        it 'cannot handle builds without tags' do
+          expect(runner.can_pick?(build)).to be_falsey
+        end
+
+        it_behaves_like 'tagged build picker'
+      end
+    end
+
+    context 'when runner is locked' do
+      before do
+        runner.locked = true
+      end
+
+      shared_examples 'locked build picker' do
+        context 'when runner cannot pick untagged jobs' do
+          before do
+            runner.run_untagged = false
+          end
+
+          it 'cannot handle builds without tags' do
+            expect(runner.can_pick?(build)).to be_falsey
+          end
+        end
+
+        context 'when having runner tags' do
+          before do
+            runner.tag_list = ['bb', 'cc']
+          end
+
+          it 'cannot handle it for builds without matching tags' do
+            build.tag_list = ['aa']
+
+            expect(runner.can_pick?(build)).to be_falsey
+          end
+        end
+      end
+
+      context 'when serving the same project' do
+        it 'can handle it' do
+          expect(runner.can_pick?(build)).to be_truthy
+        end
+
+        it_behaves_like 'locked build picker'
+
+        context 'when having runner tags' do
+          before do
+            runner.tag_list = ['bb', 'cc']
+            build.tag_list = ['bb']
+          end
+
+          it 'can handle it for matching tags' do
+            expect(runner.can_pick?(build)).to be_truthy
+          end
+        end
+      end
+
+      context 'serving a different project' do
+        before do
+          runner.runner_projects.destroy_all
+        end
+
+        it 'cannot handle it' do
+          expect(runner.can_pick?(build)).to be_falsey
+        end
+
+        it_behaves_like 'locked build picker'
+
+        context 'when having runner tags' do
+          before do
+            runner.tag_list = ['bb', 'cc']
+            build.tag_list = ['bb']
+          end
+
+          it 'cannot handle it for matching tags' do
+            expect(runner.can_pick?(build)).to be_falsey
+          end
+        end
+      end
+    end
+  end
+
+  describe '#status' do
     let(:runner) { FactoryGirl.create(:ci_runner, :shared, contacted_at: 1.second.ago) }
 
     subject { runner.status }
 
     context 'never connected' do
-      before { runner.contacted_at = nil }
+      before do
+        runner.contacted_at = nil
+      end
 
       it { is_expected.to eq(:not_connected) }
     end
 
     context 'contacted 1s ago' do
-      before { runner.contacted_at = 1.second.ago }
+      before do
+        runner.contacted_at = 1.second.ago
+      end
 
       it { is_expected.to eq(:online) }
     end
 
     context 'contacted long time ago' do
-      before { runner.contacted_at = 1.year.ago }
+      before do
+        runner.contacted_at = 1.year.ago
+      end
 
       it { is_expected.to eq(:offline) }
     end
 
     context 'inactive' do
-      before { runner.active = false }
+      before do
+        runner.active = false
+      end
 
       it { is_expected.to eq(:paused) }
     end
   end
 
+  describe '.assignable_for' do
+    let(:runner) { create(:ci_runner) }
+    let(:project) { create(:project) }
+    let(:another_project) { create(:project) }
+
+    before do
+      project.runners << runner
+    end
+
+    context 'with shared runners' do
+      before do
+        runner.update(is_shared: true)
+      end
+
+      context 'does not give owned runner' do
+        subject { Ci::Runner.assignable_for(project) }
+
+        it { is_expected.to be_empty }
+      end
+
+      context 'does not give shared runner' do
+        subject { Ci::Runner.assignable_for(another_project) }
+
+        it { is_expected.to be_empty }
+      end
+    end
+
+    context 'with unlocked runner' do
+      context 'does not give owned runner' do
+        subject { Ci::Runner.assignable_for(project) }
+
+        it { is_expected.to be_empty }
+      end
+
+      context 'does give a specific runner' do
+        subject { Ci::Runner.assignable_for(another_project) }
+
+        it { is_expected.to contain_exactly(runner) }
+      end
+    end
+
+    context 'with locked runner' do
+      before do
+        runner.update(locked: true)
+      end
+
+      context 'does not give owned runner' do
+        subject { Ci::Runner.assignable_for(project) }
+
+        it { is_expected.to be_empty }
+      end
+
+      context 'does not give a locked runner' do
+        subject { Ci::Runner.assignable_for(another_project) }
+
+        it { is_expected.to be_empty }
+      end
+    end
+  end
+
   describe "belongs_to_one_project?" do
     it "returns false if there are two projects runner assigned to" do
       runner = FactoryGirl.create(:ci_runner)
diff --git a/spec/requests/api/runners_spec.rb b/spec/requests/api/runners_spec.rb
index 73ae8ef631c..b4c826522a5 100644
--- a/spec/requests/api/runners_spec.rb
+++ b/spec/requests/api/runners_spec.rb
@@ -187,14 +187,16 @@ describe API::Runners, api: true  do
           update_runner(shared_runner.id, admin, description: "#{description}_updated",
                                                  active: !active,
                                                  tag_list: ['ruby2.1', 'pgsql', 'mysql'],
-                                                 run_untagged: 'false')
+                                                 run_untagged: 'false',
+                                                 locked: 'true')
           shared_runner.reload
 
           expect(response.status).to eq(200)
           expect(shared_runner.description).to eq("#{description}_updated")
           expect(shared_runner.active).to eq(!active)
           expect(shared_runner.tag_list).to include('ruby2.1', 'pgsql', 'mysql')
-          expect(shared_runner.run_untagged?).to be false
+          expect(shared_runner.run_untagged?).to be(false)
+          expect(shared_runner.locked?).to be(true)
         end
       end
 
@@ -360,11 +362,13 @@ describe API::Runners, api: true  do
 
   describe 'POST /projects/:id/runners' do
     context 'authorized user' do
-      it 'should enable specific runner' do
-        specific_runner2 = create(:ci_runner).tap do |runner|
+      let(:specific_runner2) do
+        create(:ci_runner).tap do |runner|
           create(:ci_runner_project, runner: runner, project: project2)
         end
+      end
 
+      it 'should enable specific runner' do
         expect do
           post api("/projects/#{project.id}/runners", user), runner_id: specific_runner2.id
         end.to change{ project.runners.count }.by(+1)
@@ -375,7 +379,17 @@ describe API::Runners, api: true  do
         expect do
           post api("/projects/#{project.id}/runners", user), runner_id: specific_runner.id
         end.to change{ project.runners.count }.by(0)
-        expect(response.status).to eq(201)
+        expect(response.status).to eq(409)
+      end
+
+      it 'should not enable locked runner' do
+        specific_runner2.update(locked: true)
+
+        expect do
+          post api("/projects/#{project.id}/runners", user), runner_id: specific_runner2.id
+        end.to change{ project.runners.count }.by(0)
+
+        expect(response.status).to eq(403)
       end
 
       it 'should not enable shared runner' do