Remove usage of DockerClient

lib/runner/strategy/docker_container_pool.rb

@@ -9,7 +9,9 @@ class Runner::Strategy::DockerContainerPool < Runner::Strategy
   end
 
   def self.initialize_environment
-    DockerClient.initialize_environment unless Rails.env.test? && `which docker`.blank?
+    raise Error.new('Docker configuration missing!') unless config[:host] && config[:workspace_root]
+
+    FileUtils.mkdir_p(File.expand_path(config[:workspace_root]))
   end
 
   def self.sync_environment(_environment)

spec/controllers/execution_environments_controller_spec.rb

@@ -12,8 +12,6 @@ describe ExecutionEnvironmentsController do
   end
 
   describe 'POST #create' do
-    before { allow(DockerClient).to receive(:image_tags).at_least(:once).and_return([]) }
-
     context 'with a valid execution environment' do
       let(:perform_request) { proc { post :create, params: {execution_environment: FactoryBot.build(:ruby).attributes} } }
 
@@ -61,7 +59,6 @@ describe ExecutionEnvironmentsController do
 
   describe 'GET #edit' do
     before do
-      allow(DockerClient).to receive(:image_tags).at_least(:once).and_return([])
       get :edit, params: {id: execution_environment.id}
     end
 
@@ -99,7 +96,6 @@ describe ExecutionEnvironmentsController do
 
   describe 'GET #new' do
     before do
-      allow(DockerClient).to receive(:image_tags).at_least(:once).and_return([])
       get :new
     end
 
@@ -115,8 +111,7 @@ describe ExecutionEnvironmentsController do
 
       before do
         allow(Runner).to receive(:strategy_class).and_return Runner::Strategy::DockerContainerPool
-        allow(DockerClient).to receive(:check_availability!).at_least(:once)
+        allow(Runner::Strategy::DockerContainerPool).to receive(:available_images).and_return(docker_images)
-        allow(DockerClient).to receive(:image_tags).and_return(docker_images)
         controller.send(:set_docker_images)
       end
 
@@ -128,7 +123,7 @@ describe ExecutionEnvironmentsController do
 
       before do
         allow(Runner).to receive(:strategy_class).and_return Runner::Strategy::DockerContainerPool
-        allow(DockerClient).to receive(:check_availability!).at_least(:once).and_raise(DockerClient::Error.new(error_message))
+        allow(Runner::Strategy::DockerContainerPool).to receive(:available_images).and_raise(Runner::Error::InternalServerError.new(error_message))
         controller.send(:set_docker_images)
       end
 
@@ -168,7 +163,6 @@ describe ExecutionEnvironmentsController do
   describe 'PUT #update' do
     context 'with a valid execution environment' do
       before do
-        allow(DockerClient).to receive(:image_tags).at_least(:once).and_return([])
         allow(controller).to receive(:sync_to_runner_management).and_return(nil)
         put :update, params: {execution_environment: FactoryBot.attributes_for(:ruby), id: execution_environment.id}
       end

spec/controllers/submissions_controller_spec.rb

@@ -156,7 +156,6 @@ describe SubmissionsController do
 
     context 'when no errors occur during execution' do
       before do
-        allow_any_instance_of(DockerClient).to receive(:execute_run_command).with(submission, filename).and_return({})
         perform_request
       end
 
@@ -219,7 +218,6 @@ describe SubmissionsController do
     let(:output) { {} }
 
     before do
-      allow_any_instance_of(DockerClient).to receive(:execute_test_command).with(submission, filename)
       get :test, params: {filename: filename, id: submission.id}
     end
 

spec/db/seeds_spec.rb

@@ -16,6 +16,7 @@ describe 'seeds' do
     allow(ActiveRecord::Base).to receive(:establish_connection).with(:development) {
       ActiveRecord::Base.establish_connection(:test)
     }
+    allow_any_instance_of(ExecutionEnvironment).to receive(:working_docker_image?).and_return true
   end
 
   describe 'execute db:seed', cleaning_strategy: :truncation do

spec/features/factories_spec.rb

@@ -3,7 +3,7 @@
 require 'rails_helper'
 
 describe 'Factories' do
-  it 'are all valid', docker: true, permitted_execution_time: 30 do
+  it 'are all valid', permitted_execution_time: 30 do
     expect { FactoryBot.lint }.not_to raise_error
   end
 end

spec/lib/docker_client_spec.rb

@@ -5,7 +5,7 @@ require 'seeds_helper'
 
 WORKSPACE_PATH = Rails.root.join('tmp', 'files', Rails.env, 'code_ocean_test')
 
-describe DockerClient, docker: true do
+describe DockerClient do
   let(:command) { 'whoami' }
   let(:docker_client) { described_class.new(execution_environment: FactoryBot.build(:java), user: FactoryBot.build(:admin)) }
   let(:execution_environment) { FactoryBot.build(:java) }
@@ -71,13 +71,15 @@ describe DockerClient, docker: true do
 
     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
+      container = create_container
+      described_class.destroy_container(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
+      container = create_container
+      described_class.destroy_container(container)
     end
 
     it 'creates a container' do
@@ -91,22 +93,26 @@ describe DockerClient, docker: true do
         result
       end
       expect(Docker::Container).to receive(:create).with(kind_of(Hash)).and_call_original
-      create_container
+      container = create_container
+      described_class.destroy_container(container)
     end
 
     it 'starts the container' do
       expect_any_instance_of(Docker::Container).to receive(:start).and_call_original
-      create_container
+      container = create_container
+      described_class.destroy_container(container)
     end
 
     it 'configures mapped directories' do
       expect(described_class).to receive(:mapped_directories).and_call_original
-      create_container
+      container = create_container
+      described_class.destroy_container(container)
     end
 
     it 'configures mapped ports' do
       expect(described_class).to receive(:mapped_ports).with(execution_environment).and_call_original
-      create_container
+      container = create_container
+      described_class.destroy_container(container)
     end
 
     context 'when an error occurs' do
@@ -118,7 +124,9 @@ describe DockerClient, docker: true do
         end
 
         it 'retries to create a container' do
-          expect(create_container).to be_a(Docker::Container)
+          container = create_container
+          expect(container).to be_a(Docker::Container)
+          described_class.destroy_container(container)
         end
       end
 
@@ -162,6 +170,7 @@ describe DockerClient, docker: true do
   end
 
   describe '#create_workspace_file' do
+    let(:container) { Docker::Container.send(:new, Docker::Connection.new('http://example.org', {}), 'id' => SecureRandom.hex) }
     let(:file) { FactoryBot.build(:file, content: 'puts 42') }
     let(:file_path) { File.join(workspace_path, file.name_with_extension) }
 
@@ -170,7 +179,7 @@ describe DockerClient, docker: true do
     it 'creates a file' do
       expect(described_class).to receive(:local_workspace_path).at_least(:once).and_return(workspace_path)
       FileUtils.mkdir_p(workspace_path)
-      docker_client.send(:create_workspace_file, container: CONTAINER, file: file)
+      docker_client.send(:create_workspace_file, container: container, file: file)
       expect(File.exist?(file_path)).to be true
       expect(File.new(file_path, 'r').read).to eq(file.content)
     end
@@ -197,13 +206,15 @@ describe DockerClient, docker: true do
     end
 
     it 'deletes the container' do
-      expect(container).to receive(:delete).with(force: true, v: true)
+      expect(container).to receive(:delete).with(force: true, v: true).and_call_original
     end
   end
 
   describe '#execute_arbitrary_command' do
     let(:execute_arbitrary_command) { docker_client.execute_arbitrary_command(command) }
 
+    after { described_class.destroy_container(docker_client.container) }
+
     it 'creates a new container' do
       expect(described_class).to receive(:create_container).and_call_original
       execute_arbitrary_command
@@ -246,7 +257,10 @@ describe DockerClient, docker: true do
   describe '#execute_run_command' do
     let(:filename) { submission.exercise.files.detect {|file| file.role == 'main_file' }.name_with_extension }
 
-    after { docker_client.send(:execute_run_command, submission, filename) }
+    after do
+      docker_client.send(:execute_run_command, submission, filename)
+      described_class.destroy_container(docker_client.container)
+    end
 
     it 'creates a new container' do
       expect(described_class).to receive(:create_container).with(submission.execution_environment).and_call_original
@@ -266,7 +280,10 @@ describe DockerClient, docker: true do
   describe '#execute_test_command' do
     let(:filename) { submission.exercise.files.detect {|file| file.role == 'teacher_defined_test' || file.role == 'teacher_defined_linter' }.name_with_extension }
 
-    after { docker_client.send(:execute_test_command, submission, filename) }
+    after do
+      docker_client.send(:execute_test_command, submission, filename)
+      described_class.destroy_container(docker_client.container)
+    end
 
     it 'creates a new container' do
       expect(described_class).to receive(:create_container).with(submission.execution_environment).and_call_original
@@ -314,6 +331,8 @@ describe DockerClient, docker: true do
     let(:container) { described_class.create_container(execution_environment) }
     let(:local_workspace_path) { described_class.local_workspace_path(container) }
 
+    after { described_class.destroy_container(container) }
+
     it 'returns a path' do
       expect(local_workspace_path).to be_a(Pathname)
     end
@@ -358,7 +377,10 @@ describe DockerClient, docker: true do
     let(:container) { described_class.create_container(execution_environment) }
     let(:send_command) { docker_client.send(:send_command, command, container, &block) }
 
-    after { send_command }
+    after do
+      send_command
+      described_class.destroy_container(container)
+    end
 
     it 'limits the execution time' do
       expect(Timeout).to receive(:timeout).at_least(:once).with(kind_of(Numeric)).and_call_original

spec/lib/docker_container_mixin_spec.rb

@@ -3,6 +3,8 @@
 require 'rails_helper'
 
 describe DockerContainerMixin do
+  let(:container) { Docker::Container.send(:new, Docker::Connection.new('http://example.org', {}), 'id' => SecureRandom.hex) }
+
   describe '#binds' do
     let(:binds) { [] }
 
@@ -11,8 +13,8 @@ describe DockerContainerMixin do
     end
 
     it 'returns the correct information' do
-      allow(CONTAINER).to receive(:json).and_return('HostConfig' => {'Binds' => binds})
+      allow(container).to receive(:json).and_return('HostConfig' => {'Binds' => binds})
-      expect(CONTAINER.binds).to eq(binds)
+      expect(container.binds).to eq(binds)
     end
   end
 
@@ -25,8 +27,8 @@ describe DockerContainerMixin do
     end
 
     it 'returns the correct information' do
-      allow(CONTAINER).to receive(:json).and_return('HostConfig' => {'PortBindings' => port_bindings})
+      allow(container).to receive(:json).and_return('HostConfig' => {'PortBindings' => port_bindings})
-      expect(CONTAINER.port_bindings).to eq(port => port)
+      expect(container.port_bindings).to eq(port => port)
     end
   end
 end

spec/models/execution_environment_spec.rb

@@ -5,10 +5,11 @@ require 'rails_helper'
 describe ExecutionEnvironment do
   let(:execution_environment) { described_class.create.tap {|execution_environment| execution_environment.update(network_enabled: nil) } }
 
-  it 'validates that the Docker image works', docker: true do
+  it 'validates that the Docker image works' do
     allow(execution_environment).to receive(:validate_docker_image?).and_return(true)
-    expect(execution_environment).to receive(:working_docker_image?)
+    allow(execution_environment).to receive(:working_docker_image?).and_return(true)
     execution_environment.update(docker_image: FactoryBot.attributes_for(:ruby)[:docker_image])
+    expect(execution_environment).to have_received(:working_docker_image?)
   end
 
   it 'validates the presence of a Docker image name' do
@@ -144,27 +145,29 @@ describe ExecutionEnvironment do
     end
   end
 
-  describe '#working_docker_image?', docker: true do
+  describe '#working_docker_image?' do
     let(:working_docker_image?) { execution_environment.send(:working_docker_image?) }
+    let(:runner) { instance_double 'runner' }
 
-    before { allow(DockerClient).to receive(:find_image_by_tag).and_return(Object.new) }
+    before do
+      allow(Runner).to receive(:for).with(execution_environment.author, execution_environment).and_return runner
+    end
 
     it 'instantiates a Runner' do
-      runner = instance_double 'runner'
-      allow(Runner).to receive(:for).with(execution_environment.author, execution_environment).and_return runner
       allow(runner).to receive(:execute_command).and_return({})
       working_docker_image?
       expect(runner).to have_received(:execute_command).once
     end
 
     it 'executes the validation command' do
-      allow_any_instance_of(DockerClient).to receive(:execute_arbitrary_command).with(ExecutionEnvironment::VALIDATION_COMMAND).and_return({})
+      allow(runner).to receive(:execute_command).and_return({})
       working_docker_image?
+      expect(runner).to have_received(:execute_command).with(ExecutionEnvironment::VALIDATION_COMMAND, raise_exception: true)
     end
 
     context 'when the command produces an error' do
       it 'adds an error' do
-        allow_any_instance_of(DockerClient).to receive(:execute_arbitrary_command).and_return(stderr: 'command not found')
+        allow(runner).to receive(:execute_command).and_return(stderr: 'command not found')
         working_docker_image?
         expect(execution_environment.errors[:docker_image]).to be_present
       end
@@ -172,7 +175,7 @@ describe ExecutionEnvironment do
 
     context 'when the Docker client produces an error' do
       it 'adds an error' do
-        allow_any_instance_of(DockerClient).to receive(:execute_arbitrary_command).and_raise(DockerClient::Error)
+        allow(runner).to receive(:execute_command).and_raise(Runner::Error)
         working_docker_image?
         expect(execution_environment.errors[:docker_image]).to be_present
       end

spec/support/docker.rb

@@ -1,27 +0,0 @@
-# frozen_string_literal: true
-
-CONTAINER = Docker::Container.send(:new, Docker::Connection.new('http://example.org', {}), 'id' => SecureRandom.hex)
-IMAGE = Docker::Image.new(Docker::Connection.new('http://example.org', {}), 'id' => SecureRandom.hex, 'RepoTags' => [FactoryBot.attributes_for(:ruby)[:docker_image]])
-
-RSpec.configure do |config|
-  config.before(:each) do |example|
-    unless example.metadata[:docker]
-      allow(DockerClient).to receive(:check_availability!).and_return(true)
-      allow(DockerClient).to receive(:create_container).and_return(CONTAINER)
-      allow(DockerClient).to receive(:find_image_by_tag).and_return(IMAGE)
-      allow(DockerClient).to receive(:image_tags).and_return([IMAGE.info['RepoTags']].flatten)
-      allow(DockerClient).to receive(:local_workspace_path).and_return(Dir.mktmpdir)
-      allow_any_instance_of(DockerClient).to receive(:send_command).and_return({})
-      allow_any_instance_of(ExecutionEnvironment).to receive(:working_docker_image?)
-    end
-  end
-
-  config.after(:suite) do
-    examples = RSpec.world.filtered_examples.values.flatten
-    has_docker_tests = examples.any? {|example| example.metadata[:docker] }
-    next unless has_docker_tests
-
-    FileUtils.rm_rf(Rails.root.join('tmp/files/test'))
-    `which docker && test -n "$(docker ps --all --quiet)" && docker rm --force $(docker ps --all --quiet)`
-  end
-end