From daedca4aec33c02938bdc4936639a44b59df1da1 Mon Sep 17 00:00:00 2001 From: Anton Chuchkalov Date: Fri, 15 Apr 2016 11:08:20 +0300 Subject: [PATCH] CID-472: simplify building results in group_bootstrapper --- .../server_operation_result.rb | 1 - ..._bootstrapper.rb => group_bootstrapper.rb} | 32 ++----- .../prioritized_groups_bootstrapper.rb | 21 +++-- .../stack_executor/stack_creation_waiter.rb | 1 - devops-service/lib/helpers/result_object.rb | 2 +- devops-service/messages/en.yml | 13 ++- ...per_spec.rb => group_bootstrapper_spec.rb} | 8 +- .../prioritized_groups_bootstrapper_spec.rb | 85 ++++++++++--------- .../workers/stack_bootstrap_worker_spec.rb | 16 ++-- 9 files changed, 86 insertions(+), 93 deletions(-) rename devops-service/lib/executors/stack_executor/{servers_bootstrapper.rb => group_bootstrapper.rb} (66%) rename devops-service/spec/executors/stack_executor/{servers_bootstrapper_spec.rb => group_bootstrapper_spec.rb} (87%) diff --git a/devops-service/lib/executors/server_executor/server_operation_result.rb b/devops-service/lib/executors/server_executor/server_operation_result.rb index a19dad0..5c015f1 100644 --- a/devops-service/lib/executors/server_executor/server_operation_result.rb +++ b/devops-service/lib/executors/server_executor/server_operation_result.rb @@ -5,7 +5,6 @@ module Devops class ServerOperationResult < Helpers::ResultObject set_result_codes( - ok: 0, server_bootstrap_fail: 2, server_cannot_update_tags: 3, server_bootstrap_private_ip_unset: 4, diff --git a/devops-service/lib/executors/stack_executor/servers_bootstrapper.rb b/devops-service/lib/executors/stack_executor/group_bootstrapper.rb similarity index 66% rename from devops-service/lib/executors/stack_executor/servers_bootstrapper.rb rename to devops-service/lib/executors/stack_executor/group_bootstrapper.rb index 098eba0..7341fae 100644 --- a/devops-service/lib/executors/stack_executor/servers_bootstrapper.rb +++ b/devops-service/lib/executors/stack_executor/group_bootstrapper.rb @@ -3,17 +3,15 @@ require 'workers/bootstrap_worker' # Starts bootstrap workers for each server in group and wait for them to end (synchroniously). class Devops::Executor::StackExecutor - class ServersBootstrapper + class GroupBootstrapper include PutsAndFlush attr_reader :out class Result < Devops::Helpers::ResultObject - set_result_codes( - ok: 0, - bootstrap_error: 2, - deploy_error: 3, - timeout_reached: 4 - ) + set_result_codes timeout_reached: 4 + def one_of_bootstrap_errors? + false + end end def initialize(out, jid, servers) @@ -28,7 +26,7 @@ class Devops::Executor::StackExecutor @server_bootstrap_jobs.map do |server_id, job_id| result = wait_for_bootstrap_job(job_id) - puts_and_flush Devops::Messages.t("stack_executor.servers_bootstrapper.result.#{result.reason}", server_id: server_id, job_id: job_id) + puts_and_flush Devops::Messages.t("stack_executor.group_bootstrapper.result.#{result.reason}", server_id: server_id, job_id: job_id) result end end @@ -50,26 +48,12 @@ class Devops::Executor::StackExecutor puts_and_flush "\n\n\n" end - def result(reason) - Result.from_reason(reason) - end - def wait_for_bootstrap_job(job_id) result_code = Devops::Helpers::JobWaiter.new(job_id).wait - result_from_job_code(result_code) + Devops::Executor::ServerOperationResult.new(result_code) rescue Devops::Helpers::JobWaiter::TimeoutReached - result(:timeout_reached) + Result.from_reason(:timeout_reached) end - def result_from_job_code(result_code) - job_result = Devops::Executor::ServerOperationResult.new(result_code) - if job_result.ok? - result(:ok) - elsif job_result.one_of_bootstrap_errors? - result(:bootstrap_error) - else - result(:deploy_error) - end - end end end \ No newline at end of file diff --git a/devops-service/lib/executors/stack_executor/prioritized_groups_bootstrapper.rb b/devops-service/lib/executors/stack_executor/prioritized_groups_bootstrapper.rb index 9fde355..d7c6e01 100644 --- a/devops-service/lib/executors/stack_executor/prioritized_groups_bootstrapper.rb +++ b/devops-service/lib/executors/stack_executor/prioritized_groups_bootstrapper.rb @@ -1,4 +1,4 @@ -require_relative 'servers_bootstrapper' +require_relative 'group_bootstrapper' # Bootstrap groups of servers based on priorities: higher first. # Doesn't start bootstrap of next group if bootstrap of previous group failed. @@ -7,6 +7,10 @@ class Devops::Executor::StackExecutor include PutsAndFlush attr_reader :out + class Result < Devops::Helpers::ResultObject + set_result_codes bootstrap_error: 1, deploy_error: 2 + end + def initialize(out, jid, servers_with_priorities) @out, @jid, @servers_with_priorities = out, jid, servers_with_priorities end @@ -15,14 +19,15 @@ class Devops::Executor::StackExecutor # {1 => [server1, server2]} # Starts bootstrapping another group only after successful bootstrapping of previous. def bootstrap_servers_by_priority + # don't try to use detect here: it returns priority instead of error sorted_priorities.each do |priority| puts_and_flush "Bootstrap servers with priority '#{priority}':" - bootstrapper = ServersBootstrapper.new(@out, @jid, @servers_with_priorities[priority]) + bootstrapper = GroupBootstrapper.new(@out, @jid, @servers_with_priorities[priority]) bootstrap_results = bootstrapper.bootstrap_group - error = most_critical_error(bootstrap_results) + error = find_error(bootstrap_results) return error if error end - ServersBootstrapper::Result.from_reason(:ok) + Result.from_reason(:ok) end private @@ -31,8 +36,12 @@ class Devops::Executor::StackExecutor @servers_with_priorities.keys.sort.reverse end - def most_critical_error(results) - results.detect(&:bootstrap_error?) || results.detect(&:failed?) + def find_error(results) + if results.detect(&:one_of_bootstrap_errors?) + Result.from_reason(:bootstrap_error) + elsif results.detect(&:failed?) + Result.from_reason(:deploy_error) + end end end end \ No newline at end of file diff --git a/devops-service/lib/executors/stack_executor/stack_creation_waiter.rb b/devops-service/lib/executors/stack_executor/stack_creation_waiter.rb index 0e63a61..ea92933 100644 --- a/devops-service/lib/executors/stack_executor/stack_creation_waiter.rb +++ b/devops-service/lib/executors/stack_executor/stack_creation_waiter.rb @@ -6,7 +6,6 @@ class Devops::Executor::StackExecutor class SyncResult < Devops::Helpers::ResultObject set_result_codes( - ok: 0, stack_rolled_back: 1, unkown_status: 2, timeout: 3, diff --git a/devops-service/lib/helpers/result_object.rb b/devops-service/lib/helpers/result_object.rb index e9cf634..df3bb51 100644 --- a/devops-service/lib/helpers/result_object.rb +++ b/devops-service/lib/helpers/result_object.rb @@ -33,7 +33,7 @@ module Devops # defines methods like :bootstrap_error? def set_result_codes(new_result_codes) - @result_codes = new_result_codes + @result_codes = new_result_codes.merge(ok: 0) @result_codes.each do |pretendent_reason, pretendent_code| define_method "#{pretendent_reason}?" do code == pretendent_code diff --git a/devops-service/messages/en.yml b/devops-service/messages/en.yml index 6cdac42..d1d3e58 100644 --- a/devops-service/messages/en.yml +++ b/devops-service/messages/en.yml @@ -12,7 +12,6 @@ en: deploy_error: | Stack was launched, but an error occured during deploying stack servers. You can redeploy stack after fixing the error. - timeout_reached: Bootstrap or deploy wasn't completed due to timeout. stack_sync: bootstrap_result: ok: "Stack has been successfully synced." @@ -20,12 +19,20 @@ en: deploy_error: An error occured during new stack servers deploy. timeout_reached: Bootstrap or deploy of new servers wasn't completed due to timeout. stack_executor: - servers_bootstrapper: + group_bootstrapper: result: ok: "Server '%{server_id}' has been bootstraped (job %{job_id})." + server_bootstrap_fail: "Server '%{server_id}' bootstrapping failed (job %{job_id})." + server_not_in_chef_nodes: "Server '%{server_id}' bootstrapping failed: server not in chef nodes (job %{job_id})." + server_bootstrap_unknown_error: "Unkown error occured during server '%{server_id}' bootstrap (job %{job_id})." timeout_reached: "Waiting for bootstrapping '%{server_id}' (job %{job_id}) halted: timeout reached." - bootstrap_error: "Server '%{server_id}' bootstrapping failed (job %{job_id})." deploy_error: "Server '%{server_id}' deploy failed (job %{job_id})." + server_cannot_update_tags: "Server '%{server_id}' deploy failed: can't update tags (job %{job_id})." + server_bootstrap_private_ip_unset: "Server '%{server_id}' deploy failed: private ip is unset (job %{job_id})." + deploy_unknown_error: "Unknown error occured during server '%{server_id}' deploy (job %{job_id})." + deploy_failed: "Server '%{server_id}' deploy failed (job %{job_id})." + creating_server_unknown_error: "Unknown error occured during server '%{server_id}' creation (job %{job_id})." + creating_server_in_cloud_failed: "Server '%{server_id}' creation in cloud failed (job %{job_id})." stack_creation_waiter: result: ok: | diff --git a/devops-service/spec/executors/stack_executor/servers_bootstrapper_spec.rb b/devops-service/spec/executors/stack_executor/group_bootstrapper_spec.rb similarity index 87% rename from devops-service/spec/executors/stack_executor/servers_bootstrapper_spec.rb rename to devops-service/spec/executors/stack_executor/group_bootstrapper_spec.rb index c5ce466..12db01b 100644 --- a/devops-service/spec/executors/stack_executor/servers_bootstrapper_spec.rb +++ b/devops-service/spec/executors/stack_executor/group_bootstrapper_spec.rb @@ -1,6 +1,6 @@ -require 'lib/executors/stack_executor/servers_bootstrapper' +require 'lib/executors/stack_executor/group_bootstrapper' class Devops::Executor::StackExecutor - RSpec.describe ServersBootstrapper, stubbed_connector: true, init_messages: true do + RSpec.describe GroupBootstrapper, stubbed_connector: true, init_messages: true do let(:out) { double(:out, puts: nil, flush: nil) } let(:jid) { 1000 } let(:servers) { [build(:server, id: 'a'), build(:server, id: 'b')] } @@ -25,11 +25,11 @@ class Devops::Executor::StackExecutor expect( bootstrapper.bootstrap_group.first.reason ).to eq :ok end - it 'returns proper error results' do + it "returns ServerOperationResult error results" do waiter = instance_double(Devops::Helpers::JobWaiter) allow(waiter).to receive(:wait).and_return(2, 8) allow(Devops::Helpers::JobWaiter).to receive(:new) { waiter } - expect( bootstrapper.bootstrap_group.map(&:reason) ).to eq [:bootstrap_error, :deploy_error] + expect( bootstrapper.bootstrap_group.map(&:reason) ).to eq [:server_bootstrap_fail, :deploy_failed] end it "returns :timeout_reached result if bootstrap and deploy hasn't been finished in 5000 seconds" do diff --git a/devops-service/spec/executors/stack_executor/prioritized_groups_bootstrapper_spec.rb b/devops-service/spec/executors/stack_executor/prioritized_groups_bootstrapper_spec.rb index 04039c4..5c7d09d 100644 --- a/devops-service/spec/executors/stack_executor/prioritized_groups_bootstrapper_spec.rb +++ b/devops-service/spec/executors/stack_executor/prioritized_groups_bootstrapper_spec.rb @@ -1,50 +1,51 @@ require 'lib/executors/stack_executor/prioritized_groups_bootstrapper' -class Devops::Executor::StackExecutor - RSpec.describe PrioritizedGroupsBootstrapper, stubbed_connector: true do - let(:out) { double(:out, puts: nil, flush: nil) } - let(:jid) { 1000 } - let(:groups_bootstrapper) { described_class.new(out, jid, @servers_by_priority) } - before do - @array1 = []; @array2 = []; @array3 = [] - @servers_by_priority = {2 => @array2, 1 => @array1, 3 => @array3} - end - - describe '#bootstrap_servers_by_priority' do - subject { groups_bootstrapper.bootstrap_servers_by_priority } - - it 'bootstraps servers in order by priorities, separately' do - allow(ServersBootstrapper).to receive(:new) { instance_double(ServersBootstrapper, bootstrap_group: []) } - expect(ServersBootstrapper).to receive(:new).with(out, jid, @array3).ordered - expect(ServersBootstrapper).to receive(:new).with(out, jid, @array2).ordered - expect(ServersBootstrapper).to receive(:new).with(out, jid, @array1).ordered - expect(subject).to be_ok +module Devops::Executor + class StackExecutor + RSpec.describe PrioritizedGroupsBootstrapper, stubbed_connector: true do + let(:out) { double(:out, puts: nil, flush: nil) } + let(:jid) { 1000 } + let(:groups_bootstrapper) { described_class.new(out, jid, @servers_by_priority) } + before do + @array1 = []; @array2 = []; @array3 = [] + @servers_by_priority = {2 => @array2, 1 => @array1, 3 => @array3} end - it 'it returns :bootstrap_error result if error occured during bootstrap' do - allow_any_instance_of(ServersBootstrapper).to receive(:bootstrap_group) { - [ServersBootstrapper::Result.from_reason(:deploy_error), ServersBootstrapper::Result.from_reason(:bootstrap_error)] - } - expect(subject).to be_bootstrap_error + describe '#bootstrap_servers_by_priority' do + subject { groups_bootstrapper.bootstrap_servers_by_priority } + + it 'bootstraps servers in order by priorities, separately' do + allow(GroupBootstrapper).to receive(:new) { instance_double(GroupBootstrapper, bootstrap_group: []) } + expect(GroupBootstrapper).to receive(:new).with(out, jid, @array3).ordered + expect(GroupBootstrapper).to receive(:new).with(out, jid, @array2).ordered + expect(GroupBootstrapper).to receive(:new).with(out, jid, @array1).ordered + expect(subject).to be_ok + end + + it 'it returns :bootstrap_error result if error occured during bootstrap' do + allow_any_instance_of(GroupBootstrapper).to receive(:bootstrap_group) { + [ServerOperationResult.from_reason(:deploy_failed), ServerOperationResult.from_reason(:server_bootstrap_fail)] + } + expect(subject).to be_bootstrap_error + end + + it 'it returns :deploy_error result if error occured during deploy' do + allow_any_instance_of(GroupBootstrapper).to receive(:bootstrap_group) { + [ServerOperationResult.from_reason(:deploy_failed)] + } + expect(subject).to be_deploy_error + end + + it "doesn't bootstrap group if previous one failed" do + allow_any_instance_of(GroupBootstrapper).to receive(:bootstrap_group) { + [ServerOperationResult.from_reason(:deploy_failed)] + } + allow(GroupBootstrapper).to receive(:new).and_call_original + expect(GroupBootstrapper).to receive(:new).once + subject + end + end - - it 'it returns :deploy_error result if error occured during deploy' do - allow_any_instance_of(ServersBootstrapper).to receive(:bootstrap_group) { - [ServersBootstrapper::Result.from_reason(:deploy_error)] - } - expect(subject).to be_deploy_error - end - - it "doesn't bootstrap group if previous one failed" do - allow_any_instance_of(ServersBootstrapper).to receive(:bootstrap_group) { - [ServersBootstrapper::Result.from_reason(:deploy_error)] - } - allow(ServersBootstrapper).to receive(:new).and_call_original - expect(ServersBootstrapper).to receive(:new).once - subject - end - - end end end \ No newline at end of file diff --git a/devops-service/spec/workers/stack_bootstrap_worker_spec.rb b/devops-service/spec/workers/stack_bootstrap_worker_spec.rb index b0c17ad..93038a2 100644 --- a/devops-service/spec/workers/stack_bootstrap_worker_spec.rb +++ b/devops-service/spec/workers/stack_bootstrap_worker_spec.rb @@ -17,7 +17,7 @@ RSpec.describe StackBootstrapWorker, type: :worker, stubbed_connector: true, ini } def bootstrap_result(reason) - Devops::Executor::StackExecutor::ServersBootstrapper::Result.from_reason(reason) + Devops::Executor::StackExecutor::PrioritizedGroupsBootstrapper::Result.from_reason(reason) end before do @@ -68,23 +68,17 @@ RSpec.describe StackBootstrapWorker, type: :worker, stubbed_connector: true, ini expect(perform_with_bootstrap).to eq 0 end - it 'rollbacks stack and returns 2 when a known error occured during servers bootstrap' do + it 'rollbacks stack and returns 1 when a known error occured during servers bootstrap' do allow(executor).to receive(:bootstrap_just_persisted) { bootstrap_result(:bootstrap_error) } expect(executor).to receive(:delete_stack) perform_with_bootstrap - expect(perform_with_bootstrap).to eq 2 + expect(perform_with_bootstrap).to eq 1 end - it "doesn't rollback stack and returns 3 when a known error occured during servers deploy" do + it "doesn't rollback stack and returns 2 when a known error occured during servers deploy" do allow(executor).to receive(:bootstrap_just_persisted) { bootstrap_result(:deploy_error) } expect(worker).not_to receive(:rollback_stack!) - expect(perform_with_bootstrap).to eq 3 - end - - it "doesn't rollback stack and returns 3 when a servers bootstrap & deploy haven't been finished due to timeout" do - allow(executor).to receive(:bootstrap_just_persisted) { bootstrap_result(:timeout_reached) } - expect(worker).not_to receive(:rollback_stack!) - expect(perform_with_bootstrap).to eq 4 + expect(perform_with_bootstrap).to eq 2 end it 'rollbacks stack and reraises that error when an unknown error occured during servers bootsrap and deploy' do