Find network binaries with the which helper

We're doing something very similar to which in these methods. We should just use which directly and avoid a bit more complexity

Signed-off-by: Tim Smith <tsmith@chef.io>

2 files changed, 14 insertions, 34 deletions

diff --git a/lib/ohai/plugins/linux/network.rb b/lib/ohai/plugins/linux/network.rb
index 11a6d227..87364499 100644
--- a/lib/ohai/plugins/linux/network.rb
+++ b/lib/ohai/plugins/linux/network.rb
@@ -37,12 +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) }
+  def iproute2_binary_path
+    which("ip")
   end
 
-  def find_ethtool_binary
-    ["/sbin/ethtool", "/usr/sbin/ethtool"].find { |path| File.exist?(path) }
+  def ethtool_binary_path
+    which("ethtool")
   end
 
   def is_openvz?
@@ -151,7 +151,7 @@ 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 = 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"
@@ -175,7 +175,7 @@ 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 = ethtool_binary_path
     iface.each_key do |tmp_int|
       next unless iface[tmp_int][:encapsulation] == "Ethernet"
       so = shell_out("#{ethtool_binary} -g #{tmp_int}")
@@ -471,7 +471,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 iproute2_binary_path
       # 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..3691d974 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(:iproute2_binary_path).and_return( network_method == "ifconfig" ? false : "/sbin/ip" )
+        allow(plugin).to receive(:ethtool_binary_path).and_return( "/sbin/ethtool" )
         plugin.run
       end
 
@@ -674,7 +654,7 @@ 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(:iproute2_binary_path).and_return( "/sbin/ip" )
         plugin.run
       end
 
@@ -710,9 +690,9 @@ EOM
       end
     end
 
-    describe "setting the node's default IP address attribute with #{network_method}" do
+    describe "setting the node's default IP address attribute with iproute2" do
       before(:each) do
-        allow(plugin).to receive(:iproute2_binary_available?).and_return( network_method == "iproute2" )
+        allow(plugin).to receive(:iproute2_binary_path).and_return( "/sbin/ip" )
         plugin.run
       end
 
@@ -790,9 +770,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(:iproute2_binary_path).and_return( "/sbin/ip" )
+      allow(plugin).to receive(:ethtool_binary_path).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