diff --git a/lib/runner/strategy/docker_container_pool.rb b/lib/runner/strategy/docker_container_pool.rb index 977fe02d..72b76d92 100644 --- a/lib/runner/strategy/docker_container_pool.rb +++ b/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) diff --git a/spec/controllers/execution_environments_controller_spec.rb b/spec/controllers/execution_environments_controller_spec.rb index 08e1f67d..3d4a3896 100644 --- a/spec/controllers/execution_environments_controller_spec.rb +++ b/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(DockerClient).to receive(:image_tags).and_return(docker_images) + allow(Runner::Strategy::DockerContainerPool).to receive(:available_images).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 diff --git a/spec/controllers/submissions_controller_spec.rb b/spec/controllers/submissions_controller_spec.rb index a176fdc9..b92f92d8 100644 --- a/spec/controllers/submissions_controller_spec.rb +++ b/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 diff --git a/spec/db/seeds_spec.rb b/spec/db/seeds_spec.rb index 7c2191db..07ab8e4f 100644 --- a/spec/db/seeds_spec.rb +++ b/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 diff --git a/spec/features/factories_spec.rb b/spec/features/factories_spec.rb index 4b65cc3d..21f4fbd1 100644 --- a/spec/features/factories_spec.rb +++ b/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 diff --git a/spec/lib/docker_client_spec.rb b/spec/lib/docker_client_spec.rb index 45dfdfca..70f56eea 100644 --- a/spec/lib/docker_client_spec.rb +++ b/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 diff --git a/spec/lib/docker_container_mixin_spec.rb b/spec/lib/docker_container_mixin_spec.rb index 8faaf8da..6f9267e0 100644 --- a/spec/lib/docker_container_mixin_spec.rb +++ b/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}) - expect(CONTAINER.binds).to eq(binds) + allow(container).to receive(:json).and_return('HostConfig' => {'Binds' => 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}) - expect(CONTAINER.port_bindings).to eq(port => port) + allow(container).to receive(:json).and_return('HostConfig' => {'PortBindings' => port_bindings}) + expect(container.port_bindings).to eq(port => port) end end end diff --git a/spec/models/execution_environment_spec.rb b/spec/models/execution_environment_spec.rb index 09fd1c0a..2d7b1e48 100644 --- a/spec/models/execution_environment_spec.rb +++ b/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 diff --git a/spec/support/docker.rb b/spec/support/docker.rb deleted file mode 100644 index 5d86d951..00000000 --- a/spec/support/docker.rb +++ /dev/null @@ -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