Merge pull request #4231 from chef/lcg/zypper-multipackage

zypper multipackage patch

2 files changed, 97 insertions, 76 deletions

diff --git a/lib/chef/provider/package/zypper.rb b/lib/chef/provider/package/zypper.rb
index 9b0aaf322a..fe8358c654 100644
--- a/lib/chef/provider/package/zypper.rb
+++ b/lib/chef/provider/package/zypper.rb
@@ -2,7 +2,7 @@
 #
 # Authors:: Adam Jacob (<adam@opscode.com>)
 #           Ionuț Arțăriși (<iartarisi@suse.cz>)
-# Copyright:: Copyright (c) 2008 Opscode, Inc.
+# Copyright:: Copyright (c) 2008-2015 Chef Software, Inc.
 #             Copyright (c) 2013 SUSE Linux GmbH
 # License:: Apache License, Version 2.0
 #
@@ -20,70 +20,74 @@
 #
 
 require "chef/provider/package"
-require "chef/mixin/command"
-require "chef/resource/package"
-require "singleton"
+require "chef/resource/zypper_package"
 
 class Chef
   class Provider
     class Package
       class Zypper < Chef::Provider::Package
+        use_multipackage_api
 
         provides :package, platform_family: "suse"
         provides :zypper_package, os: "linux"
 
-        def load_current_resource
-          @current_resource = Chef::Resource::ZypperPackage.new(new_resource.name)
-          current_resource.package_name(new_resource.package_name)
-
-          is_installed=false
-          is_out_of_date=false
-          version=""
-          oud_version=""
+        def get_versions(package_name)
+          candidate_version = current_version = nil
+          is_installed = false
           Chef::Log.debug("#{new_resource} checking zypper")
-          status = shell_out_with_timeout("zypper --non-interactive info #{new_resource.package_name}")
+          status = shell_out_with_timeout!("zypper --non-interactive info #{package_name}")
           status.stdout.each_line do |line|
             case line
             when /^Version: (.+)$/
-              version = $1
+              candidate_version = $1
               Chef::Log.debug("#{new_resource} version #{$1}")
             when /^Installed: Yes$/
               is_installed=true
               Chef::Log.debug("#{new_resource} is installed")
-
-            when /^Installed: No$/
-              is_installed=false
-              Chef::Log.debug("#{new_resource} is not installed")
             when /^Status: out-of-date \(version (.+) installed\)$/
-              is_out_of_date=true
-              oud_version=$1
+              current_version=$1
               Chef::Log.debug("#{new_resource} out of date version #{$1}")
             end
           end
+          current_version = candidate_version if is_installed
+          { current_version: current_version, candidate_version: candidate_version }
+        end
 
-          if is_installed==false
-            @candidate_version=version
-          end
-
-          if is_installed==true
-            if is_out_of_date==true
-              current_resource.version(oud_version)
-              @candidate_version=version
-            else
-              current_resource.version(version)
-              @candidate_version=version
+        def versions
+          @versions =
+            begin
+              raw_versions = package_name_array.map do |package_name|
+                get_versions(package_name)
+              end
+              Hash[*package_name_array.zip(raw_versions).flatten]
             end
+        end
+
+        def get_candidate_versions
+          package_name_array.map do |package_name|
+            versions[package_name][:candidate_version]
           end
+        end
 
-          unless status.exitstatus == 0
-            raise Chef::Exceptions::Package, "zypper failed - #{status.inspect}!"
+        def get_current_versions
+          package_name_array.map do |package_name|
+            versions[package_name][:current_version]
           end
+        end
+
+        def load_current_resource
+          @current_resource = Chef::Resource::ZypperPackage.new(new_resource.name)
+          current_resource.package_name(new_resource.package_name)
+
+          @candidate_version = get_candidate_versions
+          current_resource.version(get_current_versions)
 
           current_resource
         end
 
-        def zypper_version()
-          `zypper -V 2>&1`.scan(/\d+/).join(".").to_f
+        def zypper_version
+          @zypper_version ||=
+            `zypper -V 2>&1`.scan(/\d+/).join(".").to_f
         end
 
         def install_package(name, version)
@@ -91,6 +95,7 @@ class Chef
         end
 
         def upgrade_package(name, version)
+          # `zypper install` upgrades packages, we rely on the idempotency checks to get action :install behavior
           install_package(name, version)
         end
 
@@ -103,13 +108,19 @@ class Chef
         end
 
         private
-        def zypper_package(command, pkgname, version)
-          version = "=#{version}" unless version.nil? || version.empty?
+
+        def zip(names, versions)
+          names.zip(versions).map do |n, v|
+            (v.nil? || v.empty?) ? n : "#{n}=#{v}"
+          end
+        end
+
+        def zypper_package(command, names, versions)
+          zipped_names = zip(names, versions)
           if zypper_version < 1.0
-            shell_out_with_timeout!("zypper#{gpg_checks} #{command} -y #{pkgname}")
+            shell_out_with_timeout!(a_to_s("zypper", gpg_checks, command, "-y", names))
           else
-            shell_out_with_timeout!("zypper --non-interactive#{gpg_checks} "+
-                      "#{command} #{pkgname}#{version}")
+            shell_out_with_timeout!(a_to_s("zypper --non-interactive", gpg_checks, command, zipped_names))
           end
         end
 
@@ -118,12 +129,12 @@ class Chef
           when true
             ""
           when false
-            " --no-gpg-checks"
+            "--no-gpg-checks"
           when nil
             Chef::Log.warn("Chef::Config[:zypper_check_gpg] was not set. " +
               "All packages will be installed without gpg signature checks. " +
               "This is a security hazard.")
-            " --no-gpg-checks"
+            "--no-gpg-checks"
           end
         end
       end
diff --git a/spec/unit/provider/package/zypper_spec.rb b/spec/unit/provider/package/zypper_spec.rb
index 0f39fde5cf..34cd5fe0ca 100644
--- a/spec/unit/provider/package/zypper_spec.rb
+++ b/spec/unit/provider/package/zypper_spec.rb
@@ -34,7 +34,7 @@ describe Chef::Provider::Package::Zypper do
 
   before(:each) do
     allow(Chef::Resource::Package).to receive(:new).and_return(current_resource)
-    allow(provider).to receive(:shell_out).and_return(status)
+    allow(provider).to receive(:shell_out!).and_return(status)
     allow(provider).to receive(:`).and_return("2.0")
   end
 
@@ -60,7 +60,7 @@ describe Chef::Provider::Package::Zypper do
     end
 
     it "should run zypper info with the package name" do
-      shell_out_expectation(
+      shell_out_expectation!(
         "zypper --non-interactive info #{new_resource.package_name}"
       ).and_return(status)
       provider.load_current_resource
@@ -68,33 +68,24 @@ describe Chef::Provider::Package::Zypper do
 
     it "should set the installed version to nil on the current resource if zypper info installed version is (none)" do
       allow(provider).to receive(:shell_out).and_return(status)
+      expect(current_resource).to receive(:version).with([nil]).and_return(true)
       provider.load_current_resource
     end
 
     it "should set the installed version if zypper info has one" do
       status = double(:stdout => "Version: 1.0\nInstalled: Yes\n", :exitstatus => 0)
 
-      allow(provider).to receive(:shell_out).and_return(status)
-      expect(current_resource).to receive(:version).with("1.0").and_return(true)
+      allow(provider).to receive(:shell_out!).and_return(status)
+      expect(current_resource).to receive(:version).with(["1.0"]).and_return(true)
       provider.load_current_resource
     end
 
     it "should set the candidate version if zypper info has one" do
       status = double(:stdout => "Version: 1.0\nInstalled: No\nStatus: out-of-date (version 0.9 installed)", :exitstatus => 0)
 
-      allow(provider).to receive(:shell_out).and_return(status)
+      allow(provider).to receive(:shell_out!).and_return(status)
       provider.load_current_resource
-      expect(provider.candidate_version).to eql("1.0")
-    end
-
-    it "should raise an exception if zypper info fails" do
-      expect(status).to receive(:exitstatus).and_return(1)
-      expect { provider.load_current_resource }.to raise_error(Chef::Exceptions::Package)
-    end
-
-    it "should not raise an exception if zypper info succeeds" do
-      expect(status).to receive(:exitstatus).and_return(0)
-      expect { provider.load_current_resource }.not_to raise_error
+      expect(provider.candidate_version).to eql(["1.0"])
     end
 
     it "should return the current resouce" do
@@ -108,7 +99,7 @@ describe Chef::Provider::Package::Zypper do
       shell_out_expectation!(
         "zypper --non-interactive install --auto-agree-with-licenses emacs=1.0"
       )
-      provider.install_package("emacs", "1.0")
+      provider.install_package(["emacs"], ["1.0"])
     end
     it "should run zypper install without gpg checks" do
       allow(Chef::Config).to receive(:[]).with(:zypper_check_gpg).and_return(false)
@@ -116,7 +107,7 @@ describe Chef::Provider::Package::Zypper do
         "zypper --non-interactive --no-gpg-checks install "+
         "--auto-agree-with-licenses emacs=1.0"
       )
-      provider.install_package("emacs", "1.0")
+      provider.install_package(["emacs"], ["1.0"])
     end
     it "should warn about gpg checks on zypper install" do
       expect(Chef::Log).to receive(:warn).with(
@@ -126,7 +117,7 @@ describe Chef::Provider::Package::Zypper do
         "zypper --non-interactive --no-gpg-checks install "+
         "--auto-agree-with-licenses emacs=1.0"
       )
-      provider.install_package("emacs", "1.0")
+      provider.install_package(["emacs"], ["1.0"])
     end
   end
 
@@ -136,7 +127,7 @@ describe Chef::Provider::Package::Zypper do
       shell_out_expectation!(
         "zypper --non-interactive install --auto-agree-with-licenses emacs=1.0"
       )
-      provider.upgrade_package("emacs", "1.0")
+      provider.upgrade_package(["emacs"], ["1.0"])
     end
     it "should run zypper update without gpg checks" do
       allow(Chef::Config).to receive(:[]).with(:zypper_check_gpg).and_return(false)
@@ -144,7 +135,7 @@ describe Chef::Provider::Package::Zypper do
         "zypper --non-interactive --no-gpg-checks install "+
         "--auto-agree-with-licenses emacs=1.0"
       )
-      provider.upgrade_package("emacs", "1.0")
+      provider.upgrade_package(["emacs"], ["1.0"])
     end
     it "should warn about gpg checks on zypper upgrade" do
       expect(Chef::Log).to receive(:warn).with(
@@ -154,14 +145,14 @@ describe Chef::Provider::Package::Zypper do
         "zypper --non-interactive --no-gpg-checks install "+
         "--auto-agree-with-licenses emacs=1.0"
       )
-      provider.upgrade_package("emacs", "1.0")
+      provider.upgrade_package(["emacs"], ["1.0"])
     end
     it "should run zypper upgrade without gpg checks" do
       shell_out_expectation!(
         "zypper --non-interactive --no-gpg-checks install "+
         "--auto-agree-with-licenses emacs=1.0"
       )
-      provider.upgrade_package("emacs", "1.0")
+      provider.upgrade_package(["emacs"], ["1.0"])
     end
   end
 
@@ -173,7 +164,7 @@ describe Chef::Provider::Package::Zypper do
         shell_out_expectation!(
             "zypper --non-interactive remove emacs"
         )
-        provider.remove_package("emacs", nil)
+        provider.remove_package(["emacs"], [nil])
       end
     end
 
@@ -183,14 +174,14 @@ describe Chef::Provider::Package::Zypper do
         shell_out_expectation!(
           "zypper --non-interactive remove emacs=1.0"
         )
-        provider.remove_package("emacs", "1.0")
+        provider.remove_package(["emacs"], ["1.0"])
       end
       it "should run zypper remove without gpg checks" do
         allow(Chef::Config).to receive(:[]).with(:zypper_check_gpg).and_return(false)
         shell_out_expectation!(
             "zypper --non-interactive --no-gpg-checks remove emacs=1.0"
         )
-        provider.remove_package("emacs", "1.0")
+        provider.remove_package(["emacs"], ["1.0"])
       end
       it "should warn about gpg checks on zypper remove" do
         expect(Chef::Log).to receive(:warn).with(
@@ -199,24 +190,24 @@ describe Chef::Provider::Package::Zypper do
         shell_out_expectation!(
           "zypper --non-interactive --no-gpg-checks remove emacs=1.0"
         )
-        provider.remove_package("emacs", "1.0")
+        provider.remove_package(["emacs"], ["1.0"])
       end
     end
   end
 
   describe "purge_package" do
-    it "should run remove_package with the name and version" do
+    it "should run remove with the name and version and --clean-deps" do
       shell_out_expectation!(
         "zypper --non-interactive --no-gpg-checks remove --clean-deps emacs=1.0"
       )
-      provider.purge_package("emacs", "1.0")
+      provider.purge_package(["emacs"], ["1.0"])
     end
     it "should run zypper purge without gpg checks" do
       allow(Chef::Config).to receive(:[]).with(:zypper_check_gpg).and_return(false)
       shell_out_expectation!(
         "zypper --non-interactive --no-gpg-checks remove --clean-deps emacs=1.0"
       )
-      provider.purge_package("emacs", "1.0")
+      provider.purge_package(["emacs"], ["1.0"])
     end
     it "should warn about gpg checks on zypper purge" do
       expect(Chef::Log).to receive(:warn).with(
@@ -225,7 +216,7 @@ describe Chef::Provider::Package::Zypper do
       shell_out_expectation!(
         "zypper --non-interactive --no-gpg-checks remove --clean-deps emacs=1.0"
       )
-      provider.purge_package("emacs", "1.0")
+      provider.purge_package(["emacs"], ["1.0"])
     end
   end
 
@@ -239,7 +230,7 @@ describe Chef::Provider::Package::Zypper do
         shell_out_expectation!(
           "zypper --no-gpg-checks install --auto-agree-with-licenses -y emacs"
         )
-        provider.install_package("emacs", "1.0")
+        provider.install_package(["emacs"], ["1.0"])
       end
     end
 
@@ -248,7 +239,7 @@ describe Chef::Provider::Package::Zypper do
         shell_out_expectation!(
           "zypper --no-gpg-checks install --auto-agree-with-licenses -y emacs"
         )
-        provider.upgrade_package("emacs", "1.0")
+        provider.upgrade_package(["emacs"], ["1.0"])
       end
     end
 
@@ -257,8 +248,27 @@ describe Chef::Provider::Package::Zypper do
         shell_out_expectation!(
            "zypper --no-gpg-checks remove -y emacs"
         )
-        provider.remove_package("emacs", "1.0")
+        provider.remove_package(["emacs"], ["1.0"])
       end
     end
   end
+
+  describe "when installing multiple packages" do # https://github.com/chef/chef/issues/3570
+    it "should install an array of package names and versions" do
+      allow(Chef::Config).to receive(:[]).with(:zypper_check_gpg).and_return(false)
+      shell_out_expectation!(
+        "zypper --non-interactive --no-gpg-checks install "+
+        "--auto-agree-with-licenses emacs=1.0 vim=2.0"
+      )
+      provider.install_package(["emacs", "vim"], ["1.0", "2.0"])
+    end
+
+    it "should remove an array of package names and versions" do
+      allow(Chef::Config).to receive(:[]).with(:zypper_check_gpg).and_return(false)
+      shell_out_expectation!(
+        "zypper --non-interactive --no-gpg-checks remove emacs=1.0 vim=2.0"
+      )
+      provider.remove_package(["emacs", "vim"], ["1.0", "2.0"])
+    end
+  end
 end