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
=== modified file 'juju/lib/service.py'
--- juju/lib/service.py 2012-08-03 12:28:29 +0000
+++ juju/lib/service.py 2012-09-19 20:35:23 +0000
@@ -1,4 +1,4 @@
1from twisted.internet.defer import inlineCallbacks1from twisted.internet.defer import inlineCallbacks, Deferred
2from twisted.internet.threads import deferToThread2from twisted.internet.threads import deferToThread
33
4from juju.errors import ServiceError4from juju.errors import ServiceError
@@ -9,20 +9,31 @@
99
10def _check_call(args, env=None, output_path=None):10def _check_call(args, env=None, output_path=None):
11 if not output_path:11 if not output_path:
12 output_path = os.devnull12 f = open(os.devnull, "a")
1313 else:
14 with open(output_path, "a") as f:14 check_path = os.path.exists(output_path) and output_path \
15 or os.path.dirname(output_path)
16 if os.access(check_path, os.W_OK):
17 f = open(output_path, "a")
18 else:
19 raise ValueError(
20 "Output path inaccessible %s" % output_path)
21 try:
15 return subprocess.check_call(22 return subprocess.check_call(
16 args,23 args,
17 stdout=f.fileno(), stderr=f.fileno(),24 stdout=f.fileno(), stderr=f.fileno(),
18 env=env)25 env=env)
26 finally:
27 f.close()
1928
2029
21def _cat(filename, use_sudo=False):30def _cat(filename, use_sudo=False):
22 args = ("cat", filename)31 args = ("cat", filename)
23 if use_sudo and not os.access(filename, os.R_OK):32 if use_sudo and not os.access(filename, os.R_OK):
24 args = ("sudo",) + args33 args = ("sudo",) + args
2534 elif os.path.exists(filename):
35 with open(filename) as fh:
36 return (0, fh.read())
26 p = subprocess.Popen(37 p = subprocess.Popen(
27 args, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)38 args, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
28 stdout_data, _ = p.communicate()39 stdout_data, _ = p.communicate()
@@ -37,48 +48,45 @@
37 specifies the location of the pid file that is used to track this service.48 specifies the location of the pid file that is used to track this service.
3849
39 """50 """
40 def __init__(self, name, pidfile, use_sudo=False):51 def __init__(self, name, pidfile, output_path=None, use_sudo=False):
41 self._name = name52 self._name = name
42 self._use_sudo = use_sudo53 self._use_sudo = use_sudo
43 self._description = None54 self._description = None
44 self._environ = None55 self._environ = None
45 self._command = None56 self._command = None
46 self._output_path = None57 self._daemon = True
4758 self.output_path = output_path
48 self._pid_path = pidfile59 self._pid_path = pidfile
49 self._pid = None60 self._pid = None
5061
51 @property
52 def output_path(self):
53 if self._output_path is not None:
54 return self._output_path
55 return "/tmp/%s.output" % self._name
56
57 @output_path.setter
58 def output_path(self, path):
59 self._output_path = path
6062
61 def set_description(self, description):63 def set_description(self, description):
62 self._description = description64 self._description = description
6365
66 def set_daemon(self, value):
67 self._daemon = bool(value)
68
64 def set_environ(self, environ):69 def set_environ(self, environ):
65 for k, v in environ.items():70 for k, v in environ.items():
66 environ[k] = str(v)71 environ[k] = str(v)
67 self._environ = environ72 self._environ = environ
6873
74 def set_output_path(self, output_path):
75 self._output_path = output_path
76
69 def set_command(self, command):77 def set_command(self, command):
70 if "--pidfile" not in command:78 if self._daemon:
71 command += ["--pidfile", self._pid_path]79 if "--pidfile" not in command:
72 else:80 command.extend(["--pidfile", self._pid_path])
73 # pid file is in command (consume it for get_pid)81 else:
74 idx = command.index("--pidfile")82 # pid file is in command (consume it for get_pid)
75 self._pid_path = command[idx+1]83 idx = command.index("--pidfile")
7684 self._pid_path = command[idx+1]
77 self._command = command85 self._command = command
7886
79 @inlineCallbacks87 @inlineCallbacks
80 def _trash_output(self):88 def _trash_output(self):
81 if os.path.exists(self.output_path):89 if self.output_path and os.path.exists(self.output_path):
82 # Just using os.unlink will fail when we're running TEST_SUDO90 # Just using os.unlink will fail when we're running TEST_SUDO
83 # tests which hit this code path (because root will own91 # tests which hit this code path (because root will own
84 # self.output_path)92 # self.output_path)
@@ -88,51 +96,52 @@
88 yield self._call("rm", "-f", self._pid_path)96 yield self._call("rm", "-f", self._pid_path)
8997
90 def _call(self, *args, **kwargs):98 def _call(self, *args, **kwargs):
99 # sudo evn with -E will strip pythonpath so pass it to the command.
91 if self._use_sudo:100 if self._use_sudo:
92 args = ("sudo", "-E") + args101 if self._environ:
102 _args = ["%s=%s" % (k, v) for k, v in self._environ.items()]
103 else:
104 _args = []
105 _args.insert(0, "sudo")
106 _args.extend(args)
107 args = _args
93 return deferToThread(_check_call, args, env=self._environ,108 return deferToThread(_check_call, args, env=self._environ,
94 output_path=self.output_path)109 output_path=self.output_path)
95110
96 def install(self):
97 if self._command is None:
98 raise ServiceError("Cannot manage agent: %s no command set" % (
99 self._name))
100
101 @inlineCallbacks111 @inlineCallbacks
102 def start(self):112 def start(self):
103 if (yield self.is_running()):113 if (yield self.is_running()):
104 raise ServiceError(114 raise ServiceError(
105 "%s already running: pid (%s)" % (115 "%s already running: pid (%s)" % (
106 self._name, self.get_pid()))116 self._name, self.get_pid()))
107
108 if not self.is_installed():
109 yield self.install()
110
111 yield self._trash_output()117 yield self._trash_output()
112 yield self._call(*self._command, output_path=self.output_path)118 yield self._call(*self._command, output_path=self.output_path)
119 yield self._sleep(0.1)
120
121 def _sleep(self, delay):
122 """Non-blocking sleep."""
123 from twisted.internet import reactor
124 deferred = Deferred()
125 reactor.callLater(delay, deferred.callback, None)
126 return deferred
113127
114 @inlineCallbacks128 @inlineCallbacks
115 def destroy(self):129 def destroy(self):
116 if (yield self.is_running()):130 if not (yield self.is_running()):
117 yield self._call("kill", self.get_pid())131 return
132 yield self._call("kill", str(self.get_pid()))
118 yield self._trash_output()133 yield self._trash_output()
119134
120 def get_pid(self):135 def get_pid(self):
121 if self._pid != None:136 if self._pid != None:
122 return self._pid137 return self._pid
123
124 if not os.path.exists(self._pid_path):
125 return None
126 r, data = _cat(self._pid_path, use_sudo=self._use_sudo)138 r, data = _cat(self._pid_path, use_sudo=self._use_sudo)
127 if r != 0:139 if r != 0:
128 return None140 return None
129141 data = data.strip()
130 # verify that pid is a number but leave142 if not data.isdigit():
131 # it as a string suitable for passing to kill
132 if not data.strip().isdigit():
133 return None143 return None
134 pid = data.strip()144 self._pid = int(data)
135 self._pid = pid
136 return self._pid145 return self._pid
137146
138 def is_running(self):147 def is_running(self):
@@ -143,6 +152,3 @@
143 if not os.path.exists(proc_file):152 if not os.path.exists(proc_file):
144 return False153 return False
145 return True154 return True
146
147 def is_installed(self):
148 return False
149155
=== modified file 'juju/lib/tests/test_service.py'
--- juju/lib/tests/test_service.py 2012-08-03 12:28:29 +0000
+++ juju/lib/tests/test_service.py 2012-09-19 20:35:23 +0000
@@ -3,31 +3,25 @@
3from juju.lib.testing import TestCase3from juju.lib.testing import TestCase
4from juju.lib.mocker import MATCH, KWARGS4from juju.lib.mocker import MATCH, KWARGS
5from juju.lib.service import TwistedDaemonService5from juju.lib.service import TwistedDaemonService
6from juju.lib.lxc.tests.test_lxc import uses_sudo
7from juju.state.utils import get_open_port
68
7import os9import os
810import subprocess
911
10class TwistedDaemonServiceTest(TestCase):12class TwistedDaemonServiceTest(TestCase):
1113
12 @inlineCallbacks14 @inlineCallbacks
13 def setUp(self):15 def setUp(self):
14 yield super(TwistedDaemonServiceTest, self).setUp()16 yield super(TwistedDaemonServiceTest, self).setUp()
15 self.setup_service()17 self.pid_path = self.makeFile()
1618 service = TwistedDaemonService(
17 def setup_service(self):19 "juju-machine-agent", self.pid_path, use_sudo=False)
18 service = TwistedDaemonService("juju-machine-agent",
19 "/tmp/.juju-test.pid",
20 use_sudo=False)
21 service.set_description("Juju machine agent")20 service.set_description("Juju machine agent")
22 service.set_environ({"JUJU_MACHINE_ID": 0})21 service.set_environ({"JUJU_MACHINE_ID": 0})
23 service.set_command(["/bin/true", ])22 service.set_command(["/bin/true"])
24 self.service = service23 self.service = service
2524
26 if os.path.exists("/tmp/.juju-test.pid"):
27 os.remove("/tmp/.juju-test.pid")
28
29 return service
30
31 def setup_mock(self):25 def setup_mock(self):
32 self.check_call = self.mocker.replace("subprocess.check_call")26 self.check_call = self.mocker.replace("subprocess.check_call")
3327
@@ -44,16 +38,13 @@
44 self.setup_mock()38 self.setup_mock()
45 self.mock_call("/bin/true")39 self.mock_call("/bin/true")
46 self.mocker.replay()40 self.mocker.replay()
47
48 yield self.service.start()41 yield self.service.start()
4942
50 def test_set_output_path(self):43 def test_simple_service_failure(self):
51 # defaults work44 self.service.set_command(["/bin/false"])
52 self.assertEqual(self.service.output_path,45 return self.assertFailure(
53 "/tmp/juju-machine-agent.output")46 self.service.start(),
54 # override works47 subprocess.CalledProcessError)
55 self.service.output_path = "/tmp/valid.log"
56 self.assertEqual(self.service.output_path, "/tmp/valid.log")
5748
58 @inlineCallbacks49 @inlineCallbacks
59 def test_simple_service_start_destroy(self):50 def test_simple_service_start_destroy(self):
@@ -81,20 +72,57 @@
81 def test_webservice_start(self):72 def test_webservice_start(self):
82 # test using a real twisted service (with --pidfile)73 # test using a real twisted service (with --pidfile)
83 # arg ordering matters here so we set pidfile manually74 # arg ordering matters here so we set pidfile manually
75
76 pid_file = self.makeFile()
77 log_file = self.makeFile()
78 web_dir = self.makeDir()
79
84 self.service.set_command([80 self.service.set_command([
85 "env", "twistd",81 "env", "twistd",
86 "--pidfile", "/tmp/.juju-test.pid",82 "--pidfile", pid_file,
87 "--logfile", "/tmp/.juju-test.log",83 "--logfile", log_file,
88 "web",84 "web",
89 "--port", "9871",85 "--port", str(get_open_port()),
90 "--path", "/lib",86 "--path", web_dir,
91 ])87 ])
9288
93 yield self.service.start()89 yield self.service.start()
94 yield self.sleep(0.5)90 yield self.sleep(0.1)
91
95 self.assertTrue(self.service.get_pid())92 self.assertTrue(self.service.get_pid())
96 self.assertTrue(self.service.is_running())93 self.assertTrue(self.service.is_running())
97 self.assertTrue(os.path.exists("/tmp/.juju-test.pid"))94 self.assertTrue(os.path.exists(pid_file))
98 yield self.service.destroy()95 yield self.service.destroy()
99 yield self.sleep(0.1)96 yield self.sleep(0.1)
100 self.assertFalse(os.path.exists("/tmp/.juju-test.pid"))
101\ No newline at end of file97\ No newline at end of file
98 self.assertFalse(self.service.is_running())
99
100 @uses_sudo
101 @inlineCallbacks
102 def test_sudo_env_vars(self):
103 self.service.set_daemon(False)
104 self.service.set_environ(
105 {"JUJU_MACHINE_ID": 0, "PYTHONPATH": "foo2"})
106 self.service.set_command(["/usr/bin/env"])
107 self.service.output_path = self.makeFile()
108 yield self.service.start()
109
110 with open(self.service.output_path) as fh:
111 contents = fh.read()
112 self.assertIn("PYTHONPATH=foo2", contents)
113 self.assertIn("JUJU_MACHINE_ID=0", contents)
114
115
116 @uses_sudo
117 @inlineCallbacks
118 def test_command_tuple(self):
119 self.service.set_daemon(False)
120 self.service.output_path = self.makeFile()
121 self.service.set_environ(
122 {"JUJU_MACHINE_ID": 0, "PYTHONPATH": "foo2"})
123 self.service.set_command(("/usr/bin/env",))
124 yield self.service.start()
125
126 with open(self.service.output_path) as fh:
127 contents = fh.read()
128 self.assertIn("PYTHONPATH=foo2", contents)
129 self.assertIn("JUJU_MACHINE_ID=0", contents)
102130
=== modified file 'juju/providers/local/__init__.py'
--- juju/providers/local/__init__.py 2012-07-20 17:28:17 +0000
+++ juju/providers/local/__init__.py 2012-09-19 20:35:23 +0000
@@ -114,6 +114,7 @@
114 log.info("Starting storage server...")114 log.info("Starting storage server...")
115 storage_server = StorageServer(115 storage_server = StorageServer(
116 self._qualified_name,116 self._qualified_name,
117 self._directory,
117 storage_dir=os.path.join(self._directory, "files"),118 storage_dir=os.path.join(self._directory, "files"),
118 host=net_attributes["ip"]["address"],119 host=net_attributes["ip"]["address"],
119 port=get_open_port(net_attributes["ip"]["address"]),120 port=get_open_port(net_attributes["ip"]["address"]),
@@ -172,12 +173,13 @@
172173
173 # Stop the machine agent174 # Stop the machine agent
174 log.debug("Stopping machine agent...")175 log.debug("Stopping machine agent...")
175 agent = ManagedMachineAgent(self._qualified_name)176 agent = ManagedMachineAgent(self._qualified_name,
177 juju_directory=self._directory)
176 yield agent.stop()178 yield agent.stop()
177179
178 # Stop the storage server180 # Stop the storage server
179 log.debug("Stopping storage server...")181 log.debug("Stopping storage server...")
180 storage_server = StorageServer(self._qualified_name)182 storage_server = StorageServer(self._qualified_name, self._directory)
181 yield storage_server.stop()183 yield storage_server.stop()
182184
183 # Stop zookeeper185 # Stop zookeeper
184186
=== modified file 'juju/providers/local/agent.py'
--- juju/providers/local/agent.py 2012-08-03 10:55:21 +0000
+++ juju/providers/local/agent.py 2012-09-19 20:35:23 +0000
@@ -1,18 +1,13 @@
1import os
1import sys2import sys
2import tempfile3
34from twisted.internet.defer import inlineCallbacks
5
6from juju.errors import ServiceError
4from juju.lib.service import TwistedDaemonService7from juju.lib.service import TwistedDaemonService
5from juju.providers.common.cloudinit import get_default_origin, BRANCH8from juju.providers.common.cloudinit import get_default_origin, BRANCH
69
710
8def get_temp_filename(basename):
9 if basename:
10 basename += "-"
11
12 fd, fn = tempfile.mkstemp(prefix=basename, suffix=".pid")
13 return fn
14
15
16class ManagedMachineAgent(object):11class ManagedMachineAgent(object):
1712
18 agent_module = "juju.agents.machine"13 agent_module = "juju.agents.machine"
@@ -51,15 +46,18 @@
51 if public_key:46 if public_key:
52 env["JUJU_PUBLIC_KEY"] = public_key47 env["JUJU_PUBLIC_KEY"] = public_key
5348
54 pidfile = get_temp_filename(juju_unit_namespace)49 pidfile = os.path.join(juju_directory,
50 "%s-machine-agent.pid" % (
51 juju_unit_namespace))
52
55 self._service = TwistedDaemonService(53 self._service = TwistedDaemonService(
56 "juju-%s-machine-agent" % juju_unit_namespace,54 "juju-%s-machine-agent" % juju_unit_namespace,
57 pidfile,55 pidfile, use_sudo=True)
58 use_sudo=True)
59 self._service.set_description(56 self._service.set_description(
60 "Juju machine agent for %s" % juju_unit_namespace)57 "Juju machine agent for %s" % juju_unit_namespace)
61 self._service.set_environ(env)58 self._service.set_environ(env)
62 self._service.output_path = log_file59 self._service.output_path = log_file
60 self._logfile = log_file
6361
64 self._service_args = [62 self._service_args = [
65 "/usr/bin/python", "-m", self.agent_module,63 "/usr/bin/python", "-m", self.agent_module,
6664
=== modified file 'juju/providers/local/files.py'
--- juju/providers/local/files.py 2012-09-10 03:20:20 +0000
+++ juju/providers/local/files.py 2012-09-19 20:35:23 +0000
@@ -5,9 +5,14 @@
5from twisted.internet.error import ConnectionRefusedError5from twisted.internet.error import ConnectionRefusedError
6from twisted.web.client import getPage6from twisted.web.client import getPage
77
8<<<<<<< TREE
8from juju.errors import ProviderError, FileNotFound9from juju.errors import ProviderError, FileNotFound
9from juju.lib import serializer10from juju.lib import serializer
10from juju.lib.upstart import UpstartService11from juju.lib.upstart import UpstartService
12=======
13from juju.errors import ProviderError, FileNotFound, ServiceError
14from juju.lib.service import TwistedDaemonService
15>>>>>>> MERGE-SOURCE
11from juju.providers.common.files import FileStorage16from juju.providers.common.files import FileStorage
1217
1318
@@ -16,8 +21,8 @@
1621
17class StorageServer(object):22class StorageServer(object):
1823
19 def __init__(self, juju_unit_namespace, storage_dir=None,24 def __init__(self, juju_unit_namespace, juju_directory,
20 host=None, port=None, logfile=None):25 storage_dir=None, host=None, port=None, logfile=None):
21 """Management facade for a web server on top of the provider storage.26 """Management facade for a web server on top of the provider storage.
2227
23 :param juju_unit_namespace: For disambiguation.28 :param juju_unit_namespace: For disambiguation.
@@ -32,17 +37,21 @@
32 self._port = port37 self._port = port
33 self._logfile = logfile38 self._logfile = logfile
3439
35 self._service = UpstartService(40 self._pid_file = os.path.join(
36 "juju-%s-file-storage" % juju_unit_namespace, use_sudo=True)41 juju_directory, "%s-file-server.pid" % juju_unit_namespace)
42
43 self._service = TwistedDaemonService(
44 "juju-%s-file-storage" % juju_unit_namespace,
45 self._pid_file, use_sudo=False)
46 self._service.output_path = '/tmp/files.output'
37 self._service.set_description(47 self._service.set_description(
38 "Juju file storage for %s" % juju_unit_namespace)48 "Juju file storage for %s" % juju_unit_namespace)
39 self._service_args = [49 self._service_args = [
40 "twistd",50 "twistd",
41 "--nodaemon",
42 "--uid", str(os.getuid()),51 "--uid", str(os.getuid()),
43 "--gid", str(os.getgid()),52 "--gid", str(os.getgid()),
53 "--pidfile", self._pid_file,
44 "--logfile", logfile,54 "--logfile", logfile,
45 "--pidfile=",
46 "-d", self._storage_dir,55 "-d", self._storage_dir,
47 "web",56 "web",
48 "--port", "tcp:%s:interface=%s" % (self._port, self._host),57 "--port", "tcp:%s:interface=%s" % (self._port, self._host),
@@ -74,23 +83,40 @@
74 except IOError:83 except IOError:
75 raise AssertionError("logfile not writable by this user")84 raise AssertionError("logfile not writable by this user")
7685
77
78 storage = LocalStorage(self._storage_dir)86 storage = LocalStorage(self._storage_dir)
79 yield storage.put(87 yield storage.put(
80 SERVER_URL_KEY,88 SERVER_URL_KEY,
81 StringIO(serializer.dump(89 StringIO(serializer.dump(
82 {"storage-url": "http://%s:%s/" % (self._host, self._port)})))90 {"storage-url": "http://%s:%s/" % (self._host, self._port)})))
8391
84 self._service.set_command(" ".join(self._service_args))92 self._service.set_command(self._service_args)
85 yield self._service.start()93 yield self._service.start()
8694
87 def get_pid(self):95 # Capture the error for the user.
88 return self._service.get_pid()96 if not self._service.is_running():
97 content = self._capture_error()
98 raise ServiceError(
99 "Failed to start file-storage server: got output:\n"
100 "%s" % content)
101
102 def _capture_error(self):
103 if os.path.exists(self._logfile):
104 with open(self._logfile) as fh:
105 content = fh.read()
106 else:
107 content = ""
108 return content
89109
90 def stop(self):110 def stop(self):
91 """Stop the storage server."""111 """Stop the storage server."""
92 return self._service.destroy()112 return self._service.destroy()
93113
114 # Passthrough testing support
115 def is_running(self):
116 return self._service.is_running()
117
118 def get_pid(self):
119 return self._service.get_pid()
94120
95class LocalStorage(FileStorage):121class LocalStorage(FileStorage):
96122
97123
=== modified file 'juju/providers/local/tests/test_agent.py'
--- juju/providers/local/tests/test_agent.py 2012-09-10 03:20:20 +0000
+++ juju/providers/local/tests/test_agent.py 2012-09-19 20:35:23 +0000
@@ -1,9 +1,16 @@
1import os1import os
2import tempfile2import tempfile
3import subprocess3import subprocess
44<<<<<<< TREE
5from twisted.internet.defer import inlineCallbacks5
66from twisted.internet.defer import inlineCallbacks
7
8=======
9
10from twisted.internet.defer import inlineCallbacks
11
12from juju.errors import ServiceError
13>>>>>>> MERGE-SOURCE
7from juju.lib.lxc.tests.test_lxc import uses_sudo14from juju.lib.lxc.tests.test_lxc import uses_sudo
8from juju.lib.testing import TestCase15from juju.lib.testing import TestCase
9from juju.tests.common import get_test_zookeeper_address16from juju.tests.common import get_test_zookeeper_address
@@ -17,8 +24,9 @@
17 subprocess_calls = []24 subprocess_calls = []
1825
19 def intercept_args(args, **kwargs):26 def intercept_args(args, **kwargs):
27 #print "intercept", args, kwargs
20 self.assertEquals(args[0], "sudo")28 self.assertEquals(args[0], "sudo")
21 self.assertEquals(args[1], "-E")29 #self.assertEquals(args[1], "-E")
22 if args[2] == "rm":30 if args[2] == "rm":
23 return 031 return 0
24 subprocess_calls.append(args)32 subprocess_calls.append(args)
@@ -35,28 +43,26 @@
35 juju_directory=juju_directory,43 juju_directory=juju_directory,
36 log_file=log_file,44 log_file=log_file,
37 juju_origin="lp:juju/trunk")45 juju_origin="lp:juju/trunk")
38
39 try:
40 os.remove(agent._service.output_path)
41 except OSError:
42 pass # just make sure it's not there, so the .start()
43 # doesn't insert a spurious rm
44 yield agent.start()46 yield agent.start()
4547
46 start = subprocess_calls[0]48 start = subprocess_calls[0]
47 self.assertEquals(start, (49 self.assertEquals(start[start.index("/usr/bin/python"):], [
48 "sudo", "-E", "/usr/bin/python", "-m", "juju.agents.machine",50 "/usr/bin/python", "-m", "juju.agents.machine",
49 "--logfile", log_file,51 "--logfile", log_file,
50 "--zookeeper-servers", get_test_zookeeper_address(),52 "--zookeeper-servers", get_test_zookeeper_address(),
51 "--juju-directory", juju_directory,53 "--juju-directory", juju_directory,
52 "--machine-id", "0",54 "--machine-id", "0",
53 "--session-file", "/var/run/juju/ns1-machine-agent.zksession",55 "--session-file", "/var/run/juju/ns1-machine-agent.zksession",
54 "--pidfile", agent._service._pid_path))56 "--pidfile", agent._service._pid_path])
5557
56 @uses_sudo58 @uses_sudo
57 @inlineCallbacks59 @inlineCallbacks
58 def test_managed_agent_root(self):60 def test_managed_agent_root(self):
59 juju_directory = self.makeDir()61 juju_directory = self.makeDir()
62 os.mkdir(os.path.join(juju_directory, "charms"))
63 os.mkdir(os.path.join(juju_directory, "state"))
64 os.mkdir(os.path.join(juju_directory, "units"))
65
60 log_file = tempfile.mktemp()66 log_file = tempfile.mktemp()
6167
62 # The pid file and log file get written as root68 # The pid file and log file get written as root
@@ -64,6 +70,7 @@
64 subprocess.check_call(70 subprocess.check_call(
65 ["sudo", "rm", "-f", cleanup_file], stderr=subprocess.STDOUT)71 ["sudo", "rm", "-f", cleanup_file], stderr=subprocess.STDOUT)
66 self.addCleanup(cleanup_root_file, log_file)72 self.addCleanup(cleanup_root_file, log_file)
73 #self.addCleanup(cleanup_root_file, juju_directory)
6774
68 agent = ManagedMachineAgent(75 agent = ManagedMachineAgent(
69 "test-ns", machine_id="0",76 "test-ns", machine_id="0",
@@ -75,13 +82,15 @@
75 agent.agent_module = "juju.agents.dummy"82 agent.agent_module = "juju.agents.dummy"
76 self.assertFalse((yield agent.is_running()))83 self.assertFalse((yield agent.is_running()))
77 yield agent.start()84 yield agent.start()
85
78 # Give a moment for the process to start and write its config86 # Give a moment for the process to start and write its config
79 yield self.sleep(0.1)87 yield self.sleep(0.1)
80 self.assertTrue((yield agent.is_running()))88 self.assertTrue((yield agent.is_running()))
8189
82 # running start again is fine, detects the process is running90 # running start again raises an error, detects the process is running
83 yield agent.start()91 self.assertFailure(agent.start(), ServiceError)
84 yield agent.stop()92 yield agent.stop()
93 yield self.sleep(0.1)
85 self.assertFalse((yield agent.is_running()))94 self.assertFalse((yield agent.is_running()))
8695
87 # running stop again is fine, detects the process is stopped.96 # running stop again is fine, detects the process is stopped.
8897
=== modified file 'juju/providers/local/tests/test_files.py'
--- juju/providers/local/tests/test_files.py 2012-09-10 03:20:20 +0000
+++ juju/providers/local/tests/test_files.py 2012-09-19 20:35:23 +0000
@@ -7,8 +7,11 @@
7from twisted.web.client import getPage7from twisted.web.client import getPage
88
9from juju.errors import ProviderError, ServiceError9from juju.errors import ProviderError, ServiceError
10<<<<<<< TREE
10from juju.lib import serializer11from juju.lib import serializer
11from juju.lib.lxc.tests.test_lxc import uses_sudo12from juju.lib.lxc.tests.test_lxc import uses_sudo
13=======
14>>>>>>> MERGE-SOURCE
12from juju.lib.testing import TestCase15from juju.lib.testing import TestCase
13from juju.lib.upstart import UpstartService16from juju.lib.upstart import UpstartService
14from juju.providers.local.files import (17from juju.providers.local.files import (
@@ -21,12 +24,15 @@
21 @inlineCallbacks24 @inlineCallbacks
22 def setUp(self):25 def setUp(self):
23 yield super(WebFileStorageTest, self).setUp()26 yield super(WebFileStorageTest, self).setUp()
24 self._storage_path = self.makeDir()27 self._juju_dir = self.makeDir()
28 self._storage_path = os.path.join(self._juju_dir, "files")
29 os.mkdir(self._storage_path)
25 self._logfile = self.makeFile()30 self._logfile = self.makeFile()
26 self._storage = LocalStorage(self._storage_path)31 self._storage = LocalStorage(self._storage_path)
27 self._port = get_open_port()32 self._port = get_open_port()
28 self._server = StorageServer(33 self._server = StorageServer(
29 "ns1", self._storage_path, "localhost", self._port, self._logfile)34 "ns1", self._juju_dir,
35 self._storage_path, "localhost", self._port, self._logfile)
3036
31 @inlineCallbacks37 @inlineCallbacks
32 def wait_for_server(self, server):38 def wait_for_server(self, server):
@@ -34,7 +40,7 @@
34 yield self.sleep(0.1)40 yield self.sleep(0.1)
3541
36 def test_start_missing_args(self):42 def test_start_missing_args(self):
37 server = StorageServer("ns1", self._storage_path)43 server = StorageServer("ns1", self._juju_dir, self._storage_path)
38 return self.assertFailure(server.start(), AssertionError)44 return self.assertFailure(server.start(), AssertionError)
3945
40 def test_start_invalid_directory(self):46 def test_start_invalid_directory(self):
@@ -42,7 +48,7 @@
42 return self.assertFailure(self._server.start(), AssertionError)48 return self.assertFailure(self._server.start(), AssertionError)
4349
44 @inlineCallbacks50 @inlineCallbacks
45 def test_upstart(self):51 def xtest_upstart(self):
46 subprocess_calls = []52 subprocess_calls = []
4753
48 def intercept_args(args, **kwargs):54 def intercept_args(args, **kwargs):
@@ -96,13 +102,15 @@
96 self._port, self._storage_path))102 self._port, self._storage_path))
97 self.assertEquals(exec_, expect_exec)103 self.assertEquals(exec_, expect_exec)
98104
99 @uses_sudo
100 @inlineCallbacks105 @inlineCallbacks
101 def test_start_stop(self):106 def test_start_stop(self):
102 yield self._storage.put("abc", StringIO("hello world"))107 yield self._storage.put("abc", StringIO("hello world"))
108
103 yield self._server.start()109 yield self._server.start()
110
104 # Starting multiple times is fine.111 # Starting multiple times is fine.
105 yield self._server.start()112 yield self.assertFailure(self._server.start(), ServiceError)
113
106 storage_url = yield self._storage.get_url("abc")114 storage_url = yield self._storage.get_url("abc")
107115
108 # It might not have started actually accepting connections yet...116 # It might not have started actually accepting connections yet...
@@ -112,28 +120,26 @@
112 # Check that it can be killed by the current user (ie, is not running120 # Check that it can be killed by the current user (ie, is not running
113 # as root) and still comes back up121 # as root) and still comes back up
114 old_pid = yield self._server.get_pid()122 old_pid = yield self._server.get_pid()
123
115 os.kill(old_pid, signal.SIGKILL)124 os.kill(old_pid, signal.SIGKILL)
116 new_pid = yield self._server.get_pid()125 yield self.sleep(0.1)
117 self.assertNotEquals(old_pid, new_pid)126 self.assertFalse(self._server.is_running())
118
119 # Give it a moment to actually start serving again
120 yield self.wait_for_server(self._server)
121 self.assertEqual((yield getPage(storage_url)), "hello world")
122127
123 yield self._server.stop()128 yield self._server.stop()
124 # Stopping multiple times is fine too.129 # Stopping multiple times is fine too.
125 yield self._server.stop()130 yield self._server.stop()
126131
127 @uses_sudo
128 @inlineCallbacks132 @inlineCallbacks
129 def test_namespacing(self):133 def test_namespacing(self):
134 alt_juju_dir = self.makeDir()
130 alt_storage_path = self.makeDir()135 alt_storage_path = self.makeDir()
131 alt_storage = LocalStorage(alt_storage_path)136 alt_storage = LocalStorage(alt_storage_path)
132 yield alt_storage.put("some-path", StringIO("alternative"))137 yield alt_storage.put("some-path", StringIO("alternative"))
133 yield self._storage.put("some-path", StringIO("original"))138 yield self._storage.put("some-path", StringIO("original"))
134139
135 alt_server = StorageServer(140 alt_server = StorageServer(
136 "ns2", alt_storage_path, "localhost", get_open_port(),141 "ns2", alt_juju_dir,
142 alt_storage_path, "localhost", get_open_port(),
137 self.makeFile())143 self.makeFile())
138 yield alt_server.start()144 yield alt_server.start()
139 yield self._server.start()145 yield self._server.start()
@@ -150,18 +156,17 @@
150 yield alt_server.stop()156 yield alt_server.stop()
151 yield self._server.stop()157 yield self._server.stop()
152158
153 @uses_sudo
154 @inlineCallbacks159 @inlineCallbacks
155 def test_capture_errors(self):160 def test_capture_errors(self):
156 self._port = get_open_port()161 self._port = get_open_port()
157 self._server = StorageServer(162 self._server = StorageServer(
158 "borken", self._storage_path, "lol borken", self._port,163 "borken", self._juju_dir, self._storage_path,
159 self._logfile)164 "lol borken", self._port, self._logfile)
165
160 d = self._server.start()166 d = self._server.start()
161 e = yield self.assertFailure(d, ServiceError)167 e = yield self.assertFailure(d, ServiceError)
162 self.assertTrue(str(e).startswith(168 self.assertTrue(str(e).startswith(
163 "Failed to start job juju-borken-file-storage; got output:\n"))169 "Failed to start file-storage server: got output:\n"))
164 self.assertIn("Wrong number of arguments", str(e))
165 yield self._server.stop()170 yield self._server.stop()
166171
167172

Subscribers

People subscribed via source and target branches

to status/vote changes: