Merge lp:~pjdc/glance-simplestreams-sync-charm/image-modifier-hook into lp:glance-simplestreams-sync-charm

[Paul Collins]

Canonical IS uses glance-simplestreams-sync for an internal project.

We have made some changes to g-s-s support this project, as follows:

 * add a new hook, image-modifier, using a new interface, script-provider

   We use this hook to relate a subordinate charm that contains
   scripts used to pre-process images as they are synced to support
   our use cases. (One of these use cases involves spawning
   single-use instances to perform jobs that may take less than a
   minute, so we pre-install daemon packages, etc. to avoid losing too
   much time to things that be done to the image ahead of time.)

 * add a subclass of GlanceMirror, FilteredGlanceMirror, and use it

   GlanceMirror's filtering logic is hardcoded to a value that does
   not cover all of our anticipated use cases.

 * add two new config items, name_prefix and content_id_template

   name_prefix is used by our image-selection logic, and
   content_id_template is used so that multiple disjoint sets of
   images can be mirrored from the same source without interfering
   with each other. For example, one set of images via a g-s-s
   deployed with an image-modifier subordinate charm (see above) and a
   second g-s-s mirroring unmodified vanilla images.

 * run exec.d/*/charm-pre-install at the appropriate times

Please let me know what you think.

[Mike McCracken (mikemc)]

Hi Paul, thanks for the work on this, I'm glad it's proving useful for you in IS.
Sorry for the slow response, this MP got buried in email at first.

The LP repo is a mirror - the main development of the sync charm is on github here:
https://github.com/Ubuntu-Solutions-Engineering/glance-simplestreams-sync-charm
If you're comfortable using git/github, it'd be easiest to merge things via PR's there.
If you'd prefer to use bzr&lp, I have no problems with reviewing
branches here and doing the patches myself in git.

As a general comment, I'd prefer to review separate branches for
separate changes: eg, one for adding the image-modifier relation and
one for adding filter config, etc. It makes it easier to approve one
change and get it merged while asking for feedback or changes on the others.

 - The image modifier script subordinate charm is a nice approach, we
   were thinking about how to support modify_hook here, and did not
   like the possibility of writing scripts quoted inside the yaml
   config.

 - As for the item filter, that subclass is a solid workaround for
   simplestreams not supporting it, but we probably don't need it - I
   fixed it in simplestreams upstream, so GlanceMirror in
   simplestreams in utopic now does pay attention to the item_filter
   args, while I'm pushing an SRU through to trusty to get it added
   there: https://bugs.launchpad.net/simplestreams/+bug/1339842 (Your
   adding a comment there might be useful to help push it, I'm not
   familiar with the process) If you need it on any other series,
   let's discuss.

   I also have a trusty build with that change (and one other, to
   support per-file download progress updates in glancemirror) in this
   PPA:
   https://launchpad.net/~cloud-installer/+archive/ubuntu/simplestreams-testing

 - It's nice to have the prefix and content-id settings fixed by
   someone who knows what they need there. Thanks!

 - Thanks for adding the execd support, too. I wasn't aware of its
   existence.

review: Needs Fixing

[Mike McCracken (mikemc)]

I'm currently breaking this branch into separate branches on the github repo, and will post the links here when I'm done.

[Mike McCracken (mikemc)]

FYI, those three branches are here:

https://github.com/Ubuntu-Solutions-Engineering/glance-simplestreams-sync-charm/pull/17

https://github.com/Ubuntu-Solutions-Engineering/glance-simplestreams-sync-charm/pull/18

https://github.com/Ubuntu-Solutions-Engineering/glance-simplestreams-sync-charm/pull/19

-- I've left the filter support out for now, but am happy to discuss what to do with it.

[Tom Haddon (mthaddon)]

Hi Mike,

Thanks for doing those. Is there anything we/Paul should be doing on this end of things, or should we just monitor these pull requests and see as updates get applied or otherwise in github and then pulled back into LP?

Thanks, Tom

[Mike McCracken (mikemc)]

Hi Tom, I'd appreciate if someone (I guess Paul, or anyone else who's familiar with these changes) took a quick look at the pull requests and gave a +1.
There were a couple of minor changes, so I'd like to get a sanity check.

Also, just one clarification question:

Are you OK with waiting for a simplestreams SRU to get filtering on trusty or do you need a version of this charm that does its own filtering?

[Paul Collins (pjdc)]

Hi Mike,

I've eyeballed a diff between a Hulk-smash of your three PR branches and our version of the charm and the result looks reasonable to me.

Re glance filtering, we're happy to maintain GlanceFilteredMirror in our branch of the charm until the SRU is approved.

Cheers,
Paul

[Paul Collins (pjdc)]

These substance of these changes was upstreamed some time ago, so I'll mark this MP as merged.

=== modified file 'config.yaml'
--- config.yaml	2014-07-08 22:49:07 +0000
+++ config.yaml	2014-07-10 04:09:22 +0000
@@ -29,3 +29,28 @@
     type: string
     default: "glance-simplestreams-sync-openstack"
     description: "Cloud name to be used in simplestreams index file"
+  name_prefix:
+    type: string
+    default: "auto-sync/"
+    description: "This is prefixed to the object name when uploading to glance."
+  content_id_template:
+    type: string
+    default: "auto.sync"
+    description: >
+      A Python-style .format() template to use when generating
+      content_id properties for images uploaded to glance.
+
+      The content_id is considered when matching images between the
+      source and destination to decide which images to mirror.  By
+      varying this value you can mirror disjoint sets of images from
+      the same source into a single glance, either by using multiple
+      deployments of this charm, or by using a tool such as
+      sstream-mirror-glance, and they will not interfere with each
+      other.
+
+      Here is a more interesting example value:
+
+          com.example.customstack.{region}:ubuntu:celery-worker
+
+      Currently the only available substitution is "region".  Any
+      other attempted substitutions will break the sync script.

=== added directory 'exec.d'
=== modified file 'hooks/hooks.py'
--- hooks/hooks.py	2014-07-02 18:49:19 +0000
+++ hooks/hooks.py	2014-07-10 04:09:22 +0000
@@ -26,8 +26,9 @@
 import sys
 import shutil
 
+from charmhelpers.core import hookenv
 from charmhelpers.fetch import apt_install
-from charmhelpers.core import hookenv
+from charmhelpers.payload.execd import execd_preinstall
 
 from charmhelpers.contrib.openstack.context import (IdentityServiceContext,
                                                     OSContextGenerator)
@@ -49,15 +50,42 @@
 hooks = hookenv.Hooks()
 
 
+class MultipleImageModifierSubordinatesIsNotSupported(Exception):
+    """Raise this if multiple image-modifier subordinates are related to this charm."""
+    pass
+
+
 class MirrorsConfigServiceContext(OSContextGenerator):
-    """Context for mirrors.yaml template - does not use relation info.
+    """Context for mirrors.yaml template.
     """
     interfaces = ['simplestreams-image-service']
 
     def __call__(self):
         hookenv.log("Generating template ctxt for simplestreams-image-service")
         config = hookenv.config()
+
+        modify_hook_scripts = []
+        image_modifiers = hookenv.relations_of_type('image-modifier')
+        if len(image_modifiers) > 1:
+            raise MultipleImageModifierSubordinatesIsNotSupported()
+
+        try:
+            modify_hook_scripts.append(image_modifiers[0]['script-path'])
+        except IndexError:      # no subordinate
+            pass
+        except KeyError as ke:
+            print 'relation {} yielded exception {} - forging ahead'.format(
+                repr(relation), repr(ke))
+
+        # default no-op so that None still means "missing" for config
+        # validation (see elsewhere)
+        if len(modify_hook_scripts) == 0:
+            modify_hook_scripts.append('/bin/true')
+
         return dict(mirror_list=config['mirror_list'],
+                    modify_hook_scripts=', '.join(modify_hook_scripts),
+                    name_prefix=config['name_prefix'],
+                    content_id_template=config['content_id_template'],
                     use_swift=config['use_swift'],
                     region=config['region'],
                     cloud_name=config['cloud_name'])
@@ -135,6 +163,7 @@
 
 @hooks.hook('install')
 def install():
+    execd_preinstall()
     for directory in [CONF_FILE_DIR, USR_SHARE_DIR]:
         hookenv.log("creating config dir at {}".format(directory))
         if not os.path.isdir(directory):
@@ -151,7 +180,9 @@
     hookenv.log('end install hook.')
 
 
[e-mail]('config-changed')
[e-mail]('config-changed',
+            'image-modifier-relation-changed',
+            'image-modifier-relation-joined')
 def config_changed():
     hookenv.log('begin config-changed hook.')
 

=== added symlink 'hooks/image-modifier-relation-changed'
=== target is u'hooks.py'
=== added symlink 'hooks/image-modifier-relation-joined'
=== target is u'hooks.py'
=== modified file 'metadata.yaml'
--- metadata.yaml	2014-05-27 13:36:53 +0000
+++ metadata.yaml	2014-07-10 04:09:22 +0000
@@ -10,6 +10,9 @@
 provides:
   simplestreams-image-service:
     interface: glance-simplestreams-sync
+  image-modifier:
+    scope: container
+    interface: script-provider
 requires:
   identity-service:
     interface: keystone

=== modified file 'scripts/glance-simplestreams-sync.py'
--- scripts/glance-simplestreams-sync.py	2014-06-18 18:15:29 +0000
+++ scripts/glance-simplestreams-sync.py	2014-07-10 04:09:22 +0000
@@ -46,6 +46,7 @@
 import atexit
 from keystoneclient.v2_0 import client as keystone_client
 import os
+from simplestreams import filters
 from simplestreams.mirrors import glance, UrlMirrorReader
 from simplestreams.objectstores.swift import SwiftObjectStore
 from simplestreams.util import read_signed, path_from_mirror_url
@@ -71,13 +72,30 @@
 
 CRON_POLL_FILENAME = '/etc/cron.d/glance_simplestreams_sync_fastpoll'
 
+# As hardcoded in GlanceMirror:
+DEFAULT_GLANCE_FILTERS = ['arch~(x86_64|amd64|i386)',
+                          'ftype~(disk1.img|disk.img)']
+
+class FilteredGlanceMirror(glance.GlanceMirror):
+    def __init__(self, config, objectstore=None, region=None, name_prefix=None):
+        super(FilteredGlanceMirror, self).__init__(config=config,
+                                                   objectstore=objectstore,
+                                                   region=region,
+                                                   name_prefix=name_prefix)
+        try:
+            self.filters = filters.get_filters(config['filters'])
+        except:
+            self.filters = filters.get_filters(DEFAULT_GLANCE_FILTERS)
+
+    def filter_item(self, data, src, target, pedigree):
+        return filters.filter_item(self.filters, data, src, pedigree)
+
+
 # TODOs:
 #   - allow people to specify their own policy, since they can specify
 #     their own mirrors.
 #   - potentially allow people to specify backup mirrors?
 #   - debug keyring support
-#   - figure out what content_id is and whether we should allow users to
-#     set it
 
 
 def policy(content, path):
@@ -142,13 +160,18 @@
             store = None
 
         config = {'max_items': mirror_info['max'],
+                  'modify_hook': charm_conf['modify_hook_scripts'],
                   'keep_items': False,
-                  'content_id': 'auto.sync',
+                  'content_id': charm_conf['content_id_template'].format(
+                      region=charm_conf['region'],
+                  ),
                   'cloud_name': charm_conf['cloud_name'],
-                  'item_filters': mirror_info['item_filters']}
+                  'filters': mirror_info['item_filters']}
 
-        tmirror = glance.GlanceMirror(config=config, objectstore=store)
-        log.info("calling GlanceMirror.sync")
+        tmirror = FilteredGlanceMirror(config=config,
+                                       objectstore=store,
+                                       name_prefix=charm_conf['name_prefix'])
+        log.info("calling FilteredGlanceMirror.sync")
         tmirror.sync(smirror, path=initial_path)
 
 

=== modified file 'templates/mirrors.yaml'
--- templates/mirrors.yaml	2014-06-18 17:48:54 +0000
+++ templates/mirrors.yaml	2014-07-10 04:09:22 +0000
@@ -1,4 +1,8 @@
 mirror_list: {{ mirror_list }}
+modify_hook_scripts: {{ modify_hook_scripts }}
+name_prefix: {{ name_prefix }}
 use_swift: {{ use_swift }}
 region: {{ region }}
 cloud_name: {{ cloud_name }}
+content_id_template: {{ content_id_template }}
+