Merge lp:~fwereade/pyjuju/add-upstart-class into lp:pyjuju

[William Reade]

This is just the class we will be using in lp:~fwereade/juju/upstartify-services, but extracted so as to make the diffs a little less daunting.

[lp:~fwereade/pyjuju/add-upstart-class]

440. By William Reade on 2011-12-12

merge parent

441. By William Reade on 2011-12-15

merge parent

442. By William Reade on 2011-12-16

merge parent

[Jim Baker (jimbaker)]

+1. Just minors here:

[1]

+description "blah"
+author "Juju Team <email address hidden>"
+
+start on runlevel [2345]
+stop on runlevel [!2345]
+respawn
+
+env BLAH="blah blah"
+
+exec /bin/imagination-failure --no-ideas

Having a generating family of dummy data (eg your Star Wars theme or really whatever)
can be helpful for seeing the expected patterns. "blah blah" is unfortunately
just too much noise when looking at these tests.

The other thing is that putting these files inline might work much better, to avoid one
more place to look to see what the expected output should be.

[2]

+class UpstartServiceTest(TestCase):
+
+ def setUp(self):
+ self.root = self.makeDir()
+ init_dir = os.path.join(self.root, "etc", "init")
+ os.makedirs(init_dir)
+ self.conf = os.path.join(init_dir, "some-name.conf")
+ self.patch(UpstartService, "root_path", self.root)
+ self.service = UpstartService("some-name")

It's unlikely this will be mixed in with other tests, but it's likely best to preserve
the existing cooperative super inheritance support, including returning a Deferred
(usually by inlineCallbacks).

[3]

+ def test_is_installed(self):

Doc strings for tests!

+ self.assertFalse(self.service.is_installed())
+ self.write_dummy_conf()
+ self.assertTrue(self.service.is_installed())
+

[4]

+ @inlineCallbacks
+ def test_is_running(self):

Docs/comments especially here. Best to be explicit about how the mocks are
setting things up, especially when there's a sequence of multiple calls as seen here.

review: Approve

[lp:~fwereade/pyjuju/add-upstart-class]

443. By William Reade on 2012-01-10

merge parent

444. By William Reade on 2012-01-10

address review points

445. By William Reade on 2012-01-11

merge parent

446. By William Reade on 2012-01-11

merge parent

[Kapil Thangavelu (hazmat)]

lgtm +1, some minors

[0]

    # on class for ease of mocking

s/mocking/testing

[1]

+ self.conf = os.path.join(init_dir, "some-name.conf")

pep8 cheese, double space after =

[2]

+ def set_start_on_filesystem(self):
+ self._start_on_filesystem = True

this line is never tested, nor entirely clear on usage. There are several upstart gurus in the room if this needs clarification.

[3] more untested lines

+ self.root_path = root_path

its not clear why this mocked out as a class attribute, when the constructor takes the arg.

+ raise ServiceError("Cannot render .conf: no description set")
+ raise ServiceError("Cannot render .conf: no command set")

review: Approve

[William Reade (fwereade)]

[0,1]

Cool, thanks

[2,3]

...gaah, missing @inlineCallbacks~s led to some untestedness; root_path tested explicitly. After consultation with bcsaller (original author), it's fine to drop the "start on filesystem or" complication, so it's gone.

[lp:~fwereade/pyjuju/add-upstart-class]

447. By William Reade on 2012-01-11

address review points

448. By William Reade on 2012-01-12

merge parent

449. By William Reade on 2012-01-16

merge parent

450. By William Reade on 2012-01-16

merge parent

451. By William Reade on 2012-01-23

merge parent

452. By William Reade on 2012-02-02

merge parent

453. By William Reade on 2012-02-08

merge parent

454. By William Reade on 2012-02-21

merge parent

=== modified file 'juju/errors.py'
--- juju/errors.py	2012-02-21 10:36:56 +0000
+++ juju/errors.py	2012-02-21 10:36:57 +0000
@@ -174,3 +174,6 @@
                 self.user_policy,
                 self.provider_type,
                 ", ".join(self.provider_policies)))
+
+class ServiceError(JujuError):
+    """Some problem with an upstart service"""
 
=== added directory 'juju/lib/tests/data'
=== added file 'juju/lib/tests/data/test_basic_install'
--- juju/lib/tests/data/test_basic_install	1970-01-01 00:00:00 +0000
+++ juju/lib/tests/data/test_basic_install	2012-02-21 10:36:57 +0000
@@ -0,0 +1,10 @@
+description "uninteresting service"
+author "Juju Team <[e-mail]>"
+
+start on runlevel [2345]
+stop on runlevel [!2345]
+respawn
+
+
+
+exec /bin/false
 
=== added file 'juju/lib/tests/data/test_less_basic_install'
--- juju/lib/tests/data/test_less_basic_install	1970-01-01 00:00:00 +0000
+++ juju/lib/tests/data/test_less_basic_install	2012-02-21 10:36:57 +0000
@@ -0,0 +1,11 @@
+description "pew pew pew blam"
+author "Juju Team <[e-mail]>"
+
+start on runlevel [2345]
+stop on runlevel [!2345]
+respawn
+
+env FOO="bar baz qux"
+env PEW="pew"
+
+exec /bin/deathstar --ignore-ewoks endor
 
=== added file 'juju/lib/tests/data/test_standard_install'
--- juju/lib/tests/data/test_standard_install	1970-01-01 00:00:00 +0000
+++ juju/lib/tests/data/test_standard_install	2012-02-21 10:36:57 +0000
@@ -0,0 +1,10 @@
+description "a wretched hive of scum and villainy"
+author "Juju Team <[e-mail]>"
+
+start on runlevel [2345]
+stop on runlevel [!2345]
+respawn
+
+env LIGHTSABER="civilised weapon"
+
+exec /bin/imagination-failure --no-ideas
 
=== added file 'juju/lib/tests/test_upstart.py'
--- juju/lib/tests/test_upstart.py	1970-01-01 00:00:00 +0000
+++ juju/lib/tests/test_upstart.py	2012-02-21 10:36:57 +0000
@@ -0,0 +1,229 @@
+import os
+
+from twisted.internet.defer import inlineCallbacks, succeed
+
+from juju.errors import ServiceError
+from juju.lib.mocker import ANY
+from juju.lib.testing import TestCase
+from juju.lib.upstart import UpstartService
+
+
+DATA_DIR = os.path.join(os.path.abspath(os.path.dirname(__file__)), "data")
+
+
+class UpstartServiceTest(TestCase):
+
+    @inlineCallbacks
+    def setUp(self):
+        yield super(UpstartServiceTest, self).setUp()
+        self.root = self.makeDir()
+        init_dir = os.path.join(self.root, "etc", "init")
+        os.makedirs(init_dir)
+        self.conf = os.path.join(init_dir, "some-name.conf")
+        self.patch(UpstartService, "root_path", self.root)
+        self.service = UpstartService("some-name")
+
+    def setup_service(self):
+        self.service.set_description("a wretched hive of scum and villainy")
+        self.service.set_command("/bin/imagination-failure --no-ideas")
+        self.service.set_environ({"LIGHTSABER": "civilised weapon"})
+
+    def setup_mock(self):
+        self.check_call = self.mocker.replace("subprocess.check_call")
+        self.getProcessOutput = self.mocker.replace(
+            "twisted.internet.utils.getProcessOutput")
+
+    def mock_status(self, result):
+        self.getProcessOutput("/sbin/status", ["some-name"])
+        self.mocker.result(result)
+
+    def mock_call(self, args):
+        self.check_call(args)
+        self.mocker.result(0)
+
+    def mock_start(self):
+        self.mock_call(("/sbin/start", "some-name"))
+
+    def mock_stop(self):
+        self.mock_call(("/sbin/stop", "some-name"))
+
+    def write_dummy_conf(self):
+        with open(self.conf, "w") as f:
+            f.write("dummy")
+
+    def assert_dummy_conf(self):
+        with open(self.conf) as f:
+            self.assertEquals(f.read(), "dummy")
+
+    def assert_no_conf(self):
+        self.assertFalse(os.path.exists(self.conf))
+
+    def assert_conf(self, name="test_standard_install"):
+        with open(os.path.join(DATA_DIR, name)) as expected:
+            with open(self.conf) as actual:
+                self.assertEquals(actual.read(), expected.read())
+
+    def test_is_installed(self):
+        """Check is_installed depends on conf file existence"""
+        self.assertFalse(self.service.is_installed())
+        self.write_dummy_conf()
+        self.assertTrue(self.service.is_installed())
+
+    def test_root_path(self):
+        """
+        Check is_installed still works when root_path specified explicitly
+        """
+        self.patch(UpstartService, "root_path", "/BAD/PATH")
+        self.service = UpstartService("some-name", root_path=self.root)
+        self.setup_service()
+
+        self.assertFalse(self.service.is_installed())
+        self.write_dummy_conf()
+        self.assertTrue(self.service.is_installed())
+
+    @inlineCallbacks
+    def test_is_running(self):
+        """
+        Check is_running interprets status output (when service is installed)
+        """
+        self.setup_mock()
+        self.mock_status(succeed("blah stop/waiting blah"))
+        self.mock_status(succeed("blah blob/gibbering blah"))
+        self.mock_status(succeed("blah start/running blah"))
+        self.mocker.replay()
+
+        # Won't hit status; conf is not installed
+        self.assertFalse((yield self.service.is_running()))
+        self.write_dummy_conf()
+
+        # These 3 calls correspond to the 3 mock_status calls above
+        self.assertFalse((yield self.service.is_running()))
+        self.assertFalse((yield self.service.is_running()))
+        self.assertTrue((yield self.service.is_running()))
+
+    @inlineCallbacks
+    def test_basic_install(self):
+        """Check a simple UpstartService writes expected conf file"""
+        e = yield self.assertFailure(self.service.install(), ServiceError)
+        self.assertEquals(str(e), "Cannot render .conf: no description set")
+        self.service.set_description("uninteresting service")
+        e = yield self.assertFailure(self.service.install(), ServiceError)
+        self.assertEquals(str(e), "Cannot render .conf: no command set")
+        self.service.set_command("/bin/false")
+        yield self.service.install()
+
+        self.assert_conf("test_basic_install")
+
+    @inlineCallbacks
+    def test_less_basic_install(self):
+        """Check conf for a different UpstartService (which sets an env var)"""
+        self.service.set_description("pew pew pew blam")
+        self.service.set_command("/bin/deathstar --ignore-ewoks endor")
+        self.service.set_environ({"FOO": "bar baz qux", "PEW": "pew"})
+        yield self.service.install()
+
+        self.assert_conf("test_less_basic_install")
+
+    def test_install_via_script(self):
+        """Check that the output-as-script form does the right thing"""
+        self.setup_service()
+        install, start = self.service.get_cloud_init_commands()
+
+        os.system(install)
+        self.assert_conf()
+        self.assertEquals(start, "/sbin/start some-name")
+
+    @inlineCallbacks
+    def test_start_not_installed(self):
+        """Check that .start() also installs if necessary"""
+        self.setup_mock()
+        self.mock_status(succeed("blah stop/waiting blah"))
+        self.mock_start()
+        self.mocker.replay()
+
+        self.setup_service()
+        yield self.service.start()
+        self.assert_conf()
+
+    @inlineCallbacks
+    def test_start_not_started(self):
+        """Check that .start() starts if stopped"""
+        self.write_dummy_conf()
+        self.setup_mock()
+        self.mock_status(succeed("blah stop/waiting blah"))
+        self.mock_start()
+        self.mocker.replay()
+
+        self.setup_service()
+        yield self.service.start()
+        self.assert_dummy_conf()
+
+    @inlineCallbacks
+    def test_start_started(self):
+        """Check that .start() does nothing if already running"""
+        self.write_dummy_conf()
+        self.setup_mock()
+        self.mock_status(succeed("blah start/running blah"))
+        self.mocker.replay()
+
+        self.setup_service()
+        yield self.service.start()
+        self.assert_dummy_conf()
+
+    @inlineCallbacks
+    def test_destroy_not_installed(self):
+        """Check .destroy() does nothing if not installed"""
+        yield self.service.destroy()
+        self.assert_no_conf()
+
+    @inlineCallbacks
+    def test_destroy_not_started(self):
+        """Check .destroy just deletes conf if not running"""
+        self.write_dummy_conf()
+        self.setup_mock()
+        self.mock_status(succeed("blah stop/waiting blah"))
+        self.mocker.replay()
+
+        yield self.service.destroy()
+        self.assert_no_conf()
+
+    @inlineCallbacks
+    def test_destroy_started(self):
+        """Check .destroy() stops running service and deletes conf file"""
+        self.write_dummy_conf()
+        self.setup_mock()
+        self.mock_status(succeed("blah start/running blah"))
+        self.mock_stop()
+        self.mocker.replay()
+
+        yield self.service.destroy()
+        self.assert_no_conf()
+
+    @inlineCallbacks
+    def test_use_sudo(self):
+        """Check that expected commands are generated when use_sudo is set"""
+        self.setup_mock()
+        self.service = UpstartService("some-name", use_sudo=True)
+        self.setup_service()
+
+        def verify_cp(args):
+            sudo, cp, src, dst = args
+            self.assertEquals(sudo, "sudo")
+            self.assertEquals(cp, "cp")
+            with open(os.path.join(DATA_DIR, "test_standard_install")) as exp:
+                with open(src) as actual:
+                    self.assertEquals(actual.read(), exp.read())
+            self.assertEquals(dst, self.conf)
+            self.write_dummy_conf()
+        self.check_call(ANY)
+        self.mocker.call(verify_cp)
+        self.mock_call(("sudo", "chmod", "644", self.conf))
+        self.mock_status(succeed("blah stop/waiting blah"))
+        self.mock_call(("sudo", "/sbin/start", "some-name"))
+        self.mock_status(succeed("blah start/running blah"))
+        self.mock_call(("sudo", "/sbin/stop", "some-name"))
+        self.mock_call(("sudo", "rm", self.conf))
+
+        self.mocker.replay()
+        yield self.service.start()
+        yield self.service.destroy()
 
=== added file 'juju/lib/upstart.py'
--- juju/lib/upstart.py	1970-01-01 00:00:00 +0000
+++ juju/lib/upstart.py	2012-02-21 10:36:57 +0000
@@ -0,0 +1,105 @@
+import os
+import subprocess
+from tempfile import NamedTemporaryFile
+
+from twisted.internet.defer import inlineCallbacks, returnValue
+from twisted.internet.threads import deferToThread
+from twisted.internet.utils import getProcessOutput
+
+from juju.errors import ServiceError
+
+
+_UPSTART_TEMPLATE = """\
+description "%s"
+author "Juju Team <[e-mail]>"
+
+start on runlevel [2345]
+stop on runlevel [!2345]
+respawn
+
+%s
+
+exec %s
+"""
+
+
+class UpstartService(object):
+
+    # on class for ease of testing
+    root_path = "/"
+
+    def __init__(self, name, root_path=None, use_sudo=False):
+        self._name = name
+        self._use_sudo = use_sudo
+        if root_path is not None:
+            self.root_path = root_path
+        self._description = None
+        self._environ = {}
+        self._command = None
+
+    def set_description(self, description):
+        self._description = description
+
+    def set_environ(self, environ):
+        self._environ = environ
+
+    def set_command(self, command):
+        self._command = command
+
+    def _render(self):
+        if self._description is None:
+            raise ServiceError("Cannot render .conf: no description set")
+        if self._command is None:
+            raise ServiceError("Cannot render .conf: no command set")
+        return _UPSTART_TEMPLATE % (
+            self._description,
+            "\n".join('env %s="%s"' % kv
+                      for kv in sorted(self._environ.items())),
+            self._command)
+
+    @property
+    def _conf_path(self):
+        return os.path.join(
+            self.root_path, "etc", "init", "%s.conf" % self._name)
+
+    def _call(self, *args):
+        if self._use_sudo:
+            args = ("sudo",) + args
+        return deferToThread(subprocess.check_call, args)
+
+    def get_cloud_init_commands(self):
+        return ["cat >> %s <<EOF\n%sEOF\n" % (self._conf_path, self._render()),
+                "/sbin/start %s" % self._name]
+
+    @inlineCallbacks
+    def install(self):
+        with NamedTemporaryFile() as f:
+            f.write(self._render())
+            f.flush()
+            yield self._call("cp", f.name, self._conf_path)
+        yield self._call("chmod", "644", self._conf_path)
+
+    @inlineCallbacks
+    def start(self):
+        if not self.is_installed():
+            yield self.install()
+        if (yield self.is_running()):
+            return
+        yield self._call("/sbin/start", self._name)
+
+    @inlineCallbacks
+    def destroy(self):
+        if (yield self.is_running()):
+            yield self._call("/sbin/stop", self._name)
+        if self.is_installed():
+            yield self._call("rm", self._conf_path)
+
+    @inlineCallbacks
+    def is_running(self):
+        if not self.is_installed():
+            returnValue(False)
+        status = yield getProcessOutput("/sbin/status", [self._name])
+        returnValue("start/running" in status)
+
+    def is_installed(self):
+        return os.path.exists(self._conf_path)
 
=== modified file 'juju/tests/test_errors.py'
--- juju/tests/test_errors.py	2012-02-21 10:36:56 +0000
+++ juju/tests/test_errors.py	2012-02-21 10:36:57 +0000
@@ -3,7 +3,8 @@
     CharmInvocationError, CharmUpgradeError, NoConnection, InvalidHost,
     InvalidUser, ProviderError, CloudInitError, ProviderInteractionError,
     CannotTerminateMachine, MachinesNotFound, EnvironmentPending,
-    EnvironmentNotFound, IncompatibleVersion, InvalidPlacementPolicy)
+    EnvironmentNotFound, IncompatibleVersion, InvalidPlacementPolicy,
+    ServiceError)
 
 from juju.lib.testing import TestCase
 
@@ -111,6 +112,11 @@
             ("Unsupported placement policy: 'x' for provider: 'foobar', "
             "supported policies a, b, c"))
 
+    def test_ServiceError(self):
+        error = ServiceError("blah")
+        self.assertEquals(str(error), "blah")
+        self.assertIsJujuError(error)
+
     def test_CharmError(self):
         error = CharmError("/foo/bar", "blah blah")
         self.assertIsJujuError(error)