diff --git a/lib/astute/cobbler_manager.rb b/lib/astute/cobbler_manager.rb index 76727738..b5104e27 100644 --- a/lib/astute/cobbler_manager.rb +++ b/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| - if @engine.system_exists?(name) + nodes_to_remove.select! do |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.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 + Astute.logger.error("Cannot remove nodes from cobbler: #{nodes_to_remove.pretty_inspect}") if nodes_to_remove.present? sync end diff --git a/spec/unit/cobbler_manager_spec.rb b/spec/unit/cobbler_manager_spec.rb index 88ae6e7a..c30997ef 100644 --- a/spec/unit/cobbler_manager_spec.rb +++ b/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.expects(:remove_system).once + engine.stubs(:system_exists?).returns(*system_exists_return_seq) + 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