Remove 'previous' DockerContainerPool implementation

lib/docker_client.rb

@@ -119,13 +119,11 @@ class DockerClient
     container.start_time = Time.zone.now
     container.status = :created
     container.execution_environment = execution_environment
-    container.re_use = true
     container.docker_client = new(execution_environment: execution_environment)
 
     Thread.new do
       timeout = Random.rand(MINIMUM_CONTAINER_LIFETIME..MAXIMUM_CONTAINER_LIFETIME) # seconds
       sleep(timeout)
-      container.re_use = false
       if container.status == :executing
         Thread.new do
           timeout = SELF_DESTROY_GRACE_PERIOD.to_i
@@ -227,7 +225,7 @@ class DockerClient
     Rails.logger.info("destroying container #{container}")
 
     # Checks only if container assignment is not nil and not whether the container itself is still present.
-    if container && !DockerContainerPool.active?
+    if container
       container.kill
       container.port_bindings.each_value {|port| PortPool.release(port) }
       begin
@@ -240,8 +238,6 @@ class DockerClient
 
       # Checks only if container assignment is not nil and not whether the container itself is still present.
       container&.delete(force: true, v: true)
-    elsif container
-      DockerContainerPool.destroy_container(container)
     end
   rescue Docker::Error::NotFoundError => e
     Rails.logger.error("destroy_container: Rescued from Docker::Error::NotFoundError: #{e}")
@@ -261,7 +257,7 @@ class DockerClient
   def execute_command(command, before_execution_block, output_consuming_block)
     # tries ||= 0
     container_request_time = Time.zone.now
-    @container = DockerContainerPool.get_container(@execution_environment)
+    @container = self.class.create_container(@execution_environment)
     waiting_for_container_time = Time.zone.now - container_request_time
     if @container
       @container.status = :executing
@@ -285,7 +281,7 @@ container_execution_time: nil}
 
   # called when the user clicks the "Run" button
   def open_websocket_connection(command, before_execution_block, _output_consuming_block)
-    @container = DockerContainerPool.get_container(@execution_environment)
+    @container = self.class.create_container(@execution_environment)
     if @container
       @container.status = :executing
       # do not use try here, directly call the passed proc and rescue from the error in order to log the problem.
@@ -351,13 +347,7 @@ container_execution_time: nil}
     # exit the timeout thread if it is still alive
     exit_thread_if_alive
     @socket.close
-    # if we use pooling and recylce the containers, put it back. otherwise, destroy it.
+    self.class.destroy_container(container)
-    if DockerContainerPool.active? && RECYCLE_CONTAINERS
-      self.class.return_container(container,
-        @execution_environment)
-    else
-      self.class.destroy_container(container)
-    end
   end
 
   def kill_container(container)
@@ -413,7 +403,6 @@ container_execution_time: nil}
   end
 
   def self.initialize_environment
-    # TODO: Move to DockerContainerPool
     raise Error.new('Docker configuration missing!') unless config[:connection_timeout] && config[:workspace_root]
 
     Docker.url = config[:host] if config[:host]
@@ -455,21 +444,6 @@ container_execution_time: nil}
     `docker pull #{docker_image}` if docker_image
   end
 
-  def self.return_container(container, execution_environment)
-    Rails.logger.debug { "returning container #{container}" }
-    begin
-      clean_container_workspace(container)
-    rescue Docker::Error::NotFoundError => e
-      # FIXME: Create new container?
-      Rails.logger.info("return_container: Rescued from Docker::Error::NotFoundError: #{e}")
-      Rails.logger.info('Nothing is done here additionally. The container will be exchanged upon its next retrieval.')
-    end
-    DockerContainerPool.return_container(container, execution_environment)
-    container.status = :available
-  end
-
-  # private :return_container
-
   def send_command(command, container)
     result = {status: :failed, stdout: '', stderr: ''}
     output = nil
@@ -489,12 +463,7 @@ container_execution_time: nil}
       result = {status: (output[2])&.zero? ? :ok : :failed, stdout: output[0].join.force_encoding('utf-8'), stderr: output[1].join.force_encoding('utf-8')}
     end
 
-    # if we use pooling and recylce the containers, put it back. otherwise, destroy it.
+    self.class.destroy_container(container)
-    if DockerContainerPool.active? && RECYCLE_CONTAINERS
-      self.class.return_container(container, @execution_environment)
-    else
-      self.class.destroy_container(container)
-    end
     result
   rescue Timeout::Error
     Rails.logger.info("got timeout error for container #{container}")

lib/docker_container_mixin.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 module DockerContainerMixin
-  attr_accessor :start_time, :status, :re_use, :execution_environment, :docker_client
+  attr_accessor :start_time, :status, :execution_environment, :docker_client
 
   def binds
     host_config['Binds']

lib/docker_container_pool.rb

@@ -1,78 +0,0 @@
-# frozen_string_literal: true
-
-# get_container, destroy_container was moved to lib/runner/strategy/docker_container_pool.rb.
-# return_container is not used anymore because runners are not shared between users anymore.
-# create_container is done by the DockerContainerPool.
-# dump_info and quantities are still in use.
-
-class DockerContainerPool
-  def self.active?
-    # TODO: Refactor config and merge with code_ocean.yml
-    config[:active] && Runner.management_active? && Runner.strategy_class == Runner::Strategy::DockerContainerPool
-  end
-
-  def self.config
-    # TODO: Why erb?
-    @config ||= CodeOcean::Config.new(:docker).read(erb: true)[:pool]
-  end
-
-  def self.create_container(execution_environment)
-    Rails.logger.info("trying to create container for execution environment: #{execution_environment}")
-    container = DockerClient.create_container(execution_environment)
-    container.status = 'available' # FIXME: String vs Symbol usage?
-    # Rails.logger.debug('created container ' + container.to_s + ' for execution environment ' + execution_environment.to_s)
-    container
-  rescue StandardError => e
-    Sentry.set_extras({container: container.inspect, execution_environment: execution_environment.inspect,
-      config: config.inspect})
-    Sentry.capture_exception(e)
-    nil
-  end
-
-  # not in use because DockerClient::RECYCLE_CONTAINERS == false
-  def self.return_container(container, execution_environment)
-    Faraday.get("#{config[:location]}/docker_container_pool/return_container/#{container.id}")
-  rescue StandardError => e
-    Sentry.set_extras({container: container.inspect, execution_environment: execution_environment.inspect,
-      config: config.inspect})
-    Sentry.capture_exception(e)
-    nil
-  end
-
-  def self.get_container(execution_environment)
-    # if pooling is active, do pooling, otherwise just create an container and return it
-    if active?
-      begin
-        container_id = JSON.parse(Faraday.get("#{config[:location]}/docker_container_pool/get_container/#{execution_environment.id}").body)['id']
-        Docker::Container.get(container_id) if container_id.present?
-      rescue StandardError => e
-        Sentry.set_extras({container_id: container_id.inspect, execution_environment: execution_environment.inspect,
-          config: config.inspect})
-        Sentry.capture_exception(e)
-        nil
-      end
-    else
-      create_container(execution_environment)
-    end
-  end
-
-  def self.destroy_container(container)
-    Faraday.get("#{config[:location]}/docker_container_pool/destroy_container/#{container.id}")
-  end
-
-  def self.quantities
-    response = JSON.parse(Faraday.get("#{config[:location]}/docker_container_pool/quantities").body)
-    response.transform_keys(&:to_i)
-  rescue StandardError => e
-    Sentry.set_extras({response: response.inspect})
-    Sentry.capture_exception(e)
-    []
-  end
-
-  def self.dump_info
-    JSON.parse(Faraday.get("#{config[:location]}/docker_container_pool/dump_info").body)
-  rescue StandardError => e
-    Sentry.capture_exception(e)
-    nil
-  end
-end

spec/lib/docker_client_spec.rb

@@ -204,8 +204,8 @@ describe DockerClient, docker: true do
   describe '#execute_arbitrary_command' do
     let(:execute_arbitrary_command) { docker_client.execute_arbitrary_command(command) }
 
-    it 'takes a container from the pool' do
+    it 'creates a new container' do
-      expect(DockerContainerPool).to receive(:get_container).and_call_original
+      expect(described_class).to receive(:create_container).and_call_original
       execute_arbitrary_command
     end
 
@@ -248,8 +248,8 @@ describe DockerClient, docker: true do
 
     after { docker_client.send(:execute_run_command, submission, filename) }
 
-    it 'takes a container from the pool' do
+    it 'creates a new container' do
-      expect(DockerContainerPool).to receive(:get_container).with(submission.execution_environment).and_call_original
+      expect(described_class).to receive(:create_container).with(submission.execution_environment).and_call_original
     end
 
     it 'creates the workspace files' do
@@ -268,8 +268,8 @@ describe DockerClient, docker: true do
 
     after { docker_client.send(:execute_test_command, submission, filename) }
 
-    it 'takes a container from the pool' do
+    it 'creates a new container' do
-      expect(DockerContainerPool).to receive(:get_container).with(submission.execution_environment).and_call_original
+      expect(described_class).to receive(:create_container).with(submission.execution_environment).and_call_original
     end
 
     it 'creates the workspace files' do