Generalize method and constant names for runner management

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?
+    success = ExecutionEnvironment.all.map do |execution_environment|
-    if success
+      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
+  def sync_to_runner_management
-    unless RUNNER_MANAGEMENT_PRESENT && @execution_environment.copy_to_poseidon
+    unless Runner.management_active? && Runner.strategy_class.sync_environment(@execution_environment)
-      t('execution_environments.form.errors.not_synced_to_poseidon')
+      t('execution_environments.form.errors.not_synced_to_runner_management')
     end
   end
-  private :copy_execution_environment_to_poseidon
+  private :sync_to_runner_management
 end

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,

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)

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

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
+- if Runner.management_active?
-  = button_to( { action: :synchronize_all_to_poseidon, method: :post }, { form_class: 'float-right mb-2', class: 'btn btn-success' })
+  = 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')
 

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

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

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 <a href="https://hub.docker.com/" target="_blank">DockerHub</a> 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:

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 <a href="https://hub.docker.com/" target="_blank">DockerHub</a>.
         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:

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'

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
 

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"))

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"
+    url = "#{config[:url]}/runners"
-    body = {executionEnvironmentId: environment.id, inactivityTimeout: Runner::UNUSED_EXPIRATION_TIME}
+    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

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
+      it 'registers the execution environment with the runner management' do
-        expect(controller).to have_received(:copy_execution_environment_to_poseidon)
+        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
+      it 'does not register the execution environment with the runner management' do
-        expect(controller).not_to have_received(:copy_execution_environment_to_poseidon)
+        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
+      it 'updates the execution environment at the runner management' do
-        expect(controller).to have_received(:copy_execution_environment_to_poseidon)
+        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
+      it 'does not update the execution environment at the runner management' do
-        expect(controller).not_to have_received(:copy_execution_environment_to_poseidon)
+        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
+      post :sync_all_to_runner_management
-
-      expect(execution_environments).to all(have_received(:copy_to_poseidon).once)
     end
   end
 end

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'}

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

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
 

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