Merge lp:~tribaal/charms/trusty/landscape-client/put-install-hook-in-its-own-file into lp:charms/trusty/landscape-client

[Chris Glass]

This branch takes the install hook out to its own file, to prevent the install hook from depending on the landscape client code (that should by definition not be installed).

It did not create a problem so far because by default the cloud images have the landscape-common package installed. It will be a problem if the client's API changes in the future.

[Fernando Correa Neto (fcorrea)]

Nice! Still works +1

review: Approve

[Adam Collard (adam-collard)]

LGTM. +1 (sorry forgot to review this)

review: Approve

=== modified file 'hooks/hooks.py'
--- hooks/hooks.py	2014-06-13 15:43:58 +0000
+++ hooks/hooks.py	2014-06-19 10:48:12 +0000
@@ -4,13 +4,9 @@
 import socket
 import os
 import base64
-import subprocess
 
 from charmhelpers.core.hookenv import (
     Hooks, UnregisteredHookError, log)
-from charmhelpers.fetch import (
-    apt_install, _run_apt_command, add_source, apt_update)
-from charmhelpers.core.hookenv import config
 from shutil import rmtree
 from subprocess import CalledProcessError
 
@@ -29,31 +25,6 @@
 hooks = Hooks()
 
 
[e-mail]("install")
-def install():
-    charm_config = config()
-    apt_install(["apt-transport-https", "wget"])
-
-    origin = charm_config.get("origin")
-    if origin == "distro":
-        origin = None
-
-    if origin is not None:
-        add_apt_source(origin)
-
-    apt_update()
-    apt_install(["landscape-client"])
-    data_path = charm_config.get("data-path")
-    if not data_path:
-        data_path = "/var/lib/landscape/client"
-
-    result = subprocess.call(["landscape-config", "--init", "-d", data_path])
-    if result != 0:
-        result = subprocess.check_call([
-            "install", "-o", "landscape", "-g", "root", "-m", "755", "-d",
-            data_path])
-
-
 @hooks.hook("registration-relation-joined", "registration-relation-changed")
 def registration_relation(juju_broker=JUJU_BROKER,
                           landscape_broker=LANDSCAPE_BROKER):
@@ -228,48 +199,6 @@
     return 0
 
 
-def build_from_launchpad(url):
-    """The charm will install the code from the passed lp branch.
-    """
-    apt_install(["devscripts", "bzr", "pbuilder"], fatal=True)
-    subprocess.check_call(["rm", "-rf", "landscape-client-source"])
-    subprocess.check_call(["bzr", "branch", url, "landscape-client-source"])
-    os.chdir("landscape-client-source")
-    subprocess.check_call("/usr/lib/pbuilder/pbuilder-satisfydepends")
-    env = {"DEBUILD_OPTS": "-uc -us"}
-    subprocess.check_call(["make", "package"], env=env)
-    #TODO: The following call should be retried (potential race condition to
-    # acquire the dpkg lock).
-    subprocess.call(["dpkg", "-i", "../landscape-client_*.deb",
-                     "../landscape-common_*.deb"])
-    # The _run_apt_command will ensure the command is retried in case we cannot
-    # acquire the lock for some reason.
-    _run_apt_command(["apt-get", "-f", "install"], fatal=True)
-    os.chdir("..")
-
-
-def add_apt_source(url):
-    """Add an apt source entry, with passed in key.
-
-    This is a thin wrapper over the charmhelper "add_source" method to allow
-    for the URLs to contain the key explicitely, like:
-        - https://blah/blah|KEY
-        - deb https://blah/blah|KEY
-        - ppa:landscape/blah
-    """
-    key = None
-    if url.startswith("lp"):
-        return build_from_launchpad(url)
-
-    if "|" in url:
-        # We have a particular key specified. Url = url without the key now.
-        url, key = url.split("|", 1)  # Maximum of 1 split.
-        if key == "":
-            log("Archive key for '%s' is empty.", level="WARNING")
-
-    return add_source(url, key)
-
-
 def _write_certificate(certificate, filename):
     """
     @param certificate Text of the certificate, base64 encoded.
 
=== added symlink 'hooks/install'
=== target is u'install.py'
=== removed symlink 'hooks/install'
=== target was u'hooks.py'
=== added file 'hooks/install.py'
--- hooks/install.py	1970-01-01 00:00:00 +0000
+++ hooks/install.py	2014-06-19 10:48:12 +0000
@@ -0,0 +1,90 @@
+#!/usr/bin/python
+"""This is the install hook file.
+
+It is its own file because the rest of the hooks import landscape classes and
+functions, and landscape is not installed until this hook runs.
+"""
+
+import os
+import subprocess
+import sys
+from charmhelpers.core.hookenv import (
+    Hooks, UnregisteredHookError, log)
+from charmhelpers.fetch import (
+    apt_install, _run_apt_command, add_source, apt_update)
+from charmhelpers.core.hookenv import config
+
+
+hooks = Hooks()
+
+
[e-mail]("install")
+def install():
+    charm_config = config()
+    apt_install(["apt-transport-https", "wget"])
+
+    origin = charm_config.get("origin")
+    if origin == "distro":
+        origin = None
+
+    if origin is not None:
+        add_apt_source(origin)
+
+    apt_update()
+    apt_install(["landscape-client"])
+    data_path = charm_config.get("data-path")
+    if not data_path:
+        data_path = "/var/lib/landscape/client"
+
+    result = subprocess.call(["landscape-config", "--init", "-d", data_path])
+    if result != 0:
+        result = subprocess.check_call([
+            "install", "-o", "landscape", "-g", "root", "-m", "755", "-d",
+            data_path])
+
+def build_from_launchpad(url):
+    """The charm will install the code from the passed lp branch.
+    """
+    apt_install(["devscripts", "bzr", "pbuilder"], fatal=True)
+    subprocess.check_call(["rm", "-rf", "landscape-client-source"])
+    subprocess.check_call(["bzr", "branch", url, "landscape-client-source"])
+    os.chdir("landscape-client-source")
+    subprocess.check_call("/usr/lib/pbuilder/pbuilder-satisfydepends")
+    env = {"DEBUILD_OPTS": "-uc -us"}
+    subprocess.check_call(["make", "package"], env=env)
+    #TODO: The following call should be retried (potential race condition to
+    # acquire the dpkg lock).
+    subprocess.call(["dpkg", "-i", "../landscape-client_*.deb",
+                     "../landscape-common_*.deb"])
+    # The _run_apt_command will ensure the command is retried in case we cannot
+    # acquire the lock for some reason.
+    _run_apt_command(["apt-get", "-f", "install"], fatal=True)
+    os.chdir("..")
+
+
+def add_apt_source(url):
+    """Add an apt source entry, with passed in key.
+
+    This is a thin wrapper over the charmhelper "add_source" method to allow
+    for the URLs to contain the key explicitely, like:
+        - https://blah/blah|KEY
+        - deb https://blah/blah|KEY
+        - ppa:landscape/blah
+    """
+    key = None
+    if url.startswith("lp"):
+        return build_from_launchpad(url)
+
+    if "|" in url:
+        # We have a particular key specified. Url = url without the key now.
+        url, key = url.split("|", 1)  # Maximum of 1 split.
+        if key == "":
+            log("Archive key for '%s' is empty.", level="WARNING")
+
+    return add_source(url, key)
+
+if __name__ == '__main__':
+    try:
+        sys.exit(hooks.execute(sys.argv))
+    except UnregisteredHookError as e:
+        log('Unknown hook {} - skipping.'.format(e))