Prevent stop deployment fail in case of post-deployments hooks

After main deploy part we mark all nodes as 'ready'. If we
run stop deployment in this case, Nailgun will send to Astute
empty nodes list to stop. Before this change we fail to
run mcollective on empty nodes list.

Change-Id: Ib5b8298eb761f8f6123efc0dedce538c4f63b68d
Closes-Bug: #1463881
(cherry picked from commit d4f2bd1129)

lib/astute/orchestrator.rb

@@ -97,6 +97,10 @@ module Astute
     end
 
     def remove_nodes(reporter, task_id, engine_attrs, nodes, options={})
+      # FIXME(vsharshov): bug/1463881: In case of post deployment we mark all nodes
+      # as ready. In this case we will get empty nodes.
+      return if nodes.empty?
+
       options[:reboot] = true unless options.has_key?(:reboot)
       options[:raise_if_error] = false unless options.has_key?(:raise_if_error)
 
@@ -115,6 +119,10 @@ module Astute
     end
 
     def stop_puppet_deploy(reporter, task_id, nodes)
+      # FIXME(vsharshov): bug/1463881: In case of post deployment we mark all nodes
+      # as ready. If we run stop deployment we will get empty nodes.
+      return if nodes.empty?
+
       nodes_uids = nodes.map { |n| n['uid'] }.uniq
       puppetd = MClient.new(Context.new(task_id, reporter), "puppetd", nodes_uids, check_result=false)
       puppetd.stop_and_disable

spec/unit/orchestrator_spec.rb

@@ -152,4 +152,110 @@ describe Astute::Orchestrator do
     end
 
   end #execute_tasks
+
+  context 'stop deployment' do
+    let(:data) do
+      {
+        "engine"=>{
+          "url"=>"http://10.109.0.2:80/cobbler_api",
+          "username"=>"cobbler",
+          "password"=>"JTcu4VoM",
+          "master_ip"=>"10.109.0.2"
+        },
+        "nodes"=>[],
+        "stop_task_uuid"=>"26a5cfb5-797d-4385-9262-da88ae7a0e14",
+        "task_uuid"=>"3958fe00-5969-44e2-bb21-413993cfbd6b"
+      }
+    end
+
+    let(:nodes) { [{'uid' => '1'}, {'uid' => '2'}] }
+
+    let(:mclient) do
+      mclient = mock_rpcclient(nodes)
+      Astute::MClient.any_instance.stubs(:rpcclient).returns(mclient)
+      Astute::MClient.any_instance.stubs(:log_result).returns(mclient)
+      Astute::MClient.any_instance.stubs(:check_results_with_retries).returns(mclient)
+      mclient
+    end
+
+    describe '#stop_puppet_deploy' do
+
+      it 'should do nothing if nodes list is empty' do
+        result = @orchestrator.stop_puppet_deploy(@reporter, 'task_id', data['nodes'])
+        expect(result).to eql(nil)
+      end
+
+      it 'should stop puppet' do
+        mclient.expects(:stop_and_disable)
+        @orchestrator.stop_puppet_deploy(@reporter, 'task_id', nodes)
+      end
+    end #stop_puppet_deploy
+
+    describe '#remove_nodes' do
+
+      it 'should do nothing if nodes list is empty' do
+        expect(@orchestrator.remove_nodes(
+          @reporter,
+          'task_id',
+          data['engine'],
+          data['nodes'],
+          options={}
+        )).to eql(nil)
+      end
+
+      it 'should remove nodes' do
+        Astute::Provisioner.any_instance.expects(:remove_nodes).once
+        @orchestrator.expects(:perform_pre_deletion_tasks)
+          .returns('status' => 'ready')
+
+        @orchestrator.remove_nodes(
+          @reporter,
+          'task_id',
+          data['engine'],
+          nodes,
+          options={}
+        )
+      end
+
+      it 'should run pre deletion tasks' do
+        Astute::Provisioner.any_instance.stubs(:remove_nodes)
+        @orchestrator.expects(:perform_pre_deletion_tasks).with(
+          @reporter,
+          'task_id',
+          nodes,
+          {:reboot => true, :raise_if_error => false}
+        ).returns('status' => 'ready')
+
+        @orchestrator.remove_nodes(
+          @reporter,
+          'task_id',
+          data['engine'],
+          nodes,
+          options={}
+        )
+      end
+
+      it 'should deletion if run pre deletion tasks fail' do
+        @orchestrator.expects(:perform_pre_deletion_tasks).with(
+          @reporter,
+          'task_id',
+          nodes,
+          {:reboot => true, :raise_if_error => false}
+        ).returns('status' => 'error')
+
+        Astute::Provisioner.any_instance.expects(:remove_nodes).never
+
+        @orchestrator.remove_nodes(
+          @reporter,
+          'task_id',
+          data['engine'],
+          nodes,
+          options={}
+        )
+      end
+    end
+
+  end #stop deployment
+
+
 end