Merge ~simpoir/ubuntu/+source/landscape-client:disco-landscape-client-backports-1788219 into ubuntu/+source/landscape-client:ubuntu/disco-devel
- Git
- lp:~simpoir/ubuntu/+source/landscape-client
- disco-landscape-client-backports-1788219
- Merge into ubuntu/disco-devel
Proposed by
Simon Poirier
Status: | Merged | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Approved by: | Andreas Hasenack | ||||||||||||||||
Approved revision: | 63600cb26e13cd244833860d0366c26fb657125d | ||||||||||||||||
Merged at revision: | 63600cb26e13cd244833860d0366c26fb657125d | ||||||||||||||||
Proposed branch: | ~simpoir/ubuntu/+source/landscape-client:disco-landscape-client-backports-1788219 | ||||||||||||||||
Merge into: | ubuntu/+source/landscape-client:ubuntu/disco-devel | ||||||||||||||||
Diff against target: |
548 lines (+502/-0) 7 files modified
debian/changelog (+16/-0) debian/patches/1616116-resync-loop.patch (+142/-0) debian/patches/nutanix-kvm.patch (+17/-0) debian/patches/post-upgrade-reboot.patch (+132/-0) debian/patches/release-upgrade-success.patch (+23/-0) debian/patches/series (+5/-0) debian/patches/unicode-tags-script.patch (+167/-0) |
||||||||||||||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Andreas Hasenack | Approve | ||
Canonical Server | Pending | ||
git-ubuntu developers | Pending | ||
Review via email: mp+358799@code.launchpad.net |
Commit message
Description of the change
To post a comment you must log in.
Revision history for this message
Andreas Hasenack (ahasenack) : | # |
Revision history for this message
Andreas Hasenack (ahasenack) : | # |
Revision history for this message
Andreas Hasenack (ahasenack) : | # |
Revision history for this message
Andreas Hasenack (ahasenack) wrote : | # |
Thanks for the changes, +1
review:
Approve
Revision history for this message
Andreas Hasenack (ahasenack) wrote : | # |
Tagged and uploaded.
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 7615a82..5d64e8c 100644 |
3 | --- a/debian/changelog |
4 | +++ b/debian/changelog |
5 | @@ -1,3 +1,19 @@ |
6 | +landscape-client (18.01-0ubuntu6) disco; urgency=medium |
7 | + |
8 | + * debian/patches/nutanix-kvm.patch: Update vm_info.py to include Nutanix |
9 | + hypervisor. |
10 | + * Fixes for release-upgrade (LP: #1699179). |
11 | + - debian/patches/release-upgrade-success.patch: Enable landscape-client to |
12 | + survive trusty upgrade. (LP: #1670291) |
13 | + - debian/patches/post-upgrade-reboot.patch: Force reboot operation in |
14 | + case systemd fails. (LP: #1670291) |
15 | + * debian/patches/unicode-tags-script.patch: Permit environments |
16 | + containing unicode chars for script execution. (LP: #1765518) |
17 | + * debian/patches/1616116-resync-loop.patch: |
18 | + Clear hash id database on package resync. (LP: #1616116) |
19 | + |
20 | + -- Simon Poirier <simon.poirier@canonical.com> Tue, 27 Nov 2018 09:24:22 -0500 |
21 | + |
22 | landscape-client (18.01-0ubuntu5) disco; urgency=medium |
23 | |
24 | * No-change rebuild to build for python3.7 as the default. |
25 | diff --git a/debian/patches/1616116-resync-loop.patch b/debian/patches/1616116-resync-loop.patch |
26 | new file mode 100644 |
27 | index 0000000..a3f055b |
28 | --- /dev/null |
29 | +++ b/debian/patches/1616116-resync-loop.patch |
30 | @@ -0,0 +1,142 @@ |
31 | +Description: Clear hash id database on package resync. |
32 | + This should avoid loop when inconsistencies are present in the database |
33 | + through either corruption or server-restore. |
34 | +Author: Simon Poirier <simon.poirier@canonical.com> |
35 | +Origin: upstream, https://github.com/CanonicalLtd/landscape-client/commit/666cd32dbdedcc29c90154015bbe2ea0e52924e8 |
36 | +Bug-Ubuntu: https://bugs.launchpad.net/bugs/1616116 |
37 | +Last-Update: 2018-11-23 |
38 | + |
39 | +diff --git a/landscape/client/package/reporter.py b/landscape/client/package/reporter.py |
40 | +index afca838c..c48a4c04 100644 |
41 | +--- a/landscape/client/package/reporter.py |
42 | ++++ b/landscape/client/package/reporter.py |
43 | +@@ -400,7 +400,10 @@ class PackageReporter(PackageTaskHandler): |
44 | + |
45 | + return result |
46 | + |
47 | ++ @inlineCallbacks |
48 | + def _handle_resynchronize(self): |
49 | ++ self._store.clear_hash_ids() |
50 | ++ yield self._remove_hash_id_db() |
51 | + self._store.clear_available() |
52 | + self._store.clear_available_upgrades() |
53 | + self._store.clear_installed() |
54 | +@@ -408,8 +411,6 @@ class PackageReporter(PackageTaskHandler): |
55 | + self._store.clear_hash_id_requests() |
56 | + self._store.clear_autoremovable() |
57 | + |
58 | +- return succeed(None) |
59 | +- |
60 | + def _handle_unknown_packages(self, hashes): |
61 | + |
62 | + self._facade.ensure_channels_reloaded() |
63 | +@@ -445,6 +446,18 @@ class PackageReporter(PackageTaskHandler): |
64 | + |
65 | + return result |
66 | + |
67 | ++ def _remove_hash_id_db(self): |
68 | ++ |
69 | ++ def _remove_it(hash_id_db_filename): |
70 | ++ if hash_id_db_filename and os.path.exists(hash_id_db_filename): |
71 | ++ logging.warning( |
72 | ++ "Removing cached hash=>id database %s", |
73 | ++ hash_id_db_filename) |
74 | ++ os.remove(hash_id_db_filename) |
75 | ++ result = self._determine_hash_id_db_filename() |
76 | ++ result.addCallback(_remove_it) |
77 | ++ return result |
78 | ++ |
79 | + def remove_expired_hash_id_requests(self): |
80 | + now = time.time() |
81 | + timeout = now - HASH_ID_REQUEST_TIMEOUT |
82 | +diff --git a/landscape/client/package/tests/test_reporter.py b/landscape/client/package/tests/test_reporter.py |
83 | +index 03d2e368..851c8eb9 100644 |
84 | +--- a/landscape/client/package/tests/test_reporter.py |
85 | ++++ b/landscape/client/package/tests/test_reporter.py |
86 | +@@ -1204,10 +1204,11 @@ class PackageReporterAptTest(LandscapeTest): |
87 | + |
88 | + self.assertEqual(expected, command) |
89 | + |
90 | ++ @inlineCallbacks |
91 | + def test_resynchronize(self): |
92 | + """ |
93 | + When a resynchronize task arrives, the reporter should clear |
94 | +- out all the data in the package store, except the hash ids. |
95 | ++ out all the data in the package store, including the hash ids. |
96 | + |
97 | + This is done in the reporter so that we know it happens when |
98 | + no other reporter is possibly running at the same time. |
99 | +@@ -1220,6 +1221,16 @@ class PackageReporterAptTest(LandscapeTest): |
100 | + self.facade.reload_channels() |
101 | + message_store = self.broker_service.message_store |
102 | + message_store.set_accepted_types(["package-locks"]) |
103 | ++ |
104 | ++ # create hash_id file |
105 | ++ message_store.set_server_uuid("uuid") |
106 | ++ self.facade.set_arch("arch") |
107 | ++ hash_id_file = os.path.join( |
108 | ++ self.config.hash_id_directory, "uuid_codename_arch") |
109 | ++ os.makedirs(self.config.hash_id_directory) |
110 | ++ with open(hash_id_file, "w"): |
111 | ++ pass |
112 | ++ |
113 | + self.store.set_hash_ids({foo_hash: 3, HASH2: 4}) |
114 | + self.store.add_available([1]) |
115 | + self.store.add_available_upgrades([2]) |
116 | +@@ -1243,30 +1254,33 @@ class PackageReporterAptTest(LandscapeTest): |
117 | + self.store.add_task("reporter", {"type": "resynchronize"}) |
118 | + |
119 | + deferred = self.reporter.run() |
120 | +- |
121 | +- def check_result(result): |
122 | +- # The hashes should not go away. |
123 | +- hash1 = self.store.get_hash_id(foo_hash) |
124 | +- hash2 = self.store.get_hash_id(HASH2) |
125 | +- self.assertEqual([hash1, hash2], [3, 4]) |
126 | +- |
127 | +- # But the other data should. |
128 | +- self.assertEqual(self.store.get_available_upgrades(), []) |
129 | +- |
130 | +- # After running the resychronize task, detect_packages_changes is |
131 | +- # called, and the existing known hashes are made available. |
132 | +- self.assertEqual(self.store.get_available(), [4]) |
133 | +- self.assertEqual(self.store.get_installed(), [3]) |
134 | +- self.assertEqual(self.store.get_locked(), [3]) |
135 | +- |
136 | +- # The two original hash id requests are gone, and a new hash id |
137 | +- # request should be detected for HASH3. |
138 | +- [request] = self.store.iter_hash_id_requests() |
139 | +- self.assertEqual(request.hashes, [HASH3, HASH1]) |
140 | +- |
141 | +- deferred.addCallback(check_result) |
142 | + self.reactor.advance(0) |
143 | +- return deferred |
144 | ++ with mock.patch( |
145 | ++ "landscape.client.package.taskhandler.parse_lsb_release", |
146 | ++ side_effect=lambda _: {"code-name": "codename"} |
147 | ++ ): |
148 | ++ yield deferred |
149 | ++ |
150 | ++ # The hashes should go away to avoid loops |
151 | ++ # when server gets restored to an early state. |
152 | ++ hash1 = self.store.get_hash_id(foo_hash) |
153 | ++ hash2 = self.store.get_hash_id(HASH2) |
154 | ++ self.assertNotEqual(hash1, 3) |
155 | ++ self.assertNotEqual(hash2, 4) |
156 | ++ self.assertFalse(os.path.exists(hash_id_file)) |
157 | ++ self.assertEqual(self.store.get_available_upgrades(), []) |
158 | ++ |
159 | ++ # After running the resychronize task, the hash db is empty, |
160 | ++ # thus detect_packages_changes will generate an empty set until |
161 | ++ # next run when hash-id db is populated again. |
162 | ++ self.assertEqual(self.store.get_available(), []) |
163 | ++ self.assertEqual(self.store.get_installed(), []) |
164 | ++ self.assertEqual(self.store.get_locked(), []) |
165 | ++ |
166 | ++ # The two original hash id requests are gone, and a new hash id |
167 | ++ # request should be detected for HASH3. |
168 | ++ [request] = self.store.iter_hash_id_requests() |
169 | ++ self.assertEqual(request.hashes, [HASH3, HASH2, foo_hash, HASH1]) |
170 | + |
171 | + @mock.patch("logging.warning") |
172 | + def test_run_apt_update(self, warning_mock): |
173 | diff --git a/debian/patches/nutanix-kvm.patch b/debian/patches/nutanix-kvm.patch |
174 | new file mode 100644 |
175 | index 0000000..cf8bbba |
176 | --- /dev/null |
177 | +++ b/debian/patches/nutanix-kvm.patch |
178 | @@ -0,0 +1,17 @@ |
179 | +Description: Update vm_info.py to include Nutanix hypervisor |
180 | +Author: Simon Poirier <simon.poirier@canonical.com> |
181 | +Origin: upstream, https://github.com/CanonicalLtd/landscape-client/commit/6aa197e083e08734ab84e77039ce4fd057ec268c |
182 | +Last-Update: 2018-11-23 |
183 | + |
184 | +Index: sru/landscape/lib/vm_info.py |
185 | +=================================================================== |
186 | +--- sru.orig/landscape/lib/vm_info.py 2018-11-23 10:08:28.936161178 -0500 |
187 | ++++ sru/landscape/lib/vm_info.py 2018-11-23 10:08:28.932161181 -0500 |
188 | +@@ -73,6 +73,7 @@ |
189 | + ("google", b"gce"), |
190 | + ("innotek", b"virtualbox"), |
191 | + ("microsoft", b"hyperv"), |
192 | ++ ("nutanix", b"kvm"), |
193 | + ("openstack", b"kvm"), |
194 | + ("qemu", b"kvm"), |
195 | + ("vmware", b"vmware")) |
196 | diff --git a/debian/patches/post-upgrade-reboot.patch b/debian/patches/post-upgrade-reboot.patch |
197 | new file mode 100644 |
198 | index 0000000..f4deedb |
199 | --- /dev/null |
200 | +++ b/debian/patches/post-upgrade-reboot.patch |
201 | @@ -0,0 +1,132 @@ |
202 | +Description: Force reboot operation in case systemd fails. |
203 | +Author: Simon Poirier <simon.poirier@canonical.com> |
204 | +Origin: upstream, https://github.com/CanonicalLtd/landscape-client/commit/e5e086565df841688e435890445104bdb3197c75 |
205 | +Bug-Ubuntu: https://bugs.launchpad.net/bugs/1670291 |
206 | +Last-Update: 2018-11-23 |
207 | + |
208 | +diff --git a/landscape/client/manager/shutdownmanager.py b/landscape/client/manager/shutdownmanager.py |
209 | +index 2f4b6cc1..8c6f7224 100644 |
210 | +--- a/landscape/client/manager/shutdownmanager.py |
211 | ++++ b/landscape/client/manager/shutdownmanager.py |
212 | +@@ -45,7 +45,7 @@ class ShutdownManager(ManagerPlugin): |
213 | + protocol = ShutdownProcessProtocol() |
214 | + protocol.set_timeout(self.registry.reactor) |
215 | + protocol.result.addCallback(self._respond_success, operation_id) |
216 | +- protocol.result.addErrback(self._respond_failure, operation_id) |
217 | ++ protocol.result.addErrback(self._respond_failure, operation_id, reboot) |
218 | + command, args = self._get_command_and_args(protocol, reboot) |
219 | + self._process_factory.spawnProcess(protocol, command, args=args) |
220 | + |
221 | +@@ -58,9 +58,31 @@ class ShutdownManager(ManagerPlugin): |
222 | + lambda _: self.registry.broker.stop_exchanger()) |
223 | + return deferred |
224 | + |
225 | +- def _respond_failure(self, failure, operation_id): |
226 | ++ def _respond_failure(self, failure, operation_id, reboot): |
227 | + logging.info("Shutdown request failed.") |
228 | +- return self._respond(FAILED, failure.value.data, operation_id) |
229 | ++ failure_report = '\n'.join([ |
230 | ++ failure.value.data, |
231 | ++ "", |
232 | ++ "Attempting to force {operation}. Please note that if this " |
233 | ++ "succeeds, Landscape will have no way of knowing and will still " |
234 | ++ "mark this activity as having failed. It is recommended you check " |
235 | ++ "the state of the machine manually to determine whether " |
236 | ++ "{operation} succeeded.".format( |
237 | ++ operation="reboot" if reboot else "shutdown") |
238 | ++ ]) |
239 | ++ deferred = self._respond(FAILED, failure_report, operation_id) |
240 | ++ # Add another callback spawning the poweroff or reboot command (which |
241 | ++ # seem more reliable in aberrant situations like a post-trusty release |
242 | ++ # upgrade where upstart has been replaced with systemd). If this |
243 | ++ # succeeds, we won't have any opportunity to report it and if it fails |
244 | ++ # we'll already have responded indicating we're attempting to force |
245 | ++ # the operation so either way there's no sense capturing output |
246 | ++ protocol = ProcessProtocol() |
247 | ++ command, args = self._get_command_and_args(protocol, reboot, True) |
248 | ++ deferred.addCallback( |
249 | ++ lambda _: self._process_factory.spawnProcess( |
250 | ++ protocol, command, args=args)) |
251 | ++ return deferred |
252 | + |
253 | + def _respond(self, status, data, operation_id): |
254 | + message = {"type": "operation-result", |
255 | +@@ -70,19 +92,23 @@ class ShutdownManager(ManagerPlugin): |
256 | + return self.registry.broker.send_message( |
257 | + message, self._session_id, True) |
258 | + |
259 | +- def _get_command_and_args(self, protocol, reboot): |
260 | ++ def _get_command_and_args(self, protocol, reboot, force=False): |
261 | + """ |
262 | + Returns a C{command, args} 2-tuple suitable for use with |
263 | + L{IReactorProcess.spawnProcess}. |
264 | + """ |
265 | +- minutes = "+%d" % (protocol.delay // 60,) |
266 | +- if reboot: |
267 | +- args = ["/sbin/shutdown", "-r", minutes, |
268 | +- "Landscape is rebooting the system"] |
269 | +- else: |
270 | +- args = ["/sbin/shutdown", "-h", minutes, |
271 | +- "Landscape is shutting down the system"] |
272 | +- return "/sbin/shutdown", args |
273 | ++ minutes = None if force else "+%d" % (protocol.delay // 60,) |
274 | ++ args = { |
275 | ++ (False, False): [ |
276 | ++ "/sbin/shutdown", "-h", minutes, |
277 | ++ "Landscape is shutting down the system"], |
278 | ++ (False, True): [ |
279 | ++ "/sbin/shutdown", "-r", minutes, |
280 | ++ "Landscape is rebooting the system"], |
281 | ++ (True, False): ["/sbin/poweroff"], |
282 | ++ (True, True): ["/sbin/reboot"], |
283 | ++ }[force, reboot] |
284 | ++ return args[0], args |
285 | + |
286 | + |
287 | + class ShutdownProcessProtocol(ProcessProtocol): |
288 | +diff --git a/landscape/client/manager/tests/test_shutdownmanager.py b/landscape/client/manager/tests/test_shutdownmanager.py |
289 | +index 0d31bdff..54ff4f04 100644 |
290 | +--- a/landscape/client/manager/tests/test_shutdownmanager.py |
291 | ++++ b/landscape/client/manager/tests/test_shutdownmanager.py |
292 | +@@ -1,5 +1,6 @@ |
293 | + from twisted.python.failure import Failure |
294 | + from twisted.internet.error import ProcessTerminated, ProcessDone |
295 | ++from twisted.internet.protocol import ProcessProtocol |
296 | + |
297 | + from landscape.lib.testing import StubProcessFactory |
298 | + from landscape.client.manager.plugin import SUCCEEDED, FAILED |
299 | +@@ -76,16 +77,28 @@ class ShutdownManagerTest(LandscapeTest): |
300 | + be failed. Data printed by the process prior to the failure is |
301 | + included in the activity's result text. |
302 | + """ |
303 | +- message = {"type": "shutdown", "reboot": False, "operation-id": 100} |
304 | ++ message = {"type": "shutdown", "reboot": True, "operation-id": 100} |
305 | + self.plugin.perform_shutdown(message) |
306 | + |
307 | + def restart_failed(message_id): |
308 | + self.assertTrue(self.broker_service.exchanger.is_urgent()) |
309 | +- self.assertEqual( |
310 | +- self.broker_service.message_store.get_pending_messages(), |
311 | +- [{"type": "operation-result", "api": b"3.2", |
312 | +- "operation-id": 100, "timestamp": 0, "status": FAILED, |
313 | +- "result-text": u"Failure text is reported."}]) |
314 | ++ messages = self.broker_service.message_store.get_pending_messages() |
315 | ++ self.assertEqual(len(messages), 1) |
316 | ++ message = messages[0] |
317 | ++ self.assertEqual(message["type"], "operation-result") |
318 | ++ self.assertEqual(message["api"], b"3.2") |
319 | ++ self.assertEqual(message["operation-id"], 100) |
320 | ++ self.assertEqual(message["timestamp"], 0) |
321 | ++ self.assertEqual(message["status"], FAILED) |
322 | ++ self.assertIn(u"Failure text is reported.", message["result-text"]) |
323 | ++ |
324 | ++ # Check that after failing, we attempt to force the shutdown by |
325 | ++ # switching the binary called |
326 | ++ [spawn1_args, spawn2_args] = self.process_factory.spawns |
327 | ++ protocol = spawn2_args[0] |
328 | ++ self.assertIsInstance(protocol, ProcessProtocol) |
329 | ++ self.assertEqual(spawn2_args[1:3], |
330 | ++ ("/sbin/reboot", ["/sbin/reboot"])) |
331 | + |
332 | + [arguments] = self.process_factory.spawns |
333 | + protocol = arguments[0] |
334 | diff --git a/debian/patches/release-upgrade-success.patch b/debian/patches/release-upgrade-success.patch |
335 | new file mode 100644 |
336 | index 0000000..7a2ce1f |
337 | --- /dev/null |
338 | +++ b/debian/patches/release-upgrade-success.patch |
339 | @@ -0,0 +1,23 @@ |
340 | +Description: Enable landscape-client to survive trusty upgrade |
341 | + At present, landscape-client fails to report the completion (or failure) |
342 | + of a release upgrade from trusty to xenial due to the delayed import of |
343 | + twisted.internet.unix (part of LP: #1670291) |
344 | +Author: Simon Poirier <simon.poirier@canonical.com> |
345 | +Origin: https://github.com/CanonicalLtd/landscape-client/commit/d2bd25caafd0406ef762c718cac66a7f6ad94596 |
346 | +Bug-Ubuntu: https://bugs.launchpad.net/bugs/1670291 |
347 | +Last-Update: 2018-11-23 |
348 | + |
349 | +Index: sru/landscape/client/package/releaseupgrader.py |
350 | +=================================================================== |
351 | +--- sru.orig/landscape/client/package/releaseupgrader.py 2018-11-23 10:29:20.563300247 -0500 |
352 | ++++ sru/landscape/client/package/releaseupgrader.py 2018-11-23 10:29:20.559300250 -0500 |
353 | +@@ -165,6 +165,9 @@ |
354 | + |
355 | + @param current_code_name: The code-name of the current release. |
356 | + """ |
357 | ++ # This bizarre (and apparently unused) import is a workaround for |
358 | ++ # LP: #1670291 -- see comments in that ticket for an explanation |
359 | ++ import twisted.internet.unix # noqa: F401 |
360 | + upgrade_tool_directory = self._config.upgrade_tool_directory |
361 | + |
362 | + # On some releases the upgrade-tool doesn't support the allow third |
363 | diff --git a/debian/patches/series b/debian/patches/series |
364 | index 1cf42b5..8fe2ee6 100644 |
365 | --- a/debian/patches/series |
366 | +++ b/debian/patches/series |
367 | @@ -1,3 +1,8 @@ |
368 | remove-landscape-link.patch |
369 | detect-cloudstack-kvm-1754073.patch |
370 | apt-sources-replace-1771443.patch |
371 | +nutanix-kvm.patch |
372 | +release-upgrade-success.patch |
373 | +post-upgrade-reboot.patch |
374 | +unicode-tags-script.patch |
375 | +1616116-resync-loop.patch |
376 | diff --git a/debian/patches/unicode-tags-script.patch b/debian/patches/unicode-tags-script.patch |
377 | new file mode 100644 |
378 | index 0000000..7282de7 |
379 | --- /dev/null |
380 | +++ b/debian/patches/unicode-tags-script.patch |
381 | @@ -0,0 +1,167 @@ |
382 | +Description: Permit environments containing unicode chars for script execution |
383 | +Author: Simon Poirier <simon.poirier@canonical.com> |
384 | +Origin: upstream, https://github.com/CanonicalLtd/landscape-client/commit/d5c37413254f32e10f63f83279b92ec5ba377f22 |
385 | +Bug-Ubuntu: https://bugs.launchpad.net/bugs/1765518 |
386 | +Last-Update: 2018-11-23 |
387 | + |
388 | +diff --git a/landscape/client/manager/scriptexecution.py b/landscape/client/manager/scriptexecution.py |
389 | +index 203d31df..4481e17e 100644 |
390 | +--- a/landscape/client/manager/scriptexecution.py |
391 | ++++ b/landscape/client/manager/scriptexecution.py |
392 | +@@ -4,6 +4,7 @@ Functionality for running arbitrary shell scripts. |
393 | + @var ALL_USERS: A token indicating all users should be allowed. |
394 | + """ |
395 | + import os |
396 | ++import sys |
397 | + import os.path |
398 | + import tempfile |
399 | + import shutil |
400 | +@@ -105,6 +106,13 @@ class ScriptRunnerMixin(object): |
401 | + uid = None |
402 | + if gid == os.getgid(): |
403 | + gid = None |
404 | ++ env = { |
405 | ++ key: ( |
406 | ++ value.encode(sys.getfilesystemencoding(), errors='replace') |
407 | ++ if isinstance(value, unicode) else value |
408 | ++ ) |
409 | ++ for key, value in env.items() |
410 | ++ } |
411 | + |
412 | + pp = ProcessAccumulationProtocol( |
413 | + self.registry.reactor, self.size_limit, self.truncation_indicator) |
414 | +@@ -249,7 +257,14 @@ class ScriptExecutionPlugin(ManagerPlugin, ScriptRunnerMixin): |
415 | + self.write_script_file( |
416 | + script_file, filename, shell, code, uid, gid) |
417 | + |
418 | +- env = {"PATH": UBUNTU_PATH, "USER": user or "", "HOME": path or ""} |
419 | ++ env = { |
420 | ++ "PATH": UBUNTU_PATH, |
421 | ++ "USER": user or "", |
422 | ++ "HOME": path or "", |
423 | ++ } |
424 | ++ for locale_var in ("LANG", "LC_ALL", "LC_CTYPE"): |
425 | ++ if locale_var in os.environ: |
426 | ++ env[locale_var] = os.environ[locale_var] |
427 | + if server_supplied_env: |
428 | + env.update(server_supplied_env) |
429 | + old_umask = os.umask(0o022) |
430 | +diff --git a/landscape/client/manager/tests/test_scriptexecution.py b/landscape/client/manager/tests/test_scriptexecution.py |
431 | +index 744ea006..2f219490 100644 |
432 | +--- a/landscape/client/manager/tests/test_scriptexecution.py |
433 | ++++ b/landscape/client/manager/tests/test_scriptexecution.py |
434 | +@@ -25,10 +25,22 @@ from landscape.client.tests.helpers import LandscapeTest, ManagerHelper |
435 | + def get_default_environment(): |
436 | + username = pwd.getpwuid(os.getuid())[0] |
437 | + uid, gid, home = get_user_info(username) |
438 | +- return { |
439 | ++ env = { |
440 | + "PATH": UBUNTU_PATH, |
441 | + "USER": username, |
442 | +- "HOME": home} |
443 | ++ "HOME": home, |
444 | ++ } |
445 | ++ for var in {"LANG", "LC_ALL", "LC_CTYPE"}: |
446 | ++ if var in os.environ: |
447 | ++ env[var] = os.environ[var] |
448 | ++ return env |
449 | ++ |
450 | ++ |
451 | ++def encoded_default_environment(): |
452 | ++ return { |
453 | ++ key: value.encode('ascii', 'replace') |
454 | ++ for key, value in get_default_environment().items() |
455 | ++ } |
456 | + |
457 | + |
458 | + class RunScriptTests(LandscapeTest): |
459 | +@@ -99,6 +111,35 @@ class RunScriptTests(LandscapeTest): |
460 | + result.addCallback(check_environment) |
461 | + return result |
462 | + |
463 | ++ def test_server_supplied_env_with_funky_chars(self): |
464 | ++ """ |
465 | ++ Server-supplied environment variables can be unicode strings; client |
466 | ++ should pass these to the script's environment encoded appropriately |
467 | ++ (encoding from Python's sys.getfilesystemencoding). |
468 | ++ """ |
469 | ++ patch_fs_encoding = mock.patch( |
470 | ++ 'sys.getfilesystemencoding', return_value='UTF-8' |
471 | ++ ) |
472 | ++ patch_fs_encoding.start() |
473 | ++ |
474 | ++ server_supplied_env = { |
475 | ++ "LEMMY": u"Mot\N{LATIN SMALL LETTER O WITH DIAERESIS}rhead", |
476 | ++ # Somehow it's just not as cool... |
477 | ++ } |
478 | ++ result = self.plugin.run_script( |
479 | ++ "/bin/sh", "echo $LEMMY", |
480 | ++ server_supplied_env=server_supplied_env) |
481 | ++ |
482 | ++ def check_environment(results): |
483 | ++ self.assertEqual(server_supplied_env["LEMMY"] + u'\n', results) |
484 | ++ |
485 | ++ def cleanup(result): |
486 | ++ patch_fs_encoding.stop() |
487 | ++ return result |
488 | ++ |
489 | ++ result.addCallback(check_environment).addBoth(cleanup) |
490 | ++ return result |
491 | ++ |
492 | + def test_server_supplied_env_overrides_client(self): |
493 | + """ |
494 | + Server-supplied environment variables override client default |
495 | +@@ -424,7 +465,7 @@ class RunScriptTests(LandscapeTest): |
496 | + self.assertEqual(len(factory.spawns), 1) |
497 | + spawn = factory.spawns[0] |
498 | + self.assertIn("LANDSCAPE_ATTACHMENTS", spawn[3]) |
499 | +- attachment_dir = spawn[3]["LANDSCAPE_ATTACHMENTS"] |
500 | ++ attachment_dir = spawn[3]["LANDSCAPE_ATTACHMENTS"].decode('ascii') |
501 | + self.assertEqual(stat.S_IMODE(os.stat(attachment_dir).st_mode), 0o700) |
502 | + filename = os.path.join(attachment_dir, "file 1") |
503 | + self.assertEqual(stat.S_IMODE(os.stat(filename).st_mode), 0o600) |
504 | +@@ -598,7 +639,7 @@ class RunScriptTests(LandscapeTest): |
505 | + def spawnProcess(protocol, filename, args, env, path, uid, gid): |
506 | + self.assertIsNone(uid) |
507 | + self.assertIsNone(gid) |
508 | +- self.assertEqual(get_default_environment(), env) |
509 | ++ self.assertEqual(encoded_default_environment(), env) |
510 | + protocol.result_deferred.callback(None) |
511 | + |
512 | + process_factory = mock.Mock() |
513 | +@@ -737,7 +778,7 @@ class ScriptExecutionMessageTests(LandscapeTest): |
514 | + self.assertIn("USER", factory.spawns[0][3]) |
515 | + self.assertIn("PATH", factory.spawns[0][3]) |
516 | + self.assertIn("Dog", factory.spawns[0][3]) |
517 | +- self.assertEqual("Woof", factory.spawns[0][3]["Dog"]) |
518 | ++ self.assertEqual(b"Woof", factory.spawns[0][3]["Dog"]) |
519 | + |
520 | + self.assertMessages( |
521 | + self.broker_service.message_store.get_pending_messages(), []) |
522 | +@@ -777,7 +818,7 @@ class ScriptExecutionMessageTests(LandscapeTest): |
523 | + def check(_): |
524 | + process_factory.spawnProcess.assert_called_with( |
525 | + mock.ANY, mock.ANY, args=mock.ANY, uid=None, gid=None, |
526 | +- path=mock.ANY, env=get_default_environment()) |
527 | ++ path=mock.ANY, env=encoded_default_environment()) |
528 | + |
529 | + return result.addCallback(check) |
530 | + |
531 | +@@ -876,7 +917,7 @@ class ScriptExecutionMessageTests(LandscapeTest): |
532 | + "status": SUCCEEDED}]) |
533 | + process_factory.spawnProcess.assert_called_with( |
534 | + mock.ANY, mock.ANY, args=mock.ANY, uid=None, gid=None, |
535 | +- path=mock.ANY, env=get_default_environment()) |
536 | ++ path=mock.ANY, env=encoded_default_environment()) |
537 | + |
538 | + result = self._send_script(sys.executable, "print 'hi'") |
539 | + return result.addCallback(got_result) |
540 | +@@ -907,7 +948,7 @@ class ScriptExecutionMessageTests(LandscapeTest): |
541 | + u"\x7fELF\x01\x01\x01\x00\x00\x00\ufffd\x01") |
542 | + process_factory.spawnProcess.assert_called_with( |
543 | + mock.ANY, mock.ANY, args=mock.ANY, uid=None, gid=None, |
544 | +- path=mock.ANY, env=get_default_environment()) |
545 | ++ path=mock.ANY, env=encoded_default_environment()) |
546 | + |
547 | + result = self._send_script(sys.executable, "print 'hi'") |
548 | + return result.addCallback(got_result) |
Your fixes are changing upstream code directly. You need to create debian/patches/ files for each fix instead, and each patch file needs a DEP3 header. Here is a template of such a header:
Description: <short description, required>
<long description that can span multiple lines, optional>
Author: <name and email of author, optional>
Origin: upstream, <URL to commit>
Bug: <URL to the upstream bug report if any, implies patch has been forwarded, optional>
Bug-Ubuntu: <URL to the vendor bug report if any, optional>
Last-Update: 2018-05-10 <YYYY-MM-DD, last update of the meta-information, optional>
I tweaked it a bit for the landscape-client use case already.
Quick example: /wiki.debian. org/UsingQuilt# Using_. quiltrc_ configuration_ file patches/ <name-of- patch-file- you-want> .patch patches/ series and add the name to the ordered list patch-file> )
- create quiltrc config file per https:/
- patch upstream code as you have
- git diff > debian/
- edit debian/
- git add the patch and the series file
- revert the source code change
- update the dep3 header per above template (you can also run quilt header -e --dep3 <name-of-
- commit
- apply quilt patch: quilt push -a
- refresh patch, so that the flags specified in quiltrc apply: quilt refresh <name>
- remove patch: quilt pop -a
For d/changelog. please try to use this formatting: of-patch> : <short description> (LP: #xxxxxx)
* d/p/<name-
The dep3changelog tool can do the above for you, extracting the information from the dep3 header of the patch. It will also tell you if the dep3 header is correct or not.
If a fix needs multiple entries, then what you did with indenting is correct, using "-" for the leve immediatelly after "*".