diff --git a/app/controllers/execution_environments_controller.rb b/app/controllers/execution_environments_controller.rb index 308425f7..2eed11a4 100644 --- a/app/controllers/execution_environments_controller.rb +++ b/app/controllers/execution_environments_controller.rb @@ -29,7 +29,7 @@ class ExecutionEnvironmentsController < ApplicationController end def execution_environment_params - params[:execution_environment].permit(:docker_image, :exposed_ports, :editor_mode, :file_extension, :file_type_id, :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) + params[:execution_environment].permit(:docker_image, :exposed_ports, :editor_mode, :file_extension, :file_type_id, :help, :indent_size, :memory_limit, :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 diff --git a/app/models/execution_environment.rb b/app/models/execution_environment.rb index 0ecc2424..dd8ee8b4 100644 --- a/app/models/execution_environment.rb +++ b/app/models/execution_environment.rb @@ -15,6 +15,7 @@ class ExecutionEnvironment < ActiveRecord::Base validate :valid_test_setup? validate :working_docker_image?, if: :validate_docker_image? validates :docker_image, presence: true + validates :memory_limit, numericality: {greater_than_or_equal_to: DockerClient::MINIMUM_MEMORY_LIMIT, only_integer: true}, 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 diff --git a/app/views/execution_environments/_form.html.slim b/app/views/execution_environments/_form.html.slim index dc4a759d..67bf216b 100644 --- a/app/views/execution_environments/_form.html.slim +++ b/app/views/execution_environments/_form.html.slim @@ -17,6 +17,9 @@ = f.label(:exposed_ports) = f.text_field(:exposed_ports, class: 'form-control', placeholder: '3000, 4000') .help-block == t('.hints.exposed_ports') + .form-group + = f.label(:memory_limit) + = f.number_field(:memory_limit, class: 'form-control', min: DockerClient::MINIMUM_MEMORY_LIMIT, value: f.object.memory_limit || DockerClient::DEFAULT_MEMORY_LIMIT) .form-group = f.label(:permitted_execution_time) = f.number_field(:permitted_execution_time, class: 'form-control', min: 1) diff --git a/app/views/execution_environments/show.html.slim b/app/views/execution_environments/show.html.slim index 60c01364..b0cc38b2 100644 --- a/app/views/execution_environments/show.html.slim +++ b/app/views/execution_environments/show.html.slim @@ -5,7 +5,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)) = row(label: 'execution_environment.file_type', value: @execution_environment.file_type.present? ? link_to(@execution_environment.file_type, @execution_environment.file_type) : nil) -- [:docker_image, :exposed_ports, :permitted_execution_time, :pool_size, :run_command, :test_command].each do |attribute| +- [:docker_image, :exposed_ports, :memory_limit, :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)) diff --git a/config/locales/de.yml b/config/locales/de.yml index 0bacc13e..800a1497 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -13,6 +13,7 @@ de: file_type: Standard-Dateityp file_type_id: Standard-Dateityp help: Hilfetext + memory_limit: Speicher-Limit (in MB) name: Name permitted_execution_time: Erlaubte Ausführungszeit (in Sekunden) pool_size: Docker-Container-Pool-Größe diff --git a/config/locales/en.yml b/config/locales/en.yml index e43dee29..33e101f2 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -13,6 +13,7 @@ en: file_type: Default File Type file_type_id: Default File Type help: Help Text + memory_limit: Memory Limit (in MB) name: Name permitted_execution_time: Permitted Execution Time (in Seconds) pool_size: Docker Container Pool Size diff --git a/db/migrate/20150317083739_add_memory_limit_to_execution_environments.rb b/db/migrate/20150317083739_add_memory_limit_to_execution_environments.rb new file mode 100644 index 00000000..b2ee957c --- /dev/null +++ b/db/migrate/20150317083739_add_memory_limit_to_execution_environments.rb @@ -0,0 +1,11 @@ +class AddMemoryLimitToExecutionEnvironments < ActiveRecord::Migration + def change + add_column :execution_environments, :memory_limit, :integer + + reversible do |direction| + direction.up do + ExecutionEnvironment.update_all(memory_limit: DockerClient::DEFAULT_MEMORY_LIMIT) + end + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 94538e85..3cfd596c 100644 --- a/db/schema.rb +++ b/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: 20150310150712) do +ActiveRecord::Schema.define(version: 20150317083739) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -46,6 +46,7 @@ ActiveRecord::Schema.define(version: 20150310150712) do t.string "user_type" t.integer "pool_size" t.integer "file_type_id" + t.integer "memory_limit" end create_table "exercises", force: true do |t| diff --git a/lib/docker_client.rb b/lib/docker_client.rb index 360508f9..f47dad26 100644 --- a/lib/docker_client.rb +++ b/lib/docker_client.rb @@ -2,7 +2,9 @@ require 'concurrent' class DockerClient CONTAINER_WORKSPACE_PATH = '/workspace' + DEFAULT_MEMORY_LIMIT = 256 LOCAL_WORKSPACE_ROOT = Rails.root.join('tmp', 'files', Rails.env) + MINIMUM_MEMORY_LIMIT = 4 RETRY_COUNT = 2 attr_reader :assigned_ports @@ -23,16 +25,32 @@ class DockerClient @config ||= CodeOcean::Config.new(:docker).read(erb: true) end + def self.container_creation_options(execution_environment) + { + 'Image' => find_image_by_tag(execution_environment.docker_image).info['RepoTags'].first, + 'Memory' => execution_environment.memory_limit.megabytes, + 'OpenStdin' => true, + 'StdinOnce' => true + } + end + + def self.container_start_options(execution_environment, local_workspace_path) + { + 'Binds' => mapped_directories(local_workspace_path), + 'PortBindings' => mapped_ports(execution_environment) + } + end + def copy_file_to_workspace(options = {}) FileUtils.cp(options[:file].native_file.path, local_file_path(options)) end def self.create_container(execution_environment) tries ||= 0 - container = Docker::Container.create('Image' => find_image_by_tag(execution_environment.docker_image).info['RepoTags'].first, 'OpenStdin' => true, 'StdinOnce' => true) + container = Docker::Container.create(container_creation_options(execution_environment)) local_workspace_path = generate_local_workspace_path FileUtils.mkdir(local_workspace_path) - container.start('Binds' => mapped_directories(local_workspace_path), 'PortBindings' => mapped_ports(execution_environment)) + container.start(container_start_options(execution_environment, local_workspace_path)) container rescue Docker::Error::NotFoundError => error destroy_container(container) diff --git a/spec/factories/execution_environment.rb b/spec/factories/execution_environment.rb index be78029e..1e1b10fb 100644 --- a/spec/factories/execution_environment.rb +++ b/spec/factories/execution_environment.rb @@ -1,6 +1,7 @@ FactoryGirl.define do factory :coffee_script, class: ExecutionEnvironment do created_by_teacher + default_memory_limit docker_image 'hklement/ubuntu-coffee:latest' association :file_type, factory: :dot_coffee help @@ -13,6 +14,7 @@ FactoryGirl.define do factory :html, class: ExecutionEnvironment do created_by_teacher + default_memory_limit docker_image 'hklement/ubuntu-html:latest' association :file_type, factory: :dot_html help @@ -27,6 +29,7 @@ FactoryGirl.define do factory :java, class: ExecutionEnvironment do created_by_teacher + default_memory_limit docker_image 'hklement/ubuntu-java:latest' association :file_type, factory: :dot_java help @@ -41,6 +44,7 @@ FactoryGirl.define do factory :jruby, class: ExecutionEnvironment do created_by_teacher + default_memory_limit docker_image 'hklement/ubuntu-jruby:latest' association :file_type, factory: :dot_rb help @@ -55,6 +59,7 @@ FactoryGirl.define do factory :node_js, class: ExecutionEnvironment do created_by_teacher + default_memory_limit docker_image 'hklement/ubuntu-node:latest' association :file_type, factory: :dot_js help @@ -67,6 +72,7 @@ FactoryGirl.define do factory :python, class: ExecutionEnvironment do created_by_teacher + default_memory_limit docker_image 'hklement/ubuntu-python:latest' association :file_type, factory: :dot_py help @@ -81,6 +87,7 @@ FactoryGirl.define do factory :ruby, class: ExecutionEnvironment do created_by_teacher + default_memory_limit docker_image 'hklement/ubuntu-ruby:latest' association :file_type, factory: :dot_rb help @@ -95,6 +102,7 @@ FactoryGirl.define do factory :sinatra, class: ExecutionEnvironment do created_by_teacher + default_memory_limit docker_image 'hklement/ubuntu-sinatra:latest' association :file_type, factory: :dot_rb exposed_ports '4567' @@ -110,6 +118,7 @@ FactoryGirl.define do factory :sqlite, class: ExecutionEnvironment do created_by_teacher + default_memory_limit docker_image 'hklement/ubuntu-sqlite:latest' association :file_type, factory: :dot_sql help @@ -122,6 +131,10 @@ FactoryGirl.define do testing_framework 'SqlResultSetComparatorAdapter' end + trait :default_memory_limit do + memory_limit DockerClient::DEFAULT_MEMORY_LIMIT + end + trait :help do help { Forgery(:lorem_ipsum).words(Forgery(:basic).number(at_least: 50, at_most: 100)) } end diff --git a/spec/lib/docker_client_spec.rb b/spec/lib/docker_client_spec.rb index 9af7b90b..287891c7 100644 --- a/spec/lib/docker_client_spec.rb +++ b/spec/lib/docker_client_spec.rb @@ -25,6 +25,34 @@ describe DockerClient, docker: true do end end + describe '.container_creation_options' do + let(:container_creation_options) { described_class.container_creation_options(execution_environment) } + + it 'specifies the Docker image' do + expect(container_creation_options).to include('Image' => described_class.find_image_by_tag(execution_environment.docker_image).info['RepoTags'].first) + end + + it 'specifies the memory limit' do + expect(container_creation_options).to include('Memory' => execution_environment.memory_limit.megabytes) + end + + it 'specifies to open the standard input stream once' do + expect(container_creation_options).to include('OpenStdin' => true, 'StdinOnce' => true) + end + end + + describe '.container_start_options' do + let(:container_start_options) { described_class.container_start_options(execution_environment, '') } + + it 'specifies mapped directories' do + expect(container_start_options).to include('Binds' => kind_of(Array)) + end + + it 'specifies mapped ports' do + expect(container_start_options).to include('PortBindings' => kind_of(Hash)) + end + end + describe '.create_container' do let(:create_container) { described_class.create_container(execution_environment) } @@ -39,25 +67,25 @@ describe DockerClient, docker: true do create_container end - it 'creates a container waiting for input' do - expect(Docker::Container).to receive(:create).with('Image' => kind_of(String), 'OpenStdin' => true, 'StdinOnce' => true).and_call_original + it 'creates a container' do + expect(described_class).to receive(:container_creation_options).with(execution_environment).and_call_original + expect(Docker::Container).to receive(:create).with(kind_of(Hash)).and_call_original create_container end it 'starts the container' do - expect_any_instance_of(Docker::Container).to receive(:start) + expect(described_class).to receive(:container_start_options).with(execution_environment, kind_of(String)).and_call_original + expect_any_instance_of(Docker::Container).to receive(:start).with(kind_of(Hash)).and_call_original create_container end it 'configures mapped directories' do expect(described_class).to receive(:mapped_directories).and_call_original - expect_any_instance_of(Docker::Container).to receive(:start).with(hash_including('Binds' => kind_of(Array))) create_container end it 'configures mapped ports' do expect(described_class).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))) create_container end diff --git a/spec/models/execution_environment_spec.rb b/spec/models/execution_environment_spec.rb index ad603741..a5ad2ae8 100644 --- a/spec/models/execution_environment_spec.rb +++ b/spec/models/execution_environment_spec.rb @@ -13,6 +13,21 @@ describe ExecutionEnvironment do expect(execution_environment.errors[:docker_image]).to be_present end + it 'validates the minimum value of the memory limit' do + execution_environment.update(memory_limit: DockerClient::MINIMUM_MEMORY_LIMIT / 2) + expect(execution_environment.errors[:memory_limit]).to be_present + end + + it 'validates the numericality of the memory limit' do + execution_environment.update(memory_limit: Math::PI) + expect(execution_environment.errors[:memory_limit]).to be_present + end + + it 'validates the presence of a memory limit' do + execution_environment.update(memory_limit: nil) + expect(execution_environment.errors[:memory_limit]).to be_present + end + it 'validates the presence of a name' do expect(execution_environment.errors[:name]).to be_present end