Merge pull request #1009 from chef/network_which

Find network binaries with the which helper

2 files changed, 16 insertions, 38 deletions

diff --git a/lib/ohai/plugins/linux/network.rb b/lib/ohai/plugins/linux/network.rb
index 11a6d227..32a89133 100644
--- a/lib/ohai/plugins/linux/network.rb
+++ b/lib/ohai/plugins/linux/network.rb
@@ -37,16 +37,12 @@ Ohai.plugin(:Network) do
     File.exist? "/proc/net/if_inet6"
   end
 
-  def iproute2_binary_available?
-    ["/sbin/ip", "/usr/bin/ip", "/bin/ip"].any? { |path| File.exist?(path) }
-  end
-
-  def find_ethtool_binary
-    ["/sbin/ethtool", "/usr/sbin/ethtool"].find { |path| File.exist?(path) }
+  def ethtool_binary_path
+    @ethtool ||= which("ethtool")
   end
 
   def is_openvz?
-    ::File.directory?("/proc/vz")
+    @openvz ||= ::File.directory?("/proc/vz")
   end
 
   def is_openvz_host?
@@ -151,11 +147,11 @@ Ohai.plugin(:Network) do
 
   # determine layer 1 details for the interface using ethtool
   def ethernet_layer_one(iface)
-    return iface unless ethtool_binary = find_ethtool_binary
+    return iface unless ethtool_binary_path
     keys = %w{ Speed Duplex Port Transceiver Auto-negotiation MDI-X }
     iface.each_key do |tmp_int|
       next unless iface[tmp_int][:encapsulation] == "Ethernet"
-      so = shell_out("#{ethtool_binary} #{tmp_int}")
+      so = shell_out("#{ethtool_binary_path} #{tmp_int}")
       so.stdout.lines do |line|
         line.chomp!
         Ohai::Log.debug("Plugin Network: Parsing ethtool output: #{line}")
@@ -175,10 +171,10 @@ Ohai.plugin(:Network) do
 
   # determine ring parameters for the interface using ethtool
   def ethernet_ring_parameters(iface)
-    return iface unless ethtool_binary = find_ethtool_binary
+    return iface unless ethtool_binary_path
     iface.each_key do |tmp_int|
       next unless iface[tmp_int][:encapsulation] == "Ethernet"
-      so = shell_out("#{ethtool_binary} -g #{tmp_int}")
+      so = shell_out("#{ethtool_binary_path} -g #{tmp_int}")
       Ohai::Log.debug("Plugin Network: Parsing ethtool output: #{so.stdout}")
       type = nil
       iface[tmp_int]["ring_params"] = {}
@@ -471,7 +467,7 @@ Ohai.plugin(:Network) do
     # The '@eth0:' portion doesn't exist on primary interfaces and thus is optional in the regex
     IPROUTE_INT_REGEX = /^(\d+): ([0-9a-zA-Z@:\.\-_]*?)(@[0-9a-zA-Z]+|):\s/ unless defined? IPROUTE_INT_REGEX
 
-    if iproute2_binary_available?
+    if which("ip")
       # families to get default routes from
       families = [{
                     :name => "inet",
diff --git a/spec/unit/plugins/linux/network_spec.rb b/spec/unit/plugins/linux/network_spec.rb
index 03b16d9f..b0c5ad06 100644
--- a/spec/unit/plugins/linux/network_spec.rb
+++ b/spec/unit/plugins/linux/network_spec.rb
@@ -377,26 +377,6 @@ EOM
     allow(plugin).to receive(:shell_out).with(/ethtool [^\-]/).and_return(mock_shell_out(0, linux_ethtool, ""))
   end
 
-  describe "#iproute2_binary_available?" do
-    ["/sbin/ip", "/usr/bin/ip", "/bin/ip"].each do |path|
-      it "accepts #{path}" do
-        allow(File).to receive(:exist?).and_return(false)
-        allow(File).to receive(:exist?).with(path).and_return(true)
-        expect(plugin.iproute2_binary_available?).to be_truthy
-      end
-    end
-  end
-
-  describe "#find_ethtool_binary" do
-    ["/sbin/ethtool", "/usr/sbin/ethtool"].each do |path|
-      it "accepts #{path}" do
-        allow(File).to receive(:exist?).and_return(false)
-        allow(File).to receive(:exist?).with(path).and_return(true)
-        expect(plugin.find_ethtool_binary).to end_with("/ethtool")
-      end
-    end
-  end
-
   describe "#interface_has_no_addresses_in_family?" do
     context "when interface has no addresses" do
       let(:iface) { {} }
@@ -555,8 +535,8 @@ EOM
   %w{ifconfig iproute2}.each do |network_method|
     describe "gathering IP layer address info via #{network_method}" do
       before(:each) do
-        allow(plugin).to receive(:iproute2_binary_available?).and_return( network_method == "iproute2" )
-        allow(plugin).to receive(:find_ethtool_binary).and_return( "/sbin/ethtool" )
+        allow(plugin).to receive(:which).with("ip").and_return( network_method == "iproute2" ? "/sbin/ip" : false )
+        allow(plugin).to receive(:which).with("ethtool").and_return( "/sbin/ethtool" )
         plugin.run
       end
 
@@ -674,7 +654,8 @@ EOM
 
     describe "gathering interface counters via #{network_method}" do
       before(:each) do
-        allow(plugin).to receive(:iproute2_binary_available?).and_return( network_method == "iproute2" )
+        allow(plugin).to receive(:which).with("ip").and_return(network_method == "iproute2" ? "/sbin/ip" : false)
+        allow(plugin).to receive(:which).with("ethtool").and_return("/sbin/ethtool")
         plugin.run
       end
 
@@ -712,7 +693,8 @@ EOM
 
     describe "setting the node's default IP address attribute with #{network_method}" do
       before(:each) do
-        allow(plugin).to receive(:iproute2_binary_available?).and_return( network_method == "iproute2" )
+        allow(plugin).to receive(:which).with("ip").and_return(network_method == "iproute2" ? "/sbin/ip" : false)
+        allow(plugin).to receive(:which).with("ethtool").and_return("/sbin/ethtool")
         plugin.run
       end
 
@@ -790,9 +772,9 @@ EOM
 
   describe "for newer network features using iproute2 only" do
     before(:each) do
-      allow(File).to receive(:exist?).with("/sbin/ip").and_return(true) # iproute2 only
+      allow(plugin).to receive(:which).with("ip").and_return("/sbin/ip")
+      allow(plugin).to receive(:which).with("ethtool").and_return( "/sbin/ethtool" )
       allow(File).to receive(:exist?).with("/proc/net/if_inet6").and_return(true) # ipv6 is enabled
-      allow(File).to receive(:exist?).with("/sbin/ethtool").and_return(true) # ethtool is available
       plugin.run
     end