Permalink
Browse files

[CHEF-1333] don't include 'via' in route when a gateway isn't provide…

…d. also refactored route provider a bit and added some additional specs.
  • Loading branch information...
1 parent e098dfb commit 05cba30e64e5bac3957738e0cecb1bd655a4c832 @thbishop thbishop committed with danielsdeleo Jul 30, 2010
Showing with 110 additions and 13 deletions.
  1. +36 −12 chef/lib/chef/provider/route.rb
  2. +74 −1 chef/spec/unit/provider/route_spec.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
@@ -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,21 +39,26 @@
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
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

0 comments on commit 05cba30

Please sign in to comment.