Merge lp:~bloodearnest/charms/precise/gunicorn/upgrade-path into lp:~charmers/charms/precise/gunicorn/trunk

[Simon Davy]

Allow an easier upgrade path from old bash version of charm by adding support for old env_extra relation data format, and removing old service. Plus, and fix bug/wart in config-changed hook.

add support for old env_extra relation data format, to allow an upgrade path.

The old format was a partial python dict format, e.g.

'A': 1, 'B': 'C'

The new format is standard env format, e.g

A=1 B="C"

This update adds support back for the old format, to allow an upgrade path, as it's impossible to upgrade a subordinate and its principle in lock-step.

It also removes the older-style gunicorn config and service from previous versions of the charm, if found.

Also, using remote_unit as job name doesn't work well in config-changed hooks, so switched to local_unit, as avoids unnecessary complications.

[lp:~bloodearnest/charms/precise/gunicorn/upgrade-path]

32. By Simon Davy on 2014-03-28

catch additional missing exception

[Michael Nelson (michael.nelson)]

I ran into the issue related to remote_unit. Updated to this branch to fix that. Everything works except that I had to manually `juju ssh my-service/0 "sudo service gunicorn stop" (but that might just be related to the *very* old gunicorn charm I was using.

review: Approve

[lp:~bloodearnest/charms/precise/gunicorn/upgrade-path]

33. By Simon Davy on 2014-04-22

remove old gunicorn config if exists

[Simon Davy (bloodearnest)]

I had added code to remove the old non-upstart service on upgrade, if it exists.

[lp:~bloodearnest/charms/precise/gunicorn/upgrade-path]

34. By Simon Davy on 2014-04-22

bump to revision 4

[Wes Mason (wesmason)]

review: Approve

[lp:~bloodearnest/charms/precise/gunicorn/upgrade-path]

35. By Simon Davy on 2014-04-22

fix string/int issue for wsgi_worker config

[Simon Davy (bloodearnest)]

I found 3 gunicorn MPs pending review, which this MP supercedes:

https://code.launchpad.net/~patrick-hetu/charms/precise/gunicorn/fix_quotes/+merge/216358
https://code.launchpad.net/~thedac/charms/precise/gunicorn/fix-config-changed/+merge/211638
https://code.launchpad.net/~canonical-ci-engineering/charms/precise/gunicorn/fix-config-changed-departed/+merge/213338

See the discussion on the last one.

[lp:~bloodearnest/charms/precise/gunicorn/upgrade-path]

36. By Simon Davy on 2014-04-22

better fix for wsgi_workers quoting issue

[Matt Bruzek (mbruzek)]

+1 LGTM

review: Approve

=== modified file 'hooks/hooks.py'
--- hooks/hooks.py	2014-02-27 14:54:18 +0000
+++ hooks/hooks.py	2014-04-22 13:27:23 +0000
@@ -1,10 +1,12 @@
 #!/usr/bin/env python
 # vim: et ai ts=4 sw=4:
 
+import ast
 import os
 import sys
 from multiprocessing import cpu_count
 import shlex
+import glob
 
 from charmhelpers.core import hookenv
 from charmhelpers.core import host
@@ -30,7 +32,7 @@
 
 
 def sanitized_service_name():
-    return sanitize(hookenv.remote_unit().split('/')[0])
+    return sanitize(hookenv.local_unit().split('/')[0])
 
 
 def upstart_conf_path(name):
@@ -55,13 +57,39 @@
 # Hook functions
 ###############################################################################
 
-
[e-mail]('install', 'upgrade-charm')
-def install():
+def ensure_packages():
     fetch.apt_update()
     fetch.apt_install(CHARM_PACKAGES)
 
 
[e-mail]('install')
+def install():
+    ensure_packages()
+
+
[e-mail]('upgrade-charm')
+def upgrade():
+    ensure_packages()
+    # if we are upgrading from older charm that used gunicorn runner rather
+    # than upstart, remove that job/config
+    # sadly, we don't 100% know the name of the job, as it depends on the
+    # remote unit name, which we don't know in upgrade-charm hook
+    # TODO: remove this at somepoint
+    files = glob.glob('/etc/gunicorn.d/*.conf')
+    if files:
+        try:
+            hookenv.log('stopping system gunicorn service')
+            host.service_stop('gunicorn')
+        except:
+            pass
+    for file in files:
+        try:
+            hookenv.log('removing old guncorn config: %s' % file)
+            os.remove(file)
+        except:
+            pass
+
+
 @hooks.hook(
     "config-changed",
     "wsgi-file-relation-joined",
@@ -100,13 +128,26 @@
     elif wsgi_config['wsgi_worker_class'] == 'tornado':
         fetch.apt_install('python-tornado')
 
-    if wsgi_config['wsgi_workers'] == 0:
+    if str(wsgi_config['wsgi_workers']) == '0':
         wsgi_config['wsgi_workers'] = cpu_count() + 1
 
     env_extra = wsgi_config.get('env_extra', '')
-    wsgi_config['env_extra'] = [
-        v.split('=') for v in shlex.split(env_extra) if '=' in v
-    ]
+
+    # support old python dict format for upgrade path
+    extra = []
+    # attempt dict parsing
+    try:
+        dict_str = '{' + env_extra + '}'
+        extra = [[k, str(v)] for k, v in ast.literal_eval(dict_str).items()]
+    except (SyntaxError, ValueError):
+        pass
+
+    if not extra:
+        extra = [
+            v.split('=', 1) for v in shlex.split(env_extra) if '=' in v
+        ]
+
+    wsgi_config['env_extra'] = extra
 
     process_template('upstart.tmpl', wsgi_config, project_conf)
 

=== modified file 'hooks/tests/test_hooks.py'
--- hooks/tests/test_hooks.py	2014-02-27 14:54:18 +0000
+++ hooks/tests/test_hooks.py	2014-04-22 13:27:23 +0000
@@ -87,6 +87,19 @@
         self.fetch.apt_install.assert_called_once_with(
             ['gunicorn', 'python-jinja2'])
 
+    @patch('hooks.glob.glob')
+    @patch('os.remove')
+    def test_python_upgrade_hook(self, mock_remove, mock_glob):
+        path = '/etc/gunicorn.d/unit.conf'
+        mock_glob.return_value = [path]
+        hooks.upgrade()
+        self.assertTrue(self.fetch.apt_update.called)
+        self.fetch.apt_install.assert_called_once_with(
+            ['gunicorn', 'python-jinja2'])
+
+        self.host.service_stop.assert_called_once_with('gunicorn')
+        mock_remove.assert_called_once_with(path)
+
     def test_default_configure_gunicorn(self):
         hooks.configure_gunicorn()
         expected = self.get_default_context()
@@ -129,6 +142,19 @@
 
         self.assert_wsgi_config_applied(expected)
 
+    def test_env_extra_old_style_parsing(self):
+        self.relation_data['env_extra'] = "'A': '1', 'B': 2"
+
+        hooks.configure_gunicorn()
+
+        expected = self.get_default_context()
+        expected['env_extra'] = [
+            ['A', '1'],
+            ['B', '2'],
+        ]
+
+        self.assert_wsgi_config_applied(expected)
+
     def do_worker_class(self, worker_class):
         self.relation_data['wsgi_worker_class'] = worker_class
         hooks.configure_gunicorn()
@@ -147,8 +173,6 @@
     def test_configure_worker_class_gevent(self):
         self.do_worker_class('gevent')
 
-
-
     @patch('hooks.os.remove')
     def test_wsgi_file_relation_broken(self, remove):
         hooks.wsgi_file_relation_broken()

=== modified file 'revision'
--- revision	2013-05-29 18:40:56 +0000
+++ revision	2014-04-22 13:27:23 +0000
@@ -1,1 +1,1 @@
-3
+4