Merge ~ahasenack/ubuntu/+source/landscape-client:trusty-landscape-client-1721383 into ~usd-import-team/ubuntu/+source/landscape-client:ubuntu/trusty-devel
- Git
- lp:~ahasenack/ubuntu/+source/landscape-client
- trusty-landscape-client-1721383
- Merge into ubuntu/trusty-devel
Proposed by
Andreas Hasenack
on 2017-11-23
| Status: | Merged | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Merge reported by: | ChristianEhrhardt | ||||||||||||||||
| Merged at revision: | 9a2c9d1fe4a22822f9d05903c6af0da42a4b56c2 | ||||||||||||||||
| Proposed branch: | ~ahasenack/ubuntu/+source/landscape-client:trusty-landscape-client-1721383 | ||||||||||||||||
| Merge into: | ~usd-import-team/ubuntu/+source/landscape-client:ubuntu/trusty-devel | ||||||||||||||||
| Diff against target: |
913 lines (+873/-0) 6 files modified
debian/changelog (+10/-0) debian/patches/autoremovable-report-1208393.diff (+317/-0) debian/patches/config-no-reregister-1618483.diff (+184/-0) debian/patches/iso-configuration-1699789.diff (+87/-0) debian/patches/package-reporter-proxy-1531150.diff (+271/-0) debian/patches/series (+4/-0) |
||||||||||||||||
| Related bugs: |
|
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Simon Poirier (community) | 2017-11-23 | Approve on 2017-11-24 | |
| ChristianEhrhardt | 2017-11-23 | Approve on 2017-11-24 | |
| Eric Desrochers | 2017-11-23 | Pending | |
|
Review via email:
|
|||
Commit Message
Description of the Change
To post a comment you must log in.
~ahasenack/ubuntu/+source/landscape-client:trusty-landscape-client-1721383
updated
on 2017-11-23
- 9a2c9d1... by Andreas Hasenack on 2017-11-23
-
changelog
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
| 1 | diff --git a/debian/changelog b/debian/changelog |
| 2 | index a03f5db..2ae1ca0 100644 |
| 3 | --- a/debian/changelog |
| 4 | +++ b/debian/changelog |
| 5 | @@ -1,3 +1,13 @@ |
| 6 | +landscape-client (14.12-0ubuntu6.14.04.1) trusty; urgency=medium |
| 7 | + |
| 8 | + [ Simon Poirier ] |
| 9 | + * Add proxy handling to package reporter. (LP: #1531150) |
| 10 | + * Fix regression in configuration hook under install-cd chroot (LP: #1699789) |
| 11 | + * Report autoremovable packages (LP: #1208393) |
| 12 | + * No not re-register client by default (LP: #1618483) |
| 13 | + |
| 14 | + -- Andreas Hasenack <andreas@canonical.com> Fri, 10 Nov 2017 16:21:54 -0200 |
| 15 | + |
| 16 | landscape-client (14.12-0ubuntu6.14.04) trusty; urgency=medium |
| 17 | |
| 18 | * Minor updates (LP: #1636477): |
| 19 | diff --git a/debian/patches/autoremovable-report-1208393.diff b/debian/patches/autoremovable-report-1208393.diff |
| 20 | new file mode 100644 |
| 21 | index 0000000..eab7407 |
| 22 | --- /dev/null |
| 23 | +++ b/debian/patches/autoremovable-report-1208393.diff |
| 24 | @@ -0,0 +1,317 @@ |
| 25 | +Description: Package autoremove reporting |
| 26 | + - Add autoremove state to store schema |
| 27 | + - autoremove package report |
| 28 | +Author: Simon Poirier <simpoir@gmail.com> |
| 29 | +Origin: upstream, https://github.com/CanonicalLtd/landscape-client/commit/4807ca51a92814acaa51156d7172a274309f2934 |
| 30 | +Bug-Ubuntu: https://launchpad.net/bugs/1208393 |
| 31 | +Last-Update: 2017-11-10 |
| 32 | +--- |
| 33 | +This patch header follows DEP-3: http://dep.debian.net/deps/dep3/ |
| 34 | +--- a/landscape/package/facade.py |
| 35 | ++++ b/landscape/package/facade.py |
| 36 | +@@ -422,6 +422,10 @@ |
| 37 | + return False |
| 38 | + return version > version.package.installed |
| 39 | + |
| 40 | ++ def is_package_autoremovable(self, version): |
| 41 | ++ """Was the package auto-installed, but isn't required anymore?""" |
| 42 | ++ return version.package.is_auto_removable |
| 43 | ++ |
| 44 | + def _is_main_architecture(self, package): |
| 45 | + """Is the package for the facade's main architecture?""" |
| 46 | + # package.name includes the architecture, if it's for a foreign |
| 47 | +--- a/landscape/package/reporter.py |
| 48 | ++++ b/landscape/package/reporter.py |
| 49 | +@@ -341,6 +341,7 @@ |
| 50 | + self._store.clear_installed() |
| 51 | + self._store.clear_locked() |
| 52 | + self._store.clear_hash_id_requests() |
| 53 | ++ self._store.clear_autoremovable() |
| 54 | + |
| 55 | + return succeed(None) |
| 56 | + |
| 57 | +@@ -542,11 +543,13 @@ |
| 58 | + old_available = set(self._store.get_available()) |
| 59 | + old_upgrades = set(self._store.get_available_upgrades()) |
| 60 | + old_locked = set(self._store.get_locked()) |
| 61 | ++ old_autoremovable = set(self._store.get_autoremovable()) |
| 62 | + |
| 63 | + current_installed = set() |
| 64 | + current_available = set() |
| 65 | + current_upgrades = set() |
| 66 | + current_locked = set() |
| 67 | ++ current_autoremovable = set() |
| 68 | + lsb = parse_lsb_release(LSB_RELEASE_FILENAME) |
| 69 | + backports_archive = "{}-backports".format(lsb["code-name"]) |
| 70 | + |
| 71 | +@@ -574,6 +577,8 @@ |
| 72 | + current_installed.add(id) |
| 73 | + if self._facade.is_package_available(package): |
| 74 | + current_available.add(id) |
| 75 | ++ if self._facade.is_package_autoremovable(package): |
| 76 | ++ current_autoremovable.add(id) |
| 77 | + else: |
| 78 | + current_available.add(id) |
| 79 | + |
| 80 | +@@ -591,11 +596,13 @@ |
| 81 | + new_available = current_available - old_available |
| 82 | + new_upgrades = current_upgrades - old_upgrades |
| 83 | + new_locked = current_locked - old_locked |
| 84 | ++ new_autoremovable = current_autoremovable - old_autoremovable |
| 85 | + |
| 86 | + not_installed = old_installed - current_installed |
| 87 | + not_available = old_available - current_available |
| 88 | + not_upgrades = old_upgrades - current_upgrades |
| 89 | + not_locked = old_locked - current_locked |
| 90 | ++ not_autoremovable = old_autoremovable - current_autoremovable |
| 91 | + |
| 92 | + message = {} |
| 93 | + if new_installed: |
| 94 | +@@ -611,6 +618,13 @@ |
| 95 | + message["locked"] = \ |
| 96 | + list(sequence_to_ranges(sorted(new_locked))) |
| 97 | + |
| 98 | ++ if new_autoremovable: |
| 99 | ++ message["autoremovable"] = list( |
| 100 | ++ sequence_to_ranges(sorted(new_autoremovable))) |
| 101 | ++ if not_autoremovable: |
| 102 | ++ message["not-autoremovable"] = list( |
| 103 | ++ sequence_to_ranges(sorted(not_autoremovable))) |
| 104 | ++ |
| 105 | + if not_installed: |
| 106 | + message["not-installed"] = \ |
| 107 | + list(sequence_to_ranges(sorted(not_installed))) |
| 108 | +@@ -632,12 +646,15 @@ |
| 109 | + |
| 110 | + logging.info("Queuing message with changes in known packages: " |
| 111 | + "%d installed, %d available, %d available upgrades, " |
| 112 | +- "%d locked, %d not installed, %d not available, " |
| 113 | +- "%d not available upgrades, %d not locked." |
| 114 | ++ "%d locked, %d autoremovable, %d not installed, " |
| 115 | ++ "%d not available, %d not available upgrades, " |
| 116 | ++ "%d not locked, %d not autoremovable. " |
| 117 | + % (len(new_installed), len(new_available), |
| 118 | + len(new_upgrades), len(new_locked), |
| 119 | ++ len(new_autoremovable), |
| 120 | + len(not_installed), len(not_available), |
| 121 | +- len(not_upgrades), len(not_locked))) |
| 122 | ++ len(not_upgrades), len(not_locked), |
| 123 | ++ len(not_autoremovable))) |
| 124 | + |
| 125 | + def update_currently_known(result): |
| 126 | + if new_installed: |
| 127 | +@@ -648,6 +665,8 @@ |
| 128 | + self._store.add_available(new_available) |
| 129 | + if new_locked: |
| 130 | + self._store.add_locked(new_locked) |
| 131 | ++ if new_autoremovable: |
| 132 | ++ self._store.add_autoremovable(new_autoremovable) |
| 133 | + if not_available: |
| 134 | + self._store.remove_available(not_available) |
| 135 | + if new_upgrades: |
| 136 | +@@ -656,6 +675,8 @@ |
| 137 | + self._store.remove_available_upgrades(not_upgrades) |
| 138 | + if not_locked: |
| 139 | + self._store.remove_locked(not_locked) |
| 140 | ++ if not_autoremovable: |
| 141 | ++ self._store.remove_autoremovable(not_autoremovable) |
| 142 | + # Something has changed wrt the former run, let's update the |
| 143 | + # timestamp and return True. |
| 144 | + stamp_file = self._config.detect_package_changes_stamp |
| 145 | +--- a/landscape/package/store.py |
| 146 | ++++ b/landscape/package/store.py |
| 147 | +@@ -208,6 +208,25 @@ |
| 148 | + return [row[0] for row in cursor.fetchall()] |
| 149 | + |
| 150 | + @with_cursor |
| 151 | ++ def add_autoremovable(self, cursor, ids): |
| 152 | ++ for id in ids: |
| 153 | ++ cursor.execute("REPLACE INTO autoremovable VALUES (?)", (id,)) |
| 154 | ++ |
| 155 | ++ @with_cursor |
| 156 | ++ def remove_autoremovable(self, cursor, ids): |
| 157 | ++ id_list = ",".join(str(int(id)) for id in ids) |
| 158 | ++ cursor.execute("DELETE FROM autoremovable WHERE id IN (%s)" % id_list) |
| 159 | ++ |
| 160 | ++ @with_cursor |
| 161 | ++ def clear_autoremovable(self, cursor): |
| 162 | ++ cursor.execute("DELETE FROM autoremovable") |
| 163 | ++ |
| 164 | ++ @with_cursor |
| 165 | ++ def get_autoremovable(self, cursor): |
| 166 | ++ cursor.execute("SELECT id FROM autoremovable") |
| 167 | ++ return [row[0] for row in cursor.fetchall()] |
| 168 | ++ |
| 169 | ++ @with_cursor |
| 170 | + def add_installed(self, cursor, ids): |
| 171 | + for id in ids: |
| 172 | + cursor.execute("REPLACE INTO installed VALUES (?)", (id,)) |
| 173 | +@@ -424,6 +443,8 @@ |
| 174 | + # try block. |
| 175 | + cursor = db.cursor() |
| 176 | + try: |
| 177 | ++ cursor.execute("CREATE TABLE autoremovable" |
| 178 | ++ " (id INTEGER PRIMARY KEY)") |
| 179 | + cursor.execute("CREATE TABLE locked" |
| 180 | + " (id INTEGER PRIMARY KEY)") |
| 181 | + cursor.execute("CREATE TABLE available" |
| 182 | +--- a/landscape/package/tests/test_facade.py |
| 183 | ++++ b/landscape/package/tests/test_facade.py |
| 184 | +@@ -817,6 +817,30 @@ |
| 185 | + self.assertEqual("version2-release2", version2.version) |
| 186 | + self.assertFalse(self.facade.is_package_installed(version2)) |
| 187 | + |
| 188 | ++ def test_is_package_autoremovable(self): |
| 189 | ++ """ |
| 190 | ++ Check that auto packages without dependencies on them are correctly |
| 191 | ++ detected as being autoremovable. |
| 192 | ++ """ |
| 193 | ++ self._add_system_package("dep") |
| 194 | ++ self._add_system_package("newdep") |
| 195 | ++ self._add_system_package("foo", control_fields={"Depends": "newdep"}) |
| 196 | ++ self.facade.reload_channels() |
| 197 | ++ dep, = sorted(self.facade.get_packages_by_name("dep")) |
| 198 | ++ dep.package.mark_auto(True) |
| 199 | ++ # dep should not be explicitely installed |
| 200 | ++ dep.package.mark_install(False) |
| 201 | ++ newdep, = sorted(self.facade.get_packages_by_name("newdep")) |
| 202 | ++ newdep, = sorted(self.facade.get_packages_by_name("newdep")) |
| 203 | ++ newdep.package.mark_auto(True) |
| 204 | ++ self.assertTrue(dep.package.is_installed) |
| 205 | ++ self.assertTrue(dep.package.is_auto_installed) |
| 206 | ++ self.assertTrue(newdep.package.is_installed) |
| 207 | ++ self.assertTrue(dep.package.is_auto_installed) |
| 208 | ++ |
| 209 | ++ self.assertTrue(self.facade.is_package_autoremovable(dep)) |
| 210 | ++ self.assertFalse(self.facade.is_package_autoremovable(newdep)) |
| 211 | ++ |
| 212 | + def test_is_package_available_in_channel_not_installed(self): |
| 213 | + """ |
| 214 | + A package is considered available if the package is in a |
| 215 | +--- a/landscape/package/tests/test_reporter.py |
| 216 | ++++ b/landscape/package/tests/test_reporter.py |
| 217 | +@@ -84,6 +84,15 @@ |
| 218 | + """Make it so that package "name1" is considered installed.""" |
| 219 | + self._install_deb_file(os.path.join(self.repository_dir, PKGNAME1)) |
| 220 | + |
| 221 | ++ def set_pkg1_autoremovable(self): |
| 222 | ++ """Make it so package "name1" is considered auto removable.""" |
| 223 | ++ self.set_pkg1_installed() |
| 224 | ++ self.facade.reload_channels() |
| 225 | ++ name1 = sorted(self.facade.get_packages_by_name("name1"))[0] |
| 226 | ++ # Since no other package depends on this, all that's needed |
| 227 | ++ # to have it autoremovable is to mark it as installed+auto. |
| 228 | ++ name1.package.mark_auto(True) |
| 229 | ++ |
| 230 | + def _make_fake_apt_update(self, out="output", err="error", code=0): |
| 231 | + """Create a fake apt-update executable""" |
| 232 | + self.reporter.apt_update_filename = self.makeFile( |
| 233 | +@@ -971,6 +980,53 @@ |
| 234 | + result = self.reporter.detect_packages_changes() |
| 235 | + return result.addCallback(got_result) |
| 236 | + |
| 237 | ++ def test_detect_packages_changes_with_autoremovable(self): |
| 238 | ++ message_store = self.broker_service.message_store |
| 239 | ++ message_store.set_accepted_types(["packages"]) |
| 240 | ++ |
| 241 | ++ self.store.set_hash_ids({HASH1: 1, HASH2: 2, HASH3: 3}) |
| 242 | ++ self.store.add_available([1, 2, 3]) |
| 243 | ++ self.store.add_installed([1]) |
| 244 | ++ self.set_pkg1_autoremovable() |
| 245 | ++ |
| 246 | ++ result = self.successResultOf(self.reporter.detect_packages_changes()) |
| 247 | ++ self.assertTrue(result) |
| 248 | ++ |
| 249 | ++ expected = [{"type": "packages", "autoremovable": [1]}] |
| 250 | ++ self.assertMessages(message_store.get_pending_messages(), expected) |
| 251 | ++ self.assertEqual([1], self.store.get_autoremovable()) |
| 252 | ++ |
| 253 | ++ def test_detect_packages_changes_with_not_autoremovable(self): |
| 254 | ++ message_store = self.broker_service.message_store |
| 255 | ++ message_store.set_accepted_types(["packages"]) |
| 256 | ++ |
| 257 | ++ self.store.set_hash_ids({HASH1: 1, HASH2: 2, HASH3: 3}) |
| 258 | ++ self.store.add_available([1, 2, 3]) |
| 259 | ++ # We don't care about checking other state changes in this test. |
| 260 | ++ # In reality the package would also be installed, available, etc. |
| 261 | ++ self.store.add_autoremovable([1, 2]) |
| 262 | ++ |
| 263 | ++ result = self.successResultOf(self.reporter.detect_packages_changes()) |
| 264 | ++ self.assertTrue(result) |
| 265 | ++ |
| 266 | ++ expected = [{"type": "packages", "not-autoremovable": [1, 2]}] |
| 267 | ++ self.assertMessages(message_store.get_pending_messages(), expected) |
| 268 | ++ self.assertEqual([], self.store.get_autoremovable()) |
| 269 | ++ |
| 270 | ++ def test_detect_packages_changes_with_known_autoremovable(self): |
| 271 | ++ message_store = self.broker_service.message_store |
| 272 | ++ message_store.set_accepted_types(["packages"]) |
| 273 | ++ |
| 274 | ++ self.store.set_hash_ids({HASH1: 1, HASH2: 2, HASH3: 3}) |
| 275 | ++ self.store.add_available([1, 2, 3]) |
| 276 | ++ self.store.add_installed([1]) |
| 277 | ++ self.store.add_autoremovable([1]) |
| 278 | ++ self.set_pkg1_autoremovable() |
| 279 | ++ |
| 280 | ++ result = self.successResultOf(self.reporter.detect_packages_changes()) |
| 281 | ++ self.assertFalse(result) |
| 282 | ++ self.assertEqual([1], self.store.get_autoremovable()) |
| 283 | ++ |
| 284 | + def test_detect_packages_changes_with_backports(self): |
| 285 | + """ |
| 286 | + Package versions coming from backports aren't considered to be |
| 287 | +--- a/landscape/package/tests/test_store.py |
| 288 | ++++ b/landscape/package/tests/test_store.py |
| 289 | +@@ -284,6 +284,23 @@ |
| 290 | + self.store1.clear_available_upgrades() |
| 291 | + self.assertEqual(self.store2.get_available_upgrades(), []) |
| 292 | + |
| 293 | ++ def test_add_and_get_autoremovable(self): |
| 294 | ++ self.store1.add_autoremovable([1, 2]) |
| 295 | ++ value = self.store1.get_autoremovable() |
| 296 | ++ self.assertEqual([1, 2], value) |
| 297 | ++ |
| 298 | ++ def test_clear_autoremovable(self): |
| 299 | ++ self.store1.add_autoremovable([1, 2]) |
| 300 | ++ self.store1.clear_autoremovable() |
| 301 | ++ value = self.store1.get_autoremovable() |
| 302 | ++ self.assertEqual([], value) |
| 303 | ++ |
| 304 | ++ def test_remove_autoremovable(self): |
| 305 | ++ self.store1.add_autoremovable([1, 2, 3, 4]) |
| 306 | ++ self.store1.remove_autoremovable([2, 4, 5]) |
| 307 | ++ value = self.store1.get_autoremovable() |
| 308 | ++ self.assertEqual([1, 3], value) |
| 309 | ++ |
| 310 | + def test_add_and_get_installed_packages(self): |
| 311 | + self.store1.add_installed([1, 2]) |
| 312 | + self.assertEqual(self.store2.get_installed(), [1, 2]) |
| 313 | +@@ -350,6 +367,9 @@ |
| 314 | + cursor.execute("pragma table_info(locked)") |
| 315 | + result = cursor.fetchall() |
| 316 | + self.assertTrue(len(result) > 0) |
| 317 | ++ cursor.execute("pragma table_info(autoremovable)") |
| 318 | ++ result = cursor.fetchall() |
| 319 | ++ self.assertTrue(len(result) > 0) |
| 320 | + |
| 321 | + def test_add_and_get_locked(self): |
| 322 | + """ |
| 323 | +--- a/landscape/message_schemas.py |
| 324 | ++++ b/landscape/message_schemas.py |
| 325 | +@@ -350,13 +350,15 @@ |
| 326 | + "available": package_ids_or_ranges, |
| 327 | + "available-upgrades": package_ids_or_ranges, |
| 328 | + "locked": package_ids_or_ranges, |
| 329 | ++ "autoremovable": package_ids_or_ranges, |
| 330 | ++ "not-autoremovable": package_ids_or_ranges, |
| 331 | + "not-installed": package_ids_or_ranges, |
| 332 | + "not-available": package_ids_or_ranges, |
| 333 | + "not-available-upgrades": package_ids_or_ranges, |
| 334 | + "not-locked": package_ids_or_ranges}, |
| 335 | + optional=["installed", "available", "available-upgrades", "locked", |
| 336 | + "not-available", "not-installed", "not-available-upgrades", |
| 337 | +- "not-locked"]) |
| 338 | ++ "not-locked", "autoremovable", "not-autoremovable"]) |
| 339 | + |
| 340 | + package_locks = List(Tuple(Unicode(), Unicode(), Unicode())) |
| 341 | + PACKAGE_LOCKS = Message( |
| 342 | diff --git a/debian/patches/config-no-reregister-1618483.diff b/debian/patches/config-no-reregister-1618483.diff |
| 343 | new file mode 100644 |
| 344 | index 0000000..0a564d6 |
| 345 | --- /dev/null |
| 346 | +++ b/debian/patches/config-no-reregister-1618483.diff |
| 347 | @@ -0,0 +1,184 @@ |
| 348 | +Description: Default to not register again if already registered |
| 349 | + When running the landscape-config script, change the default answer to the |
| 350 | + registration question to "No" if this client is already registered. |
| 351 | +Author: Alberto Donato <alberto.donato@canonical.com> |
| 352 | +Origin: upstream, http://bazaar.launchpad.net/~landscape/landscape-client/trunk/revision/939 |
| 353 | +Bug: https://launchpad.net/bugs/1618483 |
| 354 | +Last-Update: 2017-11-10 |
| 355 | +--- |
| 356 | +This patch header follows DEP-3: http://dep.debian.net/deps/dep3/ |
| 357 | +--- a/landscape/configuration.py |
| 358 | ++++ b/landscape/configuration.py |
| 359 | +@@ -22,8 +22,10 @@ |
| 360 | + from landscape.lib.twisted_util import gather_results |
| 361 | + from landscape.lib.fetch import fetch, FetchError |
| 362 | + from landscape.lib.bootstrap import BootstrapList, BootstrapDirectory |
| 363 | ++from landscape.lib.persist import Persist |
| 364 | + from landscape.reactor import LandscapeReactor |
| 365 | +-from landscape.broker.registration import InvalidCredentialsError |
| 366 | ++from landscape.broker.registration import Identity, InvalidCredentialsError |
| 367 | ++from landscape.broker.service import BrokerService |
| 368 | + from landscape.broker.config import BrokerConfiguration |
| 369 | + from landscape.broker.amp import RemoteBrokerConnector |
| 370 | + |
| 371 | +@@ -731,6 +733,15 @@ |
| 372 | + return 2 # An error happened |
| 373 | + |
| 374 | + |
| 375 | ++def is_registered(config): |
| 376 | ++ """Return whether the client is already registered.""" |
| 377 | ++ persist_filename = os.path.join( |
| 378 | ++ config.data_path, "{}.bpickle".format(BrokerService.service_name)) |
| 379 | ++ persist = Persist(filename=persist_filename) |
| 380 | ++ identity = Identity(config, persist) |
| 381 | ++ return bool(identity.secure_id) |
| 382 | ++ |
| 383 | ++ |
| 384 | + def main(args, print=print): |
| 385 | + """Interact with the user and the server to set up client configuration.""" |
| 386 | + |
| 387 | +@@ -769,9 +780,13 @@ |
| 388 | + report_registration_outcome(result, print=print) |
| 389 | + sys.exit(determine_exit_code(result)) |
| 390 | + else: |
| 391 | +- answer = raw_input("\nRequest a new registration for " |
| 392 | +- "this computer now? (Y/n): ") |
| 393 | +- if not answer.upper().startswith("N"): |
| 394 | ++ script = LandscapeSetupScript(config) |
| 395 | ++ default_answer = not is_registered(config) |
| 396 | ++ answer = script.prompt_yes_no( |
| 397 | ++ "\nRequest a new registration for this computer now?", |
| 398 | ++ default=default_answer) |
| 399 | ++ |
| 400 | ++ if answer: |
| 401 | + result = register(config, reactor) |
| 402 | + report_registration_outcome(result, print=print) |
| 403 | + sys.exit(determine_exit_code(result)) |
| 404 | +--- a/landscape/tests/test_configuration.py |
| 405 | ++++ b/landscape/tests/test_configuration.py |
| 406 | +@@ -7,10 +7,12 @@ |
| 407 | + import sys |
| 408 | + import unittest |
| 409 | + |
| 410 | ++import mock |
| 411 | + from twisted.internet.defer import succeed, fail, Deferred |
| 412 | + |
| 413 | + from landscape.broker.registration import InvalidCredentialsError |
| 414 | +-from landscape.broker.tests.helpers import RemoteBrokerHelper |
| 415 | ++from landscape.broker.tests.helpers import ( |
| 416 | ++ RemoteBrokerHelper, BrokerConfigurationHelper) |
| 417 | + from landscape.configuration import ( |
| 418 | + print_text, LandscapeSetupScript, LandscapeSetupConfiguration, |
| 419 | + register, setup, main, setup_init_script_and_start_client, |
| 420 | +@@ -18,15 +20,17 @@ |
| 421 | + ImportOptionError, store_public_key_data, |
| 422 | + bootstrap_tree, got_connection, success, failure, exchange_failure, |
| 423 | + handle_registration_errors, done, got_error, report_registration_outcome, |
| 424 | +- determine_exit_code) |
| 425 | ++ determine_exit_code, is_registered) |
| 426 | + from landscape.lib.amp import MethodCallError |
| 427 | + from landscape.lib.fetch import HTTPCodeError, PyCurlError |
| 428 | ++from landscape.lib.persist import Persist |
| 429 | + from landscape.sysvconfig import SysVConfig, ProcessError |
| 430 | + from landscape.tests.helpers import FakeBrokerServiceHelper |
| 431 | + from landscape.tests.helpers import LandscapeTest, EnvironSaverHelper |
| 432 | + from landscape.tests.mocker import ANY, MATCH, CONTAINS, expect |
| 433 | + |
| 434 | + |
| 435 | ++ |
| 436 | + class LandscapeConfigurationTest(LandscapeTest): |
| 437 | + |
| 438 | + def get_config(self, args, data_path=None): |
| 439 | +@@ -1167,9 +1171,9 @@ |
| 440 | + setup_mock = self.mocker.replace(setup) |
| 441 | + setup_mock(ANY) |
| 442 | + |
| 443 | +- raw_input_mock = self.mocker.replace(raw_input) |
| 444 | ++ raw_input_mock = self.mocker.replace(raw_input, passthrough=False) |
| 445 | + raw_input_mock("\nRequest a new registration for " |
| 446 | +- "this computer now? (Y/n): ") |
| 447 | ++ "this computer now? [Y/n]") |
| 448 | + self.mocker.result("n") |
| 449 | + |
| 450 | + # This must not be called. |
| 451 | +@@ -1215,7 +1219,7 @@ |
| 452 | + |
| 453 | + raw_input_mock = self.mocker.replace(raw_input) |
| 454 | + raw_input_mock("\nRequest a new registration for " |
| 455 | +- "this computer now? (Y/n): ") |
| 456 | ++ "this computer now? [Y/n]") |
| 457 | + self.mocker.result("y") |
| 458 | + |
| 459 | + # The register() function will be called. |
| 460 | +@@ -1246,7 +1250,7 @@ |
| 461 | + |
| 462 | + raw_input_mock = self.mocker.replace(raw_input) |
| 463 | + raw_input_mock("\nRequest a new registration for " |
| 464 | +- "this computer now? (Y/n): ") |
| 465 | ++ "this computer now? [Y/n]") |
| 466 | + self.mocker.result("y") |
| 467 | + |
| 468 | + # The register() function will be called. |
| 469 | +@@ -1337,13 +1341,15 @@ |
| 470 | + printed) |
| 471 | + |
| 472 | + def make_working_config(self): |
| 473 | ++ data_path = self.makeFile() |
| 474 | + return self.makeFile("[client]\n" |
| 475 | + "computer_title = Old Title\n" |
| 476 | + "account_name = Old Name\n" |
| 477 | + "registration_key = Old Password\n" |
| 478 | + "http_proxy = http://old.proxy\n" |
| 479 | + "https_proxy = https://old.proxy\n" |
| 480 | +- "url = http://url\n") |
| 481 | ++ "data_path = {}\n" |
| 482 | ++ "url = http://url\n".format(data_path)) |
| 483 | + |
| 484 | + def test_register(self): |
| 485 | + sysvconfig_mock = self.mocker.patch(SysVConfig) |
| 486 | +@@ -1357,7 +1363,7 @@ |
| 487 | + |
| 488 | + raw_input_mock = self.mocker.replace(raw_input) |
| 489 | + raw_input_mock("\nRequest a new registration for " |
| 490 | +- "this computer now? (Y/n): ") |
| 491 | ++ "this computer now? [Y/n]") |
| 492 | + self.mocker.result("") |
| 493 | + |
| 494 | + raw_input_mock("\nThe Landscape client must be started " |
| 495 | +@@ -1424,7 +1430,7 @@ |
| 496 | + setup_mock(ANY) |
| 497 | + raw_input_mock = self.mocker.replace(raw_input) |
| 498 | + raw_input_mock("\nRequest a new registration for " |
| 499 | +- "this computer now? (Y/n): ") |
| 500 | ++ "this computer now? [Y/n]") |
| 501 | + self.mocker.result("") |
| 502 | + |
| 503 | + register_mock = self.mocker.replace(register, passthrough=False) |
| 504 | +@@ -2322,3 +2328,27 @@ |
| 505 | + for code in failure_codes: |
| 506 | + result = determine_exit_code(code) |
| 507 | + self.assertEqual(2, result) |
| 508 | ++ |
| 509 | ++ |
| 510 | ++class IsRegisteredTest(LandscapeTest): |
| 511 | ++ |
| 512 | ++ helpers = [BrokerConfigurationHelper] |
| 513 | ++ |
| 514 | ++ def setUp(self): |
| 515 | ++ super(IsRegisteredTest, self).setUp() |
| 516 | ++ persist_file = os.path.join(self.config.data_path, "broker.bpickle") |
| 517 | ++ self.persist = Persist(filename=persist_file) |
| 518 | ++ |
| 519 | ++ def test_is_registered_false(self): |
| 520 | ++ """ |
| 521 | ++ If the client hasn't previouly registered, is_registered returns False. |
| 522 | ++ """ |
| 523 | ++ self.assertFalse(is_registered(self.config)) |
| 524 | ++ |
| 525 | ++ def test_is_registered_true(self): |
| 526 | ++ """ |
| 527 | ++ If the client has previouly registered, is_registered returns True. |
| 528 | ++ """ |
| 529 | ++ self.persist.set("registration.secure-id", "super-secure") |
| 530 | ++ self.persist.save() |
| 531 | ++ self.assertTrue(is_registered(self.config)) |
| 532 | diff --git a/debian/patches/iso-configuration-1699789.diff b/debian/patches/iso-configuration-1699789.diff |
| 533 | new file mode 100644 |
| 534 | index 0000000..1adcd6d |
| 535 | --- /dev/null |
| 536 | +++ b/debian/patches/iso-configuration-1699789.diff |
| 537 | @@ -0,0 +1,87 @@ |
| 538 | +Description: Stop reactor in landscape-config on broker error |
| 539 | + Handle SystemExit exception, as those don't propagate past the reactor, in |
| 540 | + order to avoid getting stuck when installed in chroot. |
| 541 | +Author: Simon Poirier <simpoir@gmail.com> |
| 542 | +Origin: upstream, https://github.com/CanonicalLtd/landscape-client/commit/e0b1b0ca43c13bc65374df13d192405abb3014e6 |
| 543 | +Bug-Ubuntu: https://launchpad.net/bugs/1699789 |
| 544 | +Last-Update: 2017-11-10 |
| 545 | +--- |
| 546 | +This patch header follows DEP-3: http://dep.debian.net/deps/dep3/ |
| 547 | +--- a/landscape/configuration.py |
| 548 | ++++ b/landscape/configuration.py |
| 549 | +@@ -639,10 +639,12 @@ |
| 550 | + return results |
| 551 | + |
| 552 | + |
| 553 | +-def got_error(failure, print=print): |
| 554 | +- """...from broker.""" |
| 555 | ++def got_error(failure, reactor, add_result, print=print): |
| 556 | ++ """Handle errors contacting broker.""" |
| 557 | + print(failure.getTraceback(), file=sys.stderr) |
| 558 | +- raise SystemExit |
| 559 | ++ # Can't just raise SystemExit; it would be ignored by the reactor. |
| 560 | ++ add_result(SystemExit()) |
| 561 | ++ reactor.stop() |
| 562 | + |
| 563 | + |
| 564 | + def register(config, reactor=None, connector_factory=RemoteBrokerConnector, |
| 565 | +@@ -671,6 +673,7 @@ |
| 566 | + """ |
| 567 | + if reactor is None: |
| 568 | + reactor = LandscapeReactor() |
| 569 | ++ |
| 570 | + if results is None: |
| 571 | + results = [] |
| 572 | + add_result = results.append |
| 573 | +@@ -679,13 +682,17 @@ |
| 574 | + connection = connector.connect(max_retries=max_retries, quiet=True) |
| 575 | + connection.addCallback( |
| 576 | + partial(got_connection, add_result, connector, reactor)) |
| 577 | +- connection.addErrback(got_error) |
| 578 | ++ connection.addErrback( |
| 579 | ++ partial(got_error, reactor=reactor, add_result=add_result)) |
| 580 | + reactor.run() |
| 581 | + |
| 582 | + assert len(results) == 1, "We expect exactly one result." |
| 583 | + # Results will be things like "success" or "ssl-error". |
| 584 | + result = results[0] |
| 585 | + |
| 586 | ++ if isinstance(result, SystemExit): |
| 587 | ++ raise result |
| 588 | ++ |
| 589 | + # If there was an error and the caller requested that errors be reported |
| 590 | + # to the on_error callable, then do so. |
| 591 | + if result != "success" and on_error is not None: |
| 592 | +--- a/landscape/tests/test_configuration.py |
| 593 | ++++ b/landscape/tests/test_configuration.py |
| 594 | +@@ -172,14 +172,19 @@ |
| 595 | + def getTraceback(self): |
| 596 | + return "traceback" |
| 597 | + |
| 598 | ++ results = [] |
| 599 | + printed = [] |
| 600 | + |
| 601 | + def faux_print(text, file): |
| 602 | + printed.append((text, file)) |
| 603 | + |
| 604 | +- with self.assertRaises(SystemExit): |
| 605 | +- got_error(FauxFailure(), print=faux_print) |
| 606 | ++ mock_reactor = mock.Mock() |
| 607 | + |
| 608 | ++ got_error(FauxFailure(), reactor=mock_reactor, |
| 609 | ++ add_result=results.append, print=faux_print) |
| 610 | ++ mock_reactor.stop.assert_called_once_with() |
| 611 | ++ |
| 612 | ++ self.assertIsInstance(results[0], SystemExit) |
| 613 | + self.assertEqual([('traceback', sys.stderr)], printed) |
| 614 | + |
| 615 | + |
| 616 | +@@ -2081,7 +2086,7 @@ |
| 617 | + self.assertTrue(1, len(connector.connection.errbacks)) |
| 618 | + self.assertEqual( |
| 619 | + 'got_error', |
| 620 | +- connector.connection.errbacks[0].__name__) |
| 621 | ++ connector.connection.errbacks[0].func.__name__) |
| 622 | + # We ask for retries because networks aren't reliable. |
| 623 | + self.assertEqual(99, connector.max_retries) |
| 624 | + |
| 625 | diff --git a/debian/patches/package-reporter-proxy-1531150.diff b/debian/patches/package-reporter-proxy-1531150.diff |
| 626 | new file mode 100644 |
| 627 | index 0000000..b154609 |
| 628 | --- /dev/null |
| 629 | +++ b/debian/patches/package-reporter-proxy-1531150.diff |
| 630 | @@ -0,0 +1,271 @@ |
| 631 | +Description: Add proxy handling to package reporter |
| 632 | +Author: Simon Poirier <simon.poirier@canonical.com> |
| 633 | +Origin: upstream, http://bazaar.launchpad.net/~landscape/landscape-client/trunk/revision/919 |
| 634 | +Bug: https://launchpad.net/bugs/1531150 |
| 635 | +Last-Update: 2017-11-10 |
| 636 | +--- |
| 637 | +This patch header follows DEP-3: http://dep.debian.net/deps/dep3/ |
| 638 | +--- a/apt-update/apt-update.c |
| 639 | ++++ b/apt-update/apt-update.c |
| 640 | +@@ -19,7 +19,7 @@ |
| 641 | + int main(int argc, char *argv[], char *envp[]) |
| 642 | + { |
| 643 | + char *apt_argv[] = {"/usr/bin/apt-get", "-q", "update", NULL}; |
| 644 | +- char *apt_envp[] = {"PATH=/bin:/usr/bin", NULL, NULL}; |
| 645 | ++ char *apt_envp[] = {"PATH=/bin:/usr/bin", NULL, NULL, NULL, NULL}; |
| 646 | + |
| 647 | + // Set the HOME environment variable |
| 648 | + struct passwd *pwd = getpwuid(geteuid()); |
| 649 | +@@ -33,6 +33,24 @@ |
| 650 | + exit(1); |
| 651 | + } |
| 652 | + |
| 653 | ++ // Pass proxy environment variables |
| 654 | ++ int proxy_arg = 2; |
| 655 | ++ char *http_proxy = getenv("http_proxy"); |
| 656 | ++ if (http_proxy) { |
| 657 | ++ if (asprintf(&apt_envp[proxy_arg], "http_proxy=%s", http_proxy) == -1) { |
| 658 | ++ perror("error: Unable to set http_proxy environment variable"); |
| 659 | ++ exit(1); |
| 660 | ++ } |
| 661 | ++ proxy_arg++; |
| 662 | ++ } |
| 663 | ++ char *https_proxy = getenv("https_proxy"); |
| 664 | ++ if (https_proxy) { |
| 665 | ++ if (asprintf(&apt_envp[proxy_arg], "https_proxy=%s", https_proxy) == -1) { |
| 666 | ++ perror("error: Unable to set https_proxy environment variable"); |
| 667 | ++ exit(1); |
| 668 | ++ } |
| 669 | ++ } |
| 670 | ++ |
| 671 | + // Drop any supplementary group |
| 672 | + if (setgroups(0, NULL) == -1) { |
| 673 | + perror("error: Unable to set supplementary groups IDs"); |
| 674 | +--- a/landscape/lib/fetch.py |
| 675 | ++++ b/landscape/lib/fetch.py |
| 676 | +@@ -45,7 +45,7 @@ |
| 677 | + |
| 678 | + def fetch(url, post=False, data="", headers={}, cainfo=None, curl=None, |
| 679 | + connect_timeout=30, total_timeout=600, insecure=False, follow=True, |
| 680 | +- user_agent=None): |
| 681 | ++ user_agent=None, proxy=None): |
| 682 | + """Retrieve a URL and return the content. |
| 683 | + |
| 684 | + @param url: The url to be fetched. |
| 685 | +@@ -61,6 +61,7 @@ |
| 686 | + during autodiscovery) |
| 687 | + @param follow: If True, follow HTTP redirects (default True). |
| 688 | + @param user_agent: The user-agent to set in the request. |
| 689 | ++ @param proxy: The proxy url to use for the request. |
| 690 | + """ |
| 691 | + import pycurl |
| 692 | + output = StringIO(data) |
| 693 | +@@ -94,6 +95,9 @@ |
| 694 | + if user_agent is not None: |
| 695 | + curl.setopt(pycurl.USERAGENT, user_agent) |
| 696 | + |
| 697 | ++ if proxy is not None: |
| 698 | ++ curl.setopt(pycurl.PROXY, proxy) |
| 699 | ++ |
| 700 | + curl.setopt(pycurl.MAXREDIRS, 5) |
| 701 | + curl.setopt(pycurl.CONNECTTIMEOUT, connect_timeout) |
| 702 | + curl.setopt(pycurl.LOW_SPEED_LIMIT, 1) |
| 703 | +--- a/landscape/lib/tests/test_fetch.py |
| 704 | ++++ b/landscape/lib/tests/test_fetch.py |
| 705 | +@@ -286,6 +286,14 @@ |
| 706 | + self.assertEqual(result, "result") |
| 707 | + self.assertEqual("user-agent", curl.options[pycurl.USERAGENT]) |
| 708 | + |
| 709 | ++ def test_pycurl_proxy(self): |
| 710 | ++ """If provided, the proxy is set in the request.""" |
| 711 | ++ curl = CurlStub("result") |
| 712 | ++ proxy = "http://my.little.proxy" |
| 713 | ++ result = fetch("http://example.com", curl=curl, proxy=proxy) |
| 714 | ++ self.assertEqual("result", result) |
| 715 | ++ self.assertEqual(proxy, curl.options[pycurl.PROXY]) |
| 716 | ++ |
| 717 | + def test_create_curl(self): |
| 718 | + curls = [] |
| 719 | + |
| 720 | +--- a/landscape/package/reporter.py |
| 721 | ++++ b/landscape/package/reporter.py |
| 722 | +@@ -38,6 +38,10 @@ |
| 723 | + parser.add_option("--force-apt-update", default=False, |
| 724 | + action="store_true", |
| 725 | + help="Force running apt-update.") |
| 726 | ++ parser.add_option("--http-proxy", metavar="URL", |
| 727 | ++ help="The URL of the HTTP proxy, if one is needed.") |
| 728 | ++ parser.add_option("--https-proxy", metavar="URL", |
| 729 | ++ help="The URL of the HTTPS proxy, if one is needed.") |
| 730 | + return parser |
| 731 | + |
| 732 | + |
| 733 | +@@ -133,8 +137,14 @@ |
| 734 | + logging.warning("Couldn't download hash=>id database: %s" % |
| 735 | + str(exception)) |
| 736 | + |
| 737 | ++ if url.startswith("https"): |
| 738 | ++ proxy = self._config.get("https_proxy") |
| 739 | ++ else: |
| 740 | ++ proxy = self._config.get("http_proxy") |
| 741 | ++ |
| 742 | + result = fetch_async(url, |
| 743 | +- cainfo=self._config.get("ssl_public_key")) |
| 744 | ++ cainfo=self._config.get("ssl_public_key"), |
| 745 | ++ proxy=proxy) |
| 746 | + result.addCallback(fetch_ok) |
| 747 | + result.addErrback(fetch_error) |
| 748 | + |
| 749 | +@@ -262,7 +272,12 @@ |
| 750 | + Run apt-update using the passed in deferred, which allows for callers |
| 751 | + to inspect the result code. |
| 752 | + """ |
| 753 | +- result = spawn_process(self.apt_update_filename) |
| 754 | ++ env = {} |
| 755 | ++ if self._config.http_proxy: |
| 756 | ++ env["http_proxy"] = self._config.http_proxy |
| 757 | ++ if self._config.https_proxy: |
| 758 | ++ env["https_proxy"] = self._config.https_proxy |
| 759 | ++ result = spawn_process(self.apt_update_filename, env=env) |
| 760 | + |
| 761 | + def callback((out, err, code), deferred): |
| 762 | + return deferred.callback((out, err, code)) |
| 763 | +--- a/landscape/package/tests/test_reporter.py |
| 764 | ++++ b/landscape/package/tests/test_reporter.py |
| 765 | +@@ -3,11 +3,11 @@ |
| 766 | + import time |
| 767 | + import apt_pkg |
| 768 | + import shutil |
| 769 | ++import mock |
| 770 | + |
| 771 | + from twisted.internet.defer import Deferred, succeed, inlineCallbacks |
| 772 | + from twisted.internet import reactor |
| 773 | + |
| 774 | +- |
| 775 | + from landscape.lib.fs import create_file, touch_file |
| 776 | + from landscape.lib.fetch import fetch_async, FetchError |
| 777 | + from landscape.lib.lsb_release import parse_lsb_release, LSB_RELEASE_FILENAME |
| 778 | +@@ -286,7 +286,7 @@ |
| 779 | + hash_id_db_url = self.config.package_hash_id_url + "uuid_codename_arch" |
| 780 | + fetch_async_mock = self.mocker.replace("landscape.lib." |
| 781 | + "fetch.fetch_async") |
| 782 | +- fetch_async_mock(hash_id_db_url, cainfo=None) |
| 783 | ++ fetch_async_mock(hash_id_db_url, cainfo=None, patch=None) |
| 784 | + fetch_async_result = Deferred() |
| 785 | + fetch_async_result.callback("hash-ids") |
| 786 | + self.mocker.result(fetch_async_result) |
| 787 | +@@ -311,6 +311,33 @@ |
| 788 | + |
| 789 | + return result |
| 790 | + |
| 791 | ++ @mock.patch("landscape.package.reporter.fetch_async", |
| 792 | ++ return_value=succeed("hash-ids")) |
| 793 | ++ @mock.patch("logging.info", return_value=None) |
| 794 | ++ def test_fetch_hash_id_db_with_proxy(self, logging_mock, mock_fetch_async): |
| 795 | ++ """fetching hash-id-db uses proxy settings""" |
| 796 | ++ # Assume package_hash_id_url is set |
| 797 | ++ self.config.data_path = self.makeDir() |
| 798 | ++ self.config.package_hash_id_url = "https://fake.url/path/" |
| 799 | ++ os.makedirs(os.path.join(self.config.data_path, "package", "hash-id")) |
| 800 | ++ |
| 801 | ++ # Fake uuid, codename and arch |
| 802 | ++ message_store = self.broker_service.message_store |
| 803 | ++ message_store.set_server_uuid("uuid") |
| 804 | ++ self.reporter.lsb_release_filename = self.makeFile(SAMPLE_LSB_RELEASE) |
| 805 | ++ self.facade.set_arch("arch") |
| 806 | ++ |
| 807 | ++ # Let's say fetch_async is successful |
| 808 | ++ hash_id_db_url = self.config.package_hash_id_url + "uuid_codename_arch" |
| 809 | ++ |
| 810 | ++ # set proxy settings |
| 811 | ++ self.config.https_proxy = "http://helloproxy:8000" |
| 812 | ++ |
| 813 | ++ result = self.reporter.fetch_hash_id_db() |
| 814 | ++ mock_fetch_async.assert_called_once_with( |
| 815 | ++ hash_id_db_url, cainfo=None, proxy="http://helloproxy:8000") |
| 816 | ++ return result |
| 817 | ++ |
| 818 | + def test_fetch_hash_id_db_does_not_download_twice(self): |
| 819 | + |
| 820 | + # Let's say that the hash=>id database is already there |
| 821 | +@@ -330,7 +357,7 @@ |
| 822 | + # Intercept any call to fetch_async |
| 823 | + fetch_async_mock = self.mocker.replace("landscape.lib." |
| 824 | + "fetch.fetch_async") |
| 825 | +- fetch_async_mock(ANY) |
| 826 | ++ fetch_async_mock(ANY, proxy=None) |
| 827 | + |
| 828 | + # Go! |
| 829 | + self.mocker.replay() |
| 830 | +@@ -430,7 +457,7 @@ |
| 831 | + "uuid_codename_arch" |
| 832 | + fetch_async_mock = self.mocker.replace("landscape.lib." |
| 833 | + "fetch.fetch_async") |
| 834 | +- fetch_async_mock(hash_id_db_url, cainfo=None) |
| 835 | ++ fetch_async_mock(hash_id_db_url, cainfo=None, proxy=None) |
| 836 | + fetch_async_result = Deferred() |
| 837 | + fetch_async_result.callback("hash-ids") |
| 838 | + self.mocker.result(fetch_async_result) |
| 839 | +@@ -462,7 +489,7 @@ |
| 840 | + hash_id_db_url = self.config.package_hash_id_url + "uuid_codename_arch" |
| 841 | + fetch_async_mock = self.mocker.replace("landscape.lib." |
| 842 | + "fetch.fetch_async") |
| 843 | +- fetch_async_mock(hash_id_db_url, cainfo=None) |
| 844 | ++ fetch_async_mock(hash_id_db_url, cainfo=None, proxy=None) |
| 845 | + fetch_async_result = Deferred() |
| 846 | + fetch_async_result.errback(FetchError("fetch error")) |
| 847 | + self.mocker.result(fetch_async_result) |
| 848 | +@@ -538,7 +565,7 @@ |
| 849 | + "uuid_codename_arch" |
| 850 | + fetch_async_mock = self.mocker.replace("landscape.lib." |
| 851 | + "fetch.fetch_async") |
| 852 | +- fetch_async_mock(hash_id_db_url, cainfo=self.config.ssl_public_key) |
| 853 | ++ fetch_async_mock(hash_id_db_url, cainfo=self.config.ssl_public_key, proxy=None) |
| 854 | + fetch_async_result = Deferred() |
| 855 | + fetch_async_result.callback("hash-ids") |
| 856 | + self.mocker.result(fetch_async_result) |
| 857 | +@@ -1621,6 +1648,44 @@ |
| 858 | + reactor.callWhenRunning(do_test) |
| 859 | + return deferred |
| 860 | + |
| 861 | ++ @mock.patch("landscape.package.reporter.spawn_process", |
| 862 | ++ return_value=succeed(("", "", 0))) |
| 863 | ++ def test_run_apt_update_honors_http_proxy(self, mock_spawn_process): |
| 864 | ++ """ |
| 865 | ++ The PackageReporter.run_apt_update method honors the http_proxy |
| 866 | ++ config when calling the apt-update wrapper. |
| 867 | ++ """ |
| 868 | ++ self.config.http_proxy = "http://proxy_server:8080" |
| 869 | ++ self.reporter.sources_list_filename = "/I/Dont/Exist" |
| 870 | ++ |
| 871 | ++ update_result = self.reporter.run_apt_update() |
| 872 | ++ # run_apt_update uses reactor.call_later so advance a bit |
| 873 | ++ self.reactor.advance(0) |
| 874 | ++ self.successResultOf(update_result) |
| 875 | ++ |
| 876 | ++ mock_spawn_process.assert_called_once_with( |
| 877 | ++ self.reporter.apt_update_filename, |
| 878 | ++ env={"http_proxy": "http://proxy_server:8080"}) |
| 879 | ++ |
| 880 | ++ @mock.patch("landscape.package.reporter.spawn_process", |
| 881 | ++ return_value=succeed(("", "", 0))) |
| 882 | ++ def test_run_apt_update_honors_https_proxy(self, mock_spawn_process): |
| 883 | ++ """ |
| 884 | ++ The PackageReporter.run_apt_update method honors the https_proxy |
| 885 | ++ config when calling the apt-update wrapper. |
| 886 | ++ """ |
| 887 | ++ self.config.https_proxy = "http://proxy_server:8443" |
| 888 | ++ self.reporter.sources_list_filename = "/I/Dont/Exist" |
| 889 | ++ |
| 890 | ++ update_result = self.reporter.run_apt_update() |
| 891 | ++ # run_apt_update uses reactor.call_later, so advance a bit |
| 892 | ++ self.reactor.advance(0) |
| 893 | ++ self.successResultOf(update_result) |
| 894 | ++ |
| 895 | ++ mock_spawn_process.assert_called_once_with( |
| 896 | ++ self.reporter.apt_update_filename, |
| 897 | ++ env={"https_proxy": "http://proxy_server:8443"}) |
| 898 | ++ |
| 899 | + def test_run_apt_update_error_on_cache_file(self): |
| 900 | + """ |
| 901 | + L{PackageReporter.run_apt_update} succeeds if the command fails because |
| 902 | diff --git a/debian/patches/series b/debian/patches/series |
| 903 | index 687f449..aaf6d9f 100644 |
| 904 | --- a/debian/patches/series |
| 905 | +++ b/debian/patches/series |
| 906 | @@ -10,3 +10,7 @@ bug-1508110-revno-824.diff |
| 907 | bug-1532887-revno-825.diff |
| 908 | bug-1508110-revno-826.diff |
| 909 | bug-1508110-revno-829.diff |
| 910 | +package-reporter-proxy-1531150.diff |
| 911 | +iso-configuration-1699789.diff |
| 912 | +autoremovable-report-1208393.diff |
| 913 | +config-no-reregister-1618483.diff |


From a packaging perspective this LGTM, as with the other one the actual code review will be by Simon/Eric.
Note: I've seen that you added SRU Templates, but as a heads up empty "Regression potential" sections are usually teasers for the SRU Team to think into it really deep and then Nack you for some potential regression (teaches maintainers to think about regressions themselves). If possible, give it a thought and outline the potential issues :-)