Merge pull request #7138 from RoboticCheese/roboticcheese/package-locking-nomethoderror

Fix NoMethodError when (un)locking single packages in apt and zypper

4 files changed, 99 insertions, 14 deletions

diff --git a/lib/chef/provider/package/apt.rb b/lib/chef/provider/package/apt.rb
index 2692ec9f15..798abf4680 100644
--- a/lib/chef/provider/package/apt.rb
+++ b/lib/chef/provider/package/apt.rb
@@ -70,12 +70,22 @@ class Chef
           @candidate_version ||= get_candidate_versions
         end
 
-        def package_locked(name, version)
-          locked = shell_out_compact_timeout!("apt-mark", "showhold")
-          locked_packages = locked.stdout.each_line.map do |line|
-            line.strip
-          end
-          name.all? { |n| locked_packages.include? n }
+        def packages_all_locked?(names, versions)
+          names.all? { |n| locked_packages.include? n }
+        end
+
+        def packages_all_unlocked?(names, versions)
+          names.all? { |n| !locked_packages.include? n }
+        end
+
+        def locked_packages
+          @locked_packages ||=
+            begin
+              locked = shell_out_compact_timeout!("apt-mark", "showhold")
+              locked.stdout.each_line.map do |line|
+                line.strip
+              end
+            end
         end
 
         def install_package(name, version)
diff --git a/lib/chef/provider/package/zypper.rb b/lib/chef/provider/package/zypper.rb
index 7d97bf310d..c2638fbfc1 100644
--- a/lib/chef/provider/package/zypper.rb
+++ b/lib/chef/provider/package/zypper.rb
@@ -75,12 +75,22 @@ class Chef
           end
         end
 
-        def package_locked(name, version)
-          locked = shell_out_compact_timeout!("zypper", "locks")
-          locked_packages = locked.stdout.each_line.map do |line|
-            line.split("|").shift(2).last.strip
-          end
-          name.all? { |n| locked_packages.include? n }
+        def packages_all_locked?(names, versions)
+          names.all? { |n| locked_packages.include? n }
+        end
+
+        def packages_all_unlocked?(names, versions)
+          names.all? { |n| !locked_packages.include? n }
+        end
+
+        def locked_packages
+          @locked_packages ||=
+            begin
+              locked = shell_out_compact_timeout!("zypper", "locks")
+              locked.stdout.each_line.map do |line|
+                line.split("|").shift(2).last.strip
+              end
+            end
         end
 
         def load_current_resource
diff --git a/spec/unit/provider/package/apt_spec.rb b/spec/unit/provider/package/apt_spec.rb
index 78c517b9a0..0700f69eb4 100644
--- a/spec/unit/provider/package/apt_spec.rb
+++ b/spec/unit/provider/package/apt_spec.rb
@@ -422,7 +422,6 @@ mpg123 1.12.1-0ubuntu1
                     )
         expect(logger).to receive(:trace).with("#{@provider.new_resource} is already locked")
 
-        @provider.new_resource.package_name = ["irssi"]
         @provider.action_lock
       end
     end
@@ -444,7 +443,6 @@ mpg123 1.12.1-0ubuntu1
                     )
         expect(logger).to receive(:trace).with("#{@provider.new_resource} is already unlocked")
 
-        @provider.new_resource.package_name = ["irssi"]
         @provider.action_unlock
       end
     end
diff --git a/spec/unit/provider/package/zypper_spec.rb b/spec/unit/provider/package/zypper_spec.rb
index 3f172dc370..819278a795 100644
--- a/spec/unit/provider/package/zypper_spec.rb
+++ b/spec/unit/provider/package/zypper_spec.rb
@@ -267,6 +267,40 @@ describe Chef::Provider::Package::Zypper do
     end
   end
 
+  describe "action_lock" do
+    it "should lock if the package is not already locked" do
+      prov = provider
+      allow(prov).to receive(:shell_out_compact_timeout!).with(
+        "zypper", "--non-interactive", "info", new_resource.package_name
+      ).and_return(status)
+      allow(prov).to receive(:shell_out_compact_timeout!).with(
+        "zypper", "locks"
+      ).and_return(instance_double(
+        Mixlib::ShellOut, stdout: "1 | somethingelse | package | (any)"
+      ))
+      expect(prov).to receive(:lock_package).with(["cups"], [nil])
+
+      prov.load_current_resource
+      prov.action_lock
+    end
+
+    it "should not lock if the package is already locked" do
+      prov = provider
+      allow(prov).to receive(:shell_out_compact_timeout!).with(
+        "zypper", "--non-interactive", "info", new_resource.package_name
+      ).and_return(status)
+      allow(prov).to receive(:shell_out_compact_timeout!).with(
+        "zypper", "locks"
+      ).and_return(instance_double(
+        Mixlib::ShellOut, stdout: "1 | cups | package | (any)"
+      ))
+      expect(prov).to_not receive(:lock_package)
+
+      prov.load_current_resource
+      prov.action_lock
+    end
+  end
+
   describe "lock_package" do
     it "should run zypper addlock with the package name" do
       shell_out_expectation!(
@@ -290,6 +324,39 @@ describe Chef::Provider::Package::Zypper do
     end
   end
 
+  describe "action_unlock" do
+    it "should unlock if the package is not already unlocked" do
+      prov = provider
+      allow(prov).to receive(:shell_out_compact_timeout!).with(
+        "zypper", "--non-interactive", "info", new_resource.package_name
+      ).and_return(status)
+      allow(prov).to receive(:shell_out_compact_timeout!).with(
+        "zypper", "locks"
+      ).and_return(instance_double(
+        Mixlib::ShellOut, stdout: "1 | cups | package | (any)"
+      ))
+      expect(prov).to receive(:unlock_package).with(["cups"], [nil])
+
+      prov.load_current_resource
+      provider.action_unlock
+    end
+    it "should not unlock if the package is already unlocked" do
+      prov = provider
+      allow(prov).to receive(:shell_out_compact_timeout!).with(
+        "zypper", "--non-interactive", "info", new_resource.package_name
+      ).and_return(status)
+      allow(prov).to receive(:shell_out_compact_timeout!).with(
+        "zypper", "locks"
+      ).and_return(instance_double(
+        Mixlib::ShellOut, stdout: "1 | somethingelse | package | (any)"
+      ))
+      expect(prov).to_not receive(:unlock_package)
+
+      prov.load_current_resource
+      provider.action_unlock
+    end
+  end
+
   describe "unlock_package" do
     it "should run zypper removelock with the package name" do
       shell_out_expectation!(