diff --git a/lib/docker_client.rb b/lib/docker_client.rb index 0517ff69..b134fa1e 100644 --- a/lib/docker_client.rb +++ b/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. - if DockerContainerPool.active? && RECYCLE_CONTAINERS - self.class.return_container(container, - @execution_environment) - else - self.class.destroy_container(container) - end + self.class.destroy_container(container) 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. - if DockerContainerPool.active? && RECYCLE_CONTAINERS - self.class.return_container(container, @execution_environment) - else - self.class.destroy_container(container) - end + self.class.destroy_container(container) result rescue Timeout::Error Rails.logger.info("got timeout error for container #{container}") diff --git a/lib/docker_container_mixin.rb b/lib/docker_container_mixin.rb index 83b77865..d2785263 100644 --- a/lib/docker_container_mixin.rb +++ b/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'] diff --git a/lib/docker_container_pool.rb b/lib/docker_container_pool.rb deleted file mode 100644 index a89bd461..00000000 --- a/lib/docker_container_pool.rb +++ /dev/null @@ -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 diff --git a/spec/lib/docker_client_spec.rb b/spec/lib/docker_client_spec.rb index b4289488..45dfdfca 100644 --- a/spec/lib/docker_client_spec.rb +++ b/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 - expect(DockerContainerPool).to receive(:get_container).and_call_original + it 'creates a new container' do + 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 - expect(DockerContainerPool).to receive(:get_container).with(submission.execution_environment).and_call_original + it 'creates a new container' do + 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 - expect(DockerContainerPool).to receive(:get_container).with(submission.execution_environment).and_call_original + it 'creates a new container' do + expect(described_class).to receive(:create_container).with(submission.execution_environment).and_call_original end it 'creates the workspace files' do