[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.

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

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,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