1 files changed, 90 insertions, 10 deletions

diff --git a/spec/lib/container_registry/client_spec.rb b/spec/lib/container_registry/client_spec.rb
index 2c08fdc1e75..9d6f4db537d 100644
--- a/spec/lib/container_registry/client_spec.rb
+++ b/spec/lib/container_registry/client_spec.rb
@@ -26,7 +26,54 @@ RSpec.describe ContainerRegistry::Client do
     }
   end
 
-  shared_examples '#repository_manifest' do |manifest_type|
+  let(:expected_faraday_headers) { { user_agent: "GitLab/#{Gitlab::VERSION}" } }
+  let(:expected_faraday_request_options) { Gitlab::HTTP::DEFAULT_TIMEOUT_OPTIONS }
+
+  shared_examples 'handling timeouts' do
+    let(:retry_options) do
+      ContainerRegistry::Client::RETRY_OPTIONS.merge(
+        interval: 0.1,
+        interval_randomness: 0,
+        backoff_factor: 0
+      )
+    end
+
+    before do
+      stub_request(method, url).to_timeout
+    end
+
+    it 'handles network timeouts' do
+      actual_retries = 0
+      retry_options_with_block = retry_options.merge(
+        retry_block: -> (_, _, _, _) { actual_retries += 1 }
+      )
+
+      stub_const('ContainerRegistry::Client::RETRY_OPTIONS', retry_options_with_block)
+
+      expect { subject }.to raise_error(Faraday::ConnectionFailed)
+      expect(actual_retries).to eq(retry_options_with_block[:max])
+    end
+
+    it 'logs the error' do
+      stub_const('ContainerRegistry::Client::RETRY_OPTIONS', retry_options)
+
+      expect(Gitlab::ErrorTracking)
+        .to receive(:log_exception)
+        .exactly(retry_options[:max] + 1)
+        .times
+        .with(
+          an_instance_of(Faraday::ConnectionFailed),
+          class: described_class.name,
+          url: URI(url)
+        )
+
+      expect { subject }.to raise_error(Faraday::ConnectionFailed)
+    end
+  end
+
+  shared_examples 'handling repository manifest' do |manifest_type|
+    let(:method) { :get }
+    let(:url) {  'http://container-registry/v2/group/test/manifests/mytag' }
     let(:manifest) do
       {
         "schemaVersion" => 2,
@@ -48,7 +95,7 @@ RSpec.describe ContainerRegistry::Client do
     end
 
     it 'GET /v2/:name/manifests/mytag' do
-      stub_request(:get, "http://container-registry/v2/group/test/manifests/mytag")
+      stub_request(method, url)
         .with(headers: {
                 'Accept' => 'application/vnd.docker.distribution.manifest.v2+json, application/vnd.oci.image.manifest.v1+json',
                 'Authorization' => "bearer #{token}",
@@ -56,14 +103,24 @@ RSpec.describe ContainerRegistry::Client do
               })
         .to_return(status: 200, body: manifest.to_json, headers: { content_type: manifest_type })
 
-      expect(client.repository_manifest('group/test', 'mytag')).to eq(manifest)
+      expect_new_faraday
+
+      expect(subject).to eq(manifest)
     end
+
+    it_behaves_like 'handling timeouts'
   end
 
-  it_behaves_like '#repository_manifest', described_class::DOCKER_DISTRIBUTION_MANIFEST_V2_TYPE
-  it_behaves_like '#repository_manifest', described_class::OCI_MANIFEST_V1_TYPE
+  describe '#repository_manifest' do
+    subject { client.repository_manifest('group/test', 'mytag') }
+
+    it_behaves_like 'handling repository manifest', described_class::DOCKER_DISTRIBUTION_MANIFEST_V2_TYPE
+    it_behaves_like 'handling repository manifest', described_class::OCI_MANIFEST_V1_TYPE
+  end
 
   describe '#blob' do
+    let(:method) { :get }
+    let(:url) { 'http://container-registry/v2/group/test/blobs/sha256:0123456789012345' }
     let(:blob_headers) do
       {
           'Accept' => 'application/octet-stream',
@@ -78,16 +135,20 @@ RSpec.describe ContainerRegistry::Client do
       }
     end
 
+    subject { client.blob('group/test', 'sha256:0123456789012345') }
+
     it 'GET /v2/:name/blobs/:digest' do
-      stub_request(:get, "http://container-registry/v2/group/test/blobs/sha256:0123456789012345")
+      stub_request(method, url)
         .with(headers: blob_headers)
         .to_return(status: 200, body: "Blob")
 
-      expect(client.blob('group/test', 'sha256:0123456789012345')).to eq('Blob')
+      expect_new_faraday
+
+      expect(subject).to eq('Blob')
     end
 
     it 'follows 307 redirect for GET /v2/:name/blobs/:digest' do
-      stub_request(:get, "http://container-registry/v2/group/test/blobs/sha256:0123456789012345")
+      stub_request(method, url)
         .with(headers: blob_headers)
         .to_return(status: 307, body: '', headers: { Location: 'http://redirected' })
       # We should probably use hash_excluding here, but that requires an update to WebMock:
@@ -98,10 +159,12 @@ RSpec.describe ContainerRegistry::Client do
         end
         .to_return(status: 200, body: "Successfully redirected")
 
-      response = client.blob('group/test', 'sha256:0123456789012345')
+      expect_new_faraday(times: 2)
 
-      expect(response).to eq('Successfully redirected')
+      expect(subject).to eq('Successfully redirected')
     end
+
+    it_behaves_like 'handling timeouts'
   end
 
   describe '#upload_blob' do
@@ -111,6 +174,8 @@ RSpec.describe ContainerRegistry::Client do
       it 'starts the upload and posts the blob' do
         stub_upload('path', 'content', 'sha256:123')
 
+        expect_new_faraday(timeout: false)
+
         expect(subject).to be_success
       end
     end
@@ -173,6 +238,8 @@ RSpec.describe ContainerRegistry::Client do
         .with(body: "{\n  \"foo\": \"bar\"\n}", headers: manifest_headers)
         .to_return(status: 200, body: "", headers: { 'docker-content-digest' => 'sha256:123' })
 
+      expect_new_faraday(timeout: false)
+
       expect(subject).to eq 'sha256:123'
     end
   end
@@ -375,4 +442,17 @@ RSpec.describe ContainerRegistry::Client do
         headers: { 'Allow' => 'DELETE' }
       )
   end
+
+  def expect_new_faraday(times: 1, timeout: true)
+    request_options = timeout ? expected_faraday_request_options : nil
+    expect(Faraday)
+      .to receive(:new)
+      .with(
+        'http://container-registry',
+        headers: expected_faraday_headers,
+        request: request_options
+      ).and_call_original
+      .exactly(times)
+      .times
+  end
 end