diff --git a/chef/lib/chef/provider/route.rb b/chef/lib/chef/provider/route.rb index 5b753e7256a..6c780c265bc 100644 --- a/chef/lib/chef/provider/route.rb +++ b/chef/lib/chef/provider/route.rb @@ -108,10 +108,7 @@ def action_add if is_running Chef::Log.debug("Route #{@new_resource.name} already active ") else - command = "ip route replace #{@new_resource.target}" - command << "/#{MASK[@new_resource.netmask.to_s]}" if @new_resource.netmask - command << " via #{@new_resource.gateway} " - command << " dev #{@new_resource.device} " if @new_resource.device + command = generate_command(:add) Chef::Log.info("Adding route: #{command} ") run_command( :command => command ) @@ -125,9 +122,7 @@ def action_add def action_delete if is_running - command = "ip route delete #{@new_resource.target}" - command << "/#{MASK[@new_resource.netmask.to_s]}" if @new_resource.netmask - command << " via #{@new_resource.gateway} " + command = generate_command(:delete) Chef::Log.info("Removing route: #{command}") run_command( :command => command ) @@ -153,13 +148,11 @@ def generate_config conf[dev] = String.new if conf[dev].nil? if resource.action == :add - conf[dev] << "#{resource.target}" - conf[dev] << "/#{resource.netmask}" if resource.netmask - conf[dev] << " via #{resource.gateway}\n" + conf[dev] = config_file_contents(:add, :target => resource.target, :netmask => resource.netmask, :gateway => resource.gateway) else # need to do this for the case when the last route on an int # is removed - conf[dev] = "" + conf[dev] = config_file_contents(:delete) end end end @@ -170,5 +163,36 @@ def generate_config network_file.close end end - end + end + + def generate_command(action) + common_route_items = '' + common_route_items << "/#{MASK[@new_resource.netmask.to_s]}" if @new_resource.netmask + common_route_items << " via #{@new_resource.gateway} " if @new_resource.gateway + + case action + when :add + command = "ip route replace #{@new_resource.target}" + command << common_route_items + command << " dev #{@new_resource.device} " if @new_resource.device + when :delete + command = "ip route delete #{@new_resource.target}" + command << common_route_items + end + + return command + end + + def config_file_contents(action, options={}) + content = '' + case action + when :add + content << "#{options[:target]}" + content << "/#{options[:netmask]}" if options[:netmask] + content << " via #{options[:gateway]}" if options[:gateway] + content << "\n" + end + + return content + end end diff --git a/chef/spec/unit/provider/route_spec.rb b/chef/spec/unit/provider/route_spec.rb index fc49b7b902c..1e889ca5a5e 100644 --- a/chef/spec/unit/provider/route_spec.rb +++ b/chef/spec/unit/provider/route_spec.rb @@ -20,7 +20,7 @@ describe Chef::Provider::Route do before do - @node = Chef::Node.new + @node = Chef::Node.new @run_context = Chef::RunContext.new(@node, {}) @new_resource = Chef::Resource::Route.new('0.0.0.0') @@ -39,14 +39,18 @@ it "should add the route if it does not exist" do @provider.stub!(:run_command).and_return(true) @current_resource.stub!(:gateway).and_return(nil) + @provider.should_receive(:generate_command).once.with(:add) @new_resource.should_receive(:updated=).with(true) + @provider.should_receive(:generate_config) @provider.action_add end it "should not add the route if it exists" do @provider.stub!(:run_command).and_return(true) @provider.stub!(:is_running).and_return(true) + @provider.should_not_receive(:generate_command).with(:add) @new_resource.should_not_receive(:updated=).with(true) + @provider.should_receive(:generate_config) @provider.action_add end end @@ -54,6 +58,7 @@ describe Chef::Provider::Route, "action_delete" do it "should delete the route if it exists" do @provider.stub!(:run_command).and_return(true) + @provider.should_receive(:generate_command).once.with(:delete) @new_resource.should_receive(:updated=).with(true) @provider.stub!(:is_running).and_return(true) @provider.action_delete @@ -62,8 +67,76 @@ it "should not delete the route if it does not exist" do @current_resource.stub!(:gateway).and_return(nil) @provider.stub!(:run_command).and_return(true) + @provider.should_not_receive(:generate_command).with(:add) @new_resource.should_not_receive(:updated=).with(true) @provider.action_delete end end + + describe Chef::Provider::Route, "generate_command for action_add" do + it "should include a netmask when a one is specified" do + @new_resource.stub!(:netmask).and_return('255.255.0.0') + @provider.generate_command(:add).should match(/\/\d{1,2}\s/) + end + + it "should not include a netmask when a one is specified" do + @new_resource.stub!(:netmask).and_return(nil) + @provider.generate_command(:add).should_not match(/\/\d{1,2}\s/) + end + + it "should include ' via $gateway ' when a gateway is specified" do + @provider.generate_command(:add).should match(/\svia\s#{@new_resource.gateway}\s/) + end + + it "should not include ' via $gateway ' when a gateway is not specified" do + @new_resource.stub!(:gateway).and_return(nil) + @provider.generate_command(:add).should_not match(/\svia\s#{@new_resource.gateway}\s/) + end + end + + describe Chef::Provider::Route, "generate_command for action_delete" do + it "should include a netmask when a one is specified" do + @new_resource.stub!(:netmask).and_return('255.255.0.0') + @provider.generate_command(:delete).should match(/\/\d{1,2}\s/) + end + + it "should not include a netmask when a one is specified" do + @new_resource.stub!(:netmask).and_return(nil) + @provider.generate_command(:delete).should_not match(/\/\d{1,2}\s/) + end + + it "should include ' via $gateway ' when a gateway is specified" do + @provider.generate_command(:delete).should match(/\svia\s#{@new_resource.gateway}\s/) + end + + it "should not include ' via $gateway ' when a gateway is not specified" do + @new_resource.stub!(:gateway).and_return(nil) + @provider.generate_command(:delete).should_not match(/\svia\s#{@new_resource.gateway}\s/) + end + end + + describe Chef::Provider::Route, "config_file_contents for action_add" do + it "should include a netmask when a one is specified" do + @new_resource.stub!(:netmask).and_return('255.255.0.0') + @provider.config_file_contents(:add, { :target => @new_resource.target, :netmask => @new_resource.netmask}).should match(/\/\d{1,2}.*\n$/) + end + + it "should not include a netmask when a one is specified" do + @provider.config_file_contents(:add, { :target => @new_resource.target}).should_not match(/\/\d{1,2}.*\n$/) + end + + it "should include ' via $gateway ' when a gateway is specified" do + @provider.config_file_contents(:add, { :target => @new_resource.target, :gateway => @new_resource.gateway}).should match(/\svia\s#{@new_resource.gateway}\n/) + end + + it "should not include ' via $gateway ' when a gateway is not specified" do + @provider.generate_command(:add).should_not match(/\svia\s#{@new_resource.gateway}\n/) + end + end + + describe Chef::Provider::Route, "config_file_contents for action_delete" do + it "should return an empty string" do + @provider.config_file_contents(:delete).should match(/^$/) + end + end end