[Pkg-puppet-devel] [facter] 112/180: (FACT-233) Fix nmcli version detection for DHCP server fact.

[Stig Sandbeck Mathisen]

This is an automated email from the git hooks/post-receive script.

ssm pushed a commit to branch master
in repository facter.

commit d628cfeab231c2da2b01248cefcf3488d37b4021
Author: Peter Huene <peter.huene at puppetlabs.com>
Date:   Fri May 30 15:10:38 2014 -0700

    (FACT-233) Fix nmcli version detection for DHCP server fact.
    
    Previously, we were doing a simple "to_i" on the extracted version
    string from nmcli to determine which command line syntax to use.  If the
    version was greater than 0.9.9.0, we need to use the "show" command
    instead of the "list" command.  This was done by checking the result
    against 990 ("0990".to_i, meaning version "0.9.9.0").
    
    However, this won't work with version strings such as "0.9.8.10" since
    "09810".to_i > 990.
    
    The fix is to extract the version components of the string and
    explicitly check against version 0.9.9.0 or higher.
---
 lib/facter/util/dhcp_servers.rb     |  8 +++++---
 spec/unit/util/dhcp_servers_spec.rb | 35 +++++++++++++++++++++++++++++------
 2 files changed, 34 insertions(+), 9 deletions(-)

diff --git a/lib/facter/util/dhcp_servers.rb b/lib/facter/util/dhcp_servers.rb
index 8fc0ffc..785064d 100644
--- a/lib/facter/util/dhcp_servers.rb
+++ b/lib/facter/util/dhcp_servers.rb
@@ -13,7 +13,9 @@ module Facter::Util::DHCPServers
 
   def self.device_dhcp_server(device)
     if Facter::Core::Execution.which('nmcli')
-      if self.nmcli_version and self.nmcli_version >= 990
+      version = self.nmcli_version
+      # If the version is >= 0.9.9, use show instead of list
+      if version && version[0] > 0 || version[1] > 9 || (version[1] == 9 && version[2] >= 9)
         Facter::Core::Execution.exec("nmcli -f all d show #{device}").scan(/dhcp_server_identifier.*?(\d+\.\d+\.\d+\.\d+)$/).flatten.first
       else
         Facter::Core::Execution.exec("nmcli -f all d list iface #{device}").scan(/dhcp_server_identifier.*?(\d+\.\d+\.\d+\.\d+)$/).flatten.first
@@ -22,8 +24,8 @@ module Facter::Util::DHCPServers
   end
 
   def self.nmcli_version
-    if version = Facter::Core::Execution.exec("nmcli --version").scan(/version\s([\d\.]+)/).flatten.first
-      version.gsub(/\./,'').to_i
+    if version = Facter::Core::Execution.exec("nmcli --version")
+      version.scan(/version\s(\d+)\.?(\d+)?\.?(\d+)?\.?(\d+)?/).flatten.map(&:to_i)
     end
   end
 end
diff --git a/spec/unit/util/dhcp_servers_spec.rb b/spec/unit/util/dhcp_servers_spec.rb
index 0a5eeaf..ec7a646 100644
--- a/spec/unit/util/dhcp_servers_spec.rb
+++ b/spec/unit/util/dhcp_servers_spec.rb
@@ -6,22 +6,45 @@ require 'facter/util/dhcp_servers'
 describe Facter::Util::DHCPServers do
   describe "nmcli_version" do
     {
-      'nmcli tool, version 0.9.8.0' => 980,
-      'nmcli tool, version 0.9.8.9' => 989,
-      'nmcli tool, version 0.9.9.0' => 990,
-      'nmcli tool, version 0.9.9.9' => 999,
-      'version 0.9.9.0-20.git20131003.fc20' => 990,
+      'nmcli tool, version 0.9.8.0' => [0, 9, 8, 0],
+      'nmcli tool, version 0.9.8.10' => [0, 9, 8, 10],
+      'nmcli tool, version 0.9.8.9' => [0, 9, 8, 9],
+      'nmcli tool, version 0.9.9.0' => [0, 9, 9, 0],
+      'nmcli tool, version 0.9.9.9' => [0, 9, 9, 9],
+      'version 0.9.9.0-20.git20131003.fc20' => [0, 9, 9, 0],
+      'nmcli tool, version 0.9.9' => [0, 9, 9, 0],
+      'nmcli tool, version 0.9' => [0, 9, 0, 0],
+      'nmcli tool, version 1' => [1, 0, 0, 0]
     }.each do |version, expected|
       it "should turn #{version} into the integer #{expected}" do
         Facter::Core::Execution.stubs(:which).with('nmcli').returns('/usr/bin/nmcli')
         Facter::Core::Execution.stubs(:exec).with('nmcli --version').returns(version)
         
         result = Facter::Util::DHCPServers.nmcli_version
-        result.is_a?(Integer).should be true
+        result.is_a?(Array).should be true
         result.should == expected
       end
     end
   end
+
+  describe "device_dhcp_server" do
+    {
+        '0.1.2.3' => false,
+        '0.9.8.10' => false,
+        '0.9.9.0' => true,
+        '0.9.10.0' => true,
+        '0.10.0.0' => true,
+        '1.0.0.0' => true
+    }.each do |version, uses_show|
+      it "should use #{if uses_show then 'show' else 'list' end} for version #{version}" do
+        command = if uses_show then 'nmcli -f all d show eth0' else 'nmcli -f all d list iface eth0' end
+        Facter::Core::Execution.stubs(:which).with('nmcli').returns('/usr/bin/nmcli')
+        Facter::Core::Execution.stubs(:exec).with('nmcli --version').returns "nmcli tool, version #{version}"
+        Facter::Core::Execution.stubs(:exec).with(command).returns 'DHCP4.OPTION[1]: dhcp_server_identifier = 192.168.1.1'
+        Facter::Util::DHCPServers.device_dhcp_server('eth0').should == '192.168.1.1'
+      end
+    end
+  end
 end
 
 

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-puppet/facter.git