Merge lp:~mattyw/charm-tools/legacy_juju_call into lp:~charmers/charm-tools/trunk

[Matthew Williams]

Added legacy_juju function with tests

A few charms I'm working on need to know if they're running under juju (python) or juju-core. This functionality is also needed in the juju-gui charm https://bazaar.launchpad.net/~juju-gui/charms/precise/juju-gui/trunk/view/head:/hooks/utils.py line 103.

It seems to me that there are a number of ways to work out which version of juju you're working under. But there should only be one correct way which should be documented (and implemented) somewhere common. I thought putting something here might be quite useful

[Kapil Thangavelu (hazmat)]

Please use CHARM_DIR env var explicitly instead of assuming the hook hasn't
change the working dir.

On Fri, Apr 26, 2013 at 6:35 AM, Matthew Williams <
<email address hidden>> wrote:

> Matthew Williams has proposed merging
> lp:~mattyw/charm-tools/legacy_juju_call into lp:charm-tools.
>
> Commit message:
> Added legacy_juju function with tests
>
> Requested reviews:
> charmers (charmers)
>
> For more details, see:
>
> https://code.launchpad.net/~mattyw/charm-tools/legacy_juju_call/+merge/161087
>
> A few charms I'm working on need to know if they're running under juju
> (python) or juju-core. This functionality is also needed in the juju-gui
> charm
> https://bazaar.launchpad.net/~juju-gui/charms/precise/juju-gui/trunk/view/head:/hooks/utils.pyline 103.
>
> It seems to me that there are a number of ways to work out which version
> of juju you're working under. But there should only be one correct way
> which should be documented (and implemented) somewhere common. I thought
> putting something here might be quite useful
> --
>
> https://code.launchpad.net/~mattyw/charm-tools/legacy_juju_call/+merge/161087
> You are subscribed to branch lp:charm-tools.
>
> === modified file 'helpers/python/charmhelpers/__init__.py'
> --- helpers/python/charmhelpers/__init__.py 2013-04-02 21:30:49 +0000
> +++ helpers/python/charmhelpers/__init__.py 2013-04-26 10:34:24 +0000
> @@ -22,6 +22,7 @@
> 'wait_for_page_contents',
> 'wait_for_relation',
> 'wait_for_unit',
> + 'legacy_juju',
> ]
>
> from collections import namedtuple
> @@ -37,6 +38,7 @@
> import urllib2
> import yaml
> from subprocess import CalledProcessError
> +import os
>
>
> SLEEP_AMOUNT = 0.1
> @@ -275,3 +277,8 @@
> if time.time() - start_time >= timeout:
> raise RuntimeError('timeout waiting for contents of ' + url)
> time.sleep(SLEEP_AMOUNT)
> +
> +
> +def legacy_juju():
> + """Return True if the charm was deployed using python juju."""
> + return not os.path.exists(os.path.join(os.getcwd(), '..',
> 'agent.conf'))
>
> === modified file 'helpers/python/charmhelpers/tests/test_charmhelpers.py'
> --- helpers/python/charmhelpers/tests/test_charmhelpers.py 2013-04-02
> 21:30:49 +0000
> +++ helpers/python/charmhelpers/tests/test_charmhelpers.py 2013-04-26
> 10:34:24 +0000
> @@ -8,6 +8,7 @@
> from testtools import TestCase
>
> import sys
> +import os
> # Path hack to ensure we test the local code, not a version installed in
> # /usr/local/lib. This is necessary since /usr/local/lib is prepended
> before
> # what is specified in PYTHONPATH.
> @@ -470,6 +471,25 @@
> charmhelpers.log_exit()
> self.assertEqual(['<-- Exiting SCRIPT-NAME'], logged)
>
> + def test_not_legacy_juju(self):
> + # The Go version of juju install an agent.conf file in the
> directory
> + # above the charm. If that file exists then we must be running
> against
> + # the go version of juju
> + agent_file_path = "%s/../agent.conf" % os.getcwd()
> + with open(agent_file_path, 'w') as agent_file:
> + agent_file.write("")
> + self.ass...

[Matthew Williams (mattyw)]

That's a much better idea thanks, should I defult to the working dir if CHARM_DIR hasn't been set?

> Please use CHARM_DIR env var explicitly instead of assuming the hook hasn't
> change the working dir.
>
>
> On Fri, Apr 26, 2013 at 6:35 AM, Matthew Williams <
> <email address hidden>> wrote:
>
> > Matthew Williams has proposed merging
> > lp:~mattyw/charm-tools/legacy_juju_call into lp:charm-tools.
> >
> > Commit message:
> > Added legacy_juju function with tests
> >
> > Requested reviews:
> > charmers (charmers)
> >
> > For more details, see:
> >
> > https://code.launchpad.net/~mattyw/charm-
> tools/legacy_juju_call/+merge/161087
> >
> > A few charms I'm working on need to know if they're running under juju
> > (python) or juju-core. This functionality is also needed in the juju-gui
> > charm
> > https://bazaar.launchpad.net/~juju-gui/charms/precise/juju-
> gui/trunk/view/head:/hooks/utils.pyline 103.
> >
> > It seems to me that there are a number of ways to work out which version
> > of juju you're working under. But there should only be one correct way
> > which should be documented (and implemented) somewhere common. I thought
> > putting something here might be quite useful
> > --
> >
> > https://code.launchpad.net/~mattyw/charm-
> tools/legacy_juju_call/+merge/161087
> > You are subscribed to branch lp:charm-tools.
> >
> > === modified file 'helpers/python/charmhelpers/__init__.py'
> > --- helpers/python/charmhelpers/__init__.py 2013-04-02 21:30:49 +0000
> > +++ helpers/python/charmhelpers/__init__.py 2013-04-26 10:34:24 +0000
> > @@ -22,6 +22,7 @@
> > 'wait_for_page_contents',
> > 'wait_for_relation',
> > 'wait_for_unit',
> > + 'legacy_juju',
> > ]
> >
> > from collections import namedtuple
> > @@ -37,6 +38,7 @@
> > import urllib2
> > import yaml
> > from subprocess import CalledProcessError
> > +import os
> >
> >
> > SLEEP_AMOUNT = 0.1
> > @@ -275,3 +277,8 @@
> > if time.time() - start_time >= timeout:
> > raise RuntimeError('timeout waiting for contents of ' + url)
> > time.sleep(SLEEP_AMOUNT)
> > +
> > +
> > +def legacy_juju():
> > + """Return True if the charm was deployed using python juju."""
> > + return not os.path.exists(os.path.join(os.getcwd(), '..',
> > 'agent.conf'))
> >
> > === modified file 'helpers/python/charmhelpers/tests/test_charmhelpers.py'
> > --- helpers/python/charmhelpers/tests/test_charmhelpers.py 2013-04-02
> > 21:30:49 +0000
> > +++ helpers/python/charmhelpers/tests/test_charmhelpers.py 2013-04-26
> > 10:34:24 +0000
> > @@ -8,6 +8,7 @@
> > from testtools import TestCase
> >
> > import sys
> > +import os
> > # Path hack to ensure we test the local code, not a version installed in
> > # /usr/local/lib. This is necessary since /usr/local/lib is prepended
> > before
> > # what is specified in PYTHONPATH.
> > @@ -470,6 +471,25 @@
> > charmhelpers.log_exit()
> > self.assertEqual(['<-- Exiting SCRIPT-NAME'], logged)
> >
> > + def test_not_legacy_juju(self):
> > + # The Go version of juju install an agent.conf file in the
> > directory
> > + # a...

[Kapil Thangavelu (hazmat)]

charm dir is set in any hook, i'd consider raising an exception if its not
set. working dir feels very implicit. ie if in an unexpected place refuse
the temptation to guess. alternatively work out detection that's against an
absolute path.

On Fri, Apr 26, 2013 at 8:50 AM, Matthew Williams <
<email address hidden>> wrote:

> That's a much better idea thanks, should I defult to the working dir if
> CHARM_DIR hasn't been set?
>
> > Please use CHARM_DIR env var explicitly instead of assuming the hook
> hasn't
> > change the working dir.
> >
> >
> > On Fri, Apr 26, 2013 at 6:35 AM, Matthew Williams <
> > <email address hidden>> wrote:
> >
> > > Matthew Williams has proposed merging
> > > lp:~mattyw/charm-tools/legacy_juju_call into lp:charm-tools.
> > >
> > > Commit message:
> > > Added legacy_juju function with tests
> > >
> > > Requested reviews:
> > > charmers (charmers)
> > >
> > > For more details, see:
> > >
> > > https://code.launchpad.net/~mattyw/charm-
> > tools/legacy_juju_call/+merge/161087
> > >
> > > A few charms I'm working on need to know if they're running under juju
> > > (python) or juju-core. This functionality is also needed in the
> juju-gui
> > > charm
> > > https://bazaar.launchpad.net/~juju-gui/charms/precise/juju-
> > gui/trunk/view/head:/hooks/utils.pyline 103.
> > >
> > > It seems to me that there are a number of ways to work out which
> version
> > > of juju you're working under. But there should only be one correct way
> > > which should be documented (and implemented) somewhere common. I
> thought
> > > putting something here might be quite useful
> > > --
> > >
> > > https://code.launchpad.net/~mattyw/charm-
> > tools/legacy_juju_call/+merge/161087
> > > You are subscribed to branch lp:charm-tools.
> > >
> > > === modified file 'helpers/python/charmhelpers/__init__.py'
> > > --- helpers/python/charmhelpers/__init__.py 2013-04-02 21:30:49
> +0000
> > > +++ helpers/python/charmhelpers/__init__.py 2013-04-26 10:34:24
> +0000
> > > @@ -22,6 +22,7 @@
> > > 'wait_for_page_contents',
> > > 'wait_for_relation',
> > > 'wait_for_unit',
> > > + 'legacy_juju',
> > > ]
> > >
> > > from collections import namedtuple
> > > @@ -37,6 +38,7 @@
> > > import urllib2
> > > import yaml
> > > from subprocess import CalledProcessError
> > > +import os
> > >
> > >
> > > SLEEP_AMOUNT = 0.1
> > > @@ -275,3 +277,8 @@
> > > if time.time() - start_time >= timeout:
> > > raise RuntimeError('timeout waiting for contents of ' +
> url)
> > > time.sleep(SLEEP_AMOUNT)
> > > +
> > > +
> > > +def legacy_juju():
> > > + """Return True if the charm was deployed using python juju."""
> > > + return not os.path.exists(os.path.join(os.getcwd(), '..',
> > > 'agent.conf'))
> > >
> > > === modified file
> 'helpers/python/charmhelpers/tests/test_charmhelpers.py'
> > > --- helpers/python/charmhelpers/tests/test_charmhelpers.py
> 2013-04-02
> > > 21:30:49 +0000
> > > +++ helpers/python/charmhelpers/tests/test_charmhelpers.py
> 2013-04-26
> > > 10:34:24 +0000
> > > @@ -8,6 +8,7 @@
> > > from testtools import TestCase
> > >
> > > import ...

[Matthew Williams (mattyw)]

I'll submit a version raising an exception. The absolute path approach does sound tempting. But it appears that checking the contents of /var/lib/juju/ is another indirect way of checking what version you have (if the agents is present juju-core else legacy)

> charm dir is set in any hook, i'd consider raising an exception if its not
> set. working dir feels very implicit. ie if in an unexpected place refuse
> the temptation to guess. alternatively work out detection that's against an
> absolute path.
>
>
> On Fri, Apr 26, 2013 at 8:50 AM, Matthew Williams <
> <email address hidden>> wrote:
>
> > That's a much better idea thanks, should I defult to the working dir if
> > CHARM_DIR hasn't been set?
> >
> > > Please use CHARM_DIR env var explicitly instead of assuming the hook
> > hasn't
> > > change the working dir.
> > >
> > >
> > > On Fri, Apr 26, 2013 at 6:35 AM, Matthew Williams <
> > > <email address hidden>> wrote:
> > >
> > > > Matthew Williams has proposed merging
> > > > lp:~mattyw/charm-tools/legacy_juju_call into lp:charm-tools.
> > > >
> > > > Commit message:
> > > > Added legacy_juju function with tests
> > > >
> > > > Requested reviews:
> > > > charmers (charmers)
> > > >
> > > > For more details, see:
> > > >
> > > > https://code.launchpad.net/~mattyw/charm-
> > > tools/legacy_juju_call/+merge/161087
> > > >
> > > > A few charms I'm working on need to know if they're running under juju
> > > > (python) or juju-core. This functionality is also needed in the
> > juju-gui
> > > > charm
> > > > https://bazaar.launchpad.net/~juju-gui/charms/precise/juju-
> > > gui/trunk/view/head:/hooks/utils.pyline 103.
> > > >
> > > > It seems to me that there are a number of ways to work out which
> > version
> > > > of juju you're working under. But there should only be one correct way
> > > > which should be documented (and implemented) somewhere common. I
> > thought
> > > > putting something here might be quite useful
> > > > --
> > > >
> > > > https://code.launchpad.net/~mattyw/charm-
> > > tools/legacy_juju_call/+merge/161087
> > > > You are subscribed to branch lp:charm-tools.
> > > >
> > > > === modified file 'helpers/python/charmhelpers/__init__.py'
> > > > --- helpers/python/charmhelpers/__init__.py 2013-04-02 21:30:49
> > +0000
> > > > +++ helpers/python/charmhelpers/__init__.py 2013-04-26 10:34:24
> > +0000
> > > > @@ -22,6 +22,7 @@
> > > > 'wait_for_page_contents',
> > > > 'wait_for_relation',
> > > > 'wait_for_unit',
> > > > + 'legacy_juju',
> > > > ]
> > > >
> > > > from collections import namedtuple
> > > > @@ -37,6 +38,7 @@
> > > > import urllib2
> > > > import yaml
> > > > from subprocess import CalledProcessError
> > > > +import os
> > > >
> > > >
> > > > SLEEP_AMOUNT = 0.1
> > > > @@ -275,3 +277,8 @@
> > > > if time.time() - start_time >= timeout:
> > > > raise RuntimeError('timeout waiting for contents of ' +
> > url)
> > > > time.sleep(SLEEP_AMOUNT)
> > > > +
> > > > +
> > > > +def legacy_juju():
> > > > + """Return True if the charm was deployed using python juju."""
> > > > + return not os.path.e...

[lp:~mattyw/charm-tools/legacy_juju_call]

179. By Matthew Williams on 2013-04-26

get charm directory from env variable, if it isn't set raise an exception

[Matthew Williams (mattyw)]

Does this look better?

[Marco Ceppi (marcoceppi)]

Hey Matty, sorry for the delay in response on this. This looks good per Kapil's original feedback, I'm going to merge it here just, however, as we're moving these charmhelpers to the charm-helpers project could you also patch lp:charm-helpers/charmhelpers/contrib/charmhelpers/__init__.py and lp:charm-helpers/tests/contrib/charmhelpers/test_charmhelpers.py ? That'll make sure these make it in to the new project for review.

=== modified file 'helpers/python/charmhelpers/__init__.py'
--- helpers/python/charmhelpers/__init__.py	2013-04-02 21:30:49 +0000
+++ helpers/python/charmhelpers/__init__.py	2013-04-26 16:11:39 +0000
@@ -22,6 +22,7 @@
            'wait_for_page_contents',
            'wait_for_relation',
            'wait_for_unit',
+           'legacy_juju',
            ]
 
 from collections import namedtuple
@@ -37,6 +38,7 @@
 import urllib2
 import yaml
 from subprocess import CalledProcessError
+import os
 
 
 SLEEP_AMOUNT = 0.1
@@ -46,6 +48,10 @@
 juju_status = lambda: command('juju')('status')
 
 
+class CantDetermineJujuVersionException(Exception):
+    """ Raised when we can't tell what version of juju we're running under."""
+
+
 def log(message, juju_log=command('juju-log')):
     return juju_log('--', message)
 
@@ -275,3 +281,12 @@
         if time.time() - start_time >= timeout:
             raise RuntimeError('timeout waiting for contents of ' + url)
         time.sleep(SLEEP_AMOUNT)
+
+
+def legacy_juju():
+    """Return True if the charm was deployed using python juju."""
+    cwd = os.getenv("CHARM_DIR")
+    if cwd == None:
+        raise CantDetermineJujuVersionException(
+            "CHARM_DIR not set, can't determine juju version")
+    return not os.path.exists(os.path.join(os.getcwd(), '..', 'agent.conf'))

=== modified file 'helpers/python/charmhelpers/tests/test_charmhelpers.py'
--- helpers/python/charmhelpers/tests/test_charmhelpers.py	2013-04-02 21:30:49 +0000
+++ helpers/python/charmhelpers/tests/test_charmhelpers.py	2013-04-26 16:11:39 +0000
@@ -8,6 +8,7 @@
 from testtools import TestCase
 
 import sys
+import os
 # Path hack to ensure we test the local code, not a version installed in
 # /usr/local/lib.  This is necessary since /usr/local/lib is prepended before
 # what is specified in PYTHONPATH.
@@ -470,6 +471,47 @@
         charmhelpers.log_exit()
         self.assertEqual(['<-- Exiting SCRIPT-NAME'], logged)
 
+    def test_not_legacy_juju(self):
+        # The Go version of juju install an agent.conf file in the directory
+        # above the charm. If that file exists then we must be running against
+        # the go version of juju
+        os.environ['CHARM_DIR'] = os.getcwd()
+
+        def cleanup():
+            del os.environ["CHARM_DIR"]
+        self.addCleanup(cleanup)
+
+        agent_file_path = "%s/../agent.conf" % os.getcwd()
+        with open(agent_file_path, 'w') as agent_file:
+            agent_file.write("")
+
+        self.assertEqual(False, charmhelpers.legacy_juju())
+        os.remove(agent_file_path)
+        self.assertEqual(False, os.path.exists(agent_file_path))
+
+    def test_legacy_juju(self):
+        # The Go version of juju install an agent.conf file in the directory
+        # above the charm. If that file doesn't exist then we must be running
+        # against the legacy version of juju
+        os.environ['CHARM_DIR'] = os.getcwd()
+
+        def cleanup():
+            del os.environ["CHARM_DIR"]
+        self.addCleanup(cleanup)
+
+        agent_file_exists = os.path.exists("%s/../agent.conf" % os.getcwd())
+        self.assertEqual(False, agent_file_exists)
+        self.assertEqual(True, charmhelpers.legacy_juju())
+
+    def test_legacy_raises_exception(self):
+        # The Go version of juju install an agent.conf file in the directory
+        # above the charm. If the env variable CHARM_DIR isn't set we can't
+        # reasonably work out which version of juju we're running
+        self.assertRaises(KeyError, lambda: os.environ["CHARM_DIR"])
+        self.assertRaises(
+            charmhelpers.CantDetermineJujuVersionException,
+            charmhelpers.legacy_juju)
+
 
 if __name__ == '__main__':
     unittest.main()