Merge branch 'CHEF-2637'

2 files changed, 121 insertions, 54 deletions

diff --git a/chef/lib/chef/provider/package/yum.rb b/chef/lib/chef/provider/package/yum.rb
index f0271c9d4a..b97a8b3a83 100644
--- a/chef/lib/chef/provider/package/yum.rb
+++ b/chef/lib/chef/provider/package/yum.rb
@@ -834,6 +834,7 @@ class Chef
           end
 
           private
+
           def version(package_name, arch=nil, is_available=false, is_installed=false)
             refresh
             packages = @rpmdb[package_name]
@@ -923,6 +924,37 @@ class Chef
           arch ? ".#{arch}" : nil
         end
 
+        def yum_command(command)
+          status, stdout, stderr = output_of_command(command, {})
+
+          # This is fun: rpm can encounter errors in the %post/%postun scripts which aren't
+          # considered fatal - meaning the rpm is still successfully installed. These issue
+          # cause yum to emit a non fatal warning but still exit(1). As there's currently no
+          # way to suppress this behavior and an exit(1) will break a Chef run we make an
+          # effort to trap these and re-run the same install command - it will either fail a
+          # second time or succeed.
+          #
+          # A cleaner solution would have to be done in python and better hook into
+          # yum/rpm to handle exceptions as we see fit.
+          if status.exitstatus == 1
+            stdout.each_line do |l|
+              # rpm-4.4.2.3 lib/psm.c line 2182
+              if l =~ %r{^error: %(post|postun)\(.*\) scriptlet failed, exit status \d+$}
+                Chef::Log.warn("#{@new_resource} caught non-fatal scriptlet issue: \"#{l}\". Can't trust yum exit status " +
+                               "so running install again to verify.")
+                status, stdout, stderr = output_of_command(command, {})
+                break
+              end
+            end
+          end
+
+          if status.exitstatus > 0
+            command_output = "STDOUT: #{stdout}"
+            command_output << "STDERR: #{stderr}"
+            handle_command_failures(status, command_output, {})
+          end
+        end
+
         # Standard Provider methods for Parent
         #
 
@@ -989,9 +1021,7 @@ class Chef
 
         def install_package(name, version)
           if @new_resource.source
-            run_command_with_systems_locale(
-              :command => "yum -d0 -e0 -y#{expand_options(@new_resource.options)} localinstall #{@new_resource.source}"
-            )
+            yum_command("yum -d0 -e0 -y#{expand_options(@new_resource.options)} localinstall #{@new_resource.source}")
           else
             # Work around yum not exiting with an error if a package doesn't exist for CHEF-2062
             if @yum.version_available?(name, version, arch)
@@ -1016,9 +1046,7 @@ class Chef
                 end
               end
 
-              run_command_with_systems_locale(
-                :command => "yum -d0 -e0 -y#{expand_options(@new_resource.options)} #{method} #{name}-#{version}#{yum_arch}"
-              )
+              yum_command("yum -d0 -e0 -y#{expand_options(@new_resource.options)} #{method} #{name}-#{version}#{yum_arch}")
             else
               raise Chef::Exceptions::Package, "Version #{version} of #{name} not found. Did you specify both version " +
                                                "and release? (version-release, e.g. 1.84-10.fc6)"
@@ -1054,13 +1082,9 @@ class Chef
 
         def remove_package(name, version)
           if version
-            run_command_with_systems_locale(
-             :command => "yum -d0 -e0 -y#{expand_options(@new_resource.options)} remove #{name}-#{version}#{yum_arch}"
-            )
+            yum_command("yum -d0 -e0 -y#{expand_options(@new_resource.options)} remove #{name}-#{version}#{yum_arch}")
           else
-            run_command_with_systems_locale(
-             :command => "yum -d0 -e0 -y#{expand_options(@new_resource.options)} remove #{name}#{yum_arch}"
-            )
+            yum_command("yum -d0 -e0 -y#{expand_options(@new_resource.options)} remove #{name}#{yum_arch}")
           end
           if flush_cache[:after]
             @yum.reload
diff --git a/chef/spec/unit/provider/package/yum_spec.rb b/chef/spec/unit/provider/package/yum_spec.rb
index 966239205d..3c58167450 100644
--- a/chef/spec/unit/provider/package/yum_spec.rb
+++ b/chef/spec/unit/provider/package/yum_spec.rb
@@ -307,17 +307,17 @@ describe Chef::Provider::Package::Yum do
     it "should run yum install with the package name and version" do
       @provider.load_current_resource
       Chef::Provider::Package::Yum::RPMUtils.stub!(:rpmvercmp).and_return(-1)
-      @provider.should_receive(:run_command_with_systems_locale).with({
-        :command => "yum -d0 -e0 -y install emacs-1.0"
-      })
+      @provider.should_receive(:yum_command).with(
+        "yum -d0 -e0 -y install emacs-1.0"
+      )
       @provider.install_package("emacs", "1.0")
     end
 
     it "should run yum localinstall if given a path to an rpm" do
       @new_resource.stub!(:source).and_return("/tmp/emacs-21.4-20.el5.i386.rpm")
-      @provider.should_receive(:run_command_with_systems_locale).with({
-        :command => "yum -d0 -e0 -y localinstall /tmp/emacs-21.4-20.el5.i386.rpm"
-      })
+      @provider.should_receive(:yum_command).with(
+        "yum -d0 -e0 -y localinstall /tmp/emacs-21.4-20.el5.i386.rpm"
+      )
       @provider.install_package("emacs", "21.4-20.el5")
     end
 
@@ -327,9 +327,9 @@ describe Chef::Provider::Package::Yum do
       @provider = Chef::Provider::Package::Yum.new(@new_resource, @run_context)
       @provider.load_current_resource
       @new_resource.source.should == "/tmp/emacs-21.4-20.el5.i386.rpm"
-      @provider.should_receive(:run_command_with_systems_locale).with({
-        :command => "yum -d0 -e0 -y localinstall /tmp/emacs-21.4-20.el5.i386.rpm"
-      })
+      @provider.should_receive(:yum_command).with(
+        "yum -d0 -e0 -y localinstall /tmp/emacs-21.4-20.el5.i386.rpm"
+      )
       @provider.install_package("/tmp/emacs-21.4-20.el5.i386.rpm", "21.4-20.el5")
     end
 
@@ -337,9 +337,9 @@ describe Chef::Provider::Package::Yum do
       @provider.load_current_resource
       @new_resource.stub!(:arch).and_return("i386")
       Chef::Provider::Package::Yum::RPMUtils.stub!(:rpmvercmp).and_return(-1)
-      @provider.should_receive(:run_command_with_systems_locale).with({
-        :command => "yum -d0 -e0 -y install emacs-21.4-20.el5.i386"
-      })
+      @provider.should_receive(:yum_command).with(
+        "yum -d0 -e0 -y install emacs-21.4-20.el5.i386"
+      )
       @provider.install_package("emacs", "21.4-20.el5")
     end
 
@@ -348,9 +348,9 @@ describe Chef::Provider::Package::Yum do
       @provider.candidate_version = '11'
       @new_resource.stub!(:options).and_return("--disablerepo epmd")
       Chef::Provider::Package::Yum::RPMUtils.stub!(:rpmvercmp).and_return(-1)
-      @provider.should_receive(:run_command_with_systems_locale).with({
-        :command => "yum -d0 -e0 -y --disablerepo epmd install cups-11"
-      })
+      @provider.should_receive(:yum_command).with(
+        "yum -d0 -e0 -y --disablerepo epmd install cups-11"
+      )
       @provider.install_package(@new_resource.name, @provider.candidate_version)
     end
 
@@ -401,9 +401,9 @@ describe Chef::Provider::Package::Yum do
       Chef::Provider::Package::Yum::YumCache.stub!(:instance).and_return(@yum_cache)
       @provider = Chef::Provider::Package::Yum.new(@new_resource, @run_context)
       @provider.load_current_resource
-      @provider.should_receive(:run_command_with_systems_locale).with({
-        :command => "yum -d0 -e0 -y install cups-1.2.4-11.15.el5"
-      })
+      @provider.should_receive(:yum_command).with(
+        "yum -d0 -e0 -y install cups-1.2.4-11.15.el5"
+      )
       @provider.install_package("cups", "1.2.4-11.15.el5")
     end
 
@@ -422,9 +422,9 @@ describe Chef::Provider::Package::Yum do
       Chef::Provider::Package::Yum::YumCache.stub!(:instance).and_return(@yum_cache)
       @provider = Chef::Provider::Package::Yum.new(@new_resource, @run_context)
       @provider.load_current_resource
-      @provider.should_receive(:run_command_with_systems_locale).with({
-        :command => "yum -d0 -e0 -y downgrade cups-1.2.4-11.15.el5"
-      })
+      @provider.should_receive(:yum_command).with(
+        "yum -d0 -e0 -y downgrade cups-1.2.4-11.15.el5"
+      )
       @provider.install_package("cups", "1.2.4-11.15.el5")
     end
 
@@ -432,9 +432,9 @@ describe Chef::Provider::Package::Yum do
       @new_resource.stub!(:flush_cache).and_return({:after => true, :before => false})
       @provider.load_current_resource
       Chef::Provider::Package::Yum::RPMUtils.stub!(:rpmvercmp).and_return(-1)
-      @provider.should_receive(:run_command_with_systems_locale).with({
-        :command => "yum -d0 -e0 -y install emacs-1.0"
-      })
+      @provider.should_receive(:yum_command).with(
+        "yum -d0 -e0 -y install emacs-1.0"
+      )
       @yum_cache.should_receive(:reload).once
       @provider.install_package("emacs", "1.0")
     end
@@ -443,9 +443,9 @@ describe Chef::Provider::Package::Yum do
       @new_resource.stub!(:flush_cache).and_return({:after => false, :before => false})
       @provider.load_current_resource
       Chef::Provider::Package::Yum::RPMUtils.stub!(:rpmvercmp).and_return(-1)
-      @provider.should_receive(:run_command_with_systems_locale).with({
-        :command => "yum -d0 -e0 -y install emacs-1.0"
-      })
+      @provider.should_receive(:yum_command).with(
+        "yum -d0 -e0 -y install emacs-1.0"
+      )
       @yum_cache.should_not_receive(:reload)
       @provider.install_package("emacs", "1.0")
     end
@@ -456,9 +456,9 @@ describe Chef::Provider::Package::Yum do
       @provider.load_current_resource
       @provider.candidate_version = '11'
       Chef::Provider::Package::Yum::RPMUtils.stub!(:rpmvercmp).and_return(-1)
-      @provider.should_receive(:run_command_with_systems_locale).with({
-        :command => "yum -d0 -e0 -y install cups-11"
-      })
+      @provider.should_receive(:yum_command).with(
+        "yum -d0 -e0 -y install cups-11"
+      )
       @provider.upgrade_package(@new_resource.name, @provider.candidate_version)
     end
 
@@ -467,9 +467,9 @@ describe Chef::Provider::Package::Yum do
       @current_resource = Chef::Resource::Package.new('cups')
       @provider.candidate_version = '11'
       Chef::Provider::Package::Yum::RPMUtils.stub!(:rpmvercmp).and_return(-1)
-      @provider.should_receive(:run_command_with_systems_locale).with({
-        :command => "yum -d0 -e0 -y install cups-11"
-      })
+      @provider.should_receive(:yum_command).with(
+        "yum -d0 -e0 -y install cups-11"
+      )
       @provider.upgrade_package(@new_resource.name, @provider.candidate_version)
     end
 
@@ -493,30 +493,73 @@ describe Chef::Provider::Package::Yum do
 
   describe "when removing a package" do
     it "should run yum remove with the package name" do
-      @provider.should_receive(:run_command_with_systems_locale).with({
-        :command => "yum -d0 -e0 -y remove emacs-1.0"
-      })
+      @provider.should_receive(:yum_command).with(
+        "yum -d0 -e0 -y remove emacs-1.0"
+      )
       @provider.remove_package("emacs", "1.0")
     end
 
     it "should run yum remove with the package name and arch" do
       @new_resource.stub!(:arch).and_return("x86_64")
-      @provider.should_receive(:run_command_with_systems_locale).with({
-        :command => "yum -d0 -e0 -y remove emacs-1.0.x86_64"
-      })
+      @provider.should_receive(:yum_command).with(
+        "yum -d0 -e0 -y remove emacs-1.0.x86_64"
+      )
       @provider.remove_package("emacs", "1.0")
     end
   end
 
   describe "when purging a package" do
     it "should run yum remove with the package name" do
-      @provider.should_receive(:run_command_with_systems_locale).with({
-        :command => "yum -d0 -e0 -y remove emacs-1.0"
-      })
+      @provider.should_receive(:yum_command).with(
+        "yum -d0 -e0 -y remove emacs-1.0"
+      )
       @provider.purge_package("emacs", "1.0")
     end
   end
 
+  describe "when running yum" do
+    it "should run yum once if it exits with a return code of 0" do
+      @status = mock("Status", :exitstatus => 0)
+      @provider.stub!(:output_of_command).and_return([@status, "", ""])
+      @provider.should_receive(:output_of_command).once.with(
+        "yum -d0 -e0 -y install emacs-1.0",
+        {}
+      )
+      @provider.yum_command("yum -d0 -e0 -y install emacs-1.0")
+    end
+
+    it "should run yum once if it exits with a return code > 0 and no scriptlet failures" do
+      @status = mock("Status", :exitstatus => 2)
+      @provider.stub!(:output_of_command).and_return([@status, "failure failure", "problem problem"])
+      @provider.should_receive(:output_of_command).once.with(
+        "yum -d0 -e0 -y install emacs-1.0",
+        {}
+      )
+      lambda { @provider.yum_command("yum -d0 -e0 -y install emacs-1.0") }.should raise_error(Chef::Exceptions::Exec)
+    end
+
+    it "should run yum once if it exits with a return code of 1 and %pre scriptlet failures" do
+      @status = mock("Status", :exitstatus => 1)
+      @provider.stub!(:output_of_command).and_return([@status, "error: %pre(demo-1-1.el5.centos.x86_64) scriptlet failed, exit status 2", ""])
+      @provider.should_receive(:output_of_command).once.with(
+        "yum -d0 -e0 -y install emacs-1.0",
+        {}
+      )
+      # will still raise an exception, can't stub out the subsequent call
+      lambda { @provider.yum_command("yum -d0 -e0 -y install emacs-1.0") }.should raise_error(Chef::Exceptions::Exec)
+    end
+
+    it "should run yum twice if it exits with a return code of 1 and %post scriptlet failures" do
+      @status = mock("Status", :exitstatus => 1)
+      @provider.stub!(:output_of_command).and_return([@status, "error: %post(demo-1-1.el5.centos.x86_64) scriptlet failed, exit status 2", ""])
+      @provider.should_receive(:output_of_command).twice.with(
+        "yum -d0 -e0 -y install emacs-1.0",
+        {}
+      )
+      # will still raise an exception, can't stub out the subsequent call
+      lambda { @provider.yum_command("yum -d0 -e0 -y install emacs-1.0") }.should raise_error(Chef::Exceptions::Exec)
+    end
+  end
 end
 
 describe Chef::Provider::Package::Yum::RPMUtils do