Fixed reference error in cobbler_manager.rb

Due to reference error some nodes were skipped when removing.

Change-Id: I3c03b161f0643eeca65a15a5fa5cd468a0b17e43
Closes-Bug: #1494446
Co-Authored-By: Alexander Gordeev <agordeev@mirantis.com>
Co-Authored-By: Bulat Gaifullin <bgaifullin@mirantis.com>

lib/astute/cobbler_manager.rb

@@ -49,27 +49,22 @@ module Astute
     def remove_nodes(nodes, retries=3, interval=2)
       nodes_to_remove = nodes.map {|node| node['slave_name']}.uniq
       Astute.logger.info("Total list of nodes to remove: #{nodes_to_remove.pretty_inspect}")
-      error_nodes = nodes_to_remove
       retries.times do
-        nodes_to_remove.each do |name|
+        nodes_to_remove.select! do |name|
-          if @engine.system_exists?(name)
+          unless @engine.system_exists?(name)
+            Astute.logger.info("System is not in cobbler: #{name}")
+            next
+          else
             Astute.logger.info("Trying to remove system from cobbler: #{name}")
             @engine.remove_system(name)
-            error_nodes.delete(name) unless @engine.system_exists?(name)
-          else
-            Astute.logger.info("System is not in cobbler: #{name}")
-            error_nodes.delete(name)
           end
+          @engine.system_exists?(name)
         end
-        return if error_nodes.empty?
+        return if nodes_to_remove.empty?
         sleep(interval) if interval > 0
       end
     ensure
-      if error_nodes.empty?
+      Astute.logger.error("Cannot remove nodes from cobbler: #{nodes_to_remove.pretty_inspect}") if nodes_to_remove.present?
-        Astute.logger.info("Systems have been successfully removed from cobbler: #{nodes_to_remove.pretty_inspect}")
-      else
-        Astute.logger.error("Cannot remove nodes from cobbler: #{error_nodes.pretty_inspect}")
-      end
       sync
     end
 

spec/unit/cobbler_manager_spec.rb

@@ -164,27 +164,40 @@ describe Astute::CobblerManager do
   end #'edit_nodes'
 
   describe '#remove_nodes' do
+    nodes = [
+             {"slave_name" => "node-1"},
+             {"slave_name" => "node-2"},
+             {"slave_name" => "node-3"},
+            ]
+    system_exists_return_seq = [true, false, true, false, true, false]
+    system_remove_with_seq = ["node-1", "node-2", "node-3"]
     before(:each) do
       cobbler_manager.stubs(:sleep)
     end
-
     it 'should try to remove nodes using cobbler engine' do
-      engine.stubs(:system_exists?).returns(true).then.returns(false)
+      engine.stubs(:system_exists?).returns(*system_exists_return_seq)
-      engine.expects(:remove_system).once
+      engine.expects(:remove_system).returns(true).times(3)
       engine.expects(:sync).once
-      cobbler_manager.remove_nodes(data['nodes'])
+      cobbler_manager.remove_nodes(nodes)
     end
     it 'should try to remove nodes three times before giving up' do
       engine.stubs(:system_exists?).returns(true)
-      engine.expects(:remove_system).times(3)
+      engine.expects(:remove_system).times(9)
       engine.expects(:sync).once
-      cobbler_manager.remove_nodes(data['nodes'])
+      cobbler_manager.remove_nodes(nodes)
     end
     it 'should not try to remove nodes if they do not exist' do
       engine.stubs(:system_exists?).returns(false)
       engine.expects(:remove_system).never
       engine.expects(:sync).once
-      cobbler_manager.remove_nodes(data['nodes'])
+      cobbler_manager.remove_nodes(nodes)
+    end
+    it 'should try to remove uniq list of nodes' do
+      nodes << {"slave_name" => "node-1"}
+      engine.stubs(:system_exists?).returns(*system_exists_return_seq)
+      engine.expects(:remove_system).times(3)
+      engine.expects(:sync).once
+      cobbler_manager.remove_nodes(nodes)
     end
   end