Merge ~simpoir/ubuntu/+source/landscape-client:disco-landscape-client-backports-1788219 into ubuntu/+source/landscape-client: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)
Reviewer Review Type Date Requested Status
Andreas Hasenack Approve
Canonical Server Pending
git-ubuntu developers Pending
Review via email: mp+358799@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Andreas Hasenack (ahasenack) wrote :

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:
- create quiltrc config file per https://wiki.debian.org/UsingQuilt#Using_.quiltrc_configuration_file
- patch upstream code as you have
- git diff > debian/patches/<name-of-patch-file-you-want>.patch
- edit debian/patches/series and add the name to the ordered list
- 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-patch-file>)
- 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:
  * d/p/<name-of-patch>: <short description> (LP: #xxxxxx)

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 "*".

review: Needs Fixing
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
1diff --git a/debian/changelog b/debian/changelog
2index 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.
25diff --git a/debian/patches/1616116-resync-loop.patch b/debian/patches/1616116-resync-loop.patch
26new file mode 100644
27index 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):
173diff --git a/debian/patches/nutanix-kvm.patch b/debian/patches/nutanix-kvm.patch
174new file mode 100644
175index 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"))
196diff --git a/debian/patches/post-upgrade-reboot.patch b/debian/patches/post-upgrade-reboot.patch
197new file mode 100644
198index 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]
334diff --git a/debian/patches/release-upgrade-success.patch b/debian/patches/release-upgrade-success.patch
335new file mode 100644
336index 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
363diff --git a/debian/patches/series b/debian/patches/series
364index 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
376diff --git a/debian/patches/unicode-tags-script.patch b/debian/patches/unicode-tags-script.patch
377new file mode 100644
378index 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)

Subscribers

People subscribed via source and target branches