Merge lp:~ahasenack/charms/precise/apache2/apache2-default-servername into lp:charms/apache2

[Andreas Hasenack]

If the config key "servername" is empty, fill it in with the public-address of the unit to match what the documentation says.
Since we are also using charm helpers, I updated the calls to unit-get to use the charm helper method.

[Andreas Hasenack (ahasenack)]

I removed this from the review queue for now because I'm chasing a similar bug in haproxy, and my test deployment needs both apache2 and haproxy working.

[Andreas Hasenack (ahasenack)]

It's working.

[Tom Haddon (mthaddon)]

Is there any reason why we wouldn't want the change you've suggested to config/config_get in charm-helpers? Seems like it could certainly be an option you pass to it rather than needing to wrap the function at all.

[Andreas Hasenack (ahasenack)]

I don't know if such an exception is common place. It felt odd to add an exception to the generic code of charm helpers just for apache.

[Marco Ceppi (marcoceppi)]

I thinks this is a bit too specific for a change in config_get for charm-helpers. However, I think having a "default" key for when config-get returns either None or possibly an empty key might be useful. Either way this change LGTM. I'll merge it in and let you decided if you want to amend charm-helpers with that functionality.

review: Approve

=== modified file 'hooks/hooks.py'
--- hooks/hooks.py	2013-10-10 22:47:57 +0000
+++ hooks/hooks.py	2013-10-23 19:22:12 +0000
@@ -16,8 +16,9 @@
     open_port,
     close_port,
     log,
-    config as config_get,
+    config as orig_config_get,
     relations_of_type,
+    unit_get
 )
 from charmhelpers.contrib.charmsupport import nrpe
 
@@ -137,6 +138,18 @@
 ###############################################################################
 # Hook functions
 ###############################################################################
+def config_get(scope=None):
+    """
+    Wrapper around charm helper's config_get to replace an empty servername
+    with the public-address.
+    """
+    result = orig_config_get(scope)
+    if scope == "servername" and len(result) == 0:
+        result = unit_get("public-address")
+    elif type(result) is dict and "servername" in result.keys():
+        result["servername"] = unit_get("public-address")
+    return result
+
 def install_hook():
     if not os.path.exists(default_apache2_service_config_dir):
         os.mkdir(default_apache2_service_config_dir, 0600)
@@ -405,8 +418,8 @@
         # ssl_cert is SELFSIGNED so generate self-signed certificate for use.
         if config_data['ssl_cert'] and config_data['ssl_cert'] == "SELFSIGNED":
             os.environ['OPENSSL_CN'] = config_data['servername']
-            os.environ['OPENSSL_PUBLIC'] = run(["unit-get", "public-address"]).rstrip()
-            os.environ['OPENSSL_PRIVATE'] = run(["unit-get", "private-address"]).rstrip()
+            os.environ['OPENSSL_PUBLIC'] = unit_get("public-address")
+            os.environ['OPENSSL_PRIVATE'] = unit_get("private-address")
             run(['openssl', 'req', '-new', '-x509', '-nodes', '-config',
                  os.path.join(os.environ['CHARM_DIR'], 'data', 'openssl.cnf'),
                  '-keyout', key_file, '-out', cert_file])