CID-472: simplify building results in group_bootstrapper

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,

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

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

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,

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

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: |

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

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

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