diff --git a/app/controllers/execution_environments_controller.rb b/app/controllers/execution_environments_controller.rb index e9a0ba0c..648f7140 100644 --- a/app/controllers/execution_environments_controller.rb +++ b/app/controllers/execution_environments_controller.rb @@ -3,8 +3,6 @@ class ExecutionEnvironmentsController < ApplicationController include CommonBehavior - RUNNER_MANAGEMENT_PRESENT = CodeOcean::Config.new(:code_ocean).read[:runner_management].present? - before_action :set_docker_images, only: %i[create edit new update] before_action :set_execution_environment, only: MEMBER_ACTIONS + %i[execute_command shell statistics] before_action :set_testing_framework_adapters, only: %i[create edit new update] @@ -18,7 +16,7 @@ class ExecutionEnvironmentsController < ApplicationController @execution_environment = ExecutionEnvironment.new(execution_environment_params) authorize! create_and_respond(object: @execution_environment) do - copy_execution_environment_to_poseidon + sync_to_runner_management end end @@ -160,27 +158,29 @@ class ExecutionEnvironmentsController < ApplicationController def update update_and_respond(object: @execution_environment, params: execution_environment_params) do - copy_execution_environment_to_poseidon + sync_to_runner_management end end - def synchronize_all_to_poseidon + def sync_all_to_runner_management authorize ExecutionEnvironment - return unless RUNNER_MANAGEMENT_PRESENT + return unless Runner.management_active? - success = ExecutionEnvironment.all.map(&:copy_to_poseidon).all? - if success + success = ExecutionEnvironment.all.map do |execution_environment| + Runner.strategy_class.sync_environment(execution_environment) + end + if success.all? redirect_to ExecutionEnvironment, notice: t('execution_environments.index.synchronize_all.success') else redirect_to ExecutionEnvironment, alert: t('execution_environments.index.synchronize_all.failure') end end - def copy_execution_environment_to_poseidon - unless RUNNER_MANAGEMENT_PRESENT && @execution_environment.copy_to_poseidon - t('execution_environments.form.errors.not_synced_to_poseidon') + def sync_to_runner_management + unless Runner.management_active? && Runner.strategy_class.sync_environment(@execution_environment) + t('execution_environments.form.errors.not_synced_to_runner_management') end end - private :copy_execution_environment_to_poseidon + private :sync_to_runner_management end diff --git a/app/models/execution_environment.rb b/app/models/execution_environment.rb index 9d420715..cee38252 100644 --- a/app/models/execution_environment.rb +++ b/app/models/execution_environment.rb @@ -7,9 +7,6 @@ class ExecutionEnvironment < ApplicationRecord include DefaultValues VALIDATION_COMMAND = 'whoami' - RUNNER_MANAGEMENT_PRESENT = CodeOcean::Config.new(:code_ocean).read[:runner_management].present? - BASE_URL = CodeOcean::Config.new(:code_ocean).read[:runner_management][:url] if RUNNER_MANAGEMENT_PRESENT - HEADERS = {'Content-Type' => 'application/json'}.freeze DEFAULT_CPU_LIMIT = 20 after_initialize :set_default_values @@ -42,20 +39,6 @@ class ExecutionEnvironment < ApplicationRecord name end - def copy_to_poseidon - return false unless RUNNER_MANAGEMENT_PRESENT - - url = "#{BASE_URL}/execution-environments/#{id}" - response = Faraday.put(url, to_json, HEADERS) - return true if [201, 204].include? response.status - - Rails.logger.warn("Could not create execution environment in Poseidon, got response: #{response.as_json}") - false - rescue Faraday::Error => e - Rails.logger.warn("Could not create execution environment because of Faraday error: #{e.inspect}") - false - end - def to_json(*_args) { id: id, diff --git a/app/models/runner.rb b/app/models/runner.rb index ee66a28b..543452d7 100644 --- a/app/models/runner.rb +++ b/app/models/runner.rb @@ -8,14 +8,15 @@ class Runner < ApplicationRecord 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] - attr_accessor :strategy def self.strategy_class - "runner/strategy/#{STRATEGY_NAME}".camelize.constantize + strategy_name = CodeOcean::Config.new(:code_ocean).read[:runner_management][:strategy] + @strategy_class ||= "runner/strategy/#{strategy_name}".camelize.constantize + end + + def self.management_active? + @management_active ||= CodeOcean::Config.new(:code_ocean).read[:runner_management][:enabled] end def self.for(user, exercise) diff --git a/app/policies/execution_environment_policy.rb b/app/policies/execution_environment_policy.rb index cf134527..a83b1f29 100644 --- a/app/policies/execution_environment_policy.rb +++ b/app/policies/execution_environment_policy.rb @@ -9,7 +9,7 @@ class ExecutionEnvironmentPolicy < AdminOnlyPolicy define_method(action) { admin? || teacher? } end - def synchronize_all_to_poseidon? + def sync_all_to_runner_management? admin? end end diff --git a/app/views/execution_environments/index.html.slim b/app/views/execution_environments/index.html.slim index 9f6d47cc..dd6deb0b 100644 --- a/app/views/execution_environments/index.html.slim +++ b/app/views/execution_environments/index.html.slim @@ -1,7 +1,7 @@ h1.d-inline-block = ExecutionEnvironment.model_name.human(count: 2) -- if ExecutionEnvironment::RUNNER_MANAGEMENT_PRESENT - = button_to( { action: :synchronize_all_to_poseidon, method: :post }, { form_class: 'float-right mb-2', class: 'btn btn-success' }) +- if Runner.management_active? + = button_to( { action: :sync_all_to_runner_management, method: :post }, { form_class: 'float-right mb-2', class: 'btn btn-success' }) i.fa.fa-upload = t('execution_environments.index.synchronize_all.button') diff --git a/config/code_ocean.yml.ci b/config/code_ocean.yml.ci index 8b8d2a64..1c531f9e 100644 --- a/config/code_ocean.yml.ci +++ b/config/code_ocean.yml.ci @@ -10,6 +10,7 @@ test: prometheus_exporter: enabled: false runner_management: + enabled: true 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 6b694123..0202bfc9 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: + enabled: false strategy: poseidon url: https://runners.example.org unused_runner_expiration_time: 180 diff --git a/config/locales/de.yml b/config/locales/de.yml index abab5b43..ea5f34eb 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -284,7 +284,7 @@ de: docker_image: 'Wählen Sie ein Docker-Image aus der Liste oder fügen Sie ein neues hinzu, welches über DockerHub verfügbar ist.' exposed_ports: Während der Ausführung sind diese Ports für den Nutzer zugänglich. Die Portnummern müssen mit Komma, aber ohne Leerzeichen voneinander getrennt sein. errors: - not_synced_to_poseidon: Die Ausführungsumgebung wurde erstellt, aber aufgrund eines Fehlers nicht zu Poseidon synchronisiert. + not_synced_to_runner_management: Die Ausführungsumgebung wurde erstellt, aber aufgrund eines Fehlers nicht zum Runnermanagement synchronisiert. index: shell: Shell synchronize_all: diff --git a/config/locales/en.yml b/config/locales/en.yml index dd241006..b2995a27 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -284,7 +284,7 @@ en: docker_image: Pick a Docker image listed above or add a new one which is available via DockerHub. exposed_ports: During code execution these ports are accessible for the user. Port numbers must be separated by a comma but no space. errors: - not_synced_to_poseidon: The ExecutionEnvironment was created but not synced to Poseidon due to an error. + not_synced_to_runner_management: The execution environment was created but not synced to the runner management due to an error. index: shell: Shell synchronize_all: diff --git a/config/routes.rb b/config/routes.rb index 1608ac05..ca546cea 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -67,7 +67,7 @@ Rails.application.routes.draw do get :statistics end - post :synchronize_all_to_poseidon, on: :collection + post :sync_all_to_runner_management, on: :collection end post '/import_exercise' => 'exercises#import_exercise' diff --git a/lib/runner/strategy.rb b/lib/runner/strategy.rb index 0007caab..702311f4 100644 --- a/lib/runner/strategy.rb +++ b/lib/runner/strategy.rb @@ -5,7 +5,15 @@ class Runner::Strategy @execution_environment = environment end - def self.request_from_management + def self.config + raise NotImplementedError + end + + def self.sync_environment(_environment) + raise NotImplementedError + end + + def self.request_from_management(_environment) raise NotImplementedError end diff --git a/lib/runner/strategy/docker_container_pool.rb b/lib/runner/strategy/docker_container_pool.rb index afa4261d..feb41a18 100644 --- a/lib/runner/strategy/docker_container_pool.rb +++ b/lib/runner/strategy/docker_container_pool.rb @@ -8,6 +8,11 @@ class Runner::Strategy::DockerContainerPool < Runner::Strategy @config ||= CodeOcean::Config.new(:docker).read(erb: true) end + def self.sync_environment(_environment) + # There is no dedicated sync mechanism yet + true + end + def self.request_from_management(environment) container_id = JSON.parse(Faraday.get("#{config[:pool][:location]}/docker_container_pool/get_container/#{environment.id}").body)['id'] container_id.presence || raise(Runner::Error::NotAvailable.new("DockerContainerPool didn't return a container id")) diff --git a/lib/runner/strategy/poseidon.rb b/lib/runner/strategy/poseidon.rb index 6a87d92b..f61d1748 100644 --- a/lib/runner/strategy/poseidon.rb +++ b/lib/runner/strategy/poseidon.rb @@ -10,13 +10,28 @@ class Runner::Strategy::Poseidon < Runner::Strategy end end + def self.config + @config ||= CodeOcean::Config.new(:code_ocean).read[:runner_management] || {} + end + def self.sync_environment(environment) - environment.copy_to_poseidon + url = "#{config[:url]}/execution-environments/#{environment.id}" + response = Faraday.put(url, environment.to_json, HEADERS) + return true if [201, 204].include? response.status + + Rails.logger.warn("Could not create execution environment in Poseidon, got response: #{response.as_json}") + false + rescue Faraday::Error => e + Rails.logger.warn("Could not create execution environment because of Faraday error: #{e.inspect}") + false end def self.request_from_management(environment) - url = "#{Runner::BASE_URL}/runners" - body = {executionEnvironmentId: environment.id, inactivityTimeout: Runner::UNUSED_EXPIRATION_TIME} + url = "#{config[:url]}/runners" + body = { + executionEnvironmentId: environment.id, + inactivityTimeout: config[:unused_runner_expiration_time].seconds, + } response = Faraday.post(url, body.to_json, HEADERS) case response.status @@ -124,7 +139,7 @@ class Runner::Strategy::Poseidon < Runner::Strategy end def runner_url - "#{Runner::BASE_URL}/runners/#{@allocation_id}" + "#{self.class.config[:url]}/runners/#{@allocation_id}" end class Connection < Runner::Connection diff --git a/spec/controllers/execution_environments_controller_spec.rb b/spec/controllers/execution_environments_controller_spec.rb index fa76b4dd..73b408b1 100644 --- a/spec/controllers/execution_environments_controller_spec.rb +++ b/spec/controllers/execution_environments_controller_spec.rb @@ -8,7 +8,7 @@ describe ExecutionEnvironmentsController do before do allow(controller).to receive(:current_user).and_return(user) - allow(controller).to receive(:copy_execution_environment_to_poseidon).and_return(nil) + allow(controller).to receive(:sync_to_runner_management).and_return(nil) end describe 'POST #create' do @@ -26,8 +26,8 @@ describe ExecutionEnvironmentsController do expect { perform_request.call }.to change(ExecutionEnvironment, :count).by(1) end - it 'registers the execution environment with Poseidon' do - expect(controller).to have_received(:copy_execution_environment_to_poseidon) + it 'registers the execution environment with the runner management' do + expect(controller).to have_received(:sync_to_runner_management) end expect_redirect(ExecutionEnvironment.last) @@ -40,8 +40,8 @@ describe ExecutionEnvironmentsController do expect_status(200) expect_template(:new) - it 'does not register the execution environment with Poseidon' do - expect(controller).not_to have_received(:copy_execution_environment_to_poseidon) + it 'does not register the execution environment with the runner management' do + expect(controller).not_to have_received(:sync_to_runner_management) end end end @@ -167,7 +167,7 @@ describe ExecutionEnvironmentsController 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(:copy_execution_environment_to_poseidon).and_return(nil) + 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 @@ -175,8 +175,8 @@ describe ExecutionEnvironmentsController do expect_assigns(execution_environment: ExecutionEnvironment) expect_redirect(:execution_environment) - it 'updates the execution environment at Poseidon' do - expect(controller).to have_received(:copy_execution_environment_to_poseidon) + it 'updates the execution environment at the runner management' do + expect(controller).to have_received(:sync_to_runner_management) end end @@ -187,25 +187,24 @@ describe ExecutionEnvironmentsController do expect_status(200) expect_template(:edit) - it 'does not update the execution environment at Poseidon' do - expect(controller).not_to have_received(:copy_execution_environment_to_poseidon) + it 'does not update the execution environment at the runner management' do + expect(controller).not_to have_received(:sync_to_runner_management) end end end - describe '#synchronize_all_to_poseidon' do + describe '#sync_all_to_runner_management' do let(:execution_environments) { FactoryBot.build_list(:ruby, 3) } - it 'copies all execution environments to Poseidon' do + it 'copies all execution environments to the runner management' do allow(ExecutionEnvironment).to receive(:all).and_return(execution_environments) execution_environments.each do |execution_environment| - allow(execution_environment).to receive(:copy_to_poseidon).and_return(true) + allow(Runner::Strategy::Poseidon).to receive(:sync_environment).with(execution_environment).and_return(true) + expect(Runner::Strategy::Poseidon).to receive(:sync_environment).with(execution_environment).once end - post :synchronize_all_to_poseidon - - expect(execution_environments).to all(have_received(:copy_to_poseidon).once) + post :sync_all_to_runner_management end end end diff --git a/spec/lib/runner/strategy/poseidon_spec.rb b/spec/lib/runner/strategy/poseidon_spec.rb index 79c4935b..52b12389 100644 --- a/spec/lib/runner/strategy/poseidon_spec.rb +++ b/spec/lib/runner/strategy/poseidon_spec.rb @@ -114,13 +114,58 @@ describe Runner::Strategy::Poseidon do end end + describe '::sync_environment' do + let(:action) { -> { described_class.sync_environment(execution_environment) } } + let(:execution_environment) { FactoryBot.create(:ruby) } + + it 'makes the correct request to Poseidon' do + allow(Faraday).to receive(:put).and_return(Faraday::Response.new(status: 201)) + action.call + expect(Faraday).to have_received(:put) do |url, body, headers| + expect(url).to match(%r{execution-environments/#{execution_environment.id}\z}) + expect(body).to eq(execution_environment.to_json) + expect(headers).to include({'Content-Type' => 'application/json'}) + end + end + + shared_examples 'returns true when the api request was successful' do |status| + it "returns true on status #{status}" do + allow(Faraday).to receive(:put).and_return(Faraday::Response.new(status: status)) + expect(action.call).to be_truthy + end + end + + shared_examples 'returns false when the api request failed' do |status| + it "returns false on status #{status}" do + allow(Faraday).to receive(:put).and_return(Faraday::Response.new(status: status)) + expect(action.call).to be_falsey + end + end + + [201, 204].each do |status| + include_examples 'returns true when the api request was successful', status + end + + [400, 500].each do |status| + include_examples 'returns false when the api request failed', status + end + + it 'returns false if Faraday raises an error' do + allow(Faraday).to receive(:put).and_raise(Faraday::TimeoutError) + expect(action.call).to be_falsey + 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") + .stub_request(:post, "#{described_class.config[:url]}/runners") .with( - body: {executionEnvironmentId: execution_environment.id, inactivityTimeout: Runner::UNUSED_EXPIRATION_TIME}, + body: { + executionEnvironmentId: execution_environment.id, + inactivityTimeout: described_class.config[:unused_runner_expiration_time].seconds, + }, headers: {'Content-Type' => 'application/json'} ) .to_return(body: response_body, status: response_status) @@ -181,7 +226,7 @@ describe Runner::Strategy::Poseidon do 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") + .stub_request(:post, "#{described_class.config[:url]}/runners/#{runner_id}/execute") .with( body: {command: command, timeLimit: execution_environment.permitted_execution_time}, headers: {'Content-Type' => 'application/json'} @@ -235,7 +280,7 @@ describe Runner::Strategy::Poseidon do let(:action) { -> { poseidon.destroy_at_management } } let!(:destroy_stub) do WebMock - .stub_request(:delete, "#{Runner::BASE_URL}/runners/#{runner_id}") + .stub_request(:delete, "#{described_class.config[:url]}/runners/#{runner_id}") .to_return(body: response_body, status: response_status) end @@ -262,7 +307,7 @@ describe Runner::Strategy::Poseidon do 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") + .stub_request(:patch, "#{described_class.config[:url]}/runners/#{runner_id}/files") .with( body: {copy: [{path: file.filepath, content: encoded_file_content}]}, headers: {'Content-Type' => 'application/json'} diff --git a/spec/models/execution_environment_spec.rb b/spec/models/execution_environment_spec.rb index 6441b896..d7b057ab 100644 --- a/spec/models/execution_environment_spec.rb +++ b/spec/models/execution_environment_spec.rb @@ -192,45 +192,4 @@ describe ExecutionEnvironment do end end end - - describe '#copy_to_poseidon' do - let(:execution_environment) { FactoryBot.create(:ruby) } - - it 'makes the correct request to Poseidon' do - allow(Faraday).to receive(:put).and_return(Faraday::Response.new(status: 201)) - execution_environment.copy_to_poseidon - expect(Faraday).to have_received(:put) do |url, body, headers| - expect(url).to match(%r{execution-environments/#{execution_environment.id}\z}) - expect(body).to eq(execution_environment.to_json) - expect(headers).to include({'Content-Type' => 'application/json'}) - end - end - - shared_examples 'returns true when the api request was successful' do |status| - it "returns true on status #{status}" do - allow(Faraday).to receive(:put).and_return(Faraday::Response.new(status: status)) - expect(execution_environment.copy_to_poseidon).to be_truthy - end - end - - shared_examples 'returns false when the api request failed' do |status| - it "returns false on status #{status}" do - allow(Faraday).to receive(:put).and_return(Faraday::Response.new(status: status)) - expect(execution_environment.copy_to_poseidon).to be_falsey - end - end - - [201, 204].each do |status| - include_examples 'returns true when the api request was successful', status - end - - [400, 500].each do |status| - include_examples 'returns false when the api request failed', status - end - - it 'returns false if Faraday raises an error' do - allow(Faraday).to receive(:put).and_raise(Faraday::TimeoutError) - expect(execution_environment.copy_to_poseidon).to be_falsey - end - end end diff --git a/spec/models/runner_spec.rb b/spec/models/runner_spec.rb index 6105f819..0a496b24 100644 --- a/spec/models/runner_spec.rb +++ b/spec/models/runner_spec.rb @@ -30,8 +30,20 @@ describe Runner do describe '::strategy_class' do shared_examples 'uses the strategy defined in the constant' do |strategy, strategy_class| + let(:codeocean_config) { instance_double(CodeOcean::Config) } + let(:runner_management_config) { {runner_management: {enabled: true, strategy: strategy}} } + + before do + allow(CodeOcean::Config).to receive(:new).with(:code_ocean).and_return(codeocean_config) + allow(codeocean_config).to receive(:read).and_return(runner_management_config) + end + + after do + # Reset the memorized helper + described_class.remove_instance_variable :@strategy_class + end + 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 @@ -41,7 +53,7 @@ describe Runner do docker_container_pool: Runner::Strategy::DockerContainerPool, } available_strategies.each do |strategy, strategy_class| - include_examples 'uses the strategy defined in the constant', strategy, strategy_class + it_behaves_like 'uses the strategy defined in the constant', strategy, strategy_class end end diff --git a/spec/policies/execution_environment_policy_spec.rb b/spec/policies/execution_environment_policy_spec.rb index 9d0b2539..588f76db 100644 --- a/spec/policies/execution_environment_policy_spec.rb +++ b/spec/policies/execution_environment_policy_spec.rb @@ -59,7 +59,7 @@ describe ExecutionEnvironmentPolicy do end end - permissions(:synchronize_all_to_poseidon?) do + permissions(:sync_all_to_runner_management?) do it 'grants access to the admin' do expect(policy).to permit(FactoryBot.build(:admin)) end