Merge lp:~ev/adt-image-mapper/fresh-keystone-client into lp:adt-image-mapper

[Evan]

Do not cache glanceclient so we get a fresh keystone auth token with every call.

I don't think it should be acceptable to use a bare exception to treat any error in talking to glance as 'expired token'. We can either explicitly look for 401 (glanceclient.exc.HTTPUnauthorized) or just get a fresh auth token from keystone with each call.

I'm proposing the latter to get the discussion moving. Is there a risk in requesting fresh tokens?

[Celso Providelo (cprov)]

Well, getting new token is just slow, as in *few* seconds ...

[Paul Larson (pwlars)]

It's true, I don't think we're going to be generating enough traffic to realize a huge gain from the caching. I figured it was good practice though, so I tried to preserve it. Checking for glanceclient.exc.HTTPUnauthorized is what I originally tried to do, but it doesn't come through as that exception. However, if it's true that this is fixed in the glanceclient that we just updated to, it could be that we can just revert my previous patch, keep the caching, and all will be happy.

[Evan (ev)]

I'll rework this with that as a comment explaining why we retry on 401
rather than just get a fresh token. Thank you.

[Francis Ginther (fginther)]

We could potentially see request storms, although with only one adt-cloud-service, this would be limited to one requester. So it would be more like a steady downpour then a storm. Not sure if this will have any adverse impact on the keystone or glance services. I would assume that they are robust enough to not fall over because we use it too much. Just thinking out loud.

Is it ok to immediately re-attempt the request, or do we need to add a delay.

candidate_image will also be None when there are no images matching the criteria (i.e. there are no vivid-i386 images available). This should be treated as a different failure mode.

review: Needs Fixing

[Evan (ev)]

review: Needs Resubmitting

[Francis Ginther (fginther)]

approve, thanks for reworking the exception handling.

review: Approve

[Ubuntu CI Bot (uci-bot)]

Attempt to merge into lp:adt-image-mapper failed due to conflicts:

text conflict in adt_image_mapper/cloud.py

[lp:~ev/adt-image-mapper/fresh-keystone-client]

16. By Evan on 2015-03-20

Merge with trunk.

=== modified file 'adt_image_mapper/cloud.py'
--- adt_image_mapper/cloud.py	2015-03-19 22:30:01 +0000
+++ adt_image_mapper/cloud.py	2015-03-20 16:46:10 +0000
@@ -16,6 +16,7 @@
 #
 
 import logging
+import time
 
 import keystoneclient
 import glanceclient
@@ -24,6 +25,10 @@
 logger = logging.getLogger(__name__)
 
 
+class CloudMapperException(Exception):
+    pass
+
+
 class CloudImageMapper(object):
 
     def __init__(self, config):
@@ -52,18 +57,25 @@
         return candidate_image
 
     def get_image_for_series(self, series_name, architecture):
-        logger.info("Searching for image with series=%r, arch=%r", series_name, architecture)
+        logger.info("Searching for image with series=%r, arch=%r", series_name,
+                    architecture)
         glance_client = self.get_glance_client()
-        try:
-            candidate_image = self._get_candidate_image(
-                glance_client, series_name, architecture)
-        except Exception as e:
-            logger.debug("The following exception is probably just the "
-                         "token needing to be refreshed")
-            logger.exception("Exception caught:")
-            glance_client = self.get_glance_client(refresh=True)
-            candidate_image = self._get_candidate_image(
-                glance_client, series_name, architecture)
+        glance_success = False
+        glance_wait = 2
+        for attempt in range(2,5):
+            try:
+                args = (glance_client, series_name, architecture)
+                candidate_image = self._get_candidate_image(*args)
+                glance_success = True
+            except glanceclient.exc.HTTPUnauthorized:
+                logger.exception("Error authenticating to Glance:")
+                glance_client = self.get_glance_client(refresh=True)
+                time.sleep(glance_wait)
+                glance_wait *= attempt
+        if not glance_success:
+            msg = 'Failed to talk to Glance too many times.'
+            raise CloudMapperException(msg)
+
         logger.info("Search finished, found image %s", candidate_image.name)
         return candidate_image.name if candidate_image else None