Add execute_command method to runner.rb

* This is now used by the score and test runs
* This also re-enables the interactive shell for execution environments

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)
+    runner = Runner.for(current_user, @execution_environment)
-    render(json: @docker_client.execute_arbitrary_command(params[:command]))
+    output = runner.execute_command(params[:command])
+    render(json: output)
   end
 
   def working_time_query

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)
+  def self.for(user, execution_environment)
-    execution_environment = exercise.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

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
+    test_command = command_for execution_environment.test_command, file.name_with_extension
-    output = {file_role: file.role, waiting_for_container_time: waiting_duration}
+    result = {file_role: file.role, waiting_for_container_time: waiting_duration}
-    stdout = +''
+    output = runner.execute_command(test_command)
-    stderr = +''
+    result.merge(output)
-    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
   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

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
+      runner = instance_double 'runner'
-      allow_any_instance_of(DockerClient).to receive(:execute_arbitrary_command).with(command)
+      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)

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
+    it 'instantiates a Runner' do
-      expect(DockerClient).to receive(:new).with(execution_environment: execution_environment).and_call_original
+      runner = instance_double 'runner'
-      allow_any_instance_of(DockerClient).to receive(:execute_arbitrary_command).and_return({})
+      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

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