Add retries to working_docker_image?
* Also synchronize images during save
This commit is contained in:
@ -15,9 +15,7 @@ class ExecutionEnvironmentsController < ApplicationController
|
|||||||
def create
|
def create
|
||||||
@execution_environment = ExecutionEnvironment.new(execution_environment_params)
|
@execution_environment = ExecutionEnvironment.new(execution_environment_params)
|
||||||
authorize!
|
authorize!
|
||||||
create_and_respond(object: @execution_environment) do
|
create_and_respond(object: @execution_environment)
|
||||||
sync_to_runner_management
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
|
|
||||||
def destroy
|
def destroy
|
||||||
@ -165,9 +163,7 @@ class ExecutionEnvironmentsController < ApplicationController
|
|||||||
end
|
end
|
||||||
|
|
||||||
def update
|
def update
|
||||||
update_and_respond(object: @execution_environment, params: execution_environment_params) do
|
update_and_respond(object: @execution_environment, params: execution_environment_params)
|
||||||
sync_to_runner_management
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
|
|
||||||
def sync_all_to_runner_management
|
def sync_all_to_runner_management
|
||||||
@ -186,11 +182,4 @@ class ExecutionEnvironmentsController < ApplicationController
|
|||||||
redirect_to ExecutionEnvironment, alert: t('execution_environments.index.synchronize_all.failure')
|
redirect_to ExecutionEnvironment, alert: t('execution_environments.index.synchronize_all.failure')
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
def sync_to_runner_management
|
|
||||||
unless Runner.management_active? && Runner.strategy_class.sync_environment(@execution_environment)
|
|
||||||
t('execution_environments.form.errors.not_synced_to_runner_management')
|
|
||||||
end
|
|
||||||
end
|
|
||||||
private :sync_to_runner_management
|
|
||||||
end
|
end
|
||||||
|
@ -79,13 +79,24 @@ class ExecutionEnvironment < ApplicationRecord
|
|||||||
private :validate_docker_image?
|
private :validate_docker_image?
|
||||||
|
|
||||||
def working_docker_image?
|
def working_docker_image?
|
||||||
runner = Runner.for(author, self)
|
sync_runner_environment
|
||||||
output = runner.execute_command(VALIDATION_COMMAND)
|
retries = 0
|
||||||
errors.add(:docker_image, "error: #{output[:stderr]}") if output[:stderr].present?
|
begin
|
||||||
rescue Runner::Error::NotAvailable => e
|
runner = Runner.for(author, self)
|
||||||
Rails.logger.info("The Docker image could not be verified: #{e}")
|
output = runner.execute_command(VALIDATION_COMMAND)
|
||||||
rescue Runner::Error => e
|
errors.add(:docker_image, "error: #{output[:stderr]}") if output[:stderr].present?
|
||||||
errors.add(:docker_image, "error: #{e}")
|
rescue Runner::Error => e
|
||||||
|
# In case of an Runner::Error, we retry multiple times before giving up.
|
||||||
|
# The time between each retry increases to allow the runner management to catch up.
|
||||||
|
if retries < 5 && !Rails.env.test?
|
||||||
|
retries += 1
|
||||||
|
sleep retries
|
||||||
|
retry
|
||||||
|
elsif errors.exclude?(:docker_image)
|
||||||
|
errors.add(:docker_image, "error: #{e}")
|
||||||
|
raise ActiveRecord::RecordInvalid.new(self)
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
def delete_runner_environment
|
def delete_runner_environment
|
||||||
|
@ -8,7 +8,6 @@ describe ExecutionEnvironmentsController do
|
|||||||
|
|
||||||
before do
|
before do
|
||||||
allow(controller).to receive(:current_user).and_return(user)
|
allow(controller).to receive(:current_user).and_return(user)
|
||||||
allow(controller).to receive(:sync_to_runner_management).and_return(nil)
|
|
||||||
allow(Runner.strategy_class).to receive(:available_images).and_return([])
|
allow(Runner.strategy_class).to receive(:available_images).and_return([])
|
||||||
end
|
end
|
||||||
|
|
||||||
@ -16,7 +15,14 @@ describe ExecutionEnvironmentsController do
|
|||||||
context 'with a valid execution environment' do
|
context 'with a valid execution environment' do
|
||||||
let(:perform_request) { proc { post :create, params: {execution_environment: FactoryBot.build(:ruby).attributes} } }
|
let(:perform_request) { proc { post :create, params: {execution_environment: FactoryBot.build(:ruby).attributes} } }
|
||||||
|
|
||||||
before { perform_request.call }
|
before do
|
||||||
|
allow(Rails.env).to receive(:test?).and_return(false, true)
|
||||||
|
allow(Runner.strategy_class).to receive(:sync_environment).and_return(true)
|
||||||
|
runner = instance_double 'runner'
|
||||||
|
allow(Runner).to receive(:for).and_return(runner)
|
||||||
|
allow(runner).to receive(:execute_command).and_return({})
|
||||||
|
perform_request.call
|
||||||
|
end
|
||||||
|
|
||||||
expect_assigns(docker_images: Array)
|
expect_assigns(docker_images: Array)
|
||||||
expect_assigns(execution_environment: ExecutionEnvironment)
|
expect_assigns(execution_environment: ExecutionEnvironment)
|
||||||
@ -26,21 +32,25 @@ describe ExecutionEnvironmentsController do
|
|||||||
end
|
end
|
||||||
|
|
||||||
it 'registers the execution environment with the runner management' do
|
it 'registers the execution environment with the runner management' do
|
||||||
expect(controller).to have_received(:sync_to_runner_management)
|
expect(Runner.strategy_class).to have_received(:sync_environment)
|
||||||
end
|
end
|
||||||
|
|
||||||
expect_redirect(ExecutionEnvironment.last)
|
expect_redirect(ExecutionEnvironment.last)
|
||||||
end
|
end
|
||||||
|
|
||||||
context 'with an invalid execution environment' do
|
context 'with an invalid execution environment' do
|
||||||
before { post :create, params: {execution_environment: {}} }
|
before do
|
||||||
|
allow(Runner.strategy_class).to receive(:sync_environment).and_return(true)
|
||||||
|
allow(Rails.env).to receive(:test?).and_return(false, true)
|
||||||
|
post :create, params: {execution_environment: {}}
|
||||||
|
end
|
||||||
|
|
||||||
expect_assigns(execution_environment: ExecutionEnvironment)
|
expect_assigns(execution_environment: ExecutionEnvironment)
|
||||||
expect_status(200)
|
expect_status(200)
|
||||||
expect_template(:new)
|
expect_template(:new)
|
||||||
|
|
||||||
it 'does not register the execution environment with the runner management' do
|
it 'does not register the execution environment with the runner management' do
|
||||||
expect(controller).not_to have_received(:sync_to_runner_management)
|
expect(Runner.strategy_class).not_to have_received(:sync_environment)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
@ -171,7 +181,11 @@ describe ExecutionEnvironmentsController do
|
|||||||
describe 'PUT #update' do
|
describe 'PUT #update' do
|
||||||
context 'with a valid execution environment' do
|
context 'with a valid execution environment' do
|
||||||
before do
|
before do
|
||||||
allow(controller).to receive(:sync_to_runner_management).and_return(nil)
|
allow(Rails.env).to receive(:test?).and_return(false, true)
|
||||||
|
allow(Runner.strategy_class).to receive(:sync_environment).and_return(true)
|
||||||
|
runner = instance_double 'runner'
|
||||||
|
allow(Runner).to receive(:for).and_return(runner)
|
||||||
|
allow(runner).to receive(:execute_command).and_return({})
|
||||||
put :update, params: {execution_environment: FactoryBot.attributes_for(:ruby), id: execution_environment.id}
|
put :update, params: {execution_environment: FactoryBot.attributes_for(:ruby), id: execution_environment.id}
|
||||||
end
|
end
|
||||||
|
|
||||||
@ -180,19 +194,23 @@ describe ExecutionEnvironmentsController do
|
|||||||
expect_redirect(:execution_environment)
|
expect_redirect(:execution_environment)
|
||||||
|
|
||||||
it 'updates the execution environment at the runner management' do
|
it 'updates the execution environment at the runner management' do
|
||||||
expect(controller).to have_received(:sync_to_runner_management)
|
expect(Runner.strategy_class).to have_received(:sync_environment)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
context 'with an invalid execution environment' do
|
context 'with an invalid execution environment' do
|
||||||
before { put :update, params: {execution_environment: {name: ''}, id: execution_environment.id} }
|
before do
|
||||||
|
allow(Runner.strategy_class).to receive(:sync_environment).and_return(true)
|
||||||
|
allow(Rails.env).to receive(:test?).and_return(true, false, true)
|
||||||
|
put :update, params: {execution_environment: {name: ''}, id: execution_environment.id}
|
||||||
|
end
|
||||||
|
|
||||||
expect_assigns(execution_environment: ExecutionEnvironment)
|
expect_assigns(execution_environment: ExecutionEnvironment)
|
||||||
expect_status(200)
|
expect_status(200)
|
||||||
expect_template(:edit)
|
expect_template(:edit)
|
||||||
|
|
||||||
it 'does not update the execution environment at the runner management' do
|
it 'does not update the execution environment at the runner management' do
|
||||||
expect(controller).not_to have_received(:sync_to_runner_management)
|
expect(Runner.strategy_class).not_to have_received(:sync_environment)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
@ -8,7 +8,7 @@ describe ExecutionEnvironment do
|
|||||||
it 'validates that the Docker image works' do
|
it 'validates that the Docker image works' do
|
||||||
allow(execution_environment).to receive(:validate_docker_image?).and_return(true)
|
allow(execution_environment).to receive(:validate_docker_image?).and_return(true)
|
||||||
allow(execution_environment).to receive(:working_docker_image?).and_return(true)
|
allow(execution_environment).to receive(:working_docker_image?).and_return(true)
|
||||||
execution_environment.update(docker_image: FactoryBot.attributes_for(:ruby)[:docker_image])
|
execution_environment.update(FactoryBot.build(:ruby).attributes)
|
||||||
expect(execution_environment).to have_received(:working_docker_image?)
|
expect(execution_environment).to have_received(:working_docker_image?)
|
||||||
end
|
end
|
||||||
|
|
||||||
@ -146,10 +146,12 @@ describe ExecutionEnvironment do
|
|||||||
end
|
end
|
||||||
|
|
||||||
describe '#working_docker_image?' do
|
describe '#working_docker_image?' do
|
||||||
|
let(:execution_environment) { FactoryBot.create(:ruby) }
|
||||||
let(:working_docker_image?) { execution_environment.send(:working_docker_image?) }
|
let(:working_docker_image?) { execution_environment.send(:working_docker_image?) }
|
||||||
let(:runner) { instance_double 'runner' }
|
let(:runner) { instance_double 'runner' }
|
||||||
|
|
||||||
before do
|
before do
|
||||||
|
allow(execution_environment).to receive(:sync_runner_environment).and_return(true)
|
||||||
allow(Runner).to receive(:for).with(execution_environment.author, execution_environment).and_return runner
|
allow(Runner).to receive(:for).with(execution_environment.author, execution_environment).and_return runner
|
||||||
end
|
end
|
||||||
|
|
||||||
@ -176,7 +178,7 @@ describe ExecutionEnvironment do
|
|||||||
context 'when the Docker client produces an error' do
|
context 'when the Docker client produces an error' do
|
||||||
it 'adds an error' do
|
it 'adds an error' do
|
||||||
allow(runner).to receive(:execute_command).and_raise(Runner::Error)
|
allow(runner).to receive(:execute_command).and_raise(Runner::Error)
|
||||||
working_docker_image?
|
expect { working_docker_image? }.to raise_error(ActiveRecord::RecordInvalid)
|
||||||
expect(execution_environment.errors[:docker_image]).to be_present
|
expect(execution_environment.errors[:docker_image]).to be_present
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
Reference in New Issue
Block a user