From 4d1a49a0927bf4112ee0e710a31ad04a1f1453c2 Mon Sep 17 00:00:00 2001 From: Joel Low Date: Sun, 24 Jan 2016 09:22:24 +0800 Subject: [PATCH] Implement collecting the exit code from the container. --- .../evaluator/docker_container.rb | 27 ++++++++++++++++ .../models/programming_evaluation.rb | 1 + .../evaluate_programming_package_service.rb | 20 ++---------- .../evaluator/docker_container_spec.rb | 31 +++++++++++++++++++ .../models/programming_evaluation_spec.rb | 4 +++ ...aluate_programming_package_service_spec.rb | 21 +++---------- 6 files changed, 70 insertions(+), 34 deletions(-) diff --git a/lib/coursemology/evaluator/docker_container.rb b/lib/coursemology/evaluator/docker_container.rb index 5d7e355..f1e434f 100644 --- a/lib/coursemology/evaluator/docker_container.rb +++ b/lib/coursemology/evaluator/docker_container.rb @@ -22,6 +22,33 @@ def pull_image(image) end end + # Waits for the container to exit the Running state. + # + # This will time out for long running operations, so keep retrying until we return. + # + # @param [Fixnum|nil] time The amount of time to wait. + # @return [Fixnum] The exit code of the container. + def wait(time = nil) + container_state = info + while container_state.fetch('State', {}).fetch('Running', true) + super + refresh! + container_state = info + end + + container_state['State']['ExitCode'] + rescue Docker::Error::TimeoutError + retry + end + + # Gets the exit code of the container. + # + # @return [Fixnum] The exit code of the container, if +wait+ was called before. + # @return [nil] If the container is still running, or +wait+ was not called. + def exit_code + info.fetch('State', {})['ExitCode'] + end + def delete ActiveSupport::Notifications.instrument('destroy.docker.evaluator.coursemology', container: id) do diff --git a/lib/coursemology/evaluator/models/programming_evaluation.rb b/lib/coursemology/evaluator/models/programming_evaluation.rb index 77da2a0..11e4ef4 100644 --- a/lib/coursemology/evaluator/models/programming_evaluation.rb +++ b/lib/coursemology/evaluator/models/programming_evaluation.rb @@ -48,6 +48,7 @@ def evaluate self.stdout = result.stdout self.stderr = result.stderr self.test_report = result.test_report + self.exit_code = result.exit_code end private diff --git a/lib/coursemology/evaluator/services/evaluate_programming_package_service.rb b/lib/coursemology/evaluator/services/evaluate_programming_package_service.rb index 65a9fc9..f4d724e 100644 --- a/lib/coursemology/evaluator/services/evaluate_programming_package_service.rb +++ b/lib/coursemology/evaluator/services/evaluate_programming_package_service.rb @@ -1,5 +1,5 @@ class Coursemology::Evaluator::Services::EvaluateProgrammingPackageService - Result = Struct.new(:stdout, :stderr, :test_report) + Result = Struct.new(:stdout, :stderr, :test_report, :exit_code) # The path to the Coursemology user home directory. HOME_PATH = '/home/coursemology'.freeze @@ -109,28 +109,14 @@ def copy_archive(zip_file, tar_file, prefix = nil) def execute_package(container) container.start! - execute_package_wait(container) - end - - # Waits for the container to exit the Running state. - # - # This will time out for long running operations, so keep retrying until we return. - def execute_package_wait(container) - container_state = container.info - while container_state.fetch('State', {}).fetch('Running', true) - container.wait - container.refresh! - container_state = container.info - end - rescue Docker::Error::TimeoutError - retry + container.wait end def extract_result(container) logs = container.logs(stdout: true, stderr: true) _, stdout, stderr = Coursemology::Evaluator::Utils.parse_docker_stream(logs) - Result.new(stdout, stderr, extract_test_report(container)) + Result.new(stdout, stderr, extract_test_report(container), container.exit_code) end def extract_test_report(container) diff --git a/spec/coursemology/evaluator/docker_container_spec.rb b/spec/coursemology/evaluator/docker_container_spec.rb index d6648ae..bb70230 100644 --- a/spec/coursemology/evaluator/docker_container_spec.rb +++ b/spec/coursemology/evaluator/docker_container_spec.rb @@ -1,6 +1,8 @@ RSpec.describe Coursemology::Evaluator::DockerContainer do let(:image) { 'coursemology/evaluator-image-python:2.7' } + let(:delete_subject) { true } subject { Coursemology::Evaluator::DockerContainer.create(image) } + after { subject.delete if delete_subject } describe '.create' do it 'instruments the pull' do @@ -12,7 +14,36 @@ end end + describe '#wait' do + it 'retries until the container finishes' do + subject.start! + expect(subject).to receive(:refresh!).and_call_original.at_least(:once) + + expect(subject.wait(0)).not_to be_nil + end + + it 'returns the exit code of the container' do + expect(subject.wait).to eq(subject.exit_code) + end + end + + describe '#exit_code' do + context 'when the container has been waited upon' do + it 'returns the exit code of the container' do + subject.wait + expect(subject.exit_code).not_to be_nil + end + end + + context 'when the container is still running' do + it 'returns nil' do + expect(subject.exit_code).to be_nil + end + end + end + describe '#delete' do + let(:delete_subject) { false } it 'instruments the destruction' do expect { subject.delete }.to instrument_notification('destroy.docker.evaluator.coursemology') end diff --git a/spec/coursemology/evaluator/models/programming_evaluation_spec.rb b/spec/coursemology/evaluator/models/programming_evaluation_spec.rb index 9f4d6e7..c6d3604 100644 --- a/spec/coursemology/evaluator/models/programming_evaluation_spec.rb +++ b/spec/coursemology/evaluator/models/programming_evaluation_spec.rb @@ -83,5 +83,9 @@ it 'sets the test_report attribute' do expect { subject.evaluate }.to change { subject.test_report } end + + it 'sets the exit_code attribute' do + expect { subject.evaluate }.to change { subject.exit_code } + end end end diff --git a/spec/coursemology/evaluator/services/evaluate_programming_package_service_spec.rb b/spec/coursemology/evaluator/services/evaluate_programming_package_service_spec.rb index b57b81a..eb31f82 100644 --- a/spec/coursemology/evaluator/services/evaluate_programming_package_service_spec.rb +++ b/spec/coursemology/evaluator/services/evaluate_programming_package_service_spec.rb @@ -82,23 +82,6 @@ def evaluate_result end end - describe '#execute_package_wait' do - let(:container) { subject.send(:create_container, image) } - after { subject.send(:destroy_container, container) } - - it 'retries until the container finishes' do - called = 0 - expect(container).to receive(:wait).and_wrap_original do |block, *args| - args.push(0.second) if called == 0 - called += 1 - - block.call(*args) - end.at_least(:twice) - - subject.send(:execute_package_wait, container) - end - end - describe '#extract_result' do let(:container) do subject.send(:create_container, image).tap do |container| @@ -112,6 +95,10 @@ def evaluate_result expect(result.stdout).not_to include("\u0000") expect(result.stderr).not_to include("\u0000") end + + it 'sets the return code of the container' do + expect(subject.send(:extract_result, container).exit_code).to eq(2) + end end describe '#extract_test_report' do