diff --git a/app/controllers/execution_environments_controller.rb b/app/controllers/execution_environments_controller.rb index 68a84625..bd5e96d0 100644 --- a/app/controllers/execution_environments_controller.rb +++ b/app/controllers/execution_environments_controller.rb @@ -30,8 +30,9 @@ class ExecutionEnvironmentsController < ApplicationController end def execute_command - @docker_client = DockerClient.new(execution_environment: @execution_environment) - render(json: @docker_client.execute_arbitrary_command(params[:command])) + runner = Runner.for(current_user, @execution_environment) + output = runner.execute_command(params[:command]) + render(json: output) end def working_time_query diff --git a/app/models/runner.rb b/app/models/runner.rb index 3aa53a22..f9f30704 100644 --- a/app/models/runner.rb +++ b/app/models/runner.rb @@ -19,9 +19,7 @@ class Runner < ApplicationRecord @management_active ||= CodeOcean::Config.new(:code_ocean).read[:runner_management][:enabled] end - def self.for(user, exercise) - execution_environment = exercise.execution_environment - + def self.for(user, execution_environment) runner = find_by(user: user, execution_environment: execution_environment) if runner.nil? runner = Runner.create(user: user, execution_environment: execution_environment) @@ -62,6 +60,45 @@ class Runner < ApplicationRecord Time.zone.now - starting_time # execution duration end + def execute_command(command) + output = {} + stdout = +'' + stderr = +'' + try = 0 + begin + exit_code = 1 # default to error + execution_time = attach_to_execution(command) do |socket| + socket.on :stderr do |data| + stderr << data + end + socket.on :stdout do |data| + stdout << data + end + socket.on :exit do |received_exit_code| + exit_code = received_exit_code + end + end + output.merge!(container_execution_time: execution_time, status: exit_code.zero? ? :ok : :failed) + rescue Runner::Error::ExecutionTimeout => e + Rails.logger.debug { "Running command `#{command}` timed out: #{e.message}" } + output.merge!(status: :timeout, container_execution_time: e.execution_duration) + rescue Runner::Error::RunnerNotFound => e + Rails.logger.debug { "Running command `#{command}` failed for the first time: #{e.message}" } + try += 1 + request_new_id + save + retry if try == 1 + + Rails.logger.debug { "Running command `#{command}` failed for the second time: #{e.message}" } + output.merge!(status: :failed, container_execution_time: e.execution_duration) + rescue Runner::Error => e + Rails.logger.debug { "Running command `#{command}` failed: #{e.message}" } + output.merge!(status: :failed, container_execution_time: e.execution_duration) + ensure + output.merge!(stdout: stdout, stderr: stderr) + end + end + def destroy_at_management @strategy.destroy_at_management end diff --git a/app/models/submission.rb b/app/models/submission.rb index f48217fd..ea67a351 100644 --- a/app/models/submission.rb +++ b/app/models/submission.rb @@ -172,33 +172,10 @@ class Submission < ApplicationRecord end def run_test_file(file, runner, waiting_duration) - score_command = command_for execution_environment.test_command, file.name_with_extension - output = {file_role: file.role, waiting_for_container_time: waiting_duration} - stdout = +'' - stderr = +'' - begin - exit_code = 1 # default to error - execution_time = runner.attach_to_execution(score_command) do |socket| - socket.on :stderr do |data| - stderr << data - end - socket.on :stdout do |data| - stdout << data - end - socket.on :exit do |received_exit_code| - exit_code = received_exit_code - end - end - output.merge!(container_execution_time: execution_time, status: exit_code.zero? ? :ok : :failed) - rescue Runner::Error::ExecutionTimeout => e - Rails.logger.debug { "Running tests in #{file.name_with_extension} for submission #{id} timed out: #{e.message}" } - output.merge!(status: :timeout, container_execution_time: e.execution_duration) - rescue Runner::Error => e - Rails.logger.debug { "Running tests in #{file.name_with_extension} for submission #{id} failed: #{e.message}" } - output.merge!(status: :failed, container_execution_time: e.execution_duration) - ensure - output.merge!(stdout: stdout, stderr: stderr) - end + test_command = command_for execution_environment.test_command, file.name_with_extension + result = {file_role: file.role, waiting_for_container_time: waiting_duration} + output = runner.execute_command(test_command) + result.merge(output) end private @@ -206,7 +183,7 @@ class Submission < ApplicationRecord def prepared_runner request_time = Time.zone.now begin - runner = Runner.for(user, exercise) + runner = Runner.for(user, exercise.execution_environment) runner.copy_files(collect_files) rescue Runner::Error => e e.waiting_duration = Time.zone.now - request_time diff --git a/spec/controllers/execution_environments_controller_spec.rb b/spec/controllers/execution_environments_controller_spec.rb index eacb0fb5..e01b300d 100644 --- a/spec/controllers/execution_environments_controller_spec.rb +++ b/spec/controllers/execution_environments_controller_spec.rb @@ -75,12 +75,12 @@ describe ExecutionEnvironmentsController do let(:command) { 'which ruby' } before do - allow(DockerClient).to receive(:new).with(execution_environment: execution_environment).and_call_original - allow_any_instance_of(DockerClient).to receive(:execute_arbitrary_command).with(command) + runner = instance_double 'runner' + allow(Runner).to receive(:for).with(user, execution_environment).and_return runner + allow(runner).to receive(:execute_command).and_return({}) post :execute_command, params: {command: command, id: execution_environment.id} end - expect_assigns(docker_client: DockerClient) expect_assigns(execution_environment: :execution_environment) expect_json expect_status(200) diff --git a/spec/models/execution_environment_spec.rb b/spec/models/execution_environment_spec.rb index 6525869d..e4dba06c 100644 --- a/spec/models/execution_environment_spec.rb +++ b/spec/models/execution_environment_spec.rb @@ -149,10 +149,12 @@ describe ExecutionEnvironment do before { allow(DockerClient).to receive(:find_image_by_tag).and_return(Object.new) } - it 'instantiates a Docker client' do - expect(DockerClient).to receive(:new).with(execution_environment: execution_environment).and_call_original - allow_any_instance_of(DockerClient).to receive(:execute_arbitrary_command).and_return({}) + 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 diff --git a/spec/models/runner_spec.rb b/spec/models/runner_spec.rb index bd99f4bb..c5d7bf90 100644 --- a/spec/models/runner_spec.rb +++ b/spec/models/runner_spec.rb @@ -247,7 +247,7 @@ describe Runner do before { allow(strategy_class).to receive(:request_from_management).and_return(nil) } it 'raises an error' do - expect { described_class.for(user, exercise) }.to raise_error(Runner::Error::Unknown, /could not be saved/) + expect { described_class.for(user, exercise.execution_environment) }.to raise_error(Runner::Error::Unknown, /could not be saved/) end end @@ -255,12 +255,12 @@ describe Runner do let!(:existing_runner) { FactoryBot.create(:runner, user: user, execution_environment: exercise.execution_environment) } it 'returns the existing runner' do - new_runner = described_class.for(user, exercise) + new_runner = described_class.for(user, exercise.execution_environment) expect(new_runner).to eq(existing_runner) end it 'sets the strategy' do - runner = described_class.for(user, exercise) + runner = described_class.for(user, exercise.execution_environment) expect(runner.strategy).to be_present end end @@ -269,7 +269,7 @@ describe Runner do before { allow(strategy_class).to receive(:request_from_management).and_return(runner_id) } it 'returns a new runner' do - runner = described_class.for(user, exercise) + runner = described_class.for(user, exercise.execution_environment) expect(runner).to be_valid end end