Merge pull request #3235 from jaymzh/yum_arch

Make multipackage and arch play nicely together

4 files changed, 101 insertions, 32 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index d32217f08f..ada16f72c7 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -16,7 +16,6 @@
   Ensure knife ssh doesn't use a non-existant field for hostname #3131
 * [**Tom Hughes**](https://github.com/tomhughes)
   Ensure searches progress in the face of incomplete responses #3135
-
 * [pr#3162](https://github.com/chef/chef/pull/3162): Add
   `--minimal-ohai` flag to client/solo/apply; restricts ohai to only the
   bare minimum of plugins.
@@ -28,6 +27,8 @@
 * Fix nil pointer for windows event logger #3200
 * Use partial search for knife status
 * Ensure chef/knife properly honours proxy config
+* [**Phil Dibowitz**](https://github.com/jaymzh):
+  Fix multipackage and architectures
 
 ## 12.2.1
 * [Issue 3153](https://github.com/chef/chef/issues/3153): Fix bug where unset HOME would cause chef to crash
diff --git a/lib/chef/provider/package/yum.rb b/lib/chef/provider/package/yum.rb
index b3d3d72844..b6f93e2493 100644
--- a/lib/chef/provider/package/yum.rb
+++ b/lib/chef/provider/package/yum.rb
@@ -986,6 +986,17 @@ class Chef
         # Extra attributes
         #
 
+        def arch_for_name(n)
+          if @new_resource.respond_to?("arch")
+            @new_resource.arch
+          elsif @arch
+            idx = package_name_array.index(n)
+            as_array(@arch)[idx]
+          else
+            nil
+          end
+        end
+
         def arch
           if @new_resource.respond_to?("arch")
             @new_resource.arch
@@ -994,6 +1005,12 @@ class Chef
           end
         end
 
+        def set_arch(arch)
+          if @new_resource.respond_to?("arch")
+            @new_resource.arch(arch)
+          end
+        end
+
         def flush_cache
           if @new_resource.respond_to?("flush_cache")
             @new_resource.flush_cache
@@ -1005,11 +1022,12 @@ class Chef
         # Helpers
         #
 
-        def yum_arch
+        def yum_arch(arch)
           arch ? ".#{arch}" : nil
         end
 
         def yum_command(command)
+          Chef::Log.debug("#{@new_resource}: yum command: \"#{command}\"")
           status = shell_out(command, {:timeout => Chef::Config[:yum_timeout]})
 
           # This is fun: rpm can encounter errors in the %post/%postun scripts which aren't
@@ -1087,16 +1105,13 @@ class Chef
             end
           end
 
-          # Don't overwrite an existing arch
-          unless arch
-            parse_arch
-          end
 
           @current_resource = Chef::Resource::Package.new(@new_resource.name)
           @current_resource.package_name(@new_resource.package_name)
 
           installed_version = []
           @candidate_version = []
+          @arch = []
           if @new_resource.source
             unless ::File.exists?(@new_resource.source)
               raise Chef::Exceptions::Package, "Package #{@new_resource.name} not found: #{@new_resource.source}"
@@ -1113,24 +1128,43 @@ class Chef
             @candidate_version << @new_resource.version
             installed_version << @yum.installed_version(@current_resource.package_name, arch)
           else
-            if @new_resource.version
-              new_resource = "#{@new_resource.package_name}-#{@new_resource.version}#{yum_arch}"
-            else
-              new_resource = "#{@new_resource.package_name}#{yum_arch}"
-            end
 
-            Chef::Log.debug("#{@new_resource} checking yum info for #{new_resource}")
+            package_name_array.each_with_index do |pkg, idx|
+              # Don't overwrite an existing arch
+              if arch
+                name, parch = pkg, arch
+              else
+                name, parch = parse_arch(pkg)
+                # if we parsed an arch from the name, update the name
+                # to be just the package name.
+                if parch
+                  if @new_resource.package_name.is_a?(Array)
+                    @new_resource.package_name[idx] = name
+                  else
+                    @new_resource.package_name(name)
+                    # only set the arch if it's a single package
+                    set_arch(parch)
+                  end
+                end
+              end
 
-            package_name_array.each do |pkg|
-              installed_version << @yum.installed_version(pkg, arch)
-              @candidate_version << @yum.candidate_version(pkg, arch)
+              if @new_resource.version
+                new_resource =
+                  "#{@new_resource.package_name}-#{@new_resource.version}#{yum_arch(parch)}"
+              else
+                new_resource = "#{@new_resource.package_name}#{yum_arch(parch)}"
+              end
+              Chef::Log.debug("#{@new_resource} checking yum info for #{new_resource}")
+              installed_version << @yum.installed_version(name, parch)
+              @candidate_version << @yum.candidate_version(name, parch)
+              @arch << parch
             end
-
           end
 
           if installed_version.size == 1
             @current_resource.version(installed_version[0])
             @candidate_version = @candidate_version[0]
+            @arch = @arch[0]
           else
             @current_resource.version(installed_version)
           end
@@ -1145,7 +1179,7 @@ class Chef
           # Work around yum not exiting with an error if a package doesn't exist
           # for CHEF-2062
           all_avail = as_array(name).zip(as_array(version)).any? do |n, v|
-            @yum.version_available?(n, v, arch)
+            @yum.version_available?(n, v, arch_for_name(n))
           end
           method = log_method = nil
           methods = []
@@ -1187,16 +1221,16 @@ class Chef
 
             repos = []
             pkg_string_bits = []
-            index = 0
             as_array(name).zip(as_array(version)).each do |n, v|
+              idx = package_name_array.index(n)
+              a = arch_for_name(n)
               s = ''
-              unless v == current_version_array[index]
-                s = "#{n}-#{v}#{yum_arch}"
-                repo = @yum.package_repository(n, v, arch)
+              unless v == current_version_array[idx]
+                s = "#{n}-#{v}#{yum_arch(a)}"
+                repo = @yum.package_repository(n, v, a)
                 repos << "#{s} from #{repo} repository"
                 pkg_string_bits << s
               end
-              index += 1
             end
             pkg_string = pkg_string_bits.join(' ')
             Chef::Log.info("#{@new_resource} #{log_method} #{repos.join(' ')}")
@@ -1247,11 +1281,15 @@ class Chef
 
         def remove_package(name, version)
           if version
-            remove_str = as_array(name).zip(as_array(version)).map do |x|
-              "#{x.join('-')}#{yum_arch}"
+            remove_str = as_array(name).zip(as_array(version)).map do |n, v|
+              a = arch_for_name(n)
+              "#{[n, v].join('-')}#{yum_arch(a)}"
             end.join(' ')
           else
-            remove_str = as_array(name).map { |n| "#{n}#{yum_arch}" }.join(' ')
+            remove_str = as_array(name).map do |n|
+              a = arch_for_name(n)
+              "#{n}#{yum_arch(a)}"
+            end.join(' ')
           end
           yum_command("yum -d0 -e0 -y#{expand_options(@new_resource.options)} remove #{remove_str}")
 
@@ -1268,22 +1306,26 @@ class Chef
 
         private
 
-        def parse_arch
+        def parse_arch(package_name)
           # Allow for foo.x86_64 style package_name like yum uses in it's output
           #
-          if @new_resource.package_name =~ %r{^(.*)\.(.*)$}
+          if package_name =~ %r{^(.*)\.(.*)$}
             new_package_name = $1
             new_arch = $2
             # foo.i386 and foo.beta1 are both valid package names or expressions of an arch.
             # Ensure we don't have an existing package matching package_name, then ensure we at
             # least have a match for the new_package+new_arch before we overwrite. If neither
             # then fall through to standard package handling.
-            if (@yum.installed_version(@new_resource.package_name).nil? and @yum.candidate_version(@new_resource.package_name).nil?) and
-                 (@yum.installed_version(new_package_name, new_arch) or @yum.candidate_version(new_package_name, new_arch))
-               @new_resource.package_name(new_package_name)
-               @new_resource.arch(new_arch)
+            old_installed = @yum.installed_version(package_name)
+            old_candidate = @yum.candidate_version(package_name)
+            new_installed = @yum.installed_version(new_package_name, new_arch)
+            new_candidate = @yum.candidate_version(new_package_name, new_arch)
+            if (old_installed.nil? and old_candidate.nil?) and (new_installed or new_candidate)
+              Chef::Log.debug("Parsed out arch #{new_arch}, new package name is #{new_package_name}")
+              return new_package_name, new_arch
             end
           end
+          return package_name, nil
         end
 
         # If we don't have the package we could have been passed a 'whatprovides' feature
diff --git a/lib/chef/resource/yum_package.rb b/lib/chef/resource/yum_package.rb
index 8fbca9b097..d8be8c9748 100644
--- a/lib/chef/resource/yum_package.rb
+++ b/lib/chef/resource/yum_package.rb
@@ -38,7 +38,7 @@ class Chef
         set_or_return(
           :arch,
           arg,
-          :kind_of => [ String ]
+          :kind_of => [ String, Array ]
         )
       end
 
diff --git a/spec/unit/provider/package/yum_spec.rb b/spec/unit/provider/package/yum_spec.rb
index 8f4bde7f62..1ebd58d312 100644
--- a/spec/unit/provider/package/yum_spec.rb
+++ b/spec/unit/provider/package/yum_spec.rb
@@ -2110,5 +2110,31 @@ describe "Chef::Provider::Package::Yum - Multi" do
       allow(@new_resource).to receive(:options).and_return("--disablerepo epmd")
       @provider.install_package(["cups", "vim"], ["1.2.4-11.19.el5", '1.0'])
     end
+
+    it "should run yum install with the package name and version when name has arch" do
+      @new_resource = Chef::Resource::Package.new(['cups.x86_64', 'vim'])
+      @provider = Chef::Provider::Package::Yum.new(@new_resource, @run_context)
+      allow(Chef::Provider::Package::Yum::RPMUtils).to receive(:rpmvercmp).and_return(-1)
+
+      # Inside of load_current_resource() we'll call parse_arch for cups,
+      # and we need to craft the right response. The default mock setup above
+      # will just return valid versions all the time which won't work for this
+      # test.
+      allow(@yum_cache).to receive(:installed_version).with('cups', 'x86_64').and_return('XXXX')
+      allow(@yum_cache).to receive(:candidate_version).with('cups', 'x86_64').and_return('1.2.4-11.18.el5')
+      allow(@yum_cache).to receive(:installed_version).with('cups.x86_64').and_return(nil)
+      allow(@yum_cache).to receive(:candidate_version).with('cups.x86_64').and_return(nil)
+
+      # Normal mock's for the idempotency check
+      allow(@yum_cache).to receive(:installed_version).with('cups', nil).and_return('1.2.4-11.18.el5')
+      allow(@yum_cache).to receive(:installed_version).with('vim', nil).and_return('0.9')
+
+      @provider.load_current_resource
+      expect(@provider).to receive(:yum_command).with(
+        "yum -d0 -e0 -y install cups-1.2.4-11.19.el5.x86_64 vim-1.0"
+      )
+      @provider.install_package(["cups", "vim"], ["1.2.4-11.19.el5", '1.0'])
+    end
+
   end
 end