improved the Docker client's robustness regarding random errors

lib/docker_client.rb

@@ -3,6 +3,7 @@ require 'concurrent'
 class DockerClient
   CONTAINER_WORKSPACE_PATH = '/workspace'
   LOCAL_WORKSPACE_ROOT = Rails.root.join('tmp', 'files', Rails.env)
+  RETRY_COUNT = 2
 
   attr_reader :assigned_ports
   attr_reader :container_id
@@ -27,11 +28,15 @@ class DockerClient
   end
 
   def self.create_container(execution_environment)
+    tries ||= 0
     container = Docker::Container.create('Image' => find_image_by_tag(execution_environment.docker_image).info['RepoTags'].first, 'OpenStdin' => true, 'StdinOnce' => true)
     local_workspace_path = generate_local_workspace_path
     FileUtils.mkdir(local_workspace_path)
     container.start('Binds' => mapped_directories(local_workspace_path), 'PortBindings' => mapped_ports(execution_environment))
     container
+  rescue Docker::Error::NotFoundError => error
+    destroy_container(container)
+    (tries += 1) <= RETRY_COUNT ? retry : raise(error)
   end
 
   def create_workspace(container)
@@ -59,14 +64,17 @@ class DockerClient
       port = configuration.first['HostPort'].to_i
       PortPool.release(port)
     end
-    FileUtils.rm_rf(local_workspace_path(container))
+    FileUtils.rm_rf(local_workspace_path(container)) if local_workspace_path(container)
     container.delete(force: true)
   end
 
   def execute_arbitrary_command(command, &block)
+    tries ||= 0
     container = DockerContainerPool.get_container(@execution_environment)
     @container_id = container.id
     send_command(command, container, &block)
+  rescue Excon::Errors::SocketError => error
+    (tries += 1) <= RETRY_COUNT ? retry : raise(error)
   end
 
   [:run, :test].each do |cause|
@@ -113,7 +121,7 @@ class DockerClient
   private :local_file_path
 
   def self.local_workspace_path(container)
-    Pathname.new(container.binds.first.split(':').first.sub(config[:workspace_root], LOCAL_WORKSPACE_ROOT.to_s))
+    Pathname.new(container.binds.first.split(':').first.sub(config[:workspace_root], LOCAL_WORKSPACE_ROOT.to_s)) if container.binds.present?
   end
 
   def self.mapped_directories(local_workspace_path)

spec/lib/docker_client_spec.rb

@@ -26,33 +26,63 @@ describe DockerClient, docker: true do
   end
 
   describe '.create_container' do
-    after(:each) { described_class.create_container(execution_environment) }
+    let(:create_container) { described_class.create_container(execution_environment) }
 
     it 'uses the correct Docker image' do
       expect(described_class).to receive(:find_image_by_tag).with(execution_environment.docker_image).and_call_original
+      create_container
     end
 
     it 'creates a unique directory' do
       expect(described_class).to receive(:generate_local_workspace_path).and_call_original
       expect(FileUtils).to receive(:mkdir).with(kind_of(String)).and_call_original
+      create_container
     end
 
     it 'creates a container waiting for input' do
       expect(Docker::Container).to receive(:create).with('Image' => kind_of(String), 'OpenStdin' => true, 'StdinOnce' => true).and_call_original
+      create_container
     end
 
     it 'starts the container' do
       expect_any_instance_of(Docker::Container).to receive(:start)
+      create_container
     end
 
     it 'configures mapped directories' do
       expect(described_class).to receive(:mapped_directories).and_call_original
       expect_any_instance_of(Docker::Container).to receive(:start).with(hash_including('Binds' => kind_of(Array)))
+      create_container
     end
 
     it 'configures mapped ports' do
       expect(described_class).to receive(:mapped_ports).with(execution_environment).and_call_original
       expect_any_instance_of(Docker::Container).to receive(:start).with(hash_including('PortBindings' => kind_of(Hash)))
+      create_container
+    end
+
+    context 'when an error occurs' do
+      let(:error) { Docker::Error::NotFoundError.new }
+
+      context 'when retries are left' do
+        before(:each) do
+          expect(described_class).to receive(:mapped_directories).and_raise(error).and_call_original
+        end
+
+        it 'retries to create a container' do
+          expect(create_container).to be_a(Docker::Container)
+        end
+      end
+
+      context 'when no retries are left' do
+        before(:each) do
+          expect(described_class).to receive(:mapped_directories).exactly(DockerClient::RETRY_COUNT + 1).times.and_raise(error)
+        end
+
+        it 'raises the error' do
+          expect { create_container }.to raise_error(error)
+        end
+      end
     end
   end
 
@@ -114,7 +144,7 @@ describe DockerClient, docker: true do
     end
 
     it 'removes the mapped directory' do
-      expect(described_class).to receive(:local_workspace_path).and_return(workspace_path)
+      expect(described_class).to receive(:local_workspace_path).at_least(:once).and_return(workspace_path)
       expect(FileUtils).to receive(:rm_rf).with(workspace_path)
     end
 
@@ -124,14 +154,42 @@ describe DockerClient, docker: true do
   end
 
   describe '#execute_arbitrary_command' do
-    after(:each) { docker_client.execute_arbitrary_command(command) }
+    let(:execute_arbitrary_command) { docker_client.execute_arbitrary_command(command) }
 
     it 'takes a container from the pool' do
       expect(DockerContainerPool).to receive(:get_container).and_call_original
+      execute_arbitrary_command
     end
 
     it 'sends the command' do
       expect(docker_client).to receive(:send_command).with(command, kind_of(Docker::Container))
+      execute_arbitrary_command
+    end
+
+    context 'when a socket error occurs' do
+      let(:error) { Excon::Errors::SocketError.new(SocketError.new) }
+
+      context 'when retries are left' do
+        let(:result) { 42 }
+
+        before(:each) do
+          expect(docker_client).to receive(:send_command).and_raise(error).and_return(result)
+        end
+
+        it 'retries to execute the command' do
+          expect(execute_arbitrary_command).to eq(result)
+        end
+      end
+
+      context 'when no retries are left' do
+        before(:each) do
+          expect(docker_client).to receive(:send_command).exactly(DockerClient::RETRY_COUNT + 1).times.and_raise(error)
+        end
+
+        it 'raises the error' do
+          expect { execute_arbitrary_command }.to raise_error(error)
+        end
+      end
     end
   end