Add latest changes from gitlab-org/gitlab@master

4 files changed, 174 insertions, 59 deletions

diff --git a/spec/lib/gitlab/ci/trace/checksum_spec.rb b/spec/lib/gitlab/ci/trace/checksum_spec.rb
new file mode 100644
index 00000000000..4bd96aad4e8
--- /dev/null
+++ b/spec/lib/gitlab/ci/trace/checksum_spec.rb
@@ -0,0 +1,121 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Ci::Trace::Checksum do
+  let(:build) { create(:ci_build, :running) }
+
+  subject { described_class.new(build) }
+
+  context 'when build pending state exists' do
+    before do
+      create(:ci_build_pending_state, build: build, trace_checksum: 'crc32:3564598592')
+    end
+
+    context 'when matching persisted trace chunks exist' do
+      before do
+        create_chunk(index: 0, data: 'a' * 128.kilobytes)
+        create_chunk(index: 1, data: 'b' * 128.kilobytes)
+        create_chunk(index: 2, data: 'ccccccccccccccccc')
+      end
+
+      it 'calculates combined trace chunks CRC32 correctly' do
+        expect(subject.chunks_crc32).to eq 3564598592
+        expect(subject).to be_valid
+      end
+    end
+
+    context 'when trace chunks were persisted in a wrong order' do
+      before do
+        create_chunk(index: 0, data: 'b' * 128.kilobytes)
+        create_chunk(index: 1, data: 'a' * 128.kilobytes)
+        create_chunk(index: 2, data: 'ccccccccccccccccc')
+      end
+
+      it 'makes trace checksum invalid' do
+        expect(subject).not_to be_valid
+      end
+    end
+
+    context 'when one of the trace chunks is missing' do
+      before do
+        create_chunk(index: 0, data: 'a' * 128.kilobytes)
+        create_chunk(index: 2, data: 'ccccccccccccccccc')
+      end
+
+      it 'makes trace checksum invalid' do
+        expect(subject).not_to be_valid
+      end
+    end
+
+    context 'when checksums of persisted trace chunks do not match' do
+      before do
+        create_chunk(index: 0, data: 'a' * 128.kilobytes)
+        create_chunk(index: 1, data: 'X' * 128.kilobytes)
+        create_chunk(index: 2, data: 'ccccccccccccccccc')
+      end
+
+      it 'makes trace checksum invalid' do
+        expect(subject).not_to be_valid
+      end
+    end
+
+    context 'when persisted trace chunks are missing' do
+      it 'makes trace checksum invalid' do
+        expect(subject.state_crc32).to eq 3564598592
+        expect(subject).not_to be_valid
+      end
+    end
+  end
+
+  context 'when build pending state is missing' do
+    describe '#state_crc32' do
+      it 'returns zero' do
+        expect(subject.state_crc32).to be_zero
+      end
+    end
+
+    describe '#valid?' do
+      it { is_expected.not_to be_valid }
+    end
+  end
+
+  describe '#trace_chunks' do
+    before do
+      create_chunk(index: 0, data: 'abcdefg')
+    end
+
+    it 'does not load raw_data from a database store' do
+      subject.trace_chunks.first.then do |chunk|
+        expect(chunk).to be_database
+        expect { chunk.raw_data }
+          .to raise_error ActiveModel::MissingAttributeError
+      end
+    end
+  end
+
+  describe '#last_chunk' do
+    context 'when there are no chunks' do
+      it 'returns nil' do
+        expect(subject.last_chunk).to be_nil
+      end
+    end
+
+    context 'when there are multiple chunks' do
+      before do
+        create_chunk(index: 1, data: '1234')
+        create_chunk(index: 0, data: 'abcd')
+      end
+
+      it 'returns chunk with the highest index' do
+        expect(subject.last_chunk.chunk_index).to eq 1
+      end
+    end
+  end
+
+  def create_chunk(index:, data:)
+    create(:ci_build_trace_chunk, :persisted, build: build,
+                                              chunk_index: index,
+                                              initial_data: data)
+  end
+end
diff --git a/spec/lib/gitlab/ci/trace_spec.rb b/spec/lib/gitlab/ci/trace_spec.rb
index 171877dbaee..f037e803fb4 100644
--- a/spec/lib/gitlab/ci/trace_spec.rb
+++ b/spec/lib/gitlab/ci/trace_spec.rb
@@ -111,4 +111,13 @@ RSpec.describe Gitlab::Ci::Trace, :clean_gitlab_redis_shared_state do
       end
     end
   end
+
+  describe '#lock' do
+    it 'acquires an exclusive lease on the trace' do
+      trace.lock do
+        expect { trace.lock }
+          .to raise_error described_class::LockedError
+      end
+    end
+  end
 end
diff --git a/spec/lib/gitlab/exclusive_lease_helpers_spec.rb b/spec/lib/gitlab/exclusive_lease_helpers_spec.rb
index 01e2fe8ce17..40669f06371 100644
--- a/spec/lib/gitlab/exclusive_lease_helpers_spec.rb
+++ b/spec/lib/gitlab/exclusive_lease_helpers_spec.rb
@@ -25,13 +25,17 @@ RSpec.describe Gitlab::ExclusiveLeaseHelpers, :clean_gitlab_redis_shared_state d
       let!(:lease) { stub_exclusive_lease(unique_key, 'uuid') }
 
       it 'calls the given block' do
-        expect { |b| class_instance.in_lock(unique_key, &b) }.to yield_with_args(false)
+        expect { |b| class_instance.in_lock(unique_key, &b) }
+          .to yield_with_args(false, an_instance_of(described_class::SleepingLock))
       end
 
       it 'calls the given block continuously' do
-        expect { |b| class_instance.in_lock(unique_key, &b) }.to yield_with_args(false)
-        expect { |b| class_instance.in_lock(unique_key, &b) }.to yield_with_args(false)
-        expect { |b| class_instance.in_lock(unique_key, &b) }.to yield_with_args(false)
+        expect { |b| class_instance.in_lock(unique_key, &b) }
+          .to yield_with_args(false, an_instance_of(described_class::SleepingLock))
+        expect { |b| class_instance.in_lock(unique_key, &b) }
+          .to yield_with_args(false, an_instance_of(described_class::SleepingLock))
+        expect { |b| class_instance.in_lock(unique_key, &b) }
+          .to yield_with_args(false, an_instance_of(described_class::SleepingLock))
       end
 
       it 'cancels the exclusive lease after the block' do
@@ -74,7 +78,8 @@ RSpec.describe Gitlab::ExclusiveLeaseHelpers, :clean_gitlab_redis_shared_state d
             expect(lease).to receive(:try_obtain).exactly(3).times { nil }
             expect(lease).to receive(:try_obtain).once { unique_key }
 
-            expect { |b| class_instance.in_lock(unique_key, &b) }.to yield_with_args(true)
+            expect { |b| class_instance.in_lock(unique_key, &b) }
+              .to yield_with_args(true, an_instance_of(described_class::SleepingLock))
           end
         end
       end
diff --git a/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb b/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb
index 69b779d36eb..0c77dc540f3 100644
--- a/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb
+++ b/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb
@@ -38,69 +38,49 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware do
       end
 
       context 'request is a health check endpoint' do
-        it 'increments health endpoint counter' do
-          env['PATH_INFO'] = '/-/liveness'
+        ['/-/liveness', '/-/liveness/', '/-/%6D%65%74%72%69%63%73'].each do |path|
+          context "when path is #{path}" do
+            before do
+              env['PATH_INFO'] = path
+            end
 
-          expect(described_class).to receive_message_chain(:http_health_requests_total, :increment).with(method: 'get')
+            it 'increments health endpoint counter rather than overall counter' do
+              expect(described_class).to receive_message_chain(:http_health_requests_total, :increment).with(method: 'get')
+              expect(described_class).not_to receive(:http_request_total)
 
-          subject.call(env)
-        end
-
-        context 'with trailing slash' do
-          before do
-            env['PATH_INFO'] = '/-/liveness/'
-          end
+              subject.call(env)
+            end
 
-          it 'increments health endpoint counter' do
-            expect(described_class).to receive_message_chain(:http_health_requests_total, :increment).with(method: 'get')
+            it 'does not record the request duration' do
+              expect(described_class).not_to receive(:http_request_duration_seconds)
 
-            subject.call(env)
-          end
-        end
-
-        context 'with percent encoded values' do
-          before do
-            env['PATH_INFO'] = '/-/%6D%65%74%72%69%63%73' # /-/metrics
-          end
-
-          it 'increments health endpoint counter' do
-            expect(described_class).to receive_message_chain(:http_health_requests_total, :increment).with(method: 'get')
-
-            subject.call(env)
+              subject.call(env)
+            end
           end
         end
       end
 
       context 'request is not a health check endpoint' do
-        it 'does not increment health endpoint counter' do
-          env['PATH_INFO'] = '/-/ordinary-requests'
-
-          expect(described_class).not_to receive(:http_health_requests_total)
-
-          subject.call(env)
-        end
-
-        context 'path info is a root path' do
-          before do
-            env['PATH_INFO'] = '/-/'
-          end
-
-          it 'does not increment health endpoint counter' do
-            expect(described_class).not_to receive(:http_health_requests_total)
-
-            subject.call(env)
-          end
-        end
-
-        context 'path info is a subpath' do
-          before do
-            env['PATH_INFO'] = '/-/health/subpath'
-          end
-
-          it 'does not increment health endpoint counter' do
-            expect(described_class).not_to receive(:http_health_requests_total)
-
-            subject.call(env)
+        ['/-/ordinary-requests', '/-/', '/-/health/subpath'].each do |path|
+          context "when path is #{path}" do
+            before do
+              env['PATH_INFO'] = path
+            end
+
+            it 'increments overall counter rather than health endpoint counter' do
+              expect(described_class).to receive_message_chain(:http_request_total, :increment).with(method: 'get')
+              expect(described_class).not_to receive(:http_health_requests_total)
+
+              subject.call(env)
+            end
+
+            it 'records the request duration' do
+              expect(described_class)
+                .to receive_message_chain(:http_request_duration_seconds, :observe)
+                      .with({ method: 'get', status: '200' }, a_positive_execution_time)
+
+              subject.call(env)
+            end
           end
         end
       end