Improve and add specs for CI/CD Quotas/Limits classes

ee/lib/ee/gitlab/ci/pipeline/chain/limit/activity.rb

@@ -3,8 +3,10 @@ module EE
     module Ci
       module Pipeline
         module Chain
+          module Limit
             class Activity < ::Gitlab::Ci::Pipeline::Chain::Base
               include ::Gitlab::Ci::Pipeline::Chain::Helpers
+              include ::Gitlab::OptimisticLocking
 
               def initialize(*)
                 super
@@ -15,11 +17,22 @@ module EE
 
               def perform!
                 return unless @limit.exceeded?
-              return unless @command.save_incompleted
 
                 # TODO, add failure reason
-              # TODO, add validation error
-              @pipeline.drop
+                # TODO, transaction?
+
+                @pipeline.cancel_running
+
+                retry_optimistic_lock(@pipeline)
+                  @pipeline.drop!
+                end
+
+                # TODO, should we invalidate the pipeline
+                # while it is already persisted?
+                #
+                # Should we show info in the UI or alert/warning?
+                #
+                error(@limit.message)
               end
 
               def break?
@@ -30,4 +43,5 @@ module EE
         end
       end
     end
+  end
 end

ee/lib/ee/gitlab/ci/pipeline/chain/limit/size.rb

@@ -3,6 +3,7 @@ module EE
     module Ci
       module Pipeline
         module Chain
+          module Limit
             class Size < ::Gitlab::Ci::Pipeline::Chain::Base
               include ::Gitlab::Ci::Pipeline::Chain::Helpers
 
@@ -15,13 +16,15 @@ module EE
 
               def perform!
                 return unless @limit.exceeded?
-              return unless @command.save_incompleted
 
+                if @command.save_incompleted
                   # TODO, add failure reason
-              # TODO, add validation error
                   @pipeline.drop
                 end
 
+                error(@limit.message)
+              end
+
               def break?
                 @limit.exceeded?
               end
@@ -30,4 +33,5 @@ module EE
         end
       end
     end
+  end
 end

spec/ee/spec/lib/gitlab/ci/pipeline/chain/limit/activity_spec.rb

+require 'spec_helper'
+
+describe EE::Gitlab::Ci::Pipeline::Chain::Limit::Activity do
+  set(:namespace) { create(:namespace, plan: Namespace::GOLD_PLAN) }
+  set(:project) { create(:project, namespace: namespace) }
+  set(:user) { create(:user) }
+
+  let(:command) do
+    double('command', project: project,
+                      current_user: user)
+  end
+
+  let(:pipeline) do
+    create(:ci_pipeline, project: project)
+  end
+
+  before do
+    create(:ci_build, pipeline: pipeline)
+  end
+
+  let(:step) { described_class.new(pipeline, command) }
+
+  context 'when active pipelines limit is exceeded' do
+    before do
+      project.namespace.plan.update_column(:active_pipelines_limit, 1)
+
+      create(:ci_pipeline, project: project, status: 'pending')
+      create(:ci_pipeline, project: project, status: 'running')
+
+      step.perform!
+    end
+
+    it 'drops the pipeline' do
+      expect(pipeline.reload).to be_failed
+    end
+
+    it 'persists the pipeline' do
+      expect(pipeline).to be_persisted
+    end
+
+    it 'cancels all pipeline jobs' do
+      expect(pipeline.statuses).not_to be_empty
+      expect(pipeline.statuses).to all(be_canceled)
+    end
+
+    it 'breaks the chain' do
+      expect(step.break?).to be true
+    end
+
+    it 'appends validation error' do
+      expect(pipeline.errors.to_a)
+        .to include 'Active pipelines limit exceeded by 2 pipelines!'
+    end
+  end
+
+  context 'when pipeline size limit is not exceeded' do
+    before do
+      step.perform!
+    end
+
+    it 'does not break the chain' do
+      expect(step.break?).to be false
+    end
+
+    it 'does not invalidate the pipeline' do
+      expect(pipeline.errors).to be_empty
+    end
+  end
+end

spec/ee/spec/lib/gitlab/ci/pipeline/chain/limit/size_spec.rb

+require 'spec_helper'
+
+describe EE::Gitlab::Ci::Pipeline::Chain::Limit::Size do
+  set(:namespace) { create(:namespace, plan: Namespace::GOLD_PLAN) }
+  set(:project) { create(:project, namespace: namespace) }
+  set(:user) { create(:user) }
+
+  let(:pipeline) do
+    build(:ci_pipeline_with_one_job, project: project,
+                                     ref: 'master')
+  end
+
+  let(:command) do
+    double('command', project: project,
+                      current_user: user)
+  end
+
+  let(:step) { described_class.new(pipeline, command) }
+
+  context 'when pipeline size limit is exceeded' do
+    before do
+      project.namespace.plan.update_column(:pipeline_size_limit, 1)
+
+      step.perform!
+    end
+
+    let(:pipeline) do
+      config = { rspec: { script: 'rspec' },
+                 spinach: { script: 'spinach' } }
+
+      create(:ci_pipeline, project: project, config: config)
+    end
+
+    context 'when saving incomplete pipelines' do
+      let(:command) do
+        double('command', project: project,
+                          current_user: user,
+                          save_incompleted: true)
+      end
+
+      it 'drops the pipeline' do
+        expect(pipeline.reload).to be_failed
+      end
+
+      it 'persists the pipeline' do
+        expect(pipeline).to be_persisted
+      end
+
+      it 'breaks the chain' do
+        expect(step.break?).to be true
+      end
+
+      it 'appends validation error' do
+        expect(pipeline.errors.to_a)
+          .to include 'Pipeline size limit exceeded by 1 job!'
+      end
+    end
+
+    context 'when not saving incomplete pipelines' do
+      let(:command) do
+        double('command', project: project,
+                          current_user: user,
+                          save_incompleted: false)
+      end
+
+      it 'does not drop the pipeline' do
+        expect(pipeline).not_to be_failed
+      end
+
+      it 'breaks the chain' do
+        expect(step.break?).to be true
+      end
+    end
+  end
+
+  context 'when pipeline size limit is not exceeded' do
+    before do
+      step.perform!
+    end
+
+    it 'does not break the chain' do
+      expect(step.break?).to be false
+    end
+
+    it 'does not persist the pipeline' do
+      expect(pipeline).not_to be_persisted
+    end
+  end
+end