CID-512: pass skip_rollback option to bootstrappers

devops-service/app/api2/handlers/stack.rb

@@ -45,6 +45,7 @@ module Devops
           stack = self.stack(id)
           stack.sync_status_and_events!
           Devops::Db.connector.stack_update(stack)
+          # do not remove syncing status and events, just add stack sync worker here.
 
           stack
         end

devops-service/db/mongo/models/stack/stack_base.rb

@@ -122,8 +122,7 @@ module Devops
       end
 
       def skip_rollback?
-        result = launch_options['skip_rollback']
+        launch_options['skip_rollback'] || false
-        result.nil? ? true : result
       end
 
       class << self

devops-service/lib/executors/stack_executor.rb

@@ -53,7 +53,13 @@ module Devops
         @just_persisted_by_priority.each do |priority, servers|
           puts_and_flush "Servers with priority '#{priority}': #{servers.map(&:id).join(", ")}"
         end
-        PrioritizedGroupsBootstrapper.new(out, jid, @just_persisted_by_priority).bootstrap_servers_by_priority
+        groups_bootstrapper = PrioritizedGroupsBootstrapper.new(
+          out: out,
+          jid: jid,
+          servers_by_priority: @just_persisted_by_priority,
+          skip_rollback: @stack.skip_rollback?
+        )
+        groups_bootstrapper.bootstrap_servers_by_priority
       end
 
       def delete_stale_servers

devops-service/lib/executors/stack_executor/group_bootstrapper.rb

@@ -14,8 +14,10 @@ class Devops::Executor::StackExecutor
       end
     end
 
-    def initialize(out, jid, servers)
+    # TODO: move to keyword arguments
-      @out, @jid, @servers = out, jid, servers
+    def initialize(options)
+      @out, @jid, @servers = options.fetch(:out), options.fetch(:jid), options.fetch(:servers)
+      @skip_rollback = options.fetch(:skip_rollback)
       @server_bootstrap_jobs = {}
     end
 
@@ -40,7 +42,7 @@ class Devops::Executor::StackExecutor
           server_attrs: server.to_mongo_hash,
           bootstrap_template: 'omnibus',
           owner: server.created_by,
-          skip_rollback: true
+          skip_rollback: @skip_rollback
         )
         @out.puts "Start bootstraping server '#{server.id}' job (job id: #{job_id})."
         @server_bootstrap_jobs[server.id] = job_id

devops-service/lib/executors/stack_executor/prioritized_groups_bootstrapper.rb

@@ -11,18 +11,26 @@ class Devops::Executor::StackExecutor
       set_result_codes bootstrap_error: 1, deploy_error: 2
     end
 
-    def initialize(out, jid, servers_with_priorities)
+    # @param servers_by_priority [Hash] is a Hash like
-      @out, @jid, @servers_with_priorities = out, jid, servers_with_priorities
+    #   {1 => [server1, server2]}
+    # TODO: move to keyword arguments
+    def initialize(options)
+      @out, @jid = options.fetch(:out), options.fetch(:jid)
+      @servers_by_priority = options.fetch(:servers_by_priority)
+      @skip_rollback = options.fetch(:skip_rollback)
     end
 
-    # @param servers_with_priorities [Hash] is a Hash like
-    #   {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 = GroupBootstrapper.new(@out, @jid, @servers_with_priorities[priority])
+        bootstrapper = GroupBootstrapper.new(
+          out: @out,
+          jid: @jid,
+          servers: @servers_by_priority[priority],
+          skip_rollback: @skip_rollback
+        )
         bootstrap_results = bootstrapper.bootstrap_group
         error = find_error(bootstrap_results)
         return error if error
@@ -33,7 +41,7 @@ class Devops::Executor::StackExecutor
     private
 
     def sorted_priorities
-      @servers_with_priorities.keys.sort.reverse
+      @servers_by_priority.keys.sort.reverse
     end
 
     def find_error(results)

devops-service/spec/executors/stack_executor/group_bootstrapper_spec.rb

@@ -4,7 +4,8 @@ class Devops::Executor::StackExecutor
     let(:out) { double(:out, puts: nil, flush: nil) }
     let(:jid) { 1000 }
     let(:servers) { [build(:server, id: 'a'), build(:server, id: 'b')] }
-    let(:bootstrapper) { described_class.new(out, jid, servers ) }
+    let(:bootstrapper) { described_class.new(out: out, jid: jid, servers: servers, skip_rollback: false) }
+    let(:bootstrapper_with_skip_rollback) { described_class.new(out: out, jid: jid, servers: servers, skip_rollback: true) }
     let(:bootstrap_job_ids) { %w(100 200) }
 
     describe '#bootstrap_group' do
@@ -15,12 +16,22 @@ class Devops::Executor::StackExecutor
         allow_any_instance_of(Devops::Helpers::JobWaiter).to receive(:wait) { 0 }
       end
 
-      it 'start bootstrap workers and add subreports' do
+      it 'starts bootstrap workers and add subreports' do
-        expect(Worker).to receive(:start_async).with(BootstrapWorker, hash_including(:server_attrs, :bootstrap_template, :owner))
+        expect(Worker).to receive(:start_async).with(BootstrapWorker, hash_including(:server_attrs, :bootstrap_template, :owner, :skip_rollback))
         expect(stubbed_connector).to receive(:add_report_subreports).with(jid, bootstrap_job_ids)
         bootstrapper.bootstrap_group
       end
 
+      it 'starts workers with skip_rollback=true if was built with skip_rollback=true option' do
+        expect(Worker).to receive(:start_async).with(anything, hash_including(skip_rollback: true))
+        bootstrapper_with_skip_rollback.bootstrap_group
+      end
+
+      it 'starts workers with skip_rollback=false if was built with skip_rollback=false option' do
+        expect(Worker).to receive(:start_async).with(anything, hash_including(skip_rollback: false))
+        bootstrapper.bootstrap_group
+      end
+
       it 'returns :ok result if everything ok' do
         expect( bootstrapper.bootstrap_group.first.reason ).to eq :ok
       end

devops-service/spec/executors/stack_executor/prioritized_groups_bootstrapper_spec.rb

@@ -5,7 +5,13 @@ module Devops::Executor
     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) }
+      let(:groups_bootstrapper) {
+        described_class.new(out: out, jid: jid, servers_by_priority: @servers_by_priority, skip_rollback: false)
+      }
+      let(:groups_bootstrapper_with_skip_rollback) {
+        described_class.new(out: out, jid: jid, servers_by_priority: @servers_by_priority, skip_rollback: true)
+      }
+
       before do
         @array1 = []; @array2 = []; @array3 = []
         @servers_by_priority = {2 => @array2, 1 => @array1, 3 => @array3}
@@ -14,14 +20,27 @@ module Devops::Executor
       describe '#bootstrap_servers_by_priority' do
         subject { groups_bootstrapper.bootstrap_servers_by_priority }
 
+        context "with mocked #bootstrap_group" do
+          before { allow(GroupBootstrapper).to receive(:new) { instance_double(GroupBootstrapper, bootstrap_group: []) } }
+
           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(hash_including(servers: @array3)).ordered
-          expect(GroupBootstrapper).to receive(:new).with(out, jid, @array3).ordered
+            expect(GroupBootstrapper).to receive(:new).with(hash_including(servers: @array2)).ordered
-          expect(GroupBootstrapper).to receive(:new).with(out, jid, @array2).ordered
+            expect(GroupBootstrapper).to receive(:new).with(hash_including(servers: @array1)).ordered
-          expect(GroupBootstrapper).to receive(:new).with(out, jid, @array1).ordered
             expect(subject).to be_ok
           end
 
+          it 'passes skip_rollback: true if was build with skip_rollback: true' do
+            expect(GroupBootstrapper).to receive(:new).with(hash_including(skip_rollback: true))
+            groups_bootstrapper_with_skip_rollback.bootstrap_servers_by_priority
+          end
+
+          it 'passes skip_rollback: false if was build with skip_rollback: false' do
+            expect(GroupBootstrapper).to receive(:new).with(hash_including(skip_rollback: false))
+            subject
+          end
+        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)]

devops-service/spec/executors/stack_executor_spec.rb

@@ -76,10 +76,22 @@ class Devops::Executor::StackExecutor
     end
 
     describe '#bootstrap_just_persisted' do
-      it 'calls PrioritizedGroupsBootstrapper#bootstrap_servers_by_priority' do
+      subject {
-        expect_any_instance_of(PrioritizedGroupsBootstrapper).to receive(:bootstrap_servers_by_priority)
         executor_with_stack.persist_new_servers
         executor_with_stack.bootstrap_just_persisted(1000)
+      }
+
+      it 'passes skip_rollback option equals to @stack.skip_rollback?' do
+        allow(stack).to receive(:skip_rollback?) { 'test' }
+        expect(PrioritizedGroupsBootstrapper).to receive(:new).with(
+          hash_including(skip_rollback: 'test')
+        ).and_return(double('groups bootstrapper', bootstrap_servers_by_priority: nil))
+        subject
+      end
+
+      it 'calls PrioritizedGroupsBootstrapper#bootstrap_servers_by_priority' do
+        expect_any_instance_of(PrioritizedGroupsBootstrapper).to receive(:bootstrap_servers_by_priority)
+        subject
       end
     end
 

devops-service/spec/models/stack/stack_ec2_spec.rb

@@ -79,8 +79,8 @@ RSpec.describe Devops::Model::StackEc2, type: :model do
       expect(stack.skip_rollback?).to be true
     end
 
-    it 'returns true by default' do
+    it 'returns false by default' do
-      expect(described_class.new.skip_rollback?).to be true
+      expect(described_class.new.skip_rollback?).to be false
     end
   end