From e448e403ba4c92b483f4fe2a308b3d3afd4d5635 Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Tue, 21 Apr 2015 18:28:34 +0200 Subject: [PATCH 1/4] restart containers if they are running for more than 15 minutes (900 seconds) --- lib/docker_client.rb | 1 + lib/docker_container_mixin.rb | 4 ++++ lib/docker_container_pool.rb | 20 +++++++++++++++++++- 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/lib/docker_client.rb b/lib/docker_client.rb index 95b89e50..8bb6ce4f 100644 --- a/lib/docker_client.rb +++ b/lib/docker_client.rb @@ -53,6 +53,7 @@ class DockerClient local_workspace_path = generate_local_workspace_path FileUtils.mkdir(local_workspace_path) container.start(container_start_options(execution_environment, local_workspace_path)) + container.start_time = Time.now container rescue Docker::Error::NotFoundError => error destroy_container(container) diff --git a/lib/docker_container_mixin.rb b/lib/docker_container_mixin.rb index a8d3c8a4..fde9fe5b 100644 --- a/lib/docker_container_mixin.rb +++ b/lib/docker_container_mixin.rb @@ -1,4 +1,8 @@ module DockerContainerMixin + + attr_accessor :start_time + attr_accessor :status + def binds json['HostConfig']['Binds'] end diff --git a/lib/docker_container_pool.rb b/lib/docker_container_pool.rb index 8823d989..47a8bf5c 100644 --- a/lib/docker_container_pool.rb +++ b/lib/docker_container_pool.rb @@ -3,6 +3,8 @@ require 'concurrent/timer_task' require 'concurrent/utilities' class DockerContainerPool + TIME_TILL_RESTART = 900 + @containers = ThreadSafe::Hash[ExecutionEnvironment.all.map { |execution_environment| [execution_environment.id, ThreadSafe::Array.new] }] #as containers are not containing containers in use @all_containers = ThreadSafe::Hash[ExecutionEnvironment.all.map { |execution_environment| [execution_environment.id, ThreadSafe::Array.new] }] @@ -22,12 +24,28 @@ class DockerContainerPool end def self.return_container(container, execution_environment) + container.status = 'available' @containers[execution_environment.id].push(container) end def self.get_container(execution_environment) if config[:active] - @containers[execution_environment.id].try(:shift) || nil + container = @containers[execution_environment.id].try(:shift) || nil + + if((Time.now - container.start_time).to_i.abs > TIME_TILL_RESTART) + # remove container from @all_containers + @all_containers[execution_environment.id]-=[container] + + # destroy container + DockerClient.destroy_container(container) + + # create new container and add it to @all_containers. will be added to @containers on return_container + container = create_container(@execution_environment) + @all_containers[execution_environment.id]+=[container] + end + container.status = 'used' + container + else create_container(execution_environment) end From a7087824b10477293b6a79fe5c071d0586028db5 Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Wed, 22 Apr 2015 11:38:07 +0200 Subject: [PATCH 2/4] added a null check, removed setting status to used due to test problems, fixed mocking of container object. --- lib/docker_container_pool.rb | 24 ++++++++++++++---------- spec/lib/docker_container_pool_spec.rb | 2 +- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/lib/docker_container_pool.rb b/lib/docker_container_pool.rb index 47a8bf5c..7cc5fb51 100644 --- a/lib/docker_container_pool.rb +++ b/lib/docker_container_pool.rb @@ -20,7 +20,9 @@ class DockerContainerPool end def self.create_container(execution_environment) - DockerClient.create_container(execution_environment) + container = DockerClient.create_container(execution_environment) + container.status = 'available' + container end def self.return_container(container, execution_environment) @@ -32,18 +34,20 @@ class DockerContainerPool if config[:active] container = @containers[execution_environment.id].try(:shift) || nil - if((Time.now - container.start_time).to_i.abs > TIME_TILL_RESTART) - # remove container from @all_containers - @all_containers[execution_environment.id]-=[container] + if(!container.nil?) + if ((Time.now - container.start_time).to_i.abs > TIME_TILL_RESTART) + # remove container from @all_containers + @all_containers[execution_environment.id]-=[container] - # destroy container - DockerClient.destroy_container(container) + # destroy container + DockerClient.destroy_container(container) - # create new container and add it to @all_containers. will be added to @containers on return_container - container = create_container(@execution_environment) - @all_containers[execution_environment.id]+=[container] + # create new container and add it to @all_containers. will be added to @containers on return_container + container = create_container(@execution_environment) + @all_containers[execution_environment.id]+=[container] + end + #container.status = 'used' end - container.status = 'used' container else diff --git a/spec/lib/docker_container_pool_spec.rb b/spec/lib/docker_container_pool_spec.rb index f820a49b..464c3221 100644 --- a/spec/lib/docker_container_pool_spec.rb +++ b/spec/lib/docker_container_pool_spec.rb @@ -1,7 +1,7 @@ require 'rails_helper' describe DockerContainerPool do - let(:container) { double } + let(:container) { double(:start_time => Time.now, :status => 'available') } def reload_class load('docker_container_pool.rb') From e515afe6192241239b62472e7a624f8f7e2c2ffa Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Wed, 22 Apr 2015 15:00:01 +0200 Subject: [PATCH 3/4] updated container handling. hopefully removed potential error source. --- lib/docker_client.rb | 12 +++++++++++- lib/docker_container_pool.rb | 20 +++++++++++++++++--- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/lib/docker_client.rb b/lib/docker_client.rb index 8bb6ce4f..ba9677d1 100644 --- a/lib/docker_client.rb +++ b/lib/docker_client.rb @@ -188,7 +188,17 @@ class DockerClient {status: :ok, stderr: stderr.join, stdout: stdout.join} end rescue Timeout::Error - container.restart if RECYCLE_CONTAINERS + #container.restart if RECYCLE_CONTAINERS + DockerContainerPool.remove_from_all_containers(container, @execution_environment) + + # destroy container + destroy_container(container) + + if(RECYCLE_CONTAINERS) + # create new container and add it to @all_containers. will be added to @containers on return_container + container = create_container(execution_environment) + DockerContainerPool.add_to_all_containers(container, execution_environment) + end {status: :timeout} ensure RECYCLE_CONTAINERS ? return_container(container) : self.class.destroy_container(container) diff --git a/lib/docker_container_pool.rb b/lib/docker_container_pool.rb index 7cc5fb51..fc3aee99 100644 --- a/lib/docker_container_pool.rb +++ b/lib/docker_container_pool.rb @@ -19,6 +19,20 @@ class DockerContainerPool @config ||= CodeOcean::Config.new(:docker).read(erb: true)[:pool] end + def self.remove_from_all_containers(container, execution_environment) + @all_containers[execution_environment.id]-=[container] + if(@containers[execution_environment.id].include?(container)) + @containers[execution_environment.id]-=[container] + end + end + + def self.add_to_all_containers(container, execution_environment) + @all_containers[execution_environment.id]+=[container] + if(!@containers[execution_environment.id].include?(container)) + @containers[execution_environment.id]+=[container] + end + end + def self.create_container(execution_environment) container = DockerClient.create_container(execution_environment) container.status = 'available' @@ -37,14 +51,14 @@ class DockerContainerPool if(!container.nil?) if ((Time.now - container.start_time).to_i.abs > TIME_TILL_RESTART) # remove container from @all_containers - @all_containers[execution_environment.id]-=[container] + remove_from_all_containers(container, execution_environment) # destroy container DockerClient.destroy_container(container) # create new container and add it to @all_containers. will be added to @containers on return_container - container = create_container(@execution_environment) - @all_containers[execution_environment.id]+=[container] + container = create_container(execution_environment) + add_to_all_containers(container, execution_environment) end #container.status = 'used' end From d610155dbcc205a14fd4b546623a36582770034d Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Wed, 22 Apr 2015 17:46:10 +0200 Subject: [PATCH 4/4] access class wide variable --- lib/docker_client.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/docker_client.rb b/lib/docker_client.rb index ba9677d1..72960723 100644 --- a/lib/docker_client.rb +++ b/lib/docker_client.rb @@ -196,8 +196,8 @@ class DockerClient if(RECYCLE_CONTAINERS) # create new container and add it to @all_containers. will be added to @containers on return_container - container = create_container(execution_environment) - DockerContainerPool.add_to_all_containers(container, execution_environment) + container = create_container(@execution_environment) + DockerContainerPool.add_to_all_containers(container, @execution_environment) end {status: :timeout} ensure