Merge lp:~stub/charm-helpers/packagestatus into lp:charm-helpers

[Stuart Bishop]

I need to unhold packages, not just hold them.

Rework apt_hold into apt_hold, apt_unhold and general apt_mark.

[Tim Van Steenburgh (tvansteenburgh)]

+1, LGTM.

review: Approve

=== modified file 'charmhelpers/fetch/__init__.py'
--- charmhelpers/fetch/__init__.py	2015-04-29 12:52:18 +0000
+++ charmhelpers/fetch/__init__.py	2015-07-02 10:22:38 +0000
@@ -215,19 +215,27 @@
     _run_apt_command(cmd, fatal)
 
 
+def apt_mark(packages, mark, fatal=False):
+    """Flag one or more packages using apt-mark"""
+    cmd = ['apt-mark', mark]
+    if isinstance(packages, six.string_types):
+        cmd.append(packages)
+    else:
+        cmd.extend(packages)
+    log("Holding {}".format(packages))
+
+    if fatal:
+        subprocess.check_call(cmd, universal_newlines=True)
+    else:
+        subprocess.call(cmd, universal_newlines=True)
+
+
 def apt_hold(packages, fatal=False):
-    """Hold one or more packages"""
-    cmd = ['apt-mark', 'hold']
-    if isinstance(packages, six.string_types):
-        cmd.append(packages)
-    else:
-        cmd.extend(packages)
-    log("Holding {}".format(packages))
-
-    if fatal:
-        subprocess.check_call(cmd)
-    else:
-        subprocess.call(cmd)
+    return apt_mark(packages, 'hold', fatal=fatal)
+
+
+def apt_unhold(packages, fatal=False):
+    return apt_mark(packages, 'unhold', fatal=fatal)
 
 
 def add_source(source, key=None):

=== modified file 'tests/fetch/test_fetch.py'
--- tests/fetch/test_fetch.py	2014-11-25 15:07:02 +0000
+++ tests/fetch/test_fetch.py	2015-07-02 10:22:38 +0000
@@ -6,6 +6,7 @@
     patch,
     MagicMock,
     call,
+    sentinel,
 )
 from charmhelpers import fetch
 import os
@@ -581,51 +582,82 @@
 
     @patch('subprocess.check_call')
     @patch.object(fetch, 'log')
-    def test_hold_apt_packages_as_string_fatal(self, log, mock_call):
+    def test_mark_apt_packages_as_string_fatal(self, log, mock_call):
         packages = 'irrelevant names'
         mock_call.side_effect = OSError('fail')
 
-        mock_call.assertRaises(OSError, fetch.apt_hold, packages, fatal=True)
+        mock_call.assertRaises(OSError,
+                               fetch.apt_mark, packages,
+                               sentinel.mark, fatal=True)
         log.assert_called()
 
     @patch('subprocess.check_call')
     @patch.object(fetch, 'log')
-    def test_hold_apt_packages_fatal(self, log, mock_call):
+    def test_mark_apt_packages_fatal(self, log, mock_call):
         packages = ['irrelevant', 'names']
         mock_call.side_effect = OSError('fail')
 
-        mock_call.assertRaises(OSError, fetch.apt_hold, packages, fatal=True)
+        mock_call.assertRaises(OSError,
+                               fetch.apt_mark, packages,
+                               sentinel.mark, fatal=True)
         log.assert_called()
 
     @patch('subprocess.call')
     @patch.object(fetch, 'log')
-    def test_hold_apt_packages_as_string_nofatal(self, log, mock_call):
+    def test_mark_apt_packages_as_string_nofatal(self, log, mock_call):
         packages = 'foo bar'
 
-        fetch.apt_hold(packages)
+        fetch.apt_mark(packages, sentinel.mark)
 
         log.assert_called()
-        mock_call.assert_called_with(['apt-mark', 'hold', 'foo bar'])
+        mock_call.assert_called_with(['apt-mark', sentinel.mark, 'foo bar'],
+                                     universal_newlines=True)
 
     @patch('subprocess.call')
     @patch.object(fetch, 'log')
-    def test_hold_apt_packages_nofatal(self, log, mock_call):
+    def test_mark_apt_packages_nofatal(self, log, mock_call):
         packages = ['foo', 'bar']
 
-        fetch.apt_hold(packages)
+        fetch.apt_mark(packages, sentinel.mark)
 
         log.assert_called()
-        mock_call.assert_called_with(['apt-mark', 'hold', 'foo', 'bar'])
+        mock_call.assert_called_with(['apt-mark', sentinel.mark, 'foo', 'bar'],
+                                     universal_newlines=True)
 
     @patch('subprocess.check_call')
     @patch.object(fetch, 'log')
-    def test_hold_apt_packages_nofatal_abortonfatal(self, log, mock_call):
+    def test_mark_apt_packages_nofatal_abortonfatal(self, log, mock_call):
         packages = ['foo', 'bar']
 
-        fetch.apt_hold(packages, fatal=True)
+        fetch.apt_mark(packages, sentinel.mark, fatal=True)
 
         log.assert_called()
-        mock_call.assert_called_with(['apt-mark', 'hold', 'foo', 'bar'])
+        mock_call.assert_called_with(['apt-mark', sentinel.mark, 'foo', 'bar'],
+                                     universal_newlines=True)
+
+    @patch.object(fetch, 'apt_mark')
+    def test_apt_hold(self, apt_mark):
+        fetch.apt_hold(sentinel.packages)
+        apt_mark.assert_called_once_with(sentinel.packages, 'hold',
+                                         fatal=False)
+
+    @patch.object(fetch, 'apt_mark')
+    def test_apt_hold_fatal(self, apt_mark):
+        fetch.apt_hold(sentinel.packages, fatal=sentinel.fatal)
+        apt_mark.assert_called_once_with(sentinel.packages, 'hold',
+                                         fatal=sentinel.fatal)
+
+    @patch.object(fetch, 'apt_mark')
+    def test_apt_unhold(self, apt_mark):
+        fetch.apt_unhold(sentinel.packages)
+        apt_mark.assert_called_once_with(sentinel.packages, 'unhold',
+                                         fatal=False)
+
+    @patch.object(fetch, 'apt_mark')
+    def test_apt_unhold_fatal(self, apt_mark):
+        fetch.apt_unhold(sentinel.packages, fatal=sentinel.fatal)
+        apt_mark.assert_called_once_with(sentinel.packages, 'unhold',
+                                         fatal=sentinel.fatal)
 
     @patch('subprocess.check_call')
     def test_apt_update_fatal(self, check_call):