implemented pooling for Docker containers

Gemfile

@@ -5,6 +5,8 @@ gem 'bcrypt', '~> 3.1.7'
 gem 'bootstrap-will_paginate'
 gem 'carrierwave'
 gem 'coffee-rails', '~> 4.0.0'
+gem 'concurrent-ruby'
+gem 'concurrent-ruby-ext', platform: :ruby
 gem 'docker-api', require: 'docker'
 gem 'factory_girl_rails', '~> 4.0'
 gem 'forgery'

Gemfile.lock

@@ -84,6 +84,10 @@ GEM
       execjs
     coffee-script-source (1.9.0)
     colorize (0.7.5)
+    concurrent-ruby (0.8.0)
+      ref (~> 1.0, >= 1.0.5)
+    concurrent-ruby-ext (0.8.0)
+      concurrent-ruby (~> 0.8.0)
     database_cleaner (1.4.0)
     diff-lcs (1.2.5)
     docile (1.1.5)
@@ -198,6 +202,7 @@ GEM
       i18n
       polyamorous (~> 1.1)
     rdoc (4.2.0)
+    ref (1.0.5)
     rspec (3.1.0)
       rspec-core (~> 3.1.0)
       rspec-expectations (~> 3.1.0)
@@ -306,6 +311,8 @@ DEPENDENCIES
   carrierwave
   codeclimate-test-reporter
   coffee-rails (~> 4.0.0)
+  concurrent-ruby
+  concurrent-ruby-ext
   database_cleaner
   docker-api
   factory_girl_rails (~> 4.0)

app/controllers/execution_environments_controller.rb

@@ -25,11 +25,11 @@ class ExecutionEnvironmentsController < ApplicationController
 
   def execute_command
     @docker_client = DockerClient.new(execution_environment: @execution_environment, user: current_user)
-    render(json: @docker_client.execute_command(params[:command]))
+    render(json: @docker_client.execute_arbitrary_command(params[:command]))
   end
 
   def execution_environment_params
-    params[:execution_environment].permit(:docker_image, :exposed_ports, :editor_mode, :file_extension, :help, :indent_size, :name, :permitted_execution_time, :run_command, :test_command, :testing_framework).merge(user_id: current_user.id, user_type: current_user.class.name)
+    params[:execution_environment].permit(:docker_image, :exposed_ports, :editor_mode, :file_extension, :help, :indent_size, :name, :permitted_execution_time, :pool_size, :run_command, :test_command, :testing_framework).merge(user_id: current_user.id, user_type: current_user.class.name)
   end
   private :execution_environment_params
 
@@ -44,6 +44,7 @@ class ExecutionEnvironmentsController < ApplicationController
   end
 
   def set_docker_images
+    DockerClient.check_availability!
     @docker_images = DockerClient.image_tags.sort
   rescue DockerClient::Error => error
     @docker_images = []

app/models/execution_environment.rb

@@ -3,6 +3,8 @@ class ExecutionEnvironment < ActiveRecord::Base
 
   VALIDATION_COMMAND = 'whoami'
 
+  after_initialize :set_default_values
+
   has_many :exercises
   has_many :hints
 
@@ -13,8 +15,15 @@ class ExecutionEnvironment < ActiveRecord::Base
   validates :docker_image, presence: true
   validates :name, presence: true
   validates :permitted_execution_time, numericality: {only_integer: true}, presence: true
+  validates :pool_size, numericality: {only_integer: true}, presence: true
   validates :run_command, presence: true
 
+  def set_default_values
+    self.permitted_execution_time ||= 60
+    self.pool_size ||= 0
+  end
+  private :set_default_values
+
   def to_s
     name
   end
@@ -27,13 +36,13 @@ class ExecutionEnvironment < ActiveRecord::Base
   private :valid_test_setup?
 
   def validate_docker_image?
-    docker_image.present? && Rails.env != 'test'
+    docker_image.present? && !Rails.env.test?
   end
   private :validate_docker_image?
 
   def working_docker_image?
     DockerClient.pull(docker_image) unless DockerClient.image_tags.include?(docker_image)
-    output = DockerClient.new(execution_environment: self).execute_command(VALIDATION_COMMAND)
+    output = DockerClient.new(execution_environment: self).execute_arbitrary_command(VALIDATION_COMMAND)
     errors.add(:docker_image, "error: #{output[:stderr]}") if output[:stderr].present?
   rescue DockerClient::Error => error
     errors.add(:docker_image, "error: #{error}")

app/views/execution_environments/_form.html.slim

@@ -17,6 +17,9 @@
   .form-group
     = f.label(:permitted_execution_time)
     = f.number_field(:permitted_execution_time, class: 'form-control', min: 1)
+  .form-group
+    = f.label(:pool_size)
+    = f.number_field(:pool_size, class: 'form-control', min: 0)
   .form-group
     = f.label(:run_command)
     = f.text_field(:run_command, class: 'form-control', placeholder: 'command %{filename}', required: true)

app/views/execution_environments/show.html.slim

@@ -4,7 +4,7 @@ h1
 
 = row(label: 'execution_environment.name', value: @execution_environment.name)
 = row(label: 'execution_environment.user', value: link_to(@execution_environment.author, @execution_environment.author))
-- [:docker_image, :exposed_ports, :permitted_execution_time, :run_command, :test_command].each do |attribute|
+- [:docker_image, :exposed_ports, :permitted_execution_time, :pool_size, :run_command, :test_command].each do |attribute|
   = row(label: "execution_environment.#{attribute}", value: @execution_environment.send(attribute))
 = row(label: 'execution_environment.testing_framework', value: @testing_framework_adapter.try(:framework_name))
 = row(label: 'execution_environment.help', value: render_markdown(@execution_environment.help))

config/docker.yml.erb

@@ -1,5 +1,8 @@
 default: &default
   connection_timeout: 3
+  pool:
+    active: false
+    interval: 60
   ports: !ruby/range 4500..4600
 
 development:
@@ -9,6 +12,8 @@ development:
 
 production:
   <<: *default
+  pool:
+    active: true
   workspace_root: <%= Rails.root.join('tmp', 'files', Rails.env) %>
 
 test:

config/initializers/docker.rb

@@ -0,0 +1,3 @@
+DockerClient.initialize_environment
+DockerContainerPool.start_refill_task if DockerContainerPool.config[:active]
+at_exit { DockerContainerPool.clean_up } unless Rails.env.test?

config/locales/de.yml

@@ -13,6 +13,7 @@ de:
         help: Hilfetext
         name: Name
         permitted_execution_time: Erlaubte Ausführungszeit (in Sekunden)
+        pool_size: Docker-Container-Pool-Größe
         run_command: Ausführungsbefehl
         test_command: Testbefehl
         testing_framework: Testing-Framework

config/locales/en.yml

@@ -13,6 +13,7 @@ en:
         help: Help Text
         name: Name
         permitted_execution_time: Permitted Execution Time (in Seconds)
+        pool_size: Docker Container Pool Size
         run_command: Run Command
         test_command: Test Command
         testing_framework: Testing Framework

db/migrate/20150204080832_add_pool_size_to_execution_environments.rb

@@ -0,0 +1,11 @@
+class AddPoolSizeToExecutionEnvironments < ActiveRecord::Migration
+  def change
+    add_column :execution_environments, :pool_size, :integer
+
+    reversible do |direction|
+      direction.up do
+        ExecutionEnvironment.update_all(pool_size: 0)
+      end
+    end
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20150128093003) do
+ActiveRecord::Schema.define(version: 20150204080832) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -44,6 +44,7 @@ ActiveRecord::Schema.define(version: 20150128093003) do
     t.integer  "permitted_execution_time"
     t.integer  "user_id"
     t.string   "user_type"
+    t.integer  "pool_size"
   end
 
   create_table "exercises", force: true do |t|

lib/docker_client.rb

@@ -1,3 +1,5 @@
+require 'concurrent'
+
 class DockerClient
   CONTAINER_WORKSPACE_PATH = '/workspace'
   LOCAL_WORKSPACE_ROOT = Rails.root.join('tmp', 'files', Rails.env)
@@ -5,23 +7,12 @@ class DockerClient
   attr_reader :assigned_ports
   attr_reader :container_id
 
-  def bound_folders
-    @submission ? ["#{remote_workspace_path}:#{CONTAINER_WORKSPACE_PATH}"] : []
-  end
-  private :bound_folders
-
   def self.check_availability!
-    initialize_environment
     Timeout::timeout(config[:connection_timeout]) { Docker.version }
   rescue Excon::Errors::SocketError, Timeout::Error
     raise Error.new("The Docker host at #{Docker.url} is not reachable!")
   end
 
-  def clean_workspace
-    FileUtils.rm_rf(local_workspace_path)
-  end
-  private :clean_workspace
-
   def command_substitutions(filename)
     {class_name: File.basename(filename, File.extname(filename)).camelize, filename: filename}
   end
@@ -32,28 +23,29 @@ class DockerClient
   end
 
   def copy_file_to_workspace(options = {})
-    FileUtils.cp(options[:file].native_file.path, File.join(local_workspace_path, options[:file].path || '', options[:file].name_with_extension))
+    FileUtils.cp(options[:file].native_file.path, File.join(self.class.local_workspace_path(options[:container]), options[:file].path || '', options[:file].name_with_extension))
   end
 
-  def create_container(options = {})
+  def self.create_container(execution_environment)
-    Docker::Container.create('Cmd' => options[:command], 'Image' => @image.info['RepoTags'].first)
+    container = Docker::Container.create('Image' => find_image_by_tag(execution_environment.docker_image).info['RepoTags'].first, 'OpenStdin' => true, 'StdinOnce' => true)
+    container.start('Binds' => mapped_directories, 'PortBindings' => mapped_ports(execution_environment))
+    container
   end
-  private :create_container
 
-  def create_workspace
+  def create_workspace(container)
     @submission.collect_files.each do |file|
-      FileUtils.mkdir_p(File.join(local_workspace_path, file.path || ''))
+      FileUtils.mkdir_p(File.join(self.class.local_workspace_path(container), file.path || ''))
       if file.file_type.binary?
-        copy_file_to_workspace(file: file)
+        copy_file_to_workspace(container: container, file: file)
       else
-        create_workspace_file(file: file)
+        create_workspace_file(container: container, file: file)
       end
     end
   end
   private :create_workspace
 
   def create_workspace_file(options = {})
-    file = File.new(File.join(local_workspace_path, options[:file].path || '', options[:file].name_with_extension), 'w')
+    file = File.new(File.join(self.class.local_workspace_path(options[:container]), options[:file].path || '', options[:file].name_with_extension), 'w')
     file.write(options[:file].content)
     file.close
   end
@@ -65,51 +57,43 @@ class DockerClient
       port = configuration.first['HostPort'].to_i
       PortPool.release(port)
     end
+    FileUtils.rm_rf(local_workspace_path(container))
     container.delete(force: true)
   end
 
-  def execute_command(command, &block)
+  def execute_arbitrary_command(command, &block)
-    container = create_container(command: ['bash', '-c', command])
+    container = DockerContainerPool.get_container(@execution_environment)
     @container_id = container.id
-    start_container(container, &block)
+    send_command(command, container, &block)
   end
 
-  def execute_in_workspace(submission, &block)
+  [:run, :test].each do |cause|
-    @submission = submission
+    define_method("execute_#{cause}_command") do |submission, filename, &block|
-    create_workspace
+      container = DockerContainerPool.get_container(submission.execution_environment)
-    block.call
+      @container_id = container.id
-  ensure
+      @submission = submission
-    clean_workspace if @submission
+      create_workspace(container)
-  end
+      command = submission.execution_environment.send(:"#{cause}_command") % command_substitutions(filename)
-  private :execute_in_workspace
+      send_command(command, container, &block)
-
-  def execute_run_command(submission, filename, &block)
-    execute_in_workspace(submission) do
-      execute_command(@execution_environment.run_command % command_substitutions(filename), &block)
     end
   end
 
-  def execute_test_command(submission, filename)
+  def self.find_image_by_tag(tag)
-    execute_in_workspace(submission) do
-      execute_command(@execution_environment.test_command % command_substitutions(filename))
-    end
-  end
-
-  def find_image_by_tag(tag)
     Docker::Image.all.detect { |image| image.info['RepoTags'].flatten.include?(tag) }
   end
-  private :find_image_by_tag
+
+  def self.generate_remote_workspace_path
+    File.join(config[:workspace_root], SecureRandom.uuid)
+  end
 
   def self.image_tags
-    check_availability!
     Docker::Image.all.map { |image| image.info['RepoTags'] }.flatten.reject { |tag| tag.include?('<none>') }
   end
 
   def initialize(options = {})
-    self.class.check_availability!
     @execution_environment = options[:execution_environment]
     @user = options[:user]
-    @image = find_image_by_tag(@execution_environment.docker_image)
+    @image = self.class.find_image_by_tag(@execution_environment.docker_image)
     raise Error.new("Cannot find image #{@execution_environment.docker_image}!") unless @image
   end
 
@@ -118,38 +102,33 @@ class DockerClient
       raise Error.new('Docker configuration missing!')
     end
     Docker.url = config[:host] if config[:host]
+    check_availability!
+    FileUtils.mkdir_p(LOCAL_WORKSPACE_ROOT)
   end
 
-  def local_workspace_path
+  def self.local_workspace_path(container)
-    File.join(LOCAL_WORKSPACE_ROOT, @submission.id.to_s)
+    Pathname.new(container.binds.first.split(':').first.sub(config[:workspace_root], LOCAL_WORKSPACE_ROOT.to_s))
   end
-  private :local_workspace_path
 
-  def mapped_ports
+  def self.mapped_directories
-    @assigned_ports = []
+    ["#{generate_remote_workspace_path}:#{CONTAINER_WORKSPACE_PATH}"]
-    (@execution_environment.exposed_ports || '').gsub(/\s/, '').split(',').map do |port|
+  end
-      @assigned_ports << PortPool.available_port
+
-      ["#{port}/tcp", [{'HostPort' => @assigned_ports.last.to_s}]]
+  def self.mapped_ports(execution_environment)
+    (execution_environment.exposed_ports || '').gsub(/\s/, '').split(',').map do |port|
+      ["#{port}/tcp", [{'HostPort' => PortPool.available_port.to_s}]]
     end.to_h
   end
-  private :mapped_ports
 
   def self.pull(docker_image)
     `docker pull #{docker_image}` if docker_image
   end
 
-  def remote_workspace_path
+  def send_command(command, container, &block)
-    File.join(self.class.config[:workspace_root], @submission.id.to_s)
-  end
-  private :remote_workspace_path
-
-  def start_container(container, &block)
     Timeout::timeout(@execution_environment.permitted_execution_time) do
-      container.start('Binds' => bound_folders, 'PortBindings' => mapped_ports)
-      container.wait(@execution_environment.permitted_execution_time)
       stderr = []
       stdout = []
-      container.streaming_logs(stderr: true, stdout: true) do |stream, chunk|
+      container.attach(stdin: StringIO.new(command)) do |stream, chunk|
         block.call(stream, chunk) if block_given?
         if stream == :stderr
           stderr.push(chunk)
@@ -159,15 +138,13 @@ class DockerClient
       end
       {status: :ok, stderr: stderr.join, stdout: stdout.join}
     end
-  rescue Docker::Error::TimeoutError, Timeout::Error
+  rescue Timeout::Error
     {status: :timeout}
   ensure
-    self.class.destroy_container(container)
+    Concurrent::Future.execute { self.class.destroy_container(container) }
   end
-  private :start_container
+  private :send_command
 end
 
 class DockerClient::Error < RuntimeError
 end
-
-DockerClient.initialize_environment

lib/docker_container_pool.rb

@@ -0,0 +1,52 @@
+require 'concurrent/future'
+require 'concurrent/timer_task'
+require 'concurrent/utilities'
+
+class DockerContainerPool
+  @containers = ThreadSafe::Hash[ExecutionEnvironment.all.map { |execution_environment| [execution_environment.id, ThreadSafe::Array.new] }]
+
+  def self.clean_up
+    @refill_task.try(:shutdown)
+    @containers.each do |key, value|
+      while !value.empty? do
+        DockerClient.destroy_container(value.shift)
+      end
+    end
+  end
+
+  def self.config
+    @config ||= CodeOcean::Config.new(:docker).read(erb: true)[:pool]
+  end
+
+  def self.create_container(execution_environment)
+    DockerClient.create_container(execution_environment)
+  end
+
+  def self.get_container(execution_environment)
+    if config[:active]
+      @containers[execution_environment.id].try(:shift) || create_container(execution_environment)
+    else
+      create_container(execution_environment)
+    end
+  end
+
+  def self.quantities
+    @containers.map { |key, value| [key, value.length] }.to_h
+  end
+
+  def self.refill
+    ExecutionEnvironment.all.each do |execution_environment|
+      refill_count = execution_environment.pool_size - @containers[execution_environment.id].length
+      if refill_count > 0
+        Concurrent::Future.execute do
+          @containers[execution_environment.id] += refill_count.times.map { create_container(execution_environment) }
+        end
+      end
+    end
+  end
+
+  def self.start_refill_task
+    @refill_task = Concurrent::TimerTask.new(execution_interval: config[:interval], run_now: true) { refill }
+    @refill_task.execute
+  end
+end

spec/controllers/execution_environments_controller_spec.rb

@@ -61,7 +61,7 @@ describe ExecutionEnvironmentsController do
 
     before(:each) do
       expect(DockerClient).to receive(:new).with(execution_environment: execution_environment, user: user).and_call_original
-      expect_any_instance_of(DockerClient).to receive(:execute_command).with(command)
+      expect_any_instance_of(DockerClient).to receive(:execute_arbitrary_command).with(command)
       post :execute_command, command: command, id: execution_environment.id
     end
 

spec/factories/execution_environment.rb

@@ -5,6 +5,7 @@ FactoryGirl.define do
     help
     name 'CoffeeScript'
     permitted_execution_time 10.seconds
+    pool_size 0
     run_command 'coffee'
     singleton_execution_environment
   end
@@ -15,6 +16,7 @@ FactoryGirl.define do
     help
     name 'HTML5'
     permitted_execution_time 10.seconds
+    pool_size 0
     run_command 'touch'
     singleton_execution_environment
     test_command 'rspec %{filename} --format documentation'
@@ -27,6 +29,7 @@ FactoryGirl.define do
     help
     name 'Java 8'
     permitted_execution_time 10.seconds
+    pool_size 0
     run_command 'make run'
     singleton_execution_environment
     test_command 'make test CLASS_NAME="%{class_name}" FILENAME="%{filename}"'
@@ -39,6 +42,7 @@ FactoryGirl.define do
     help
     name 'JRuby 1.7'
     permitted_execution_time 10.seconds
+    pool_size 0
     run_command 'ruby %{filename}'
     singleton_execution_environment
     test_command 'rspec %{filename} --format documentation'
@@ -51,6 +55,7 @@ FactoryGirl.define do
     help
     name 'Node.js'
     permitted_execution_time 10.seconds
+    pool_size 0
     run_command 'node %{filename}'
     singleton_execution_environment
   end
@@ -61,6 +66,7 @@ FactoryGirl.define do
     help
     name 'Python 2.7'
     permitted_execution_time 10.seconds
+    pool_size 0
     run_command 'python %{filename}'
     singleton_execution_environment
     test_command 'python -m unittest --verbose %{module_name}'
@@ -73,6 +79,7 @@ FactoryGirl.define do
     help
     name 'Ruby 2.1'
     permitted_execution_time 10.seconds
+    pool_size 0
     run_command 'ruby %{filename}'
     singleton_execution_environment
     test_command 'rspec %{filename} --format documentation'
@@ -86,6 +93,7 @@ FactoryGirl.define do
     help
     name 'Sinatra'
     permitted_execution_time 15.minutes
+    pool_size 0
     run_command 'ruby %{filename}'
     singleton_execution_environment
     test_command 'rspec %{filename} --format documentation'
@@ -98,6 +106,7 @@ FactoryGirl.define do
     help
     name 'SQLite'
     permitted_execution_time 1.minute
+    pool_size 0
     run_command 'sqlite3 /database.db -init %{filename} -html'
     singleton_execution_environment
     test_command 'ruby %{filename}'

spec/lib/docker_client_spec.rb

@@ -4,28 +4,11 @@ require 'seeds_helper'
 describe DockerClient, docker: true do
   let(:command) { 'whoami' }
   let(:docker_client) { DockerClient.new(execution_environment: FactoryGirl.build(:ruby), user: FactoryGirl.build(:admin)) }
+  let(:execution_environment) { FactoryGirl.build(:ruby) }
   let(:image) { double }
   let(:submission) { FactoryGirl.create(:submission) }
   let(:workspace_path) { '/tmp' }
 
-  describe '#bound_folders' do
-    context 'when executing a submission' do
-      before(:each) { docker_client.instance_variable_set(:@submission, submission) }
-
-      it 'returns a submission-specific mapping' do
-        mapping = docker_client.send(:bound_folders).first
-        expect(mapping).to include(submission.id.to_s)
-        expect(mapping).to end_with(DockerClient::CONTAINER_WORKSPACE_PATH)
-      end
-    end
-
-    context 'when executing a single command' do
-      it 'returns an empty mapping' do
-        expect(docker_client.send(:bound_folders)).to eq([])
-      end
-    end
-  end
-
   describe '.check_availability!' do
     context 'when a socket error occurs' do
       it 'raises an error' do
@@ -42,111 +25,126 @@ describe DockerClient, docker: true do
     end
   end
 
-  describe '#clean_workspace' do
+  describe '.create_container' do
-    it 'removes the submission-specific directory' do
+    after(:each) { DockerClient.create_container(execution_environment) }
-      expect(docker_client).to receive(:local_workspace_path).and_return(workspace_path)
+
-      expect(FileUtils).to receive(:rm_rf).with(workspace_path)
+    it 'uses the correct Docker image' do
-      docker_client.send(:clean_workspace)
+      expect(DockerClient).to receive(:find_image_by_tag).with(execution_environment.docker_image).and_call_original
     end
-  end
 
-  describe '#create_container' do
+    it 'creates a container waiting for input' do
-    let(:image_tag) { 'tag' }
+      expect(Docker::Container).to receive(:create).with('Image' => kind_of(String), 'OpenStdin' => true, 'StdinOnce' => true).and_call_original
-    before(:each) { docker_client.instance_variable_set(:@image, image) }
+    end
 
-    it 'creates a container' do
+    it 'starts the container' do
-      expect(image).to receive(:info).and_return({'RepoTags' => [image_tag]})
+      expect_any_instance_of(Docker::Container).to receive(:start)
-      expect(Docker::Container).to receive(:create).with('Cmd' => command, 'Image' => image_tag)
+    end
-      docker_client.send(:create_container, command: command)
+
+    it 'configures mapped directories' do
+      expect(DockerClient).to receive(:mapped_directories).and_call_original
+      expect_any_instance_of(Docker::Container).to receive(:start).with(hash_including('Binds' => kind_of(Array)))
+    end
+
+    it 'configures mapped ports' do
+      expect(DockerClient).to receive(:mapped_ports).with(execution_environment).and_call_original
+      expect_any_instance_of(Docker::Container).to receive(:start).with(hash_including('PortBindings' => kind_of(Hash)))
     end
   end
 
   describe '#create_workspace' do
-    before(:each) { docker_client.instance_variable_set(:@submission, submission) }
+    let(:container) { double }
+
+    before(:each) do
+      docker_client.instance_variable_set(:@submission, submission)
+      expect(container).to receive(:binds).at_least(:once).and_return(["#{workspace_path}:#{DockerClient::CONTAINER_WORKSPACE_PATH}"])
+    end
+
+    after(:each) { docker_client.send(:create_workspace, container) }
 
     it 'creates submission-specific directories' do
-      expect(docker_client).to receive(:local_workspace_path).at_least(:once).and_return(workspace_path)
       expect(Dir).to receive(:mkdir).at_least(:once)
-      docker_client.send(:create_workspace)
+    end
+
+    it 'copies binary files' do
+      submission.collect_files.select { |file| file.file_type.binary? }.each do |file|
+        expect(docker_client).to receive(:copy_file_to_workspace).with(container: container, file: file)
+      end
+    end
+
+    it 'creates non-binary files' do
+      submission.collect_files.reject { |file| file.file_type.binary? }.each do |file|
+        expect(docker_client).to receive(:create_workspace_file).with(container: container, file: file)
+      end
     end
   end
 
   describe '#create_workspace_file' do
     let(:file) { FactoryGirl.build(:file, content: 'puts 42') }
     let(:file_path) { File.join(workspace_path, file.name_with_extension) }
+    after(:each) { File.delete(file_path) }
 
     it 'creates a file' do
-      expect(docker_client).to receive(:local_workspace_path).and_return(workspace_path)
+      expect(DockerClient).to receive(:local_workspace_path).and_return(workspace_path)
-      docker_client.send(:create_workspace_file, file: file)
+      docker_client.send(:create_workspace_file, container: CONTAINER, file: file)
       expect(File.exist?(file_path)).to be true
       expect(File.new(file_path, 'r').read).to eq(file.content)
-      File.delete(file_path)
     end
   end
 
   describe '.destroy_container' do
-    let(:container) { docker_client.send(:create_container, {command: command}) }
+    let(:container) { DockerClient.send(:create_container, execution_environment) }
     after(:each) { DockerClient.destroy_container(container) }
 
     it 'stops the container' do
       expect(container).to receive(:stop).and_return(container)
     end
 
-    it 'kills the container' do
+    it 'kills running processes' do
       expect(container).to receive(:kill)
     end
 
     it 'releases allocated ports' do
-      expect(container).to receive(:json).at_least(:once).and_return({'HostConfig' => {'PortBindings' => {foo: [{'HostPort' => '42'}]}}})
+      expect(container).to receive(:port_bindings).at_least(:once).and_return(foo: [{'HostPort' => '42'}])
       expect(PortPool).to receive(:release)
     end
 
+    it 'removes the mapped directory' do
+      expect(DockerClient).to receive(:local_workspace_path).and_return(workspace_path)
+      expect(FileUtils).to receive(:rm_rf).with(workspace_path)
+    end
+
     it 'deletes the container' do
-      expect(container).to receive(:delete)
+      expect(container).to receive(:delete).with(force: true)
     end
   end
 
-  describe '#execute_command' do
+  describe '#execute_arbitrary_command' do
-    after(:each) { docker_client.send(:execute_command, command) }
+    after(:each) { docker_client.execute_arbitrary_command(command) }
 
-    it 'creates a container' do
+    it 'takes a container from the pool' do
-      expect(docker_client).to receive(:create_container).with(command: ['bash', '-c', command]).and_call_original
+      expect(DockerContainerPool).to receive(:get_container).and_call_original
     end
 
-    it 'starts the container' do
+    it 'sends the command' do
-      expect(docker_client).to receive(:start_container)
+      expect(docker_client).to receive(:send_command).with(command, kind_of(Docker::Container))
     end
   end
 
-  describe '#execute_in_workspace' do
+  describe '#execute_run_command' do
-    let(:block) { Proc.new do; end }
+    let(:filename) { submission.exercise.files.detect { |file| file.role == 'main_file' }.name_with_extension }
-    let(:execute_in_workspace) { docker_client.send(:execute_in_workspace, submission, &block) }
+    after(:each) { docker_client.send(:execute_run_command, submission, filename) }
-    after(:each) { execute_in_workspace }
+
+    it 'takes a container from the pool' do
+      expect(DockerContainerPool).to receive(:get_container).with(submission.execution_environment).and_call_original
+    end
 
     it 'creates the workspace' do
       expect(docker_client).to receive(:create_workspace)
     end
 
-    it 'calls the block' do
-      expect(block).to receive(:call)
-    end
-
-    it 'cleans the workspace' do
-      expect(docker_client).to receive(:clean_workspace)
-    end
-  end
-
-  describe '#execute_run_command' do
-    let(:block) { Proc.new {} }
-    let(:filename) { submission.exercise.files.detect { |file| file.role == 'main_file' }.name_with_extension }
-    after(:each) { docker_client.send(:execute_run_command, submission, filename, &block) }
-
-    it 'is executed in the workspace' do
-      expect(docker_client).to receive(:execute_in_workspace)
-    end
-
     it 'executes the run command' do
-      expect(docker_client).to receive(:execute_command).with(kind_of(String), &block)
+      expect(submission.execution_environment).to receive(:run_command).and_call_original
+      expect(docker_client).to receive(:send_command).with(kind_of(String), kind_of(Docker::Container))
     end
   end
 
@@ -154,23 +152,36 @@ describe DockerClient, docker: true do
     let(:filename) { submission.exercise.files.detect { |file| file.role == 'teacher_defined_test' }.name_with_extension }
     after(:each) { docker_client.send(:execute_test_command, submission, filename) }
 
-    it 'is executed in the workspace' do
+    it 'takes a container from the pool' do
-      expect(docker_client).to receive(:execute_in_workspace)
+      expect(DockerContainerPool).to receive(:get_container).with(submission.execution_environment).and_call_original
+    end
+
+    it 'creates the workspace' do
+      expect(docker_client).to receive(:create_workspace)
     end
 
     it 'executes the test command' do
-      expect(docker_client).to receive(:execute_command).with(kind_of(String))
+      expect(submission.execution_environment).to receive(:test_command).and_call_original
+      expect(docker_client).to receive(:send_command).with(kind_of(String), kind_of(Docker::Container))
+    end
+  end
+
+  describe '.generate_remote_workspace_path' do
+    it 'includes the correct workspace root' do
+      expect(DockerClient.generate_remote_workspace_path).to start_with(DockerClient.config[:workspace_root])
+    end
+
+    it 'includes a UUID' do
+      expect(SecureRandom).to receive(:uuid).and_call_original
+      DockerClient.generate_remote_workspace_path
     end
   end
 
   describe '.initialize_environment' do
-    let(:config) { {connection_timeout: 3, host: 'tcp://8.8.8.8:2375', workspace_root: '/'} }
-
     context 'with complete configuration' do
-      before(:each) { expect(DockerClient).to receive(:config).at_least(:once).and_return(config) }
+      it 'creates the file directory' do
-
+        expect(FileUtils).to receive(:mkdir_p).with(DockerClient::LOCAL_WORKSPACE_ROOT)
-      it 'does not raise an error' do
+        DockerClient.initialize_environment
-        expect { DockerClient.initialize_environment }.not_to raise_error
       end
     end
 
@@ -183,75 +194,92 @@ describe DockerClient, docker: true do
     end
   end
 
-  describe '#local_workspace_path' do
+  describe '.local_workspace_path' do
-    before(:each) { docker_client.instance_variable_set(:@submission, submission) }
+    let(:container) { DockerClient.create_container(execution_environment) }
+    let(:local_workspace_path) { DockerClient.local_workspace_path(container) }
 
-    it 'includes the correct workspace root' do
+    it 'returns a path' do
-      expect(docker_client.send(:local_workspace_path)).to start_with(DockerClient::LOCAL_WORKSPACE_ROOT.to_s)
+      expect(local_workspace_path).to be_a(Pathname)
     end
 
-    it 'is submission-specific' do
+    it 'includes the correct workspace root' do
-      expect(docker_client.send(:local_workspace_path)).to end_with(submission.id.to_s)
+      expect(local_workspace_path.to_s).to start_with(DockerClient::LOCAL_WORKSPACE_ROOT.to_s)
     end
   end
 
-  describe '#remote_workspace_path' do
+  describe '.mapped_directories' do
-    before(:each) { docker_client.instance_variable_set(:@submission, submission) }
+    it 'returns a unique mapping' do
-
+      expect(DockerClient).to receive(:generate_remote_workspace_path).and_return(workspace_path)
-    it 'includes the correct workspace root' do
+      mapping = DockerClient.send(:mapped_directories).first
-      expect(docker_client.send(:remote_workspace_path)).to start_with(DockerClient.config[:workspace_root])
+      expect(mapping).to start_with(workspace_path)
-    end
+      expect(mapping).to end_with(DockerClient::CONTAINER_WORKSPACE_PATH)
-
-    it 'is submission-specific' do
-      expect(docker_client.send(:remote_workspace_path)).to end_with(submission.id.to_s)
     end
   end
 
-  describe '#start_container' do
+  describe '.mapped_ports' do
-    let(:container) { docker_client.send(:create_container, command: command) }
+    context 'with exposed ports' do
-    let(:start_container) { docker_client.send(:start_container, container) }
+      before(:each) { execution_environment.exposed_ports = '3000' }
 
-    it 'configures bound folders' do
+      it 'returns a mapping' do
-      expect(container).to receive(:start).with(hash_including('Binds' => kind_of(Array))).and_call_original
+        expect(DockerClient.mapped_ports(execution_environment)).to be_a(Hash)
-      start_container
+        expect(DockerClient.mapped_ports(execution_environment).length).to eq(1)
+      end
+
+      it 'retrieves available ports' do
+        expect(PortPool).to receive(:available_port)
+        DockerClient.mapped_ports(execution_environment)
+      end
     end
 
-    it 'configures bound ports' do
+    context 'without exposed ports' do
-      expect(container).to receive(:start).with(hash_including('PortBindings' => kind_of(Hash))).and_call_original
+      it 'returns an empty mapping' do
-      start_container
+        expect(DockerClient.mapped_ports(execution_environment)).to eq({})
+      end
+    end
+  end
+
+  describe '#send_command' do
+    let(:block) { Proc.new {} }
+    let(:container) { DockerClient.create_container(execution_environment) }
+    let(:send_command) { docker_client.send(:send_command, command, container, &block) }
+    after(:each) { send_command }
+
+    it 'limits the execution time' do
+      expect(Timeout).to receive(:timeout).at_least(:once).with(kind_of(Numeric)).and_call_original
     end
 
-    it 'starts the container' do
+    it 'provides the command to be executed as input' do
-      expect(container).to receive(:start).and_call_original
+      expect(container).to receive(:attach).with(stdin: kind_of(StringIO))
-      start_container
     end
 
-    it 'waits for the container to terminate' do
+    it 'calls the block' do
-      expect(container).to receive(:wait).with(kind_of(Numeric)).and_call_original
+      expect(block).to receive(:call)
-      start_container
     end
 
     context 'when a timeout occurs' do
-      before(:each) { expect(container).to receive(:wait).and_raise(Docker::Error::TimeoutError) }
+      before(:each) { expect(container).to receive(:attach).and_raise(Timeout::Error) }
 
-      it 'kills the container' do
+      it 'destroys the container asynchronously' do
-        expect(container).to receive(:kill)
+        expect(Concurrent::Future).to receive(:execute)
-        start_container
       end
 
       it 'returns a corresponding status' do
-        expect(start_container[:status]).to eq(:timeout)
+        expect(send_command[:status]).to eq(:timeout)
       end
     end
 
     context 'when the container terminates timely' do
+      it 'destroys the container asynchronously' do
+        expect(Concurrent::Future).to receive(:execute)
+      end
+
       it "returns the container's output" do
-        expect(start_container[:stderr]).to be_blank
+        expect(send_command[:stderr]).to be_blank
-        expect(start_container[:stdout]).to start_with('root')
+        expect(send_command[:stdout]).to start_with('root')
       end
 
       it 'returns a corresponding status' do
-        expect(start_container[:status]).to eq(:ok)
+        expect(send_command[:status]).to eq(:ok)
       end
     end
   end

spec/lib/docker_container_pool_spec.rb

@@ -0,0 +1,121 @@
+require 'rails_helper'
+
+describe DockerContainerPool do
+  let(:container) { double }
+
+  def reload_class
+    load('docker_container_pool.rb')
+  end
+  private :reload_class
+
+  before(:each) do
+    @execution_environment = FactoryGirl.create(:ruby)
+    reload_class
+  end
+
+  it 'uses thread-safe data structures' do
+    expect(DockerContainerPool.instance_variable_get(:@containers)).to be_a(ThreadSafe::Hash)
+    expect(DockerContainerPool.instance_variable_get(:@containers)[@execution_environment.id]).to be_a(ThreadSafe::Array)
+  end
+
+  describe '.clean_up' do
+    before(:each) { DockerContainerPool.instance_variable_set(:@refill_task, double) }
+    after(:each) { DockerContainerPool.clean_up }
+
+    it 'stops the refill task' do
+      expect(DockerContainerPool.instance_variable_get(:@refill_task)).to receive(:shutdown)
+    end
+
+    it 'destroys all containers' do
+      DockerContainerPool.instance_variable_get(:@containers).each do |key, value|
+        value.each do |container|
+          expect(DockerClient).to receive(:destroy_container).with(container)
+        end
+      end
+    end
+  end
+
+  describe '.get_container' do
+    context 'when active' do
+      before(:each) do
+        expect(DockerContainerPool).to receive(:config).and_return(active: true)
+      end
+
+      context 'with an available container' do
+        before(:each) { DockerContainerPool.instance_variable_get(:@containers)[@execution_environment.id].push(container) }
+
+        it 'takes a container from the pool' do
+          expect(DockerContainerPool).not_to receive(:create_container).with(@execution_environment)
+          expect(DockerContainerPool.get_container(@execution_environment)).to eq(container)
+        end
+      end
+
+      context 'without an available container' do
+        before(:each) do
+          expect(DockerContainerPool.instance_variable_get(:@containers)[@execution_environment.id]).to be_empty
+        end
+
+        it 'creates a new container' do
+          expect(DockerContainerPool).to receive(:create_container).with(@execution_environment)
+          DockerContainerPool.get_container(@execution_environment)
+        end
+      end
+    end
+
+    context 'when inactive' do
+      before(:each) do
+        expect(DockerContainerPool).to receive(:config).and_return(active: false)
+      end
+
+      it 'creates a new container' do
+        expect(DockerContainerPool).to receive(:create_container).with(@execution_environment)
+        DockerContainerPool.get_container(@execution_environment)
+      end
+    end
+  end
+
+  describe '.quantities' do
+    it 'maps execution environments to quantities of available containers' do
+      expect(DockerContainerPool.quantities.keys).to eq(ExecutionEnvironment.all.map(&:id))
+      expect(DockerContainerPool.quantities.values.uniq).to eq([0])
+    end
+  end
+
+  describe '.refill' do
+    after(:each) { DockerContainerPool.refill }
+
+    it 'regards all execution environments' do
+      ExecutionEnvironment.all.each do |execution_environment|
+        expect(DockerContainerPool.instance_variable_get(:@containers)).to receive(:[]).with(execution_environment.id).and_call_original
+      end
+    end
+
+    context 'with something to refill' do
+      before(:each) { @execution_environment.update(pool_size: 1) }
+
+      it 'works asynchronously' do
+        expect(Concurrent::Future).to receive(:execute)
+      end
+    end
+
+    context 'with nothing to refill' do
+      before(:each) { @execution_environment.update(pool_size: 0) }
+
+      it 'does nothing' do
+        expect(Concurrent::Future).not_to receive(:execute)
+      end
+    end
+  end
+
+  describe '.start_refill_task' do
+    after(:each) { DockerContainerPool.start_refill_task }
+
+    it 'creates an asynchronous task' do
+      expect(Concurrent::TimerTask).to receive(:new).and_call_original
+    end
+
+    it 'executes the task' do
+      expect_any_instance_of(Concurrent::TimerTask).to receive(:execute)
+    end
+  end
+end

spec/models/execution_environment_spec.rb

@@ -4,10 +4,9 @@ describe ExecutionEnvironment do
   let(:execution_environment) { ExecutionEnvironment.create }
 
   it 'validates that the Docker image works', docker: true do
-    expect(execution_environment).to receive(:working_docker_image?).and_call_original
     expect(execution_environment).to receive(:validate_docker_image?).and_return(true)
-    execution_environment.update(docker_image: 'invalid')
+    expect(execution_environment).to receive(:working_docker_image?)
-    expect(execution_environment.errors[:docker_image]).to be_present
+    execution_environment.update(docker_image: FactoryGirl.attributes_for(:ruby)[:docker_image])
   end
 
   it 'validates the presence of a Docker image name' do
@@ -18,15 +17,26 @@ describe ExecutionEnvironment do
     expect(execution_environment.errors[:name]).to be_present
   end
 
-  it 'validates the numericality of a permitted run time' do
+  it 'validates the numericality of the permitted run time' do
     execution_environment.update(permitted_execution_time: Math::PI)
     expect(execution_environment.errors[:permitted_execution_time]).to be_present
   end
 
   it 'validates the presence of a permitted run time' do
+    execution_environment.update(permitted_execution_time: nil)
     expect(execution_environment.errors[:permitted_execution_time]).to be_present
   end
 
+  it 'validates the numericality of the pool size' do
+    execution_environment.update(pool_size: Math::PI)
+    expect(execution_environment.errors[:pool_size]).to be_present
+  end
+
+  it 'validates the presence of a pool size' do
+    execution_environment.update(pool_size: nil)
+    expect(execution_environment.errors[:pool_size]).to be_present
+  end
+
   it 'validates the presence of a run command' do
     expect(execution_environment.errors[:run_command]).to be_present
   end
@@ -38,39 +48,40 @@ describe ExecutionEnvironment do
 
   describe '#validate_docker_image?' do
     it 'is false in the test environment' do
+      expect(Rails.env.test?).to be true
       expect(execution_environment.send(:validate_docker_image?)).to be false
     end
 
     it 'is false without a Docker image' do
-      allow(Rails).to receive(:env).and_return('production')
+      expect(execution_environment.docker_image).to be_blank
       expect(execution_environment.send(:validate_docker_image?)).to be false
     end
 
     it 'is true otherwise' do
-      execution_environment.docker_image = DockerClient.image_tags.first
+      execution_environment.docker_image = FactoryGirl.attributes_for(:ruby)[:docker_image]
-      expect(Rails).to receive(:env).and_return('production')
+      allow(Rails.env).to receive(:test?).and_return(false)
       expect(execution_environment.send(:validate_docker_image?)).to be true
     end
   end
 
   describe '#working_docker_image?', docker: true do
     let(:working_docker_image?) { execution_environment.send(:working_docker_image?) }
-    before(:each) { expect_any_instance_of(DockerClient).to receive(:find_image_by_tag).and_return(Object.new) }
+    before(:each) { expect(DockerClient).to receive(:find_image_by_tag).and_return(Object.new) }
 
     it 'instantiates a Docker client' do
       expect(DockerClient).to receive(:new).with(execution_environment: execution_environment).and_call_original
-      expect_any_instance_of(DockerClient).to receive(:execute_command).and_return({})
+      expect_any_instance_of(DockerClient).to receive(:execute_arbitrary_command).and_return({})
       working_docker_image?
     end
 
     it 'executes the validation command' do
-      expect_any_instance_of(DockerClient).to receive(:execute_command).with(ExecutionEnvironment::VALIDATION_COMMAND).and_return({})
+      expect_any_instance_of(DockerClient).to receive(:execute_arbitrary_command).with(ExecutionEnvironment::VALIDATION_COMMAND).and_return({})
       working_docker_image?
     end
 
     context 'when the command produces an error' do
       it 'adds an error' do
-        expect_any_instance_of(DockerClient).to receive(:execute_command).and_return({stderr: 'command not found'})
+        expect_any_instance_of(DockerClient).to receive(:execute_arbitrary_command).and_return({stderr: 'command not found'})
         working_docker_image?
         expect(execution_environment.errors[:docker_image]).to be_present
       end
@@ -78,7 +89,7 @@ describe ExecutionEnvironment do
 
     context 'when the Docker client produces an error' do
       it 'adds an error' do
-        expect_any_instance_of(DockerClient).to receive(:execute_command).and_raise(DockerClient::Error)
+        expect_any_instance_of(DockerClient).to receive(:execute_arbitrary_command).and_raise(DockerClient::Error)
         working_docker_image?
         expect(execution_environment.errors[:docker_image]).to be_present
       end

spec/support/docker.rb

@@ -1,12 +1,15 @@
-IMAGE = Docker::Image.new(Docker::Connection.new('http://example.org', {}), 'id' => SecureRandom.hex)
+CONTAINER = Docker::Container.send(:new, Docker::Connection.new('http://example.org', {}), 'id' => SecureRandom.hex)
+IMAGE = Docker::Image.new(Docker::Connection.new('http://example.org', {}), 'id' => SecureRandom.hex, 'RepoTags' => [FactoryGirl.attributes_for(:ruby)[:docker_image]])
 
 RSpec.configure do |config|
   config.before(:each) do |example|
     unless example.metadata[:docker]
       allow(DockerClient).to receive(:check_availability!).and_return(true)
+      allow(DockerClient).to receive(:create_container).and_return(CONTAINER)
+      allow(DockerClient).to receive(:find_image_by_tag).and_return(IMAGE)
       allow(DockerClient).to receive(:image_tags).and_return([IMAGE])
-      allow_any_instance_of(DockerClient).to receive(:execute_command).and_return({})
+      allow(DockerClient).to receive(:local_workspace_path).and_return(Pathname.new('/tmp'))
-      allow_any_instance_of(DockerClient).to receive(:find_image_by_tag).and_return(IMAGE)
+      allow_any_instance_of(DockerClient).to receive(:send_command).and_return({})
       allow_any_instance_of(ExecutionEnvironment).to receive(:working_docker_image?)
     end
   end