Merge lp:~bcsaller/pyjuju/lxc-repairs into lp:pyjuju

Proposed by Benjamin Saller on 2012-07-16
Status: Merged
Merged at revision: 562
Proposed branch: lp:~bcsaller/pyjuju/lxc-repairs
Merge into: lp:pyjuju
Diff against target: 634 lines (+333/-79)
12 files modified
juju/agents/machine.py (+1/-1)
juju/control/destroy_environment.py (+1/-1)
juju/lib/lxc/__init__.py (+1/-1)
juju/lib/lxc/data/juju-create (+2/-8)
juju/lib/service.py (+148/-0)
juju/lib/tests/test_service.py (+100/-0)
juju/machine/unit.py (+1/-0)
juju/providers/local/__init__.py (+21/-8)
juju/providers/local/agent.py (+23/-6)
juju/providers/local/tests/test_agent.py (+15/-51)
juju/providers/local/tests/test_provider.py (+18/-1)
setup.py (+2/-2)
To merge this branch: bzr merge lp:~bcsaller/pyjuju/lxc-repairs
Reviewer Review Type Date Requested Status
Kapil Thangavelu (community) 2012-07-16 Needs Fixing on 2012-07-18
Review via email: mp+115206@code.launchpad.net

Description of the change

Remove LXC restart partial support

This branch includes the bug fix (and a few trivials). The extra checks and features are in
the history but not included here.

https://codereview.appspot.com/6404052/

To post a comment you must log in.
Benjamin Saller (bcsaller) wrote :

Reviewers: mp+115206_code.launchpad.net,

Message:
Please take a look.

Description:
Remove LXC restart partial support

Local lxc containers were initially intended to be ephemeral. This patch
makes that official
removing the partial restart of services via upstart from local
containers. It includes
a number of other minor tweaks including warning when a /tmp dir is used
for keeping
container state (as a developer could still manually restart containers
if state was stable).

With the change to ephemeral containers the /tmp change might not make
sense but the warning
shouldn't prove an issue and can easily prove useful in the future if we
choose to fully support
restarts.

https://code.launchpad.net/~bcsaller/juju/lxc-repairs/+merge/115206

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M juju/agents/machine.py
   M juju/control/destroy_environment.py
   A juju/lib/juju.png
   M juju/lib/lxc/__init__.py
   M juju/lib/lxc/data/juju-create
   M juju/lib/lxc/tests/test_lxc.py
   A juju/lib/notifier.py
   A juju/lib/service.py
   A juju/lib/tests/test_notifier.py
   A juju/lib/tests/test_service.py
   M juju/lib/upstart.py
   M juju/machine/tests/test_unit_deployment.py
   M juju/machine/unit.py
   M juju/providers/local/__init__.py
   M juju/providers/local/agent.py
   M juju/providers/local/tests/test_agent.py
   M juju/providers/local/tests/test_provider.py
   M setup.py

Kapil Thangavelu (hazmat) wrote :

as discussed in irc, this branch has three other features in addition to the bug fix which should be in another branch. The tmp folder guards are irrelevant given the branches focus. The ephemeral stuff needs more vetting with actual charms/usage. The desktop notification is cool, but again none of those is relevant to landing the core and important of the linked bug. pls yank those out and resubmit.

Besides that the actual bug fix uses /tmp paths for pid and logfiles which feels questionable. Also the generic service name unless its truly generic rather than a twisted daemon process launcher.

review: Needs Fixing
Benjamin Saller (bcsaller) wrote :

> as discussed in irc, this branch has three other features in addition to the
> bug fix which should be in another branch. The tmp folder guards are
> irrelevant given the branches focus. The ephemeral stuff needs more vetting
> with actual charms/usage. The desktop notification is cool, but again none of
> those is relevant to landing the core and important of the linked bug. pls
> yank those out and resubmit.
>
> Besides that the actual bug fix uses /tmp paths for pid and logfiles which
> feels questionable. Also the generic service name unless its truly generic
> rather than a twisted daemon process launcher.

Thanks for the review, I'll separate out the branch as we spoke about. I couldn't parse the last line of the review though, any help?

Benjamin Saller (bcsaller) wrote :
Kapil Thangavelu (hazmat) wrote :

lgtm, with two minors.

https://codereview.appspot.com/6404052/diff/1/juju/lib/service.py
File juju/lib/service.py (right):

https://codereview.appspot.com/6404052/diff/1/juju/lib/service.py#newcode56
juju/lib/service.py:56: return self._output_path
feels like this should be required, if pid file is. hardcoded /tmp files
are a red flag.

https://codereview.appspot.com/6404052/diff/1/juju/lib/service.py#newcode72
juju/lib/service.py:72: if "--pidfile" not in command:
class should probably get renamed twisted daemon service, or class
string documentation of this the parameter requirement.

https://codereview.appspot.com/6404052/diff/2001/juju/lib/service.py
File juju/lib/service.py (right):

https://codereview.appspot.com/6404052/diff/2001/juju/lib/service.py#newcode33
juju/lib/service.py:33: class ServiceManager(object):
naming nitpick, service manager is a bit ambiguious, and sounds like its
managing multiple services. Given the description it seems more like an
AgentService.

https://codereview.appspot.com/6404052/diff/2001/juju/lib/service.py#newcode55
juju/lib/service.py:55: return "/tmp/%s.output" % self._name
this should use the data dir for output.

https://codereview.appspot.com/6404052/diff/2001/juju/lib/service.py#newcode139
juju/lib/service.py:139: return False
nice

https://codereview.appspot.com/6404052/diff/2001/juju/lib/tests/test_service.py
File juju/lib/tests/test_service.py (right):

https://codereview.appspot.com/6404052/diff/2001/juju/lib/tests/test_service.py#newcode45
juju/lib/tests/test_service.py:45:
there should also be a real tests here, twistd has several builtin
services.

https://codereview.appspot.com/6404052/diff/2001/juju/providers/local/agent.py
File juju/providers/local/agent.py (right):

https://codereview.appspot.com/6404052/diff/2001/juju/providers/local/agent.py#newcode54
juju/providers/local/agent.py:54: pidfile =
get_temp_filename(juju_unit_namespace)
these can go into the data-dir instead of /tmp, and can be named instead
of mkstemp

https://codereview.appspot.com/6404052/

lp:~bcsaller/pyjuju/lxc-repairs updated on 2012-08-03
558. By Benjamin Saller on 2012-08-03

minor review feedback pre-merge

559. By Benjamin Saller on 2012-08-03

merge trunk

560. By Benjamin Saller on 2012-08-03

fix changed import

561. By Benjamin Saller on 2012-08-03

valid webserver test via twistd to test TwistedDaemon service manager

Benjamin Saller (bcsaller) wrote :

Changes made, test add, merging.

https://codereview.appspot.com/6404052/diff/2001/juju/lib/tests/test_service.py
File juju/lib/tests/test_service.py (right):

https://codereview.appspot.com/6404052/diff/2001/juju/lib/tests/test_service.py#newcode45
juju/lib/tests/test_service.py:45:
On 2012/07/26 13:03:13, hazmat wrote:
> there should also be a real tests here, twistd has several builtin
services.
Added test using twistd to spawn a webserver, some of the arg handling
of twistd is more specific than the manager expected, this was corrected
in the code, the result is more flexible.

https://codereview.appspot.com/6404052/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'juju/agents/machine.py'
2--- juju/agents/machine.py 2012-06-21 22:12:18 +0000
3+++ juju/agents/machine.py 2012-08-03 12:30:27 +0000
4@@ -122,5 +122,5 @@
5 return parser
6
7
8-if __name__ == '__main__':
9+if __name__ == "__main__":
10 MachineAgent().run()
11
12=== modified file 'juju/control/destroy_environment.py'
13--- juju/control/destroy_environment.py 2012-04-04 13:00:13 +0000
14+++ juju/control/destroy_environment.py 2012-08-03 12:30:27 +0000
15@@ -24,7 +24,7 @@
16 value = raw_input(
17 "WARNING: this command will destroy the %r environment (type: %s).\n"
18 "This includes all machines, services, data, and other resources. "
19- "Continue [y/N]" % (
20+ "Continue [y/N] " % (
21 environment.name, environment.type))
22
23 if value.strip().lower() not in ("y", "yes"):
24
25=== modified file 'juju/lib/lxc/__init__.py'
26--- juju/lib/lxc/__init__.py 2012-05-22 22:08:15 +0000
27+++ juju/lib/lxc/__init__.py 2012-08-03 12:30:27 +0000
28@@ -61,7 +61,7 @@
29
30
31 def _lxc_start(container_name, debug_log=None, console_log=None):
32- args = ["sudo", "lxc-start", "--daemon", "-n", container_name]
33+ args = ["sudo", "lxc-start", "-d", "-n", container_name]
34 if console_log:
35 args.extend(["-c", console_log])
36 if debug_log:
37
38=== modified file 'juju/lib/lxc/data/juju-create'
39--- juju/lib/lxc/data/juju-create 2012-06-22 17:01:27 +0000
40+++ juju/lib/lxc/data/juju-create 2012-08-03 12:30:27 +0000
41@@ -107,8 +107,8 @@
42 apt-get update
43 apt-get install $APT_OPTIONS juju python-txzookeeper
44 elif [ $JUJU_ORIGIN = "proposed" ]; then
45- # Enabled testing of proposed updates
46- echo "deb http://archive.ubuntu.com/ubuntu/ $JUJU_SERIES-proposed main universe" >> /etc/apt/sources.list
47+ # Enabled testing of proposed updates
48+ echo "deb http://archive.ubuntu.com/ubuntu/ $JUJU_SERIES-proposed main universe" >> /etc/apt/sources.list
49 apt-get update
50 apt-get install $APT_OPTIONS juju python-txzookeeper
51
52@@ -147,12 +147,6 @@
53 echo "Without JUJU_PUBLIC_KEY you will not have ssh access to your container"
54 fi
55
56-
57-if [ "$(id -u)" != "0" ]; then
58- echo "This script should be run as 'root'"
59- exit 1
60-fi
61-
62 setup_apt_cache
63 setup_networking
64 setup_juju
65
66=== added file 'juju/lib/service.py'
67--- juju/lib/service.py 1970-01-01 00:00:00 +0000
68+++ juju/lib/service.py 2012-08-03 12:30:27 +0000
69@@ -0,0 +1,148 @@
70+from twisted.internet.defer import inlineCallbacks
71+from twisted.internet.threads import deferToThread
72+
73+from juju.errors import ServiceError
74+
75+import os
76+import subprocess
77+
78+
79+def _check_call(args, env=None, output_path=None):
80+ if not output_path:
81+ output_path = os.devnull
82+
83+ with open(output_path, "a") as f:
84+ return subprocess.check_call(
85+ args,
86+ stdout=f.fileno(), stderr=f.fileno(),
87+ env=env)
88+
89+
90+def _cat(filename, use_sudo=False):
91+ args = ("cat", filename)
92+ if use_sudo and not os.access(filename, os.R_OK):
93+ args = ("sudo",) + args
94+
95+ p = subprocess.Popen(
96+ args, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
97+ stdout_data, _ = p.communicate()
98+ r = p.returncode
99+ return (r, stdout_data)
100+
101+
102+class TwistedDaemonService(object):
103+ """Manage the starting and stopping of an Agent.
104+
105+ This manager tracks the agent via its --pidfile. The pidfile argument
106+ specifies the location of the pid file that is used to track this service.
107+
108+ """
109+ def __init__(self, name, pidfile, use_sudo=False):
110+ self._name = name
111+ self._use_sudo = use_sudo
112+ self._description = None
113+ self._environ = None
114+ self._command = None
115+ self._output_path = None
116+
117+ self._pid_path = pidfile
118+ self._pid = None
119+
120+ @property
121+ def output_path(self):
122+ if self._output_path is not None:
123+ return self._output_path
124+ return "/tmp/%s.output" % self._name
125+
126+ @output_path.setter
127+ def output_path(self, path):
128+ self._output_path = path
129+
130+ def set_description(self, description):
131+ self._description = description
132+
133+ def set_environ(self, environ):
134+ for k, v in environ.items():
135+ environ[k] = str(v)
136+ self._environ = environ
137+
138+ def set_command(self, command):
139+ if "--pidfile" not in command:
140+ command += ["--pidfile", self._pid_path]
141+ else:
142+ # pid file is in command (consume it for get_pid)
143+ idx = command.index("--pidfile")
144+ self._pid_path = command[idx+1]
145+
146+ self._command = command
147+
148+ @inlineCallbacks
149+ def _trash_output(self):
150+ if os.path.exists(self.output_path):
151+ # Just using os.unlink will fail when we're running TEST_SUDO
152+ # tests which hit this code path (because root will own
153+ # self.output_path)
154+ yield self._call("rm", "-f", self.output_path)
155+
156+ if os.path.exists(self._pid_path):
157+ yield self._call("rm", "-f", self._pid_path)
158+
159+ def _call(self, *args, **kwargs):
160+ if self._use_sudo:
161+ args = ("sudo", "-E") + args
162+ return deferToThread(_check_call, args, env=self._environ,
163+ output_path=self.output_path)
164+
165+ def install(self):
166+ if self._command is None:
167+ raise ServiceError("Cannot manage agent: %s no command set" % (
168+ self._name))
169+
170+ @inlineCallbacks
171+ def start(self):
172+ if (yield self.is_running()):
173+ raise ServiceError(
174+ "%s already running: pid (%s)" % (
175+ self._name, self.get_pid()))
176+
177+ if not self.is_installed():
178+ yield self.install()
179+
180+ yield self._trash_output()
181+ yield self._call(*self._command, output_path=self.output_path)
182+
183+ @inlineCallbacks
184+ def destroy(self):
185+ if (yield self.is_running()):
186+ yield self._call("kill", self.get_pid())
187+ yield self._trash_output()
188+
189+ def get_pid(self):
190+ if self._pid != None:
191+ return self._pid
192+
193+ if not os.path.exists(self._pid_path):
194+ return None
195+ r, data = _cat(self._pid_path, use_sudo=self._use_sudo)
196+ if r != 0:
197+ return None
198+
199+ # verify that pid is a number but leave
200+ # it as a string suitable for passing to kill
201+ if not data.strip().isdigit():
202+ return None
203+ pid = data.strip()
204+ self._pid = pid
205+ return self._pid
206+
207+ def is_running(self):
208+ pid = self.get_pid()
209+ if not pid:
210+ return False
211+ proc_file = "/proc/%s" % pid
212+ if not os.path.exists(proc_file):
213+ return False
214+ return True
215+
216+ def is_installed(self):
217+ return False
218
219=== added file 'juju/lib/tests/test_service.py'
220--- juju/lib/tests/test_service.py 1970-01-01 00:00:00 +0000
221+++ juju/lib/tests/test_service.py 2012-08-03 12:30:27 +0000
222@@ -0,0 +1,100 @@
223+from twisted.internet.defer import inlineCallbacks
224+
225+from juju.lib.testing import TestCase
226+from juju.lib.mocker import MATCH, KWARGS
227+from juju.lib.service import TwistedDaemonService
228+
229+import os
230+
231+
232+class TwistedDaemonServiceTest(TestCase):
233+
234+ @inlineCallbacks
235+ def setUp(self):
236+ yield super(TwistedDaemonServiceTest, self).setUp()
237+ self.setup_service()
238+
239+ def setup_service(self):
240+ service = TwistedDaemonService("juju-machine-agent",
241+ "/tmp/.juju-test.pid",
242+ use_sudo=False)
243+ service.set_description("Juju machine agent")
244+ service.set_environ({"JUJU_MACHINE_ID": 0})
245+ service.set_command(["/bin/true", ])
246+ self.service = service
247+
248+ if os.path.exists("/tmp/.juju-test.pid"):
249+ os.remove("/tmp/.juju-test.pid")
250+
251+ return service
252+
253+ def setup_mock(self):
254+ self.check_call = self.mocker.replace("subprocess.check_call")
255+
256+ def mock_call(self, args):
257+ def exe_match(cmd):
258+ cmd = " ".join(str(s) for s in cmd)
259+ return cmd.startswith(args)
260+
261+ self.check_call(MATCH(exe_match), KWARGS)
262+ self.mocker.result(0)
263+
264+ @inlineCallbacks
265+ def test_simple_service_start(self):
266+ self.setup_mock()
267+ self.mock_call("/bin/true")
268+ self.mocker.replay()
269+
270+ yield self.service.start()
271+
272+ def test_set_output_path(self):
273+ # defaults work
274+ self.assertEqual(self.service.output_path,
275+ "/tmp/juju-machine-agent.output")
276+ # override works
277+ self.service.output_path = "/tmp/valid.log"
278+ self.assertEqual(self.service.output_path, "/tmp/valid.log")
279+
280+ @inlineCallbacks
281+ def test_simple_service_start_destroy(self):
282+ self.setup_mock()
283+ mock_service = self.mocker.patch(self.service)
284+ get_pid = mock_service.get_pid
285+ get_pid()
286+ self.mocker.result(1337)
287+
288+ is_running = mock_service.is_running
289+ is_running()
290+ self.mocker.result(False)
291+
292+ is_running()
293+ self.mocker.result(True)
294+
295+ self.mock_call(("/bin/true", ))
296+ self.mock_call(("kill", "1337"))
297+ self.mocker.replay()
298+
299+ yield self.service.start()
300+ yield self.service.destroy()
301+
302+ @inlineCallbacks
303+ def test_webservice_start(self):
304+ # test using a real twisted service (with --pidfile)
305+ # arg ordering matters here so we set pidfile manually
306+ self.service.set_command([
307+ "env", "twistd",
308+ "--pidfile", "/tmp/.juju-test.pid",
309+ "--logfile", "/tmp/.juju-test.log",
310+ "web",
311+ "--port", "9871",
312+ "--path", "/lib",
313+ ])
314+
315+ yield self.service.start()
316+ yield self.sleep(0.5)
317+ self.assertTrue(self.service.get_pid())
318+ self.assertTrue(self.service.is_running())
319+ self.assertTrue(os.path.exists("/tmp/.juju-test.pid"))
320+ yield self.service.destroy()
321+ yield self.sleep(0.1)
322+ self.assertFalse(os.path.exists("/tmp/.juju-test.pid"))
323\ No newline at end of file
324
325=== modified file 'juju/machine/unit.py'
326--- juju/machine/unit.py 2012-04-04 10:13:38 +0000
327+++ juju/machine/unit.py 2012-08-03 12:30:27 +0000
328@@ -244,6 +244,7 @@
329 "Juju unit agent for %s" % self.unit_name)
330 service.set_environ(_get_environment(
331 self.unit_name, "/var/lib/juju", machine_id, zookeeper_hosts))
332+
333 service.set_output_path(
334 "/var/log/juju/unit-%s-output.log" % self.unit_path_name)
335 service.set_command(" ".join((
336
337=== modified file 'juju/providers/local/__init__.py'
338--- juju/providers/local/__init__.py 2012-04-06 16:35:31 +0000
339+++ juju/providers/local/__init__.py 2012-08-03 12:30:27 +0000
340@@ -58,6 +58,9 @@
341 def bootstrap(self, constraints):
342 """Bootstrap a local development environment.
343 """
344+ # Validate `data-dir` or raise/warn
345+ self._validate_data_dir()
346+
347 # Check for existing environment
348 state = yield self.load_state()
349 if state is not False:
350@@ -77,11 +80,6 @@
351 except LookupError, e:
352 raise ProviderError(str(e))
353
354- # Get/create directory for zookeeper and files
355- zookeeper_dir = os.path.join(self._directory, "zookeeper")
356- if not os.path.exists(zookeeper_dir):
357- os.makedirs(zookeeper_dir)
358-
359 # Start networking, and get an open port.
360 log.info("Starting networking...")
361 net = Network("default", subnet=122)
362@@ -92,8 +90,13 @@
363 net_attributes = yield net.get_attributes()
364 port = get_open_port(net_attributes["ip"]["address"])
365
366- # Start zookeeper
367- log.info("Starting zookeeper...")
368+ # Get/create directory for zookeeper and files
369+ zookeeper_dir = os.path.join(self._directory, "zookeeper")
370+ if not os.path.exists(zookeeper_dir):
371+ os.makedirs(zookeeper_dir)
372+
373+ # Start ZooKeeper
374+ log.info("Starting ZooKeeper...")
375 # Run zookeeper as the current user, unless we're being run as root
376 # in which case run zookeeper as the 'zookeeper' user.
377 zookeeper_user = None
378@@ -102,7 +105,8 @@
379 zookeeper = Zookeeper(zookeeper_dir,
380 port=port,
381 host=net_attributes["ip"]["address"],
382- user=zookeeper_user, group=zookeeper_user)
383+ user=zookeeper_user,
384+ group=zookeeper_user)
385
386 yield zookeeper.start()
387
388@@ -143,12 +147,21 @@
389 juju_origin=juju_origin,
390 public_key=public_key,
391 juju_series=self.config["default-series"])
392+
393 log.info(
394 "Starting machine agent (origin: %s)... ", agent.juju_origin)
395 yield agent.start()
396
397 log.info("Environment bootstrapped")
398
399+ def _validate_data_dir(self):
400+ data_dir = self.config["data-dir"]
401+ if not os.path.exists(data_dir):
402+ raise ProviderError("`data-dir` does not exist")
403+
404+ if not os.access(data_dir, os.W_OK):
405+ raise ProviderError("`data-dir` not writable")
406+
407 @inlineCallbacks
408 def destroy_environment(self):
409 """Shutdown the machine environment.
410
411=== modified file 'juju/providers/local/agent.py'
412--- juju/providers/local/agent.py 2012-02-21 12:14:11 +0000
413+++ juju/providers/local/agent.py 2012-08-03 12:30:27 +0000
414@@ -1,9 +1,18 @@
415 import sys
416+import tempfile
417
418-from juju.lib.upstart import UpstartService
419+from juju.lib.service import TwistedDaemonService
420 from juju.providers.common.cloudinit import get_default_origin, BRANCH
421
422
423+def get_temp_filename(basename):
424+ if basename:
425+ basename += "-"
426+
427+ fd, fn = tempfile.mkstemp(prefix=basename, suffix=".pid")
428+ return fn
429+
430+
431 class ManagedMachineAgent(object):
432
433 agent_module = "juju.agents.machine"
434@@ -11,7 +20,7 @@
435 def __init__(
436 self, juju_unit_namespace, zookeeper_hosts=None,
437 machine_id="0", log_file=None, juju_directory="/var/lib/juju",
438- public_key=None, juju_origin="ppa", juju_series=None):
439+ public_key=None, juju_origin=None, juju_series=None):
440 """
441 :param juju_series: The release series to use (maverick, natty, etc).
442 :param machine_id: machine id for the local machine.
443@@ -42,14 +51,22 @@
444 if public_key:
445 env["JUJU_PUBLIC_KEY"] = public_key
446
447- self._service = UpstartService(
448- "juju-%s-machine-agent" % juju_unit_namespace, use_sudo=True)
449+ pidfile = get_temp_filename(juju_unit_namespace)
450+ self._service = TwistedDaemonService(
451+ "juju-%s-machine-agent" % juju_unit_namespace,
452+ pidfile,
453+ use_sudo=True)
454 self._service.set_description(
455 "Juju machine agent for %s" % juju_unit_namespace)
456 self._service.set_environ(env)
457+ self._service.output_path = log_file
458+
459 self._service_args = [
460 "/usr/bin/python", "-m", self.agent_module,
461- "--nodaemon", "--logfile", log_file,
462+ "--logfile", log_file,
463+ "--zookeeper-servers", zookeeper_hosts,
464+ "--juju-directory", juju_directory,
465+ "--machine-id", machine_id,
466 "--session-file",
467 "/var/run/juju/%s-machine-agent.zksession" % juju_unit_namespace]
468
469@@ -59,7 +76,7 @@
470
471 def start(self):
472 """Start the machine agent."""
473- self._service.set_command(" ".join(self._service_args))
474+ self._service.set_command(self._service_args)
475 return self._service.start()
476
477 def stop(self):
478
479=== modified file 'juju/providers/local/tests/test_agent.py'
480--- juju/providers/local/tests/test_agent.py 2012-02-21 12:14:11 +0000
481+++ juju/providers/local/tests/test_agent.py 2012-08-03 12:30:27 +0000
482@@ -7,7 +7,6 @@
483
484 from juju.lib.lxc.tests.test_lxc import uses_sudo
485 from juju.lib.testing import TestCase
486-from juju.lib.upstart import UpstartService
487 from juju.tests.common import get_test_zookeeper_address
488 from juju.providers.local.agent import ManagedMachineAgent
489
490@@ -19,26 +18,14 @@
491 subprocess_calls = []
492
493 def intercept_args(args, **kwargs):
494+ self.assertEquals(args[0], "sudo")
495+ self.assertEquals(args[1], "-E")
496+ if args[2] == "rm":
497+ return 0
498 subprocess_calls.append(args)
499- self.assertEquals(args[0], "sudo")
500- if args[1] == "cp":
501- return real_check_call(args[1:], **kwargs)
502 return 0
503
504- real_check_call = self.patch(subprocess, "check_call", intercept_args)
505- init_dir = self.makeDir()
506- self.patch(UpstartService, "init_dir", init_dir)
507-
508- # Mock out the repeated checking for unstable pid, after an initial
509- # stop/waiting to induce the actual start
510- getProcessOutput = self.mocker.replace(
511- "twisted.internet.utils.getProcessOutput")
512- getProcessOutput("/sbin/status", ["juju-ns1-machine-agent"])
513- self.mocker.result(succeed("stop/waiting"))
514- for _ in range(5):
515- getProcessOutput("/sbin/status", ["juju-ns1-machine-agent"])
516- self.mocker.result(succeed("start/running 123"))
517- self.mocker.replay()
518+ self.patch(subprocess, "check_call", intercept_args)
519
520 juju_directory = self.makeDir()
521 log_file = self.makeFile()
522@@ -51,44 +38,21 @@
523 juju_origin="lp:juju/trunk")
524
525 try:
526- os.remove("/tmp/juju-ns1-machine-agent.output")
527+ os.remove(agent._service.output_path)
528 except OSError:
529 pass # just make sure it's not there, so the .start()
530 # doesn't insert a spurious rm
531-
532 yield agent.start()
533
534- conf_dest = os.path.join(
535- init_dir, "juju-ns1-machine-agent.conf")
536- chmod, start = subprocess_calls[1:]
537- self.assertEquals(chmod, ("sudo", "chmod", "644", conf_dest))
538- self.assertEquals(
539- start, ("sudo", "/sbin/start", "juju-ns1-machine-agent"))
540-
541- env = []
542- with open(conf_dest) as f:
543- for line in f:
544- if line.startswith("env"):
545- env.append(line[4:-1].split("=", 1))
546- if line.startswith("exec"):
547- exec_ = line[5:-1]
548-
549- expect_exec = (
550- "/usr/bin/python -m juju.agents.machine --nodaemon --logfile %s "
551- "--session-file /var/run/juju/ns1-machine-agent.zksession "
552- ">> /tmp/juju-ns1-machine-agent.output 2>&1"
553- % log_file)
554- self.assertEquals(exec_, expect_exec)
555-
556- env = dict((k, v.strip('"')) for (k, v) in env)
557- self.assertEquals(env, {
558- "JUJU_ZOOKEEPER": get_test_zookeeper_address(),
559- "JUJU_MACHINE_ID": "0",
560- "JUJU_HOME": juju_directory,
561- "JUJU_ORIGIN": "lp:juju/trunk",
562- "JUJU_UNIT_NS": "ns1",
563- "JUJU_SERIES": "precise",
564- "PYTHONPATH": ":".join(sys.path)})
565+ start = subprocess_calls[0]
566+ self.assertEquals(start, (
567+ "sudo", "-E", "/usr/bin/python", "-m", "juju.agents.machine",
568+ "--logfile", log_file,
569+ "--zookeeper-servers", get_test_zookeeper_address(),
570+ "--juju-directory", juju_directory,
571+ "--machine-id", "0",
572+ "--session-file", "/var/run/juju/ns1-machine-agent.zksession",
573+ "--pidfile", agent._service._pid_path))
574
575 @uses_sudo
576 @inlineCallbacks
577
578=== modified file 'juju/providers/local/tests/test_provider.py'
579--- juju/providers/local/tests/test_provider.py 2012-03-29 01:37:57 +0000
580+++ juju/providers/local/tests/test_provider.py 2012-08-03 12:30:27 +0000
581@@ -101,7 +101,7 @@
582 sorted(children))
583 output = self.output.getvalue()
584 self.assertIn("Starting networking...", output)
585- self.assertIn("Starting zookeeper...", output)
586+ self.assertIn("Starting ZooKeeper...", output)
587 self.assertIn("Initializing state...", output)
588 self.assertIn("Starting storage server", output)
589 self.assertIn("Starting machine agent", output)
590@@ -117,6 +117,23 @@
591 "zookeeper-instances": ["local"]})
592
593 @inlineCallbacks
594+ def test_bootstrap_bad_data_dir(self):
595+ data_dir = self.makeDir()
596+ # make it read only
597+ os.chmod(data_dir, 0444)
598+
599+ self.provider = MachineProvider(
600+ "local-test", {
601+ "admin-secret": "admin:abc", "data-dir": data_dir,
602+ "authorized-keys": "fooabc123", "default-series": "oneiric"})
603+
604+ self.mocker.replay()
605+
606+ error = yield self.assertFailure(
607+ self.provider.bootstrap(self.constraints), ProviderError)
608+ self.assertIn("`data-dir` not writable", str(error))
609+
610+ @inlineCallbacks
611 def test_boostrap_previously_bootstrapped(self):
612 """Any local state is a sign that we had a previous bootstrap.
613 """
614
615=== modified file 'setup.py'
616--- setup.py 2012-03-23 02:13:50 +0000
617+++ setup.py 2012-08-03 12:30:27 +0000
618@@ -16,14 +16,14 @@
619 pass
620 packages = []
621 for directory, subdirectories, files in os.walk("juju"):
622- if '__init__.py' in files:
623+ if "__init__.py" in files:
624 packages.append(directory.replace(os.sep, '.'))
625 return packages
626
627
628 setup(
629 name="juju",
630- version="0.5", # Change debian/changelog too, for daily builds.
631+ version="0.5", # Change debian/changelog too, for daily builds.
632 description="Cloud automation and orchestration",
633 author="Juju Developers",
634 author_email="juju@lists.ubuntu.com",

Subscribers

People subscribed via source and target branches

to status/vote changes: