Merge lp:~hazmat/pyjuju/lxc-killpid into lp:pyjuju

Proposed by Kapil Thangavelu
Status: Work in progress
Proposed branch: lp:~hazmat/pyjuju/lxc-killpid
Merge into: lp:pyjuju
Diff against target: 725 lines (+210/-136) (has conflicts)
7 files modified
juju/lib/service.py (+55/-49)
juju/lib/tests/test_service.py (+56/-28)
juju/providers/local/__init__.py (+4/-2)
juju/providers/local/agent.py (+11/-13)
juju/providers/local/files.py (+36/-10)
juju/providers/local/tests/test_agent.py (+24/-15)
juju/providers/local/tests/test_files.py (+24/-19)
Text conflict in juju/providers/local/files.py
Text conflict in juju/providers/local/tests/test_agent.py
Text conflict in juju/providers/local/tests/test_files.py
To merge this branch: bzr merge lp:~hazmat/pyjuju/lxc-killpid
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+125333@code.launchpad.net

Description of the change

clean up lxc process management

Adopted from ben's branch of the same name. Ensure better killing of processes
on destroy env, and additionally expand scope to all services utilized by local
env (ie file-server was still using upstart). Also ensures compatibility with
dev branches since py path was previously being stripped.

https://codereview.appspot.com/6533057/

To post a comment you must log in.
Revision history for this message
Kapil Thangavelu (hazmat) wrote :

Reviewers: mp+125333_code.launchpad.net,

Message:
Please take a look.

Description:
clean up lxc process management

Adopted from ben's branch of the same name. Ensure better killing of
processes
on destroy env, and additionally expand scope to all services utilized
by local
env (ie file-server was still using upstart). Also ensures compatibility
with
dev branches since py path was previously being stripped.

https://code.launchpad.net/~hazmat/juju/lxc-killpid/+merge/125333

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/6533057/

Affected files:
   A [revision details]
   M juju/lib/service.py
   M juju/lib/tests/test_service.py
   M juju/providers/local/__init__.py
   M juju/providers/local/agent.py
   M juju/providers/local/files.py
   M juju/providers/local/tests/test_agent.py
   M juju/providers/local/tests/test_files.py

Revision history for this message
Clint Byrum (clint-fewbar) wrote :

I believe much of this is superseded by lp:~clint-fewbar/juju/local-cloud-img , so I'm setting this back to Work In Progress until that branch is reviewed.

Unmerged revisions

569. By Kapil Thangavelu

pre mp cleanup

568. By Kapil Thangavelu

cleanup commented out block

567. By Kapil Thangavelu

destroy-env passes juju dir to storage server

566. By Kapil Thangavelu

file-server compatibility

565. By Kapil Thangavelu

add some real tests, and fix several broken bits

564. By Benjamin Saller

properly kill daemons, properly pass juju_dir on destroy-environment/container destruction

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'juju/lib/service.py'
2--- juju/lib/service.py 2012-08-03 12:28:29 +0000
3+++ juju/lib/service.py 2012-09-19 20:35:23 +0000
4@@ -1,4 +1,4 @@
5-from twisted.internet.defer import inlineCallbacks
6+from twisted.internet.defer import inlineCallbacks, Deferred
7 from twisted.internet.threads import deferToThread
8
9 from juju.errors import ServiceError
10@@ -9,20 +9,31 @@
11
12 def _check_call(args, env=None, output_path=None):
13 if not output_path:
14- output_path = os.devnull
15-
16- with open(output_path, "a") as f:
17+ f = open(os.devnull, "a")
18+ else:
19+ check_path = os.path.exists(output_path) and output_path \
20+ or os.path.dirname(output_path)
21+ if os.access(check_path, os.W_OK):
22+ f = open(output_path, "a")
23+ else:
24+ raise ValueError(
25+ "Output path inaccessible %s" % output_path)
26+ try:
27 return subprocess.check_call(
28 args,
29 stdout=f.fileno(), stderr=f.fileno(),
30 env=env)
31+ finally:
32+ f.close()
33
34
35 def _cat(filename, use_sudo=False):
36 args = ("cat", filename)
37 if use_sudo and not os.access(filename, os.R_OK):
38 args = ("sudo",) + args
39-
40+ elif os.path.exists(filename):
41+ with open(filename) as fh:
42+ return (0, fh.read())
43 p = subprocess.Popen(
44 args, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
45 stdout_data, _ = p.communicate()
46@@ -37,48 +48,45 @@
47 specifies the location of the pid file that is used to track this service.
48
49 """
50- def __init__(self, name, pidfile, use_sudo=False):
51+ def __init__(self, name, pidfile, output_path=None, use_sudo=False):
52 self._name = name
53 self._use_sudo = use_sudo
54 self._description = None
55 self._environ = None
56 self._command = None
57- self._output_path = None
58-
59+ self._daemon = True
60+ self.output_path = output_path
61 self._pid_path = pidfile
62 self._pid = None
63
64- @property
65- def output_path(self):
66- if self._output_path is not None:
67- return self._output_path
68- return "/tmp/%s.output" % self._name
69-
70- @output_path.setter
71- def output_path(self, path):
72- self._output_path = path
73
74 def set_description(self, description):
75 self._description = description
76
77+ def set_daemon(self, value):
78+ self._daemon = bool(value)
79+
80 def set_environ(self, environ):
81 for k, v in environ.items():
82 environ[k] = str(v)
83 self._environ = environ
84
85+ def set_output_path(self, output_path):
86+ self._output_path = output_path
87+
88 def set_command(self, command):
89- if "--pidfile" not in command:
90- command += ["--pidfile", self._pid_path]
91- else:
92- # pid file is in command (consume it for get_pid)
93- idx = command.index("--pidfile")
94- self._pid_path = command[idx+1]
95-
96+ if self._daemon:
97+ if "--pidfile" not in command:
98+ command.extend(["--pidfile", self._pid_path])
99+ else:
100+ # pid file is in command (consume it for get_pid)
101+ idx = command.index("--pidfile")
102+ self._pid_path = command[idx+1]
103 self._command = command
104
105 @inlineCallbacks
106 def _trash_output(self):
107- if os.path.exists(self.output_path):
108+ if self.output_path and os.path.exists(self.output_path):
109 # Just using os.unlink will fail when we're running TEST_SUDO
110 # tests which hit this code path (because root will own
111 # self.output_path)
112@@ -88,51 +96,52 @@
113 yield self._call("rm", "-f", self._pid_path)
114
115 def _call(self, *args, **kwargs):
116+ # sudo evn with -E will strip pythonpath so pass it to the command.
117 if self._use_sudo:
118- args = ("sudo", "-E") + args
119+ if self._environ:
120+ _args = ["%s=%s" % (k, v) for k, v in self._environ.items()]
121+ else:
122+ _args = []
123+ _args.insert(0, "sudo")
124+ _args.extend(args)
125+ args = _args
126 return deferToThread(_check_call, args, env=self._environ,
127 output_path=self.output_path)
128
129- def install(self):
130- if self._command is None:
131- raise ServiceError("Cannot manage agent: %s no command set" % (
132- self._name))
133-
134 @inlineCallbacks
135 def start(self):
136 if (yield self.is_running()):
137 raise ServiceError(
138 "%s already running: pid (%s)" % (
139 self._name, self.get_pid()))
140-
141- if not self.is_installed():
142- yield self.install()
143-
144 yield self._trash_output()
145 yield self._call(*self._command, output_path=self.output_path)
146+ yield self._sleep(0.1)
147+
148+ def _sleep(self, delay):
149+ """Non-blocking sleep."""
150+ from twisted.internet import reactor
151+ deferred = Deferred()
152+ reactor.callLater(delay, deferred.callback, None)
153+ return deferred
154
155 @inlineCallbacks
156 def destroy(self):
157- if (yield self.is_running()):
158- yield self._call("kill", self.get_pid())
159+ if not (yield self.is_running()):
160+ return
161+ yield self._call("kill", str(self.get_pid()))
162 yield self._trash_output()
163
164 def get_pid(self):
165 if self._pid != None:
166 return self._pid
167-
168- if not os.path.exists(self._pid_path):
169- return None
170 r, data = _cat(self._pid_path, use_sudo=self._use_sudo)
171 if r != 0:
172 return None
173-
174- # verify that pid is a number but leave
175- # it as a string suitable for passing to kill
176- if not data.strip().isdigit():
177+ data = data.strip()
178+ if not data.isdigit():
179 return None
180- pid = data.strip()
181- self._pid = pid
182+ self._pid = int(data)
183 return self._pid
184
185 def is_running(self):
186@@ -143,6 +152,3 @@
187 if not os.path.exists(proc_file):
188 return False
189 return True
190-
191- def is_installed(self):
192- return False
193
194=== modified file 'juju/lib/tests/test_service.py'
195--- juju/lib/tests/test_service.py 2012-08-03 12:28:29 +0000
196+++ juju/lib/tests/test_service.py 2012-09-19 20:35:23 +0000
197@@ -3,31 +3,25 @@
198 from juju.lib.testing import TestCase
199 from juju.lib.mocker import MATCH, KWARGS
200 from juju.lib.service import TwistedDaemonService
201+from juju.lib.lxc.tests.test_lxc import uses_sudo
202+from juju.state.utils import get_open_port
203
204 import os
205-
206+import subprocess
207
208 class TwistedDaemonServiceTest(TestCase):
209
210 @inlineCallbacks
211 def setUp(self):
212 yield super(TwistedDaemonServiceTest, self).setUp()
213- self.setup_service()
214-
215- def setup_service(self):
216- service = TwistedDaemonService("juju-machine-agent",
217- "/tmp/.juju-test.pid",
218- use_sudo=False)
219+ self.pid_path = self.makeFile()
220+ service = TwistedDaemonService(
221+ "juju-machine-agent", self.pid_path, use_sudo=False)
222 service.set_description("Juju machine agent")
223 service.set_environ({"JUJU_MACHINE_ID": 0})
224- service.set_command(["/bin/true", ])
225+ service.set_command(["/bin/true"])
226 self.service = service
227
228- if os.path.exists("/tmp/.juju-test.pid"):
229- os.remove("/tmp/.juju-test.pid")
230-
231- return service
232-
233 def setup_mock(self):
234 self.check_call = self.mocker.replace("subprocess.check_call")
235
236@@ -44,16 +38,13 @@
237 self.setup_mock()
238 self.mock_call("/bin/true")
239 self.mocker.replay()
240-
241 yield self.service.start()
242
243- def test_set_output_path(self):
244- # defaults work
245- self.assertEqual(self.service.output_path,
246- "/tmp/juju-machine-agent.output")
247- # override works
248- self.service.output_path = "/tmp/valid.log"
249- self.assertEqual(self.service.output_path, "/tmp/valid.log")
250+ def test_simple_service_failure(self):
251+ self.service.set_command(["/bin/false"])
252+ return self.assertFailure(
253+ self.service.start(),
254+ subprocess.CalledProcessError)
255
256 @inlineCallbacks
257 def test_simple_service_start_destroy(self):
258@@ -81,20 +72,57 @@
259 def test_webservice_start(self):
260 # test using a real twisted service (with --pidfile)
261 # arg ordering matters here so we set pidfile manually
262+
263+ pid_file = self.makeFile()
264+ log_file = self.makeFile()
265+ web_dir = self.makeDir()
266+
267 self.service.set_command([
268 "env", "twistd",
269- "--pidfile", "/tmp/.juju-test.pid",
270- "--logfile", "/tmp/.juju-test.log",
271+ "--pidfile", pid_file,
272+ "--logfile", log_file,
273 "web",
274- "--port", "9871",
275- "--path", "/lib",
276+ "--port", str(get_open_port()),
277+ "--path", web_dir,
278 ])
279
280 yield self.service.start()
281- yield self.sleep(0.5)
282+ yield self.sleep(0.1)
283+
284 self.assertTrue(self.service.get_pid())
285 self.assertTrue(self.service.is_running())
286- self.assertTrue(os.path.exists("/tmp/.juju-test.pid"))
287+ self.assertTrue(os.path.exists(pid_file))
288 yield self.service.destroy()
289 yield self.sleep(0.1)
290- self.assertFalse(os.path.exists("/tmp/.juju-test.pid"))
291\ No newline at end of file
292+ self.assertFalse(self.service.is_running())
293+
294+ @uses_sudo
295+ @inlineCallbacks
296+ def test_sudo_env_vars(self):
297+ self.service.set_daemon(False)
298+ self.service.set_environ(
299+ {"JUJU_MACHINE_ID": 0, "PYTHONPATH": "foo2"})
300+ self.service.set_command(["/usr/bin/env"])
301+ self.service.output_path = self.makeFile()
302+ yield self.service.start()
303+
304+ with open(self.service.output_path) as fh:
305+ contents = fh.read()
306+ self.assertIn("PYTHONPATH=foo2", contents)
307+ self.assertIn("JUJU_MACHINE_ID=0", contents)
308+
309+
310+ @uses_sudo
311+ @inlineCallbacks
312+ def test_command_tuple(self):
313+ self.service.set_daemon(False)
314+ self.service.output_path = self.makeFile()
315+ self.service.set_environ(
316+ {"JUJU_MACHINE_ID": 0, "PYTHONPATH": "foo2"})
317+ self.service.set_command(("/usr/bin/env",))
318+ yield self.service.start()
319+
320+ with open(self.service.output_path) as fh:
321+ contents = fh.read()
322+ self.assertIn("PYTHONPATH=foo2", contents)
323+ self.assertIn("JUJU_MACHINE_ID=0", contents)
324
325=== modified file 'juju/providers/local/__init__.py'
326--- juju/providers/local/__init__.py 2012-07-20 17:28:17 +0000
327+++ juju/providers/local/__init__.py 2012-09-19 20:35:23 +0000
328@@ -114,6 +114,7 @@
329 log.info("Starting storage server...")
330 storage_server = StorageServer(
331 self._qualified_name,
332+ self._directory,
333 storage_dir=os.path.join(self._directory, "files"),
334 host=net_attributes["ip"]["address"],
335 port=get_open_port(net_attributes["ip"]["address"]),
336@@ -172,12 +173,13 @@
337
338 # Stop the machine agent
339 log.debug("Stopping machine agent...")
340- agent = ManagedMachineAgent(self._qualified_name)
341+ agent = ManagedMachineAgent(self._qualified_name,
342+ juju_directory=self._directory)
343 yield agent.stop()
344
345 # Stop the storage server
346 log.debug("Stopping storage server...")
347- storage_server = StorageServer(self._qualified_name)
348+ storage_server = StorageServer(self._qualified_name, self._directory)
349 yield storage_server.stop()
350
351 # Stop zookeeper
352
353=== modified file 'juju/providers/local/agent.py'
354--- juju/providers/local/agent.py 2012-08-03 10:55:21 +0000
355+++ juju/providers/local/agent.py 2012-09-19 20:35:23 +0000
356@@ -1,18 +1,13 @@
357+import os
358 import sys
359-import tempfile
360-
361+
362+from twisted.internet.defer import inlineCallbacks
363+
364+from juju.errors import ServiceError
365 from juju.lib.service import TwistedDaemonService
366 from juju.providers.common.cloudinit import get_default_origin, BRANCH
367
368
369-def get_temp_filename(basename):
370- if basename:
371- basename += "-"
372-
373- fd, fn = tempfile.mkstemp(prefix=basename, suffix=".pid")
374- return fn
375-
376-
377 class ManagedMachineAgent(object):
378
379 agent_module = "juju.agents.machine"
380@@ -51,15 +46,18 @@
381 if public_key:
382 env["JUJU_PUBLIC_KEY"] = public_key
383
384- pidfile = get_temp_filename(juju_unit_namespace)
385+ pidfile = os.path.join(juju_directory,
386+ "%s-machine-agent.pid" % (
387+ juju_unit_namespace))
388+
389 self._service = TwistedDaemonService(
390 "juju-%s-machine-agent" % juju_unit_namespace,
391- pidfile,
392- use_sudo=True)
393+ pidfile, use_sudo=True)
394 self._service.set_description(
395 "Juju machine agent for %s" % juju_unit_namespace)
396 self._service.set_environ(env)
397 self._service.output_path = log_file
398+ self._logfile = log_file
399
400 self._service_args = [
401 "/usr/bin/python", "-m", self.agent_module,
402
403=== modified file 'juju/providers/local/files.py'
404--- juju/providers/local/files.py 2012-09-10 03:20:20 +0000
405+++ juju/providers/local/files.py 2012-09-19 20:35:23 +0000
406@@ -5,9 +5,14 @@
407 from twisted.internet.error import ConnectionRefusedError
408 from twisted.web.client import getPage
409
410+<<<<<<< TREE
411 from juju.errors import ProviderError, FileNotFound
412 from juju.lib import serializer
413 from juju.lib.upstart import UpstartService
414+=======
415+from juju.errors import ProviderError, FileNotFound, ServiceError
416+from juju.lib.service import TwistedDaemonService
417+>>>>>>> MERGE-SOURCE
418 from juju.providers.common.files import FileStorage
419
420
421@@ -16,8 +21,8 @@
422
423 class StorageServer(object):
424
425- def __init__(self, juju_unit_namespace, storage_dir=None,
426- host=None, port=None, logfile=None):
427+ def __init__(self, juju_unit_namespace, juju_directory,
428+ storage_dir=None, host=None, port=None, logfile=None):
429 """Management facade for a web server on top of the provider storage.
430
431 :param juju_unit_namespace: For disambiguation.
432@@ -32,17 +37,21 @@
433 self._port = port
434 self._logfile = logfile
435
436- self._service = UpstartService(
437- "juju-%s-file-storage" % juju_unit_namespace, use_sudo=True)
438+ self._pid_file = os.path.join(
439+ juju_directory, "%s-file-server.pid" % juju_unit_namespace)
440+
441+ self._service = TwistedDaemonService(
442+ "juju-%s-file-storage" % juju_unit_namespace,
443+ self._pid_file, use_sudo=False)
444+ self._service.output_path = '/tmp/files.output'
445 self._service.set_description(
446 "Juju file storage for %s" % juju_unit_namespace)
447 self._service_args = [
448 "twistd",
449- "--nodaemon",
450 "--uid", str(os.getuid()),
451 "--gid", str(os.getgid()),
452+ "--pidfile", self._pid_file,
453 "--logfile", logfile,
454- "--pidfile=",
455 "-d", self._storage_dir,
456 "web",
457 "--port", "tcp:%s:interface=%s" % (self._port, self._host),
458@@ -74,23 +83,40 @@
459 except IOError:
460 raise AssertionError("logfile not writable by this user")
461
462-
463 storage = LocalStorage(self._storage_dir)
464 yield storage.put(
465 SERVER_URL_KEY,
466 StringIO(serializer.dump(
467 {"storage-url": "http://%s:%s/" % (self._host, self._port)})))
468
469- self._service.set_command(" ".join(self._service_args))
470+ self._service.set_command(self._service_args)
471 yield self._service.start()
472
473- def get_pid(self):
474- return self._service.get_pid()
475+ # Capture the error for the user.
476+ if not self._service.is_running():
477+ content = self._capture_error()
478+ raise ServiceError(
479+ "Failed to start file-storage server: got output:\n"
480+ "%s" % content)
481+
482+ def _capture_error(self):
483+ if os.path.exists(self._logfile):
484+ with open(self._logfile) as fh:
485+ content = fh.read()
486+ else:
487+ content = ""
488+ return content
489
490 def stop(self):
491 """Stop the storage server."""
492 return self._service.destroy()
493
494+ # Passthrough testing support
495+ def is_running(self):
496+ return self._service.is_running()
497+
498+ def get_pid(self):
499+ return self._service.get_pid()
500
501 class LocalStorage(FileStorage):
502
503
504=== modified file 'juju/providers/local/tests/test_agent.py'
505--- juju/providers/local/tests/test_agent.py 2012-09-10 03:20:20 +0000
506+++ juju/providers/local/tests/test_agent.py 2012-09-19 20:35:23 +0000
507@@ -1,9 +1,16 @@
508 import os
509 import tempfile
510 import subprocess
511-
512-from twisted.internet.defer import inlineCallbacks
513-
514+<<<<<<< TREE
515+
516+from twisted.internet.defer import inlineCallbacks
517+
518+=======
519+
520+from twisted.internet.defer import inlineCallbacks
521+
522+from juju.errors import ServiceError
523+>>>>>>> MERGE-SOURCE
524 from juju.lib.lxc.tests.test_lxc import uses_sudo
525 from juju.lib.testing import TestCase
526 from juju.tests.common import get_test_zookeeper_address
527@@ -17,8 +24,9 @@
528 subprocess_calls = []
529
530 def intercept_args(args, **kwargs):
531+ #print "intercept", args, kwargs
532 self.assertEquals(args[0], "sudo")
533- self.assertEquals(args[1], "-E")
534+ #self.assertEquals(args[1], "-E")
535 if args[2] == "rm":
536 return 0
537 subprocess_calls.append(args)
538@@ -35,28 +43,26 @@
539 juju_directory=juju_directory,
540 log_file=log_file,
541 juju_origin="lp:juju/trunk")
542-
543- try:
544- os.remove(agent._service.output_path)
545- except OSError:
546- pass # just make sure it's not there, so the .start()
547- # doesn't insert a spurious rm
548 yield agent.start()
549
550 start = subprocess_calls[0]
551- self.assertEquals(start, (
552- "sudo", "-E", "/usr/bin/python", "-m", "juju.agents.machine",
553+ self.assertEquals(start[start.index("/usr/bin/python"):], [
554+ "/usr/bin/python", "-m", "juju.agents.machine",
555 "--logfile", log_file,
556 "--zookeeper-servers", get_test_zookeeper_address(),
557 "--juju-directory", juju_directory,
558 "--machine-id", "0",
559 "--session-file", "/var/run/juju/ns1-machine-agent.zksession",
560- "--pidfile", agent._service._pid_path))
561+ "--pidfile", agent._service._pid_path])
562
563 @uses_sudo
564 @inlineCallbacks
565 def test_managed_agent_root(self):
566 juju_directory = self.makeDir()
567+ os.mkdir(os.path.join(juju_directory, "charms"))
568+ os.mkdir(os.path.join(juju_directory, "state"))
569+ os.mkdir(os.path.join(juju_directory, "units"))
570+
571 log_file = tempfile.mktemp()
572
573 # The pid file and log file get written as root
574@@ -64,6 +70,7 @@
575 subprocess.check_call(
576 ["sudo", "rm", "-f", cleanup_file], stderr=subprocess.STDOUT)
577 self.addCleanup(cleanup_root_file, log_file)
578+ #self.addCleanup(cleanup_root_file, juju_directory)
579
580 agent = ManagedMachineAgent(
581 "test-ns", machine_id="0",
582@@ -75,13 +82,15 @@
583 agent.agent_module = "juju.agents.dummy"
584 self.assertFalse((yield agent.is_running()))
585 yield agent.start()
586+
587 # Give a moment for the process to start and write its config
588 yield self.sleep(0.1)
589 self.assertTrue((yield agent.is_running()))
590
591- # running start again is fine, detects the process is running
592- yield agent.start()
593+ # running start again raises an error, detects the process is running
594+ self.assertFailure(agent.start(), ServiceError)
595 yield agent.stop()
596+ yield self.sleep(0.1)
597 self.assertFalse((yield agent.is_running()))
598
599 # running stop again is fine, detects the process is stopped.
600
601=== modified file 'juju/providers/local/tests/test_files.py'
602--- juju/providers/local/tests/test_files.py 2012-09-10 03:20:20 +0000
603+++ juju/providers/local/tests/test_files.py 2012-09-19 20:35:23 +0000
604@@ -7,8 +7,11 @@
605 from twisted.web.client import getPage
606
607 from juju.errors import ProviderError, ServiceError
608+<<<<<<< TREE
609 from juju.lib import serializer
610 from juju.lib.lxc.tests.test_lxc import uses_sudo
611+=======
612+>>>>>>> MERGE-SOURCE
613 from juju.lib.testing import TestCase
614 from juju.lib.upstart import UpstartService
615 from juju.providers.local.files import (
616@@ -21,12 +24,15 @@
617 @inlineCallbacks
618 def setUp(self):
619 yield super(WebFileStorageTest, self).setUp()
620- self._storage_path = self.makeDir()
621+ self._juju_dir = self.makeDir()
622+ self._storage_path = os.path.join(self._juju_dir, "files")
623+ os.mkdir(self._storage_path)
624 self._logfile = self.makeFile()
625 self._storage = LocalStorage(self._storage_path)
626 self._port = get_open_port()
627 self._server = StorageServer(
628- "ns1", self._storage_path, "localhost", self._port, self._logfile)
629+ "ns1", self._juju_dir,
630+ self._storage_path, "localhost", self._port, self._logfile)
631
632 @inlineCallbacks
633 def wait_for_server(self, server):
634@@ -34,7 +40,7 @@
635 yield self.sleep(0.1)
636
637 def test_start_missing_args(self):
638- server = StorageServer("ns1", self._storage_path)
639+ server = StorageServer("ns1", self._juju_dir, self._storage_path)
640 return self.assertFailure(server.start(), AssertionError)
641
642 def test_start_invalid_directory(self):
643@@ -42,7 +48,7 @@
644 return self.assertFailure(self._server.start(), AssertionError)
645
646 @inlineCallbacks
647- def test_upstart(self):
648+ def xtest_upstart(self):
649 subprocess_calls = []
650
651 def intercept_args(args, **kwargs):
652@@ -96,13 +102,15 @@
653 self._port, self._storage_path))
654 self.assertEquals(exec_, expect_exec)
655
656- @uses_sudo
657 @inlineCallbacks
658 def test_start_stop(self):
659 yield self._storage.put("abc", StringIO("hello world"))
660+
661 yield self._server.start()
662+
663 # Starting multiple times is fine.
664- yield self._server.start()
665+ yield self.assertFailure(self._server.start(), ServiceError)
666+
667 storage_url = yield self._storage.get_url("abc")
668
669 # It might not have started actually accepting connections yet...
670@@ -112,28 +120,26 @@
671 # Check that it can be killed by the current user (ie, is not running
672 # as root) and still comes back up
673 old_pid = yield self._server.get_pid()
674+
675 os.kill(old_pid, signal.SIGKILL)
676- new_pid = yield self._server.get_pid()
677- self.assertNotEquals(old_pid, new_pid)
678-
679- # Give it a moment to actually start serving again
680- yield self.wait_for_server(self._server)
681- self.assertEqual((yield getPage(storage_url)), "hello world")
682+ yield self.sleep(0.1)
683+ self.assertFalse(self._server.is_running())
684
685 yield self._server.stop()
686 # Stopping multiple times is fine too.
687 yield self._server.stop()
688
689- @uses_sudo
690 @inlineCallbacks
691 def test_namespacing(self):
692+ alt_juju_dir = self.makeDir()
693 alt_storage_path = self.makeDir()
694 alt_storage = LocalStorage(alt_storage_path)
695 yield alt_storage.put("some-path", StringIO("alternative"))
696 yield self._storage.put("some-path", StringIO("original"))
697
698 alt_server = StorageServer(
699- "ns2", alt_storage_path, "localhost", get_open_port(),
700+ "ns2", alt_juju_dir,
701+ alt_storage_path, "localhost", get_open_port(),
702 self.makeFile())
703 yield alt_server.start()
704 yield self._server.start()
705@@ -150,18 +156,17 @@
706 yield alt_server.stop()
707 yield self._server.stop()
708
709- @uses_sudo
710 @inlineCallbacks
711 def test_capture_errors(self):
712 self._port = get_open_port()
713 self._server = StorageServer(
714- "borken", self._storage_path, "lol borken", self._port,
715- self._logfile)
716+ "borken", self._juju_dir, self._storage_path,
717+ "lol borken", self._port, self._logfile)
718+
719 d = self._server.start()
720 e = yield self.assertFailure(d, ServiceError)
721 self.assertTrue(str(e).startswith(
722- "Failed to start job juju-borken-file-storage; got output:\n"))
723- self.assertIn("Wrong number of arguments", str(e))
724+ "Failed to start file-storage server: got output:\n"))
725 yield self._server.stop()
726
727

Subscribers

People subscribed via source and target branches

to status/vote changes: