diff --git a/app/controllers/submissions_controller.rb b/app/controllers/submissions_controller.rb index 55972d54..05e86e4d 100644 --- a/app/controllers/submissions_controller.rb +++ b/app/controllers/submissions_controller.rb @@ -131,7 +131,7 @@ class SubmissionsController < ApplicationController end end - def handle_websockets(tubesock, runner, socket) + def handle_websockets(tubesock, socket) tubesock.send_data JSON.dump({cmd: :status, status: :container_running}) @output = +'' @@ -150,11 +150,10 @@ class SubmissionsController < ApplicationController socket.on :exit do |exit_code| EventMachine.stop_event_loop - status = runner.status if @output.empty? tubesock.send_data JSON.dump({cmd: :write, stream: :stdout, data: "#{t('exercises.implement.no_output', timestamp: l(Time.zone.now, format: :short))}\n"}) end - tubesock.send_data JSON.dump({cmd: :write, stream: :stdout, data: "#{t('exercises.implement.exit', exit_code: exit_code)}\n"}) unless status == :timeouted + tubesock.send_data JSON.dump({cmd: :write, stream: :stdout, data: "#{t('exercises.implement.exit', exit_code: exit_code)}\n"}) kill_socket(tubesock) end @@ -170,6 +169,7 @@ class SubmissionsController < ApplicationController else Rails.logger.info("Unknown command from client: #{event[:cmd]}") end + rescue JSON::ParserError Rails.logger.debug { "Data received from client is not valid json: #{data}" } Sentry.set_extras(data: data) @@ -183,15 +183,16 @@ class SubmissionsController < ApplicationController hijack do |tubesock| return kill_socket(tubesock) if @embed_options[:disable_run] - @container_execution_time = @submission.run(sanitize_filename) do |runner, socket| - @waiting_for_container_time = runner.waiting_time - handle_websockets(tubesock, runner, socket) + durations = @submission.run(sanitize_filename) do |socket| + handle_websockets(tubesock, socket) end + @container_execution_time = durations[:execution_duration] + @waiting_for_container_time = durations[:waiting_duration] save_run_output rescue Runner::Error::ExecutionTimeout => e tubesock.send_data JSON.dump({cmd: :status, status: :timeout}) kill_socket(tubesock) - Rails.logger.debug { "Running a submission failed: #{e.message}" } + Rails.logger.debug { "Running a submission timed out: #{e.message}" } rescue Runner::Error => e tubesock.send_data JSON.dump({cmd: :status, status: :container_depleted}) kill_socket(tubesock) @@ -244,7 +245,7 @@ class SubmissionsController < ApplicationController # send_hints(tubesock, StructuredError.where(submission: @submission)) rescue Runner::Error::ExecutionTimeout => e tubesock.send_data JSON.dump({cmd: :status, status: :timeout}) - Rails.logger.debug { "Running a submission failed: #{e.message}" } + Rails.logger.debug { "Scoring a submission timed out: #{e.message}" } rescue Runner::Error => e tubesock.send_data JSON.dump({cmd: :status, status: :container_depleted}) Rails.logger.debug { "Runner error while scoring a submission: #{e.message}" } diff --git a/app/models/runner.rb b/app/models/runner.rb index f8861a2a..707563bc 100644 --- a/app/models/runner.rb +++ b/app/models/runner.rb @@ -1,138 +1,57 @@ # frozen_string_literal: true +require 'forwardable' + class Runner < ApplicationRecord - BASE_URL = CodeOcean::Config.new(:code_ocean).read[:runner_management][:url] - HEADERS = {'Content-Type' => 'application/json'}.freeze - UNUSED_EXPIRATION_TIME = CodeOcean::Config.new(:code_ocean).read[:runner_management][:unused_runner_expiration_time].seconds - ERRORS = %w[NOMAD_UNREACHABLE NOMAD_OVERLOAD NOMAD_INTERNAL_SERVER_ERROR UNKNOWN].freeze - - ERRORS.each do |error| - define_singleton_method :"error_#{error.downcase}" do - error - end - end - belongs_to :execution_environment belongs_to :user, polymorphic: true - before_validation :request_remotely + before_validation :request_id validates :execution_environment, :user, :runner_id, presence: true + STRATEGY_NAME = CodeOcean::Config.new(:code_ocean).read[:runner_management][:strategy] + UNUSED_EXPIRATION_TIME = CodeOcean::Config.new(:code_ocean).read[:runner_management][:unused_runner_expiration_time].seconds + BASE_URL = CodeOcean::Config.new(:code_ocean).read[:runner_management][:url] + DELEGATED_STRATEGY_METHODS = %i[destroy_at_management attach_to_execution copy_files].freeze + + attr_accessor :strategy + + def self.strategy_class + "runner/strategy/#{STRATEGY_NAME}".camelize.constantize + end + def self.for(user, exercise) execution_environment = ExecutionEnvironment.find(exercise.execution_environment_id) - runner = find_or_create_by(user: user, execution_environment: execution_environment) - unless runner.persisted? - # runner was not saved in the database (was not valid) - raise Runner::Error::InternalServerError.new("Provided runner could not be saved: #{runner.errors.inspect}") + runner = find_by(user: user, execution_environment: execution_environment) + if runner.nil? + runner = Runner.create(user: user, execution_environment: execution_environment) + raise Runner::Error::Unknown.new("Runner could not be saved: #{runner.errors.inspect}") unless runner.persisted? + else + runner.strategy = strategy_class.new(runner.runner_id, runner.execution_environment) end runner end - def copy_files(files) - url = "#{runner_url}/files" - body = {copy: files.map {|filename, content| {path: filename, content: Base64.strict_encode64(content)} }} - response = Faraday.patch(url, body.to_json, HEADERS) - handle_error response unless response.status == 204 - end - - def execute_command(command) - url = "#{runner_url}/execute" - body = {command: command, timeLimit: execution_environment.permitted_execution_time} - response = Faraday.post(url, body.to_json, HEADERS) - if response.status == 200 - response_body = parse response - websocket_url = response_body[:websocketUrl] - if websocket_url.present? - return websocket_url - else - raise Runner::Error::Unknown.new('Runner management sent unexpected response') - end - end - - handle_error response - end - - def execute_interactively(command) - starting_time = Time.zone.now - websocket_url = execute_command(command) - EventMachine.run do - socket = Runner::Connection.new(websocket_url) - yield(self, socket) if block_given? - end - Time.zone.now - starting_time # execution time - end - - # This method is currently not used. - # This does *not* destroy the ActiveRecord model. - def destroy_remotely - response = Faraday.delete runner_url - return if response.status == 204 - - if response.status == 404 - raise Runner::Error::NotFound.new('Runner not found') - else - handle_error response + DELEGATED_STRATEGY_METHODS.each do |method| + define_method(method) do |*args, &block| + @strategy.send(method, *args, &block) + rescue Runner::Error::NotFound + update(runner_id: self.class.strategy_class.request_from_management(execution_environment)) + @strategy = self.class.strategy_class.new(runner_id, execution_environment) + @strategy.send(method, *args, &block) end end private - def request_remotely + def request_id return if runner_id.present? - url = "#{BASE_URL}/runners" - body = {executionEnvironmentId: execution_environment.id, inactivityTimeout: UNUSED_EXPIRATION_TIME} - response = Faraday.post(url, body.to_json, HEADERS) - - case response.status - when 200 - response_body = parse response - runner_id = response_body[:runnerId] - throw(:abort) if runner_id.blank? - self.runner_id = response_body[:runnerId] - when 404 - raise Runner::Error::NotFound.new('Execution environment not found') - else - handle_error response - end - end - - def handle_error(response) - case response.status - when 400 - response_body = parse response - raise Runner::Error::BadRequest.new(response_body[:message]) - when 401 - raise Runner::Error::Unauthorized.new('Authentication with runner management failed') - when 404 - # The runner does not exist in the runner management (e.g. due to an inactivity timeout). - # Delete the runner model in this case as it can not be used anymore. - destroy - raise Runner::Error::NotFound.new('Runner not found') - when 500 - response_body = parse response - error_code = response_body[:errorCode] - if error_code == Runner.error_nomad_overload - raise Runner::Error::NotAvailable.new("No runner available (#{error_code}): #{response_body[:message]}") - else - raise Runner::Error::InternalServerError.new("#{response_body[:errorCode]}: #{response_body[:message]}") - end - else - raise Runner::Error::Unknown.new('Runner management sent unexpected response') - end - end - - def runner_url - "#{BASE_URL}/runners/#{runner_id}" - end - - def parse(response) - JSON.parse(response.body).deep_symbolize_keys - rescue JSON::ParserError => e - # the runner management should not send invalid json - raise Runner::Error::Unknown.new("Error parsing response from runner management: #{e.message}") + strategy_class = self.class.strategy_class + self.runner_id = strategy_class.request_from_management(execution_environment) + @strategy = strategy_class.new(runner_id, execution_environment) end end diff --git a/app/models/submission.rb b/app/models/submission.rb index 249fdc3f..c2fd3121 100644 --- a/app/models/submission.rb +++ b/app/models/submission.rb @@ -141,13 +141,13 @@ class Submission < ApplicationRecord def calculate_score score = nil - prepared_runner do |runner| + prepared_runner do |runner, waiting_duration| scores = collect_files.select(&:teacher_defined_assessment?).map do |file| score_command = command_for execution_environment.test_command, file.name_with_extension - stdout = '' - stderr = '' + stdout = +'' + stderr = +'' exit_code = 1 # default to error - execution_time = runner.execute_interactively(score_command) do |_runner, socket| + execution_time = runner.attach_to_execution(score_command) do |socket| socket.on :stderr do |data| stderr << data end @@ -161,7 +161,7 @@ class Submission < ApplicationRecord end output = { file_role: file.role, - waiting_for_container_time: runner.waiting_time, + waiting_for_container_time: waiting_duration, container_execution_time: execution_time, status: exit_code.zero? ? :ok : :failed, stdout: stdout, @@ -176,11 +176,12 @@ class Submission < ApplicationRecord def run(file, &block) run_command = command_for execution_environment.run_command, file - execution_time = 0 - prepared_runner do |runner| - execution_time = runner.execute_interactively(run_command, &block) + durations = {} + prepared_runner do |runner, waiting_duration| + durations[:execution_duration] = runner.attach_to_execution(run_command, &block) + durations[:waiting_duration] = waiting_duration end - execution_time + durations end private @@ -197,8 +198,8 @@ class Submission < ApplicationRecord request_time = Time.zone.now runner = Runner.for(user, exercise) copy_files_to runner - runner.waiting_time = Time.zone.now - request_time - yield(runner) if block_given? + waiting_duration = Time.zone.now - request_time + yield(runner, waiting_duration) if block_given? end def command_for(template, file) diff --git a/config/code_ocean.yml.ci b/config/code_ocean.yml.ci index 6865a478..8b8d2a64 100644 --- a/config/code_ocean.yml.ci +++ b/config/code_ocean.yml.ci @@ -10,5 +10,6 @@ test: prometheus_exporter: enabled: false runner_management: + strategy: poseidon url: https://runners.example.org unused_runner_expiration_time: 180 diff --git a/config/code_ocean.yml.example b/config/code_ocean.yml.example index f948ac6a..6b694123 100644 --- a/config/code_ocean.yml.example +++ b/config/code_ocean.yml.example @@ -12,6 +12,7 @@ default: &default prometheus_exporter: enabled: false runner_management: + strategy: poseidon url: https://runners.example.org unused_runner_expiration_time: 180 diff --git a/lib/runner/strategy.rb b/lib/runner/strategy.rb new file mode 100644 index 00000000..dce76adf --- /dev/null +++ b/lib/runner/strategy.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +class Runner::Strategy + def initialize(runner_id, environment) + @runner_id = runner_id + @execution_environment = environment + end + + def self.request_from_management + raise NotImplementedError + end + + def destroy_at_management + raise NotImplementedError + end + + def copy_files(_files) + raise NotImplementedError + end + + def attach_to_execution(_command) + raise NotImplementedError + end +end diff --git a/lib/runner/strategy/docker.rb b/lib/runner/strategy/docker.rb new file mode 100644 index 00000000..fd7cfafc --- /dev/null +++ b/lib/runner/strategy/docker.rb @@ -0,0 +1,3 @@ +# frozen_string_literal: true + +class Runner::Strategy::Docker < Runner::Strategy; end diff --git a/lib/runner/strategy/poseidon.rb b/lib/runner/strategy/poseidon.rb new file mode 100644 index 00000000..6a642bb8 --- /dev/null +++ b/lib/runner/strategy/poseidon.rb @@ -0,0 +1,103 @@ +# frozen_string_literal: true + +class Runner::Strategy::Poseidon < Runner::Strategy + HEADERS = {'Content-Type' => 'application/json'}.freeze + ERRORS = %w[NOMAD_UNREACHABLE NOMAD_OVERLOAD NOMAD_INTERNAL_SERVER_ERROR UNKNOWN].freeze + + ERRORS.each do |error| + define_singleton_method :"error_#{error.downcase}" do + error + end + end + + def self.request_from_management(environment) + url = "#{Runner::BASE_URL}/runners" + body = {executionEnvironmentId: environment.id, inactivityTimeout: Runner::UNUSED_EXPIRATION_TIME} + response = Faraday.post(url, body.to_json, HEADERS) + + case response.status + when 200 + response_body = parse response + runner_id = response_body[:runnerId] + runner_id.presence || raise(Runner::Error::Unknown.new('Poseidon did not send a runner id')) + when 404 + raise Runner::Error::NotFound.new('Execution environment not found') + else + handle_error response + end + end + + def self.handle_error(response) + case response.status + when 400 + response_body = parse response + raise Runner::Error::BadRequest.new(response_body[:message]) + when 401 + raise Runner::Error::Unauthorized.new('Authentication with Poseidon failed') + when 404 + raise Runner::Error::NotFound.new('Runner not found') + when 500 + response_body = parse response + error_code = response_body[:errorCode] + if error_code == error_nomad_overload + raise Runner::Error::NotAvailable.new("Poseidon has no runner available (#{error_code}): #{response_body[:message]}") + else + raise Runner::Error::InternalServerError.new("Poseidon sent #{response_body[:errorCode]}: #{response_body[:message]}") + end + else + raise Runner::Error::Unknown.new("Poseidon sent unexpected response status code #{response.status}") + end + end + + def self.parse(response) + JSON.parse(response.body).deep_symbolize_keys + rescue JSON::ParserError => e + # Poseidon should not send invalid json + raise Runner::Error::Unknown.new("Error parsing response from Poseidon: #{e.message}") + end + + def copy_files(files) + url = "#{runner_url}/files" + body = {copy: files.map {|filename, content| {path: filename, content: Base64.strict_encode64(content)} }} + response = Faraday.patch(url, body.to_json, HEADERS) + self.class.handle_error response unless response.status == 204 + end + + def attach_to_execution(command) + starting_time = Time.zone.now + websocket_url = execute_command(command) + EventMachine.run do + socket = Runner::Connection.new(websocket_url) + yield(socket) if block_given? + end + Time.zone.now - starting_time # execution duration + end + + def destroy_at_management + response = Faraday.delete runner_url + self.class.handle_error response unless response.status == 204 + end + + private + + def execute_command(command) + url = "#{runner_url}/execute" + body = {command: command, timeLimit: @execution_environment.permitted_execution_time} + response = Faraday.post(url, body.to_json, HEADERS) + if response.status == 200 + response_body = self.class.parse response + websocket_url = response_body[:websocketUrl] + if websocket_url.present? + return websocket_url + else + raise Runner::Error::Unknown.new('Poseidon did not send websocket url') + end + end + + self.class.handle_error response + end + + def runner_url + "#{Runner::BASE_URL}/runners/#{@runner_id}" + end +end diff --git a/spec/concerns/submission_scoring_spec.rb b/spec/concerns/submission_scoring_spec.rb index 74d97ab8..8eb1506f 100644 --- a/spec/concerns/submission_scoring_spec.rb +++ b/spec/concerns/submission_scoring_spec.rb @@ -11,7 +11,7 @@ describe SubmissionScoring do before do allow(Runner).to receive(:for).and_return(runner) allow(runner).to receive(:copy_files) - allow(runner).to receive(:execute_interactively).and_return(1.0) + allow(runner).to receive(:attach_to_execution).and_return(1.0) end after { submission.calculate_score } diff --git a/spec/factories/runner.rb b/spec/factories/runner.rb index 809d134e..636a8055 100644 --- a/spec/factories/runner.rb +++ b/spec/factories/runner.rb @@ -3,7 +3,7 @@ # This factory does not request the runner management as the id is already provided. FactoryBot.define do factory :runner do - runner_id { 'test-runner-id' } + sequence(:runner_id) {|n| "test-runner-id-#{n}" } association :execution_environment, factory: :ruby association :user, factory: :external_user waiting_time { 1.0 } diff --git a/spec/lib/runner/strategy/docker_spec.rb b/spec/lib/runner/strategy/docker_spec.rb new file mode 100644 index 00000000..4110ebf9 --- /dev/null +++ b/spec/lib/runner/strategy/docker_spec.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe Runner::Strategy::Docker do + let(:runner_id) { FactoryBot.attributes_for(:runner)[:runner_id] } + let(:execution_environment) { FactoryBot.create :ruby } + let(:docker) { described_class.new(runner_id, execution_environment) } + + # TODO: add tests for these methods when implemented + it 'defines all methods all runner management strategies must define' do + expect(docker.public_methods).to include(*Runner::DELEGATED_STRATEGY_METHODS) + expect(described_class.public_methods).to include(:request_from_management) + end +end diff --git a/spec/lib/runner/strategy/poseidon_spec.rb b/spec/lib/runner/strategy/poseidon_spec.rb new file mode 100644 index 00000000..e8370351 --- /dev/null +++ b/spec/lib/runner/strategy/poseidon_spec.rb @@ -0,0 +1,273 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe Runner::Strategy::Poseidon do + let(:runner_id) { FactoryBot.attributes_for(:runner)[:runner_id] } + let(:execution_environment) { FactoryBot.create :ruby } + let(:poseidon) { described_class.new(runner_id, execution_environment) } + let(:error_message) { 'test error message' } + let(:response_body) { nil } + + # All requests handle a BadRequest (400) response the same way. + shared_examples 'BadRequest (400) error handling' do + context 'when Poseidon returns BadRequest (400)' do + let(:response_body) { {message: error_message}.to_json } + let(:response_status) { 400 } + + it 'raises an error' do + expect { action.call }.to raise_error(Runner::Error::BadRequest, /#{error_message}/) + end + end + end + + # All requests handle a Unauthorized (401) response the same way. + shared_examples 'Unauthorized (401) error handling' do + context 'when Poseidon returns Unauthorized (401)' do + let(:response_status) { 401 } + + it 'raises an error' do + expect { action.call }.to raise_error(Runner::Error::Unauthorized) + end + end + end + + # All requests except creation handle a NotFound (404) response the same way. + shared_examples 'NotFound (404) error handling' do + context 'when Poseidon returns NotFound (404)' do + let(:response_status) { 404 } + + it 'raises an error' do + expect { action.call }.to raise_error(Runner::Error::NotFound, /Runner/) + end + end + end + + # All requests handle an InternalServerError (500) response the same way. + shared_examples 'InternalServerError (500) error handling' do + context 'when Poseidon returns InternalServerError (500)' do + shared_examples 'InternalServerError (500) with error code' do |error_code, error_class| + let(:response_status) { 500 } + let(:response_body) { {message: error_message, errorCode: error_code}.to_json } + + it 'raises an error' do + expect { action.call }.to raise_error(error_class) do |error| + expect(error.message).to match(/#{error_message}/) + expect(error.message).to match(/#{error_code}/) + end + end + end + + context 'when error code is nomad overload' do + include_examples( + 'InternalServerError (500) with error code', + described_class.error_nomad_overload, Runner::Error::NotAvailable + ) + end + + context 'when error code is not nomad overload' do + include_examples( + 'InternalServerError (500) with error code', + described_class.error_unknown, Runner::Error::InternalServerError + ) + end + end + end + + # All requests handle an unknown response status the same way. + shared_examples 'unknown response status error handling' do + context 'when Poseidon returns an unknown response status' do + let(:response_status) { 1337 } + + it 'raises an error' do + expect { action.call }.to raise_error(Runner::Error::Unknown, /#{response_status}/) + end + end + end + + describe '::request_from_management' do + let(:action) { -> { described_class.request_from_management(execution_environment) } } + let!(:request_runner_stub) do + WebMock + .stub_request(:post, "#{Runner::BASE_URL}/runners") + .with( + body: {executionEnvironmentId: execution_environment.id, inactivityTimeout: Runner::UNUSED_EXPIRATION_TIME}, + headers: {'Content-Type' => 'application/json'} + ) + .to_return(body: response_body, status: response_status) + end + + context 'when Poseidon returns Ok (200) with an id' do + let(:response_body) { {runnerId: runner_id}.to_json } + let(:response_status) { 200 } + + it 'successfully requests Poseidon' do + action.call + expect(request_runner_stub).to have_been_requested.once + end + + it 'returns the received runner id' do + id = action.call + expect(id).to eq(runner_id) + end + end + + context 'when Poseidon returns Ok (200) without an id' do + let(:response_body) { {}.to_json } + let(:response_status) { 200 } + + it 'raises an error' do + expect { action.call }.to raise_error(Runner::Error::Unknown) + end + end + + context 'when Poseidon returns Ok (200) with invalid JSON' do + let(:response_body) { '{hello}' } + let(:response_status) { 200 } + + it 'raises an error' do + expect { action.call }.to raise_error(Runner::Error::Unknown) + end + end + + include_examples 'BadRequest (400) error handling' + include_examples 'Unauthorized (401) error handling' + + context 'when Poseidon returns NotFound (404)' do + let(:response_status) { 404 } + + it 'raises an error' do + expect { action.call }.to raise_error(Runner::Error::NotFound, /Execution environment/) + end + end + + include_examples 'InternalServerError (500) error handling' + include_examples 'unknown response status error handling' + end + + describe '#execute_command' do + let(:command) { 'ls' } + let(:action) { -> { poseidon.send(:execute_command, command) } } + let(:websocket_url) { 'ws://ws.example.com/path/to/websocket' } + let!(:execute_command_stub) do + WebMock + .stub_request(:post, "#{Runner::BASE_URL}/runners/#{runner_id}/execute") + .with( + body: {command: command, timeLimit: execution_environment.permitted_execution_time}, + headers: {'Content-Type' => 'application/json'} + ) + .to_return(body: response_body, status: response_status) + end + + context 'when Poseidon returns Ok (200) with a websocket url' do + let(:response_status) { 200 } + let(:response_body) { {websocketUrl: websocket_url}.to_json } + + it 'schedules an execution in Poseidon' do + action.call + expect(execute_command_stub).to have_been_requested.once + end + + it 'returns the url' do + url = action.call + expect(url).to eq(websocket_url) + end + end + + context 'when Poseidon returns Ok (200) without a websocket url' do + let(:response_body) { {}.to_json } + let(:response_status) { 200 } + + it 'raises an error' do + expect { action.call }.to raise_error(Runner::Error::Unknown) + end + end + + context 'when Poseidon returns Ok (200) with invalid JSON' do + let(:response_body) { '{hello}' } + let(:response_status) { 200 } + + it 'raises an error' do + expect { action.call }.to raise_error(Runner::Error::Unknown) + end + end + + include_examples 'BadRequest (400) error handling' + include_examples 'Unauthorized (401) error handling' + include_examples 'NotFound (404) error handling' + include_examples 'InternalServerError (500) error handling' + include_examples 'unknown response status error handling' + end + + describe '#destroy_at_management' do + let(:action) { -> { poseidon.destroy_at_management } } + let!(:destroy_stub) do + WebMock + .stub_request(:delete, "#{Runner::BASE_URL}/runners/#{runner_id}") + .to_return(body: response_body, status: response_status) + end + + context 'when Poseidon returns NoContent (204)' do + let(:response_status) { 204 } + + it 'deletes the runner from Poseidon' do + action.call + expect(destroy_stub).to have_been_requested.once + end + end + + include_examples 'Unauthorized (401) error handling' + include_examples 'NotFound (404) error handling' + include_examples 'InternalServerError (500) error handling' + include_examples 'unknown response status error handling' + end + + describe '#copy_files' do + let(:filename) { 'main.py' } + let(:file_content) { 'print("Hello World!")' } + let(:action) { -> { poseidon.copy_files({filename => file_content}) } } + let(:encoded_file_content) { Base64.strict_encode64(file_content) } + let!(:copy_files_stub) do + WebMock + .stub_request(:patch, "#{Runner::BASE_URL}/runners/#{runner_id}/files") + .with( + body: {copy: [{path: filename, content: encoded_file_content}]}, + headers: {'Content-Type' => 'application/json'} + ) + .to_return(body: response_body, status: response_status) + end + + context 'when Poseidon returns NoContent (204)' do + let(:response_status) { 204 } + + it 'sends the files to Poseidon' do + action.call + expect(copy_files_stub).to have_been_requested.once + end + end + + include_examples 'BadRequest (400) error handling' + include_examples 'Unauthorized (401) error handling' + include_examples 'NotFound (404) error handling' + include_examples 'InternalServerError (500) error handling' + include_examples 'unknown response status error handling' + end + + describe '#attach_to_execution' do + # TODO: add more tests here + + let(:command) { 'ls' } + let(:action) { -> { poseidon.attach_to_execution command } } + let(:websocket_url) { 'ws://ws.example.com/path/to/websocket' } + + it 'returns the execution time' do + allow(poseidon).to receive(:execute_command).with(command).and_return(websocket_url) + allow(EventMachine).to receive(:run) + + starting_time = Time.zone.now + execution_time = action.call + test_time = Time.zone.now - starting_time + expect(execution_time).to be_between(0.0, test_time) + end + end +end diff --git a/spec/models/runner_spec.rb b/spec/models/runner_spec.rb index 2c5c4b84..4c3043c0 100644 --- a/spec/models/runner_spec.rb +++ b/spec/models/runner_spec.rb @@ -3,92 +3,17 @@ require 'rails_helper' describe Runner do - let(:runner) { FactoryBot.create :runner } - let(:runner_id) { runner.runner_id } - let(:error_message) { 'test error message' } - let(:response_body) { nil } - let(:user) { FactoryBot.build :external_user } - let(:execution_environment) { FactoryBot.create :ruby } - - # All requests handle a BadRequest (400) response the same way. - shared_examples 'BadRequest (400) error handling' do - let(:response_body) { {message: error_message}.to_json } - let(:response_status) { 400 } - - it 'raises an error' do - expect { action.call }.to raise_error(Runner::Error::BadRequest, /#{error_message}/) - end - end - - # All requests handle a Unauthorized (401) response the same way. - shared_examples 'Unauthorized (401) error handling' do - let(:response_status) { 401 } - - it 'raises an error' do - expect { action.call }.to raise_error(Runner::Error::Unauthorized) - end - end - - # All requests except creation and destruction handle a NotFound (404) response the same way. - shared_examples 'NotFound (404) error handling' do - let(:response_status) { 404 } - - it 'raises an error' do - expect { action.call }.to raise_error(Runner::Error::NotFound, /Runner/) - end - - it 'destroys the runner locally' do - expect { action.call }.to change(described_class, :count).by(-1) - .and raise_error(Runner::Error::NotFound) - end - end - - # All requests handle an InternalServerError (500) response the same way. - shared_examples 'InternalServerError (500) error handling' do - shared_examples 'InternalServerError (500) with error code' do |error_code, error_class| - let(:response_status) { 500 } - let(:response_body) { {message: error_message, errorCode: error_code}.to_json } - - it 'raises an error' do - expect { action.call }.to raise_error(error_class) do |error| - expect(error.message).to match(/#{error_message}/) - expect(error.message).to match(/#{error_code}/) - end - end - end - - context 'when error code is nomad overload' do - include_examples( - 'InternalServerError (500) with error code', - described_class.error_nomad_overload, Runner::Error::NotAvailable - ) - end - - context 'when error code is not nomad overload' do - include_examples( - 'InternalServerError (500) with error code', - described_class.error_unknown, Runner::Error::InternalServerError - ) - end - end - - # All requests handle an unknown response status the same way. - shared_examples 'unknown response status error handling' do - let(:response_status) { 1337 } - - it 'raises an error' do - expect { action.call }.to raise_error(Runner::Error::Unknown) - end - end + let(:runner_id) { FactoryBot.attributes_for(:runner)[:runner_id] } + let(:strategy_class) { described_class.strategy_class } describe 'attribute validation' do let(:runner) { FactoryBot.create :runner } it 'validates the presence of the runner id' do - described_class.skip_callback(:validation, :before, :request_remotely) + described_class.skip_callback(:validation, :before, :request_id) runner.update(runner_id: nil) expect(runner.errors[:runner_id]).to be_present - described_class.set_callback(:validation, :before, :request_remotely) + described_class.set_callback(:validation, :before, :request_id) end it 'validates the presence of an execution environment' do @@ -102,256 +27,98 @@ describe Runner do end end - describe 'creation' do - let(:action) { -> { described_class.create(user: user, execution_environment: execution_environment) } } - let!(:create_stub) do - WebMock - .stub_request(:post, "#{Runner::BASE_URL}/runners") - .with( - body: {executionEnvironmentId: execution_environment.id, inactivityTimeout: Runner::UNUSED_EXPIRATION_TIME}, - headers: {'Content-Type' => 'application/json'} - ) - .to_return(body: response_body, status: response_status) - end - - context 'when a runner is created' do - let(:response_body) { {runnerId: runner_id}.to_json } - let(:response_status) { 200 } - - it 'requests a runner from the runner management' do - action.call - expect(create_stub).to have_been_requested.once - end - - it 'does not call the runner management again when updating' do - runner = action.call - runner.runner_id = 'another_id' - successfully_saved = runner.save - expect(successfully_saved).to be_truthy - expect(create_stub).to have_been_requested.once + describe '::strategy_class' do + shared_examples 'uses the strategy defined in the constant' do |strategy, strategy_class| + it "uses #{strategy_class} as strategy class for constant #{strategy}" do + stub_const('Runner::STRATEGY_NAME', strategy) + expect(described_class.strategy_class).to eq(strategy_class) end end - context 'when the runner management returns Ok (200) with an id' do - let(:response_body) { {runnerId: runner_id}.to_json } - let(:response_status) { 200 } + {poseidon: Runner::Strategy::Poseidon, docker: Runner::Strategy::Docker}.each do |strategy, strategy_class| + include_examples 'uses the strategy defined in the constant', strategy, strategy_class + end - it 'sets the runner id according to the response' do - runner = action.call - expect(runner.runner_id).to eq(runner_id) - expect(runner).to be_persisted + shared_examples 'delegates method sends to its strategy' do |method, *args| + context "when sending #{method}" do + let(:strategy) { instance_double(strategy_class) } + let(:runner) { described_class.create } + + before do + allow(strategy_class).to receive(:request_from_management).and_return(runner_id) + allow(strategy_class).to receive(:new).and_return(strategy) + end + + it "delegates the method #{method}" do + expect(strategy).to receive(method) + runner.send(method, *args) + end end end - context 'when the runner management returns Ok (200) without an id' do - let(:response_body) { {}.to_json } - let(:response_status) { 200 } + include_examples 'delegates method sends to its strategy', :destroy_at_management + include_examples 'delegates method sends to its strategy', :copy_files, nil + include_examples 'delegates method sends to its strategy', :attach_to_execution, nil + end - it 'does not save the runner' do - runner = action.call - expect(runner).not_to be_persisted - end + describe '#request_id' do + it 'requests a runner from the runner management when created' do + expect(strategy_class).to receive(:request_from_management) + described_class.create end - context 'when the runner management returns Ok (200) with invalid JSON' do - let(:response_body) { '{hello}' } - let(:response_status) { 200 } - - it 'raises an error' do - expect { action.call }.to raise_error(Runner::Error::Unknown) - end + it 'sets the runner id when created' do + allow(strategy_class).to receive(:request_from_management).and_return(runner_id) + runner = described_class.create + expect(runner.runner_id).to eq(runner_id) end - context 'when the runner management returns BadRequest (400)' do - include_examples 'BadRequest (400) error handling' + it 'sets the strategy when created' do + allow(strategy_class).to receive(:request_from_management).and_return(runner_id) + runner = described_class.create + expect(runner.strategy).to be_present end - context 'when the runner management returns Unauthorized (401)' do - include_examples 'Unauthorized (401) error handling' - end - - context 'when the runner management returns NotFound (404)' do - let(:response_status) { 404 } - - it 'raises an error' do - expect { action.call }.to raise_error(Runner::Error::NotFound, /Execution environment/) - end - end - - context 'when the runner management returns InternalServerError (500)' do - include_examples 'InternalServerError (500) error handling' - end - - context 'when the runner management returns an unknown response status' do - include_examples 'unknown response status error handling' + it 'does not call the runner management again when validating the model' do + expect(strategy_class).to receive(:request_from_management).and_return(runner_id).once + runner = described_class.create + runner.valid? end end - describe 'execute command' do - let(:command) { 'ls' } - let(:action) { -> { runner.execute_command(command) } } - let(:websocket_url) { 'ws://ws.example.com/path/to/websocket' } - let!(:execute_command_stub) do - WebMock - .stub_request(:post, "#{Runner::BASE_URL}/runners/#{runner_id}/execute") - .with( - body: {command: command, timeLimit: execution_environment.permitted_execution_time}, - headers: {'Content-Type' => 'application/json'} - ) - .to_return(body: response_body, status: response_status) - end + describe '::for' do + let(:user) { FactoryBot.create :external_user } + let(:exercise) { FactoryBot.create :fibonacci } - context 'when #execute_command is called' do - let(:response_status) { 200 } - let(:response_body) { {websocketUrl: websocket_url}.to_json } - - it 'schedules an execution in the runner management' do - action.call - expect(execute_command_stub).to have_been_requested.once - end - end - - context 'when the runner management returns Ok (200) with a websocket url' do - let(:response_status) { 200 } - let(:response_body) { {websocketUrl: websocket_url}.to_json } - - it 'returns the url' do - url = action.call - expect(url).to eq(websocket_url) - end - end - - context 'when the runner management returns Ok (200) without a websocket url' do - let(:response_body) { {}.to_json } - let(:response_status) { 200 } + context 'when the runner could not be saved' do + before { allow(strategy_class).to receive(:request_from_management).and_return(nil) } it 'raises an error' do - expect { action.call }.to raise_error(Runner::Error::Unknown) + expect { described_class.for(user, exercise) }.to raise_error(Runner::Error::Unknown, /could not be saved/) end end - context 'when the runner management returns Ok (200) with invalid JSON' do - let(:response_body) { '{hello}' } - let(:response_status) { 200 } + context 'when a runner already exists' do + let!(:existing_runner) { FactoryBot.create(:runner, user: user, execution_environment: exercise.execution_environment) } - it 'raises an error' do - expect { action.call }.to raise_error(Runner::Error::Unknown) + it 'returns the existing runner' do + new_runner = described_class.for(user, exercise) + expect(new_runner).to eq(existing_runner) + end + + it 'sets the strategy' do + runner = described_class.for(user, exercise) + expect(runner.strategy).to be_present end end - context 'when the runner management returns BadRequest (400)' do - include_examples 'BadRequest (400) error handling' - end + context 'when no runner exists' do + before { allow(strategy_class).to receive(:request_from_management).and_return(runner_id) } - context 'when the runner management returns Unauthorized (401)' do - include_examples 'Unauthorized (401) error handling' - end - - context 'when the runner management returns NotFound (404)' do - include_examples 'NotFound (404) error handling' - end - - context 'when the runner management returns InternalServerError (500)' do - include_examples 'InternalServerError (500) error handling' - end - - context 'when the runner management returns an unknown response status' do - include_examples 'unknown response status error handling' - end - end - - describe 'destruction' do - let(:action) { -> { runner.destroy_remotely } } - let(:response_status) { 204 } - let!(:destroy_stub) do - WebMock - .stub_request(:delete, "#{Runner::BASE_URL}/runners/#{runner_id}") - .to_return(body: response_body, status: response_status) - end - - it 'deletes the runner from the runner management' do - action.call - expect(destroy_stub).to have_been_requested.once - end - - it 'does not destroy the runner locally' do - expect { action.call }.not_to change(described_class, :count) - end - - context 'when the runner management returns NoContent (204)' do - it 'does not raise an error' do - expect { action.call }.not_to raise_error + it 'returns a new runner' do + runner = described_class.for(user, exercise) + expect(runner).to be_valid end end - - context 'when the runner management returns Unauthorized (401)' do - include_examples 'Unauthorized (401) error handling' - end - - context 'when the runner management returns NotFound (404)' do - let(:response_status) { 404 } - - it 'raises an exception' do - expect { action.call }.to raise_error(Runner::Error::NotFound, /Runner/) - end - end - - context 'when the runner management returns InternalServerError (500)' do - include_examples 'InternalServerError (500) error handling' - end - - context 'when the runner management returns an unknown response status' do - include_examples 'unknown response status error handling' - end - end - - describe 'copy files' do - let(:filename) { 'main.py' } - let(:file_content) { 'print("Hello World!")' } - let(:action) { -> { runner.copy_files({filename => file_content}) } } - let(:encoded_file_content) { Base64.strict_encode64(file_content) } - let(:response_status) { 204 } - let!(:copy_files_stub) do - WebMock - .stub_request(:patch, "#{Runner::BASE_URL}/runners/#{runner_id}/files") - .with( - body: {copy: [{path: filename, content: encoded_file_content}]}, - headers: {'Content-Type' => 'application/json'} - ) - .to_return(body: response_body, status: response_status) - end - - it 'sends the files to the runner management' do - action.call - expect(copy_files_stub).to have_been_requested.once - end - - context 'when the runner management returns NoContent (204)' do - let(:response_status) { 204 } - - it 'does not raise an error' do - expect { action.call }.not_to raise_error - end - end - - context 'when the runner management returns BadRequest (400)' do - include_examples 'BadRequest (400) error handling' - end - - context 'when the runner management returns Unauthorized (401)' do - include_examples 'Unauthorized (401) error handling' - end - - context 'when the runner management returns NotFound (404)' do - include_examples 'NotFound (404) error handling' - end - - context 'when the runner management returns InternalServerError (500)' do - include_examples 'InternalServerError (500) error handling' - end - - context 'when the runner management returns an unknown response status' do - include_examples 'unknown response status error handling' - end end end