Merge pull request #5782 from chef/lcg/dnf-rhel7

rhel7 / dnf 2.0 fixes / improved errors

4 files changed, 86 insertions, 34 deletions

diff --git a/lib/chef/mixin/which.rb b/lib/chef/mixin/which.rb
index 4fa79eeccb..fd386241a0 100644
--- a/lib/chef/mixin/which.rb
+++ b/lib/chef/mixin/which.rb
@@ -1,6 +1,6 @@
 #--
 # Author:: Lamont Granquist <lamont@chef.io>
-# Copyright:: Copyright 2010-2016, Chef Software Inc.
+# Copyright:: Copyright 2010-2017, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -18,15 +18,32 @@
 class Chef
   module Mixin
     module Which
-      def which(cmd, extra_path: nil)
+      def which(*cmds, extra_path: nil, &block)
+        where(*cmds, extra_path: extra_path, &block).first || false
+      end
+
+      def where(*cmds, extra_path: nil, &block)
         # NOTE: unnecessarily duplicates function of path_sanity
         extra_path ||= [ "/bin", "/usr/bin", "/sbin", "/usr/sbin" ]
-        paths = ENV["PATH"].split(File::PATH_SEPARATOR) + extra_path
-        paths.each do |path|
-          filename = Chef.path_to(File.join(path, cmd))
-          return filename if File.executable?(filename)
-        end
-        false
+        paths = env_path.split(File::PATH_SEPARATOR) + extra_path
+        cmds.map do |cmd|
+          paths.map do |path|
+            filename = Chef.path_to(File.join(path, cmd))
+            filename if valid_executable?(filename, &block)
+          end.compact
+        end.flatten
+      end
+
+      private
+
+      # for test stubbing
+      def env_path
+        ENV["PATH"]
+      end
+
+      def valid_executable?(filename, &block)
+        return false unless File.executable?(filename) && !File.directory?(filename)
+        block ? yield(filename) : true
       end
     end
   end
diff --git a/lib/chef/provider/package.rb b/lib/chef/provider/package.rb
index f52614672a..edd27a0439 100644
--- a/lib/chef/provider/package.rb
+++ b/lib/chef/provider/package.rb
@@ -1,6 +1,6 @@
 #
 # Author:: Adam Jacob (<adam@chef.io>)
-# Copyright:: Copyright 2008-2016, Chef Software, Inc.
+# Copyright:: Copyright 2008-2017, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
diff --git a/lib/chef/provider/package/dnf/dnf_helper.py b/lib/chef/provider/package/dnf/dnf_helper.py
index 236b967710..ef08bb54c2 100644
--- a/lib/chef/provider/package/dnf/dnf_helper.py
+++ b/lib/chef/provider/package/dnf/dnf_helper.py
@@ -53,7 +53,7 @@ def query(command):
     if len(archq.run()) > 0:
         q = archq
 
-    pkgs = dnf.query.latest_limit_pkgs(q, 1)
+    pkgs = q.latest(1).run()
 
     if not pkgs:
         sys.stdout.write('{} nil nil\n'.format(command['provides'].split().pop(0)))
diff --git a/lib/chef/provider/package/dnf/python_helper.rb b/lib/chef/provider/package/dnf/python_helper.rb
index 466114b339..d6e278a9fb 100644
--- a/lib/chef/provider/package/dnf/python_helper.rb
+++ b/lib/chef/provider/package/dnf/python_helper.rb
@@ -1,5 +1,5 @@
 #
-# Copyright:: Copyright 2016, Chef Software, Inc.
+# Copyright:: Copyright 2016-2017, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -15,6 +15,8 @@
 # limitations under the License.
 #
 
+require "chef/mixin/which"
+require "chef/mixin/shell_out"
 require "chef/provider/package/dnf/version"
 require "timeout"
 
@@ -24,7 +26,8 @@ class Chef
       class Dnf < Chef::Provider::Package
         class PythonHelper
           include Singleton
-          extend Chef::Mixin::Which
+          include Chef::Mixin::Which
+          include Chef::Mixin::ShellOut
 
           attr_accessor :stdin
           attr_accessor :stdout
@@ -32,11 +35,16 @@ class Chef
           attr_accessor :wait_thr
 
           DNF_HELPER = ::File.expand_path(::File.join(::File.dirname(__FILE__), "dnf_helper.py")).freeze
-          DNF_COMMAND = "#{which("python3")} #{DNF_HELPER}"
+
+          def dnf_command
+            @dnf_command ||= which("python", "python3", "python2", "python2.7") do |f|
+              shell_out("#{f} -c 'import dnf'").exitstatus == 0
+            end + " #{DNF_HELPER}"
+          end
 
           def start
             ENV["PYTHONUNBUFFERED"] = "1"
-            @stdin, @stdout, @stderr, @wait_thr = Open3.popen3(DNF_COMMAND)
+            @stdin, @stdout, @stderr, @wait_thr = Open3.popen3(dnf_command)
           end
 
           def reap
@@ -53,6 +61,27 @@ class Chef
             start if stdin.nil?
           end
 
+          # @returns Array<Version>
+          def query(action, provides, version = nil, arch = nil)
+            with_helper do
+              json = build_query(action, provides, version, arch)
+              Chef::Log.debug "sending '#{json}' to python helper"
+              stdin.syswrite json + "\n"
+              output = stdout.sysread(4096).chomp
+              Chef::Log.debug "got '#{output}' from python helper"
+              version = parse_response(output)
+              Chef::Log.debug "parsed #{version} from python helper"
+              version
+            end
+          end
+
+          def restart
+            reap
+            start
+          end
+
+          private
+
           # i couldn't figure out how to decompose an evr on the python side, it seems reasonably
           # painless to do it in ruby (generally massaging nevras in the ruby side is HIGHLY
           # discouraged -- this is an "every rule has an exception" exception -- any additional
@@ -83,35 +112,41 @@ class Chef
             array.each_slice(3).map { |x| Version.new(*x) }.first
           end
 
-          # @returns Array<Version>
-          def query(action, provides, version = nil, arch = nil)
-            with_helper do
-              json = build_query(action, provides, version, arch)
-              Chef::Log.debug "sending '#{json}' to python helper"
-              stdin.syswrite json + "\n"
-              output = stdout.sysread(4096).chomp
-              Chef::Log.debug "got '#{output}' from python helper"
-              version = parse_response(output)
-              Chef::Log.debug "parsed #{version} from python helper"
-              version
+          def drain_stderr
+            output = ""
+            until IO.select([stderr], nil, nil, 0).nil?
+              output += stderr.sysread(4096).chomp
             end
-          end
-
-          def restart
-            reap
-            start
+            output
+          rescue
+            # we must rescue EOFError, and we don't much care about errors on stderr anyway
+            output
           end
 
           def with_helper
             max_retries ||= 5
+            ret = nil
             Timeout.timeout(600) do
               check
-              yield
+              ret = yield
             end
+            output = drain_stderr
+            unless output.empty?
+              Chef::Log.debug "discarding output on stderr from python helper: #{output}"
+            end
+            ret
           rescue EOFError, Errno::EPIPE, Timeout::Error, Errno::ESRCH => e
-            raise e unless ( max_retries -= 1 ) > 0
-            restart
-            retry
+            output = drain_stderr
+            if ( max_retries -= 1 ) > 0
+              unless output.empty?
+                Chef::Log.debug "discarding output on stderr from python helper: #{output}"
+              end
+              restart
+              retry
+            else
+              raise e if output.empty?
+              raise "dnf-helper.py had stderr output:\n\n#{output}"
+            end
           end
         end
       end