A bit of cleanup

hooks/keystone-hooks

@@ -8,6 +8,8 @@ packages = "keystone python-mysqldb pwgen"
 service = "keystone"
 
 # used to verify joined services are valid openstack components.
+# this should reflect the current "core" components of openstack
+# and be expanded as we add support for them as a distro
 valid_services = {
     "nova": {
         "type": "compute",
@@ -21,13 +23,12 @@ valid_services = {
         "type": "storage",
         "desc": "Swift Object Storage Service"
     }
-
 }
 
-
 def install_hook():
     if config["keystone-release"] != "distro":
         setup_ppa(config["keystone-release"])
+    execute("apt-get update", die=True)
     execute("apt-get -y install %s" % packages, die=True, echo=True)
     keystone_conf_update("keystone-admin-role",
                          config["keystone-admin-role"])
@@ -74,10 +75,10 @@ def identity_changed():
         juju_log("Missing relation data.  Peer not ready, exit 0")
         exit(0)
 
-    # we do not support the service advertised on the other end of relation
+    # If we do not support the service advertised on the other end of relation
-    # return a token of -1 if it is not supported
+    # return a token of -1
     if relation_data["service"] not in valid_services.keys():
-        juju_log("Invalid service requested: '%s'" % relation_data["service"])
+        juju_log("WARN: Invalid service requested: '%s'" % relation_data["service"])
         realtion_set({ "admin_token": -1 })
         return
     import manager
@@ -90,6 +91,7 @@ def identity_changed():
                              relation_data["admin_url"],
                              relation_data["internal_url"])
     token = generate_admin_token(manager, config)
+    # we return a token and information about our API endpoints
     relation_data = {
         "admin_token": token,
         "service_host": config["hostname"],
@@ -100,13 +102,18 @@ def identity_changed():
     relation_set(relation_data)
 
 def keystone_joined():
-    """ the keystone relations are here specifically for horizon since it
+    """
-        provide an API endpoint like other services but requires a valid
+    The keystone-service relations exist specifically for horizon.  Instead of
-        role and token to exist in keystone. it also needs to be informed
+    a token and entry in the service catalog (like other services), it requires
-        of *our* api endpoints (admin and auth) """
+    a token and a role.  It doesn't expose any endpoint that keystone needs to
+    care about, thus it relates to this unit via its own service relation.
+    """
     pass
 
 def keystone_changed():
+    """ Horizon will request a default role, we create it and return a
+        token
+    """
     import manager
     options = ["role"]
     relation_data = relation_get(options)
@@ -116,6 +123,8 @@ def keystone_changed():
     # create the requested admin role
     create_role(manager, relation_data["role"], config["admin-user"])
     token = generate_admin_token(manager, config)
+    # Return token and API ports.  Let horizon find our hostname via
+    # 'relation-get private-address'
     relation_data = {
         "service_port": config["service-port"],
         "auth_port": config["admin-port"],
@@ -133,9 +142,9 @@ hooks = {
     "keystone-service-relation-changed": keystone_changed
 }
 
+# keystone-hooks gets called by symlink corresponding to the requested relation
+# hook.
 arg0 = sys.argv[0].split("/").pop()
 if arg0 not in hooks.keys():
     error_out("Unsupported hook: %s" % arg0)
-
 hooks[arg0]()
-

hooks/manager.py

@@ -1,5 +1,6 @@
 #!/usr/bin/python
-
+#
+# Bootstraps the keystone API so we can utilize its managment API natively
 import keystone.manage
 import keystone.backends as db
 from keystone import version
@@ -12,4 +13,3 @@ parser = RaisingOptionParser("", version='%%prog %s'
 (options, args) = config.parse_options(parser)
 _config_file, conf = config.load_paste_config('admin', options, args)
 db.configure_backends(conf.global_conf)
-

hooks/utils.py

@@ -10,9 +10,11 @@ stored_passwd = "/var/lib/keystone/keystone.passwd"
 
 def execute(cmd, die=False, echo=False):
     """ Executes a command 
-        if die=True, script will exit(1) if command does not return 0
+
-        if echo=True, output of command will be printed to stdout
+    if die=True, script will exit(1) if command does not return 0
-        returns a tuple: (stdout, stderr, return code)
+    if echo=True, output of command will be printed to stdout
+
+    returns a tuple: (stdout, stderr, return code)
     """
     p = subprocess.Popen(cmd.split(" "),
                          stdout=subprocess.PIPE,
@@ -49,7 +51,12 @@ def error_out(msg):
     exit(1)
 
 def setup_ppa(rel):
-    """ currently the keystone-core team only publishes a trunk PPA """
+    """ Configure a PPA prior to installing.
+    Currently, keystone-core only maintains a trunk PPA (unlike other
+    subprojects that maintain one for milestone + milestone-proposed)
+    Currently, supported options are 'trunk' or a custom PPA passed to config
+    as 'ppa:someproject/someppa'
+    """
     if rel == "trunk":
         ppa = "ppa:keystone-core/trunk"
     elif rel[:4] == "ppa:":
@@ -59,8 +66,11 @@ def setup_ppa(rel):
     execute(("add-apt-repository -y %s" % ppa), die=True, echo=True)
 
 def config_get():
-    """ return a dict representing the output of config-get 
+    """ Obtain the units config via 'config-get' 
-        private-address and IP of the unit is also tacked on """
+    Returns a dict representing current config.
+    private-address and IP of the unit is also tacked on for
+    convienence
+    """
     output = execute("config-get --format json")[0]
     config = json.loads(output)
     # make sure no config element is blank after config-get
@@ -80,11 +90,13 @@ def relation_set(relation_data):
         execute("relation-set %s=%s" % (k, relation_data[k]), die=True)
 
 def relation_get(relation_data):
-    """ takes a list of options to query from the relation
+    """ Obtain all current relation data
-        returns a k,v dict of the results. 
+    relation_data is a list of options to query from the relation
-        leave empty responses out of the results as they haven't yet been
+    Returns a k,v dict of the results. 
-        set on the other end. caller expects
+    Leave empty responses out of the results as they haven't yet been
-        len(results.keys()) == len(relation_data)
+    set on the other end. 
+    Caller can then "len(results.keys()) == len(relation_data)" to find out if
+    all relation values have been set on the other side
     """
     results = {}
     for r in relation_data:
@@ -94,10 +106,10 @@ def relation_get(relation_data):
     return results
 
 def keystone_conf_update(opt, val):
-    """ updates keystone.conf values 
+    """ Updates keystone.conf values 
-        if option exists, it is reset to new value
+    If option exists, it is reset to new value
-        if it does not, it added to the top of the config file
+    If it does not, it added to the top of the config file after the [DEFAULT]
-        after the [DEFAULT] heading
+    heading to keep it out of the paste deploy config
     """
     f = open(keystone_conf, "r+")
     orig = f.readlines()
@@ -178,7 +190,7 @@ def create_role(manager, name, user):
         juju_log("Created new role '%s'" % name)
     else:
         juju_log("A role named '%s' already exists" % name)
-    # TODO Doesn't seem to be anyway of querying current role assignments?
+    # NOTE: There does not seem to be any way of querying current role asignment
     manager.api.grant_role(name, user)
     juju_log("Granted role '%s' to '%s'" % (name, user))
 
@@ -196,6 +208,8 @@ def ensure_initial_admin(config):
         run during install as well as during db-changed.  This will maintain
         the admin tenant, user, role, service entry and endpoint across every
         datastore we might use. 
+        TODO: Possibly migrate data from one backend to another after it
+        changes?
     """
     import manager
     create_tenant(manager, "admin")