Merge lp:~bjornt/landscape-client/add-remove-dpkg-holds into lp:~landscape/landscape-client/trunk

Proposed by Björn Tillenius
Status: Merged
Approved by: Geoff Teale
Approved revision: 447
Merged at revision: 435
Proposed branch: lp:~bjornt/landscape-client/add-remove-dpkg-holds
Merge into: lp:~landscape/landscape-client/trunk
Diff against target: 438 lines (+314/-3)
6 files modified
landscape/message_schemas.py (+6/-0)
landscape/package/changer.py (+55/-1)
landscape/package/facade.py (+41/-0)
landscape/package/tests/helpers.py (+0/-1)
landscape/package/tests/test_changer.py (+123/-1)
landscape/package/tests/test_facade.py (+89/-0)
To merge this branch: bzr merge lp:~bjornt/landscape-client/add-remove-dpkg-holds
Reviewer Review Type Date Requested Status
Geoff Teale (community) Approve
Alberto Donato (community) Approve
Review via email: mp+89253@code.launchpad.net

Description of the change

Add a message for adding and removing package holds, handling the
message in the package changer.

To post a comment you must log in.
Revision history for this message
Alberto Donato (ack) wrote :

Looks good! +1

#1:
+ """Log that a package holds result is sent and sent the response."""

typo, "and send"

review: Approve
Revision history for this message
Geoff Teale (tealeg) wrote :

+1 Looks good. Please fix:

landscape/package/tests/test_changer.py:1067:1: E303 too many blank lines (3)

review: Approve
448. By Björn Tillenius

Typo.

449. By Björn Tillenius

Lint.

Revision history for this message
Björn Tillenius (bjornt) wrote :

On Thu, Jan 19, 2012 at 04:06:23PM -0000, Alberto Donato wrote:
> Review: Approve
>
> Looks good! +1
>
> #1:
> + """Log that a package holds result is sent and sent the response."""
>
> typo, "and send"

Thanks, fixed.

--
Björn Tillenius | https://launchpad.net/~bjornt

Revision history for this message
Björn Tillenius (bjornt) wrote :

On Fri, Jan 20, 2012 at 10:43:24AM -0000, Geoff Teale wrote:
> Review: Approve
>
> +1 Looks good. Please fix:
>
> landscape/package/tests/test_changer.py:1067:1: E303 too many blank lines (3)

Oops, missed that. Fixed.

--
Björn Tillenius | https://launchpad.net/~bjornt

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'landscape/message_schemas.py'
--- landscape/message_schemas.py 2012-01-11 13:37:17 +0000
+++ landscape/message_schemas.py 2012-01-20 12:58:25 +0000
@@ -294,6 +294,12 @@
294 "deleted": package_locks},294 "deleted": package_locks},
295 optional=["created", "deleted"])295 optional=["created", "deleted"])
296296
297CHANGE_PACKAGE_HOLDS = Message(
298 "change-package-holds",
299 {"created": List(utf8),
300 "deleted": List(utf8)},
301 optional=["created", "deleted"])
302
297CHANGE_PACKAGES_RESULT = Message(303CHANGE_PACKAGES_RESULT = Message(
298 "change-packages-result",304 "change-packages-result",
299 {"operation-id": Int(),305 {"operation-id": Int(),
300306
=== modified file 'landscape/package/changer.py'
--- landscape/package/changer.py 2012-01-13 14:01:43 +0000
+++ landscape/package/changer.py 2012-01-20 12:58:25 +0000
@@ -18,7 +18,7 @@
18from landscape.package.taskhandler import (18from landscape.package.taskhandler import (
19 PackageTaskHandler, PackageTaskHandlerConfiguration, PackageTaskError,19 PackageTaskHandler, PackageTaskHandlerConfiguration, PackageTaskError,
20 run_task_handler)20 run_task_handler)
21from landscape.manager.manager import SUCCEEDED21from landscape.manager.manager import FAILED, SUCCEEDED
2222
2323
24class UnknownPackageData(Exception):24class UnknownPackageData(Exception):
@@ -103,6 +103,8 @@
103 return result.addErrback(self.unknown_package_data_error, task)103 return result.addErrback(self.unknown_package_data_error, task)
104 if message["type"] == "change-package-locks":104 if message["type"] == "change-package-locks":
105 return self.handle_change_package_locks(message)105 return self.handle_change_package_locks(message)
106 if message["type"] == "change-package-holds":
107 return self.handle_change_package_holds(message)
106108
107 def unknown_package_data_error(self, failure, task):109 def unknown_package_data_error(self, failure, task):
108 """Handle L{UnknownPackageData} data errors.110 """Handle L{UnknownPackageData} data errors.
@@ -296,6 +298,58 @@
296 "exchange urgently.")298 "exchange urgently.")
297 return self._broker.send_message(response, True)299 return self._broker.send_message(response, True)
298300
301 def _send_change_package_holds_response(self, response):
302 """Log that a package holds result is sent and send the response."""
303 logging.info("Queuing message with change package holds results to "
304 "exchange urgently.")
305 return self._broker.send_message(response, True)
306
307 def handle_change_package_holds(self, message):
308 """Handle a C{change-package-holds} message.
309
310 Create and delete package holds as requested by the given C{message}.
311 """
312 if not self._facade.supports_package_holds:
313 response = {
314 "type": "operation-result",
315 "operation-id": message.get("operation-id"),
316 "status": FAILED,
317 "result-text": "This client doesn't support package holds.",
318 "result-code": 1}
319 return self._send_change_package_holds_response(response)
320
321 not_installed = set()
322 holds_to_create = message.get("create", [])
323 for name in holds_to_create:
324 versions = self._facade.get_packages_by_name(name)
325 if not versions or not versions[0].package.installed:
326 not_installed.add(name)
327 if not_installed:
328 response = {
329 "type": "operation-result",
330 "operation-id": message.get("operation-id"),
331 "status": FAILED,
332 "result-text": "Package holds not added, since the following" +
333 " packages are not installed: %s" % (
334 ", ".join(sorted(not_installed))),
335 "result-code": 1}
336 return self._send_change_package_holds_response(response)
337
338 for hold in holds_to_create:
339 self._facade.set_package_hold(hold)
340 self._facade.reload_channels()
341 for hold in message.get("delete", []):
342 self._facade.remove_package_hold(hold)
343 self._facade.reload_channels()
344
345 response = {"type": "operation-result",
346 "operation-id": message.get("operation-id"),
347 "status": SUCCEEDED,
348 "result-text": "Package holds successfully changed.",
349 "result-code": 0}
350
351 return self._send_change_package_holds_response(response)
352
299 @staticmethod353 @staticmethod
300 def find_command():354 def find_command():
301 return find_changer_command()355 return find_changer_command()
302356
=== modified file 'landscape/package/facade.py'
--- landscape/package/facade.py 2012-01-16 15:08:15 +0000
+++ landscape/package/facade.py 2012-01-20 12:58:25 +0000
@@ -1,5 +1,6 @@
1import hashlib1import hashlib
2import os2import os
3import subprocess
3import tempfile4import tempfile
4from cStringIO import StringIO5from cStringIO import StringIO
56
@@ -79,10 +80,14 @@
79 database.80 database.
80 """81 """
8182
83 supports_package_holds = True
84
82 def __init__(self, root=None):85 def __init__(self, root=None):
83 self._root = root86 self._root = root
87 self._dpkg_args = []
84 if self._root is not None:88 if self._root is not None:
85 self._ensure_dir_structure()89 self._ensure_dir_structure()
90 self._dpkg_args.extend(["--root", self._root])
86 # don't use memonly=True here because of a python-apt bug on Natty when91 # don't use memonly=True here because of a python-apt bug on Natty when
87 # sources.list contains invalid lines (LP: #886208)92 # sources.list contains invalid lines (LP: #886208)
88 self._cache = apt.cache.Cache(rootdir=root)93 self._cache = apt.cache.Cache(rootdir=root)
@@ -100,6 +105,10 @@
100 self._ensure_sub_dir("var/cache/apt/archives/partial")105 self._ensure_sub_dir("var/cache/apt/archives/partial")
101 self._ensure_sub_dir("var/lib/apt/lists/partial")106 self._ensure_sub_dir("var/lib/apt/lists/partial")
102 dpkg_dir = self._ensure_sub_dir("var/lib/dpkg")107 dpkg_dir = self._ensure_sub_dir("var/lib/dpkg")
108 self._ensure_sub_dir("var/lib/dpkg/info")
109 self._ensure_sub_dir("var/lib/dpkg/updates")
110 self._ensure_sub_dir("var/lib/dpkg/triggers")
111 create_file(os.path.join(dpkg_dir, "available"), "")
103 self._dpkg_status = os.path.join(dpkg_dir, "status")112 self._dpkg_status = os.path.join(dpkg_dir, "status")
104 if not os.path.exists(self._dpkg_status):113 if not os.path.exists(self._dpkg_status):
105 create_file(self._dpkg_status, "")114 create_file(self._dpkg_status, "")
@@ -157,6 +166,37 @@
157 """166 """
158 return []167 return []
159168
169 def get_package_holds(self):
170 """Return the name of all the packages that are on hold."""
171 return [version.package.name for version in self.get_locked_packages()]
172
173 def _set_dpkg_selections(self, selection):
174 """Set the dpkg selection.
175
176 It basically does "echo $selection | dpkg --set-selections".
177 """
178 process = subprocess.Popen(
179 ["dpkg", "--set-selections"] + self._dpkg_args,
180 stdin=subprocess.PIPE)
181 process.communicate(selection)
182
183 def set_package_hold(self, name):
184 """Add a dpkg hold for a package.
185
186 @param name: The name of the package to hold.
187 """
188 self._set_dpkg_selections(name + " hold")
189
190 def remove_package_hold(self, name):
191 """Removes a dpkg hold for a package.
192
193 @param name: The name of the package to unhold.
194 """
195 versions = self.get_packages_by_name(name)
196 if not versions or not self._is_package_held(versions[0].package):
197 return
198 self._set_dpkg_selections(name + " install")
199
160 def reload_channels(self):200 def reload_channels(self):
161 """Reload the channels and update the cache."""201 """Reload the channels and update the cache."""
162 self._cache.open(None)202 self._cache.open(None)
@@ -469,6 +509,7 @@
469 """509 """
470510
471 _deb_package_type = None511 _deb_package_type = None
512 supports_package_holds = False
472513
473 def __init__(self, smart_init_kwargs={}, sysconf_args=None):514 def __init__(self, smart_init_kwargs={}, sysconf_args=None):
474 self._smart_init_kwargs = smart_init_kwargs.copy()515 self._smart_init_kwargs = smart_init_kwargs.copy()
475516
=== modified file 'landscape/package/tests/helpers.py'
--- landscape/package/tests/helpers.py 2011-12-14 10:41:04 +0000
+++ landscape/package/tests/helpers.py 2012-01-20 12:58:25 +0000
@@ -40,7 +40,6 @@
40 Architecture: %(architecture)s40 Architecture: %(architecture)s
41 Source: source41 Source: source
42 Version: %(version)s42 Version: %(version)s
43 Config-Version: 1.0
44 Description: description43 Description: description
45 """ % {"name": name, "version": version,44 """ % {"name": name, "version": version,
46 "architecture": architecture})45 "architecture": architecture})
4746
=== modified file 'landscape/package/tests/test_changer.py'
--- landscape/package/tests/test_changer.py 2011-12-13 10:29:04 +0000
+++ landscape/package/tests/test_changer.py 2012-01-20 12:58:25 +0000
@@ -24,7 +24,7 @@
24from landscape.package.tests.helpers import (24from landscape.package.tests.helpers import (
25 SmartFacadeHelper, HASH1, HASH2, HASH3, PKGDEB1, PKGDEB2, PKGNAME2,25 SmartFacadeHelper, HASH1, HASH2, HASH3, PKGDEB1, PKGDEB2, PKGNAME2,
26 AptFacadeHelper, SimpleRepositoryHelper)26 AptFacadeHelper, SimpleRepositoryHelper)
27from landscape.manager.manager import SUCCEEDED27from landscape.manager.manager import FAILED, SUCCEEDED
2828
2929
30class PackageChangerTestMixin(object):30class PackageChangerTestMixin(object):
@@ -1037,6 +1037,31 @@
1037 self.assertIn("(2)", text)1037 self.assertIn("(2)", text)
1038 return result.addCallback(got_result)1038 return result.addCallback(got_result)
10391039
1040 def test_change_package_holds(self):
1041 """
1042 If C{SmartFacade} is used, the L{PackageChanger.handle_tasks}
1043 method fails the activity, since it can't add or remove dpkg holds.
1044 """
1045 self.facade.reload_channels()
1046 self.store.add_task("changer", {"type": "change-package-holds",
1047 "create": ["name1"],
1048 "delete": ["name2"],
1049 "operation-id": 123})
1050
1051 def assert_result(result):
1052 self.assertIn("Queuing message with change package holds results "
1053 "to exchange urgently.", self.logfile.getvalue())
1054 self.assertMessages(
1055 self.get_pending_messages(),
1056 [{"type": "operation-result",
1057 "operation-id": 123,
1058 "status": FAILED,
1059 "result-text": "This client doesn't support package holds.",
1060 "result-code": 1}])
1061
1062 result = self.changer.handle_tasks()
1063 return result.addCallback(assert_result)
1064
10401065
1041class AptPackageChangerTest(LandscapeTest, PackageChangerTestMixin):1066class AptPackageChangerTest(LandscapeTest, PackageChangerTestMixin):
10421067
@@ -1129,3 +1154,100 @@
1129 def get_package_name(self, version):1154 def get_package_name(self, version):
1130 """Return the name of the package."""1155 """Return the name of the package."""
1131 return version.package.name1156 return version.package.name
1157
1158 def test_change_package_holds(self):
1159 """
1160 The L{PackageChanger.handle_tasks} method appropriately creates and
1161 deletes package holds as requested by the C{change-package-holds}
1162 message.
1163 """
1164 self._add_system_package("foo")
1165 self._add_system_package("bar")
1166 self.facade.set_package_hold("bar")
1167 self.facade.reload_channels()
1168 self.store.add_task("changer", {"type": "change-package-holds",
1169 "create": ["foo"],
1170 "delete": ["bar"],
1171 "operation-id": 123})
1172
1173 def assert_result(result):
1174 self.assertEqual(["foo"], self.facade.get_package_holds())
1175 self.assertIn("Queuing message with change package holds results "
1176 "to exchange urgently.", self.logfile.getvalue())
1177 self.assertMessages(
1178 self.get_pending_messages(),
1179 [{"type": "operation-result",
1180 "operation-id": 123,
1181 "status": SUCCEEDED,
1182 "result-text": "Package holds successfully changed.",
1183 "result-code": 0}])
1184
1185 result = self.changer.handle_tasks()
1186 return result.addCallback(assert_result)
1187
1188 def test_change_package_holds_create_not_installed(self):
1189 """
1190 If the C{change-package-holds} message requests to add holds for
1191 packages that aren't installed, the whole activity is failed. If
1192 multiple holds are specified, those won't be added. There's no
1193 difference between a package that is available in some
1194 repository and a package that the facade doesn't know about at
1195 all.
1196 """
1197 self._add_system_package("foo")
1198 self._add_package_to_deb_dir(self.repository_dir, "bar")
1199 self.facade.reload_channels()
1200 self.store.add_task("changer", {"type": "change-package-holds",
1201 "create": ["foo", "bar", "baz"],
1202 "operation-id": 123})
1203
1204 def assert_result(result):
1205 self.facade.reload_channels()
1206 self.assertEqual([], self.facade.get_package_holds())
1207 self.assertIn("Queuing message with change package holds results "
1208 "to exchange urgently.", self.logfile.getvalue())
1209 self.assertMessages(
1210 self.get_pending_messages(),
1211 [{"type": "operation-result",
1212 "operation-id": 123,
1213 "status": FAILED,
1214 "result-text": "Package holds not added, since the "
1215 "following packages are not installed: "
1216 "bar, baz",
1217 "result-code": 1}])
1218
1219 result = self.changer.handle_tasks()
1220 return result.addCallback(assert_result)
1221
1222 def test_change_package_holds_delete_not_held(self):
1223 """
1224 If the C{change-package-holds} message requests to remove holds
1225 for packages that aren't held, the activity still succeeds. If
1226 other valid holds are specified, those will be removed.
1227 There's no difference between a package that is installed
1228 and a package that isn't installed. In either case, the
1229 result is that the package isn't held.
1230 """
1231 self._add_system_package("foo")
1232 self._add_system_package("bar")
1233 self.facade.set_package_hold("bar")
1234 self.facade.reload_channels()
1235 self.store.add_task("changer", {"type": "change-package-holds",
1236 "delete": ["foo", "bar", "baz"],
1237 "operation-id": 123})
1238
1239 def assert_result(result):
1240 self.facade.reload_channels()
1241 self.assertEqual([], self.facade.get_package_holds())
1242 self.assertIn("Queuing message with change package holds results "
1243 "to exchange urgently.", self.logfile.getvalue())
1244 self.assertMessages(
1245 self.get_pending_messages(),
1246 [{"type": "operation-result",
1247 "operation-id": 123,
1248 "status": SUCCEEDED,
1249 "result-text": "Package holds successfully changed.",
1250 "result-code": 0}])
1251
1252 result = self.changer.handle_tasks()
1253 return result.addCallback(assert_result)
11321254
=== modified file 'landscape/package/tests/test_facade.py'
--- landscape/package/tests/test_facade.py 2012-01-16 15:08:15 +0000
+++ landscape/package/tests/test_facade.py 2012-01-20 12:58:25 +0000
@@ -1385,6 +1385,95 @@
1385 sorted(error.packages, key=self.version_sortkey),1385 sorted(error.packages, key=self.version_sortkey),
1386 sorted([bar, baz], key=self.version_sortkey))1386 sorted([bar, baz], key=self.version_sortkey))
13871387
1388 def test_get_package_holds_with_no_hold(self):
1389 """
1390 If no package holds are set, C{get_package_holds} returns
1391 an empty C{list}.
1392 """
1393 self._add_system_package("foo")
1394 self.facade.reload_channels()
1395 self.assertEqual([], self.facade.get_package_holds())
1396
1397 def test_get_package_holds_with_holds(self):
1398 """
1399 If package holds are set, C{get_package_holds} returns
1400 the name of the packages that are held.
1401 """
1402 self._add_system_package(
1403 "foo", control_fields={"Status": "hold ok installed"})
1404 self._add_system_package("bar")
1405 self._add_system_package(
1406 "baz", control_fields={"Status": "hold ok installed"})
1407 self.facade.reload_channels()
1408
1409 self.assertEqual(
1410 ["baz", "foo"], sorted(self.facade.get_package_holds()))
1411
1412 def test_set_package_hold(self):
1413 """
1414 C{set_package_hold} marks a package to be on hold.
1415 """
1416 self._add_system_package("foo")
1417 self.facade.reload_channels()
1418 self.facade.set_package_hold("foo")
1419 self.facade.reload_channels()
1420
1421 self.assertEqual(["foo"], self.facade.get_package_holds())
1422
1423 def test_set_package_hold_existing_hold(self):
1424 """
1425 If a package is already hel, C{set_package_hold} doesn't return
1426 an error.
1427 """
1428 self._add_system_package(
1429 "foo", control_fields={"Status": "hold ok installed"})
1430 self.facade.reload_channels()
1431 self.facade.set_package_hold("foo")
1432 self.facade.reload_channels()
1433
1434 self.assertEqual(["foo"], self.facade.get_package_holds())
1435
1436 def test_remove_package_hold(self):
1437 """
1438 C{remove_package_hold} marks a package not to be on hold.
1439 """
1440 self._add_system_package(
1441 "foo", control_fields={"Status": "hold ok installed"})
1442 self.facade.reload_channels()
1443 self.facade.remove_package_hold("foo")
1444 self.facade.reload_channels()
1445
1446 self.assertEqual([], self.facade.get_package_holds())
1447
1448 def test_remove_package_hold_no_package(self):
1449 """
1450 If a package doesn't exist, C{remove_package_hold} doesn't
1451 return an error. It's up to the caller to make sure that the
1452 package exist, if it's important.
1453 """
1454 self._add_system_package("foo")
1455 self.facade.reload_channels()
1456 self.facade.remove_package_hold("bar")
1457 self.facade.reload_channels()
1458
1459 self.assertEqual([], self.facade.get_package_holds())
1460
1461 def test_remove_package_hold_no_hold(self):
1462 """
1463 If a package isn't held, the existing selection is retained when
1464 C{remove_package_hold} is called.
1465 """
1466 self._add_system_package(
1467 "foo", control_fields={"Status": "deinstall ok installed"})
1468 self.facade.reload_channels()
1469 self.facade.remove_package_hold("foo")
1470 self.facade.reload_channels()
1471
1472 self.assertEqual([], self.facade.get_package_holds())
1473 [foo] = self.facade.get_packages_by_name("foo")
1474 self.assertEqual(
1475 apt_pkg.SELSTATE_DEINSTALL, foo.package._pkg.selected_state)
1476
1388 if not hasattr(Package, "shortname"):1477 if not hasattr(Package, "shortname"):
1389 # The 'shortname' attribute was added when multi-arch support1478 # The 'shortname' attribute was added when multi-arch support
1390 # was added to python-apt. So if it's not there, it means that1479 # was added to python-apt. So if it's not there, it means that

Subscribers

People subscribed via source and target branches

to all changes: