Fix yum provider blocking and flushing

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

4 files changed, 52 insertions, 45 deletions

diff --git a/lib/chef/provider/package/yum.rb b/lib/chef/provider/package/yum.rb
index 76b9b15172..eefc6b939f 100644
--- a/lib/chef/provider/package/yum.rb
+++ b/lib/chef/provider/package/yum.rb
@@ -237,11 +237,8 @@ class Chef
           @installed_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.closerpmdb
         end
 
         def yum_binary
diff --git a/lib/chef/provider/package/yum/python_helper.rb b/lib/chef/provider/package/yum/python_helper.rb
index 7758383b95..288016a296 100644
--- a/lib/chef/provider/package/yum/python_helper.rb
+++ b/lib/chef/provider/package/yum/python_helper.rb
@@ -81,6 +81,10 @@ class Chef
             start if stdin.nil?
           end
 
+          def closerpmdb
+            query("closerpmdb", {})
+          end
+
           def compare_versions(version1, version2)
             query("versioncompare", { "versions" => [version1, version2] }).to_i
           end
@@ -117,12 +121,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  before and after every query with an enablerepo/disablerepo to clean the helpers internal state
+            closerpmdb 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?
+            closerpmdb unless repo_opts.empty?
             version
           end
 
@@ -209,7 +213,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["YUMHELPER_NO_RETRIES"]
               unless output.empty?
                 Chef::Log.trace "discarding output on stderr/stdout from python helper: #{output}"
               end
diff --git a/lib/chef/provider/package/yum/yum_helper.py b/lib/chef/provider/package/yum/yum_helper.py
index 6a7b481f62..5aa607b4ab 100644
--- a/lib/chef/provider/package/yum/yum_helper.py
+++ b/lib/chef/provider/package/yum/yum_helper.py
@@ -10,6 +10,8 @@ import sys
 import yum
 import signal
 import os
+import fcntl
+
 sys.path.append(os.path.join(os.path.dirname(__file__), '..', 'simplejson'))
 try: import json
 except ImportError: import simplejson as json
@@ -32,15 +34,6 @@ except ImportError:
 if not hasattr(yum.packages.FakeRepository, 'compare_providers_priority'):
     yum.packages.FakeRepository.compare_providers_priority = 99
 
-base = None
-
-def get_base():
-    global base
-    if base is None:
-        base = yum.YumBase()
-    setup_exit_handler()
-    return base
-
 def versioncompare(versions):
     arch_list = getArchList()
     candidate_arch1 = versions[0].split(".")[-1]
@@ -51,9 +44,9 @@ def versioncompare(versions):
     # then we'll chop the arch component (assuming it *is* a valid one) from the first version string
     # so we're only comparing the evr portions.
     if (candidate_arch2 not in arch_list) and (candidate_arch1 in arch_list):
-       final_version1 = versions[0].replace("." + candidate_arch1,"")
+        final_version1 = versions[0].replace("." + candidate_arch1,"")
     else:
-       final_version1 = versions[0]
+        final_version1 = versions[0]
 
     final_version2 = versions[1]
 
@@ -64,12 +57,11 @@ def versioncompare(versions):
     outpipe.write("%(e)s\n" % { 'e': evr_comparison })
     outpipe.flush()
 
-def install_only_packages(name):
-    base = get_base()
+def install_only_packages(base, name):
     if name in base.conf.installonlypkgs:
-      outpipe.write('True')
+        outpipe.write('True')
     else:
-      outpipe.write('False')
+        outpipe.write('False')
     outpipe.flush()
 
 # python2.4 / centos5 compat
@@ -82,19 +74,17 @@ except NameError:
                 return True
         return False
 
-def query(command):
-    base = get_base()
-
+def query(base, command):
     enabled_repos = base.repos.listEnabled()
 
     # Handle any repocontrols passed in with our options
 
     if 'repos' in command:
-      for repo in command['repos']:
-        if 'enable' in repo:
-          base.repos.enableRepo(repo['enable'])
+        for repo in command['repos']:
+            if 'enable' in repo:
+                base.repos.enableRepo(repo['enable'])
         if 'disable' in repo:
-          base.repos.disableRepo(repo['disable'])
+            base.repos.disableRepo(repo['disable'])
 
     args = { 'name': command['provides'] }
     do_nevra = False
@@ -136,7 +126,7 @@ def query(command):
         #    returnPackages and searchProvides and then apply the Nevra filters to those results.
         pkgs = obj.searchNevra(**args)
         if (command['action'] == "whatinstalled") and (not pkgs):
-          pkgs = obj.searchNevra(name=args['name'], arch=desired_arch)
+            pkgs = obj.searchNevra(name=args['name'], arch=desired_arch)
     else:
         pats = [command['provides']]
         pkgs = obj.returnPackages(patterns=pats)
@@ -157,13 +147,13 @@ def query(command):
 
     # Reset any repos we were passed in enablerepo/disablerepo to the original state in enabled_repos
     if 'repos' in command:
-      for repo in command['repos']:
-        if 'enable' in repo:
-          if base.repos.getRepo(repo['enable']) not in enabled_repos:
-            base.repos.disableRepo(repo['enable'])
+        for repo in command['repos']:
+            if 'enable' in repo:
+                if base.repos.getRepo(repo['enable']) not in enabled_repos:
+                    base.repos.disableRepo(repo['enable'])
         if 'disable' in repo:
-          if base.repos.getRepo(repo['disable']) in enabled_repos:
-            base.repos.enableRepo(repo['disable'])
+            if base.repos.getRepo(repo['disable']) in enabled_repos:
+                base.repos.enableRepo(repo['disable'])
 
 # the design of this helper is that it should try to be 'brittle' and fail hard and exit in order
 # to keep process tables clean.  additional error handling should probably be added to the retry loop
@@ -179,21 +169,29 @@ 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)
+
+base = None
+
 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
@@ -205,14 +203,20 @@ try:
         except ValueError, e:
             raise RuntimeError("bad json parse")
 
+        if base is None:
+            base = yum.YumBase()
+
         if command['action'] == "whatinstalled":
-            query(command)
+            query(base, command)
         elif command['action'] == "whatavailable":
-            query(command)
+            query(base, command)
         elif command['action'] == "versioncompare":
             versioncompare(command['versions'])
         elif command['action'] == "installonlypkgs":
-             install_only_packages(command['package'])
+            install_only_packages(base, command['package'])
+        elif command['action'] == "closerpmdb":
+            base.closeRpmDB()
+            base = None
         else:
             raise RuntimeError("bad command")
 finally:
diff --git a/spec/functional/resource/yum_package_spec.rb b/spec/functional/resource/yum_package_spec.rb
index 777db6ff2b..627a72841b 100644
--- a/spec/functional/resource/yum_package_spec.rb
+++ b/spec/functional/resource/yum_package_spec.rb
@@ -48,6 +48,8 @@ describe Chef::Resource::YumPackage, :requires_root, external: exclude_test do
   end
 
   before(:each) do
+    # force errors to fail and not retry
+    ENV["YUMHELPER_NO_RETRIES"] = "true"
     File.open("/etc/yum.repos.d/chef-yum-localtesting.repo", "w+") do |f|
       f.write <<~EOF
         [chef-yum-localtesting]