Fix DNF package blocking and flushing

Nearly identical fixes as #11486

Signed-off-by: Lamont Granquist <lamont@scriptkiddie.org>

4 files changed, 36 insertions, 19 deletions

diff --git a/lib/chef/provider/package/dnf.rb b/lib/chef/provider/package/dnf.rb
index 5c74ad0414..c0c7def44f 100644
--- a/lib/chef/provider/package/dnf.rb
+++ b/lib/chef/provider/package/dnf.rb
@@ -242,11 +242,8 @@ class Chef
           @current_version[index]
         end
 
-        # cache flushing is accomplished by simply restarting the python helper.  this produces a roughly
-        # 15% hit to the runtime of installing/removing/upgrading packages.  correctly using multipackage
-        # array installs (and the multipackage cookbook) can produce 600% improvements in runtime.
         def flushcache
-          python_helper.restart
+          python_helper.close_rpmdb
         end
 
         def dnf(*args)
diff --git a/lib/chef/provider/package/dnf/dnf_helper.py b/lib/chef/provider/package/dnf/dnf_helper.py
index c9d49f33b5..7b47872bce 100644
--- a/lib/chef/provider/package/dnf/dnf_helper.py
+++ b/lib/chef/provider/package/dnf/dnf_helper.py
@@ -7,6 +7,7 @@ import hawkey
 import signal
 import os
 import json
+import fcntl
 
 base = None
 
@@ -78,12 +79,12 @@ def version_tuple(versionstr):
 def versioncompare(versions):
     sack = get_sack()
     if (versions[0] is None) or (versions[1] is None):
-      outpipe.write('0\n')
-      outpipe.flush()
+        outpipe.write('0\n')
+        outpipe.flush()
     else:
-      evr_comparison = dnf.rpm.rpm.labelCompare(version_tuple(versions[0]), version_tuple(versions[1]))
-      outpipe.write('{}\n'.format(evr_comparison))
-      outpipe.flush()
+        evr_comparison = dnf.rpm.rpm.labelCompare(version_tuple(versions[0]), version_tuple(versions[1]))
+        outpipe.write('{}\n'.format(evr_comparison))
+        outpipe.flush()
 
 def query(command):
     sack = get_sack()
@@ -151,21 +152,27 @@ def setup_exit_handler():
     signal.signal(signal.SIGPIPE, exit_handler)
     signal.signal(signal.SIGQUIT, exit_handler)
 
+def set_blocking(fd):
+    old_flags = fcntl.fcntl(fd, fcntl.F_GETFL)
+    fcntl.fcntl(fd, fcntl.F_SETFL, old_flags & ~os.O_NONBLOCK)
+
 if len(sys.argv) < 3:
-  inpipe = sys.stdin
-  outpipe = sys.stdout
+    inpipe = sys.stdin
+    outpipe = sys.stdout
 else:
-  inpipe = os.fdopen(int(sys.argv[1]), "r")
-  outpipe = os.fdopen(int(sys.argv[2]), "w")
+    set_blocking(int(sys.argv[1]))
+    set_blocking(int(sys.argv[2]))
+    inpipe = os.fdopen(int(sys.argv[1]), "r")
+    outpipe = os.fdopen(int(sys.argv[2]), "w")
 
 try:
+    setup_exit_handler()
     while 1:
         # stop the process if the parent proc goes away
         ppid = os.getppid()
         if ppid == 1:
             raise RuntimeError("orphaned")
 
-        setup_exit_handler()
         line = inpipe.readline()
 
         # only way to detect EOF in python
@@ -183,6 +190,11 @@ try:
             query(command)
         elif command['action'] == "versioncompare":
             versioncompare(command['versions'])
+        elif command['action'] == "close_rpmdb":
+            base.close()
+            base = None
+            outpipe.write('nil nil nil\n')
+            outpipe.flush()
         else:
             raise RuntimeError("bad command")
 finally:
diff --git a/lib/chef/provider/package/dnf/python_helper.rb b/lib/chef/provider/package/dnf/python_helper.rb
index 1f6243b9b2..b02db340ba 100644
--- a/lib/chef/provider/package/dnf/python_helper.rb
+++ b/lib/chef/provider/package/dnf/python_helper.rb
@@ -52,6 +52,8 @@ class Chef
           end
 
           def start
+            # For some reason we have to force python to unbuffered here, and then force the input pipe back to line
+            # buffered in the python code.  XXX: I tried to remove this but hit more issues in the python side.
             ENV["PYTHONUNBUFFERED"] = "1"
             @inpipe, inpipe_write = IO.pipe
             outpipe_read, @outpipe = IO.pipe
@@ -82,6 +84,10 @@ class Chef
             start if stdin.nil?
           end
 
+          def close_rpmdb
+            query("close_rpmdb", {})
+          end
+
           def compare_versions(version1, version2)
             query("versioncompare", { "versions" => [version1, version2] }).to_i
           end
@@ -109,12 +115,12 @@ class Chef
             parameters = { "provides" => provides, "version" => version, "arch" => arch }
             repo_opts = options_params(options || {})
             parameters.merge!(repo_opts)
-            # XXX: for now we restart before and after every query with an enablerepo/disablerepo to clean the helpers internal state
-            restart unless repo_opts.empty?
+            # XXX: for now we close the rpmdb before and after every query with an enablerepo/disablerepo to clean the helpers internal state
+            close_rpmdb unless repo_opts.empty?
             query_output = query(action, parameters)
             version = parse_response(query_output.lines.last)
             Chef::Log.trace "parsed #{version} from python helper"
-            restart unless repo_opts.empty?
+            close_rpmdb unless repo_opts.empty?
             version
           end
 
@@ -151,7 +157,7 @@ class Chef
               outpipe.syswrite json + "\n"
               output = inpipe.sysread(4096).chomp
               Chef::Log.trace "got '#{output}' from python helper"
-              return output
+              output
             end
           end
 
@@ -201,7 +207,7 @@ class Chef
             ret
           rescue EOFError, Errno::EPIPE, Timeout::Error, Errno::ESRCH => e
             output = drain_fds
-            if ( max_retries -= 1 ) > 0
+            if ( max_retries -= 1 ) > 0 && !ENV["YUM_HELPER_NO_RETRIES"]
               unless output.empty?
                 Chef::Log.trace "discarding output on stderr/stdout from python helper: #{output}"
               end
diff --git a/spec/functional/resource/dnf_package_spec.rb b/spec/functional/resource/dnf_package_spec.rb
index 0aa88b7a23..97256472d6 100644
--- a/spec/functional/resource/dnf_package_spec.rb
+++ b/spec/functional/resource/dnf_package_spec.rb
@@ -48,6 +48,8 @@ describe Chef::Resource::DnfPackage, :requires_root, external: exclude_test do
   end
 
   before(:each) do
+    # force errors to fail and not retry
+    ENV["YUM_HELPER_NO_RETRIES"] = "true"
     File.open("/etc/yum.repos.d/chef-dnf-localtesting.repo", "w+") do |f|
       f.write <<~EOF
         [chef-dnf-localtesting]