Merge lp:~harlowja/cloud-init/smart-read into lp:~cloud-init-dev/cloud-init/trunk

[Joshua Harlow]

[lp:~harlowja/cloud-init/smart-read]

1003. By Joshua Harlow on 2014-08-27

Add logging around failure to read optional/mandatory paths

1004. By Joshua Harlow on 2014-08-27

Just use os.path.exists directly

1005. By Joshua Harlow on 2014-08-27

Log a warning when unable to fetch the openstack versions

1006. By Joshua Harlow on 2014-08-27

Show the available versions in the debug log message

1007. By Joshua Harlow on 2014-08-27

Fixed more of the slowness around fetching and retrying

1008. By Joshua Harlow on 2014-08-27

Fix retry cb to reflect what used to exist

=== modified file 'cloudinit/sources/helpers/openstack.py'
--- cloudinit/sources/helpers/openstack.py	2014-02-24 22:41:42 +0000
+++ cloudinit/sources/helpers/openstack.py	2014-08-27 19:59:20 +0000
@@ -150,17 +150,34 @@
         pass
 
     @abc.abstractmethod
-    def _path_exists(self, path):
-        pass
-
-    @abc.abstractmethod
     def _path_read(self, path):
         pass
 
     @abc.abstractmethod
+    def _fetch_available_versions(self):
+        pass
+
+    @abc.abstractmethod
     def _read_ec2_metadata(self):
         pass
 
+    def _find_working_version(self, version):
+        search_versions = [version] + list(OS_VERSIONS)
+        available_versions = self._fetch_available_versions()
+        for potential_version in search_versions:
+            if not potential_version:
+                continue
+            if potential_version not in available_versions:
+                continue
+            if potential_version != version:
+                LOG.debug("Version '%s' not available, attempting to use"
+                          " version '%s' instead", version,
+                          potential_version)
+                return potential_version
+        LOG.debug("Version '%s' not available, attempting to use '%s'"
+                  " instead", version, OS_LATEST)
+        return OS_LATEST
+
     def _read_content_path(self, item):
         path = item.get('content_path', '').lstrip("/")
         path_pieces = path.split("/")
@@ -170,23 +187,6 @@
         path = self._path_join(self.base_path, "openstack", *path_pieces)
         return self._path_read(path)
 
-    def _find_working_version(self, version):
-        search_versions = [version] + list(OS_VERSIONS)
-        for potential_version in search_versions:
-            if not potential_version:
-                continue
-            path = self._path_join(self.base_path, "openstack",
-                                   potential_version)
-            if self._path_exists(path):
-                if potential_version != version:
-                    LOG.debug("Version '%s' not available, attempting to use"
-                              " version '%s' instead", version,
-                              potential_version)
-                return potential_version
-        LOG.debug("Version '%s' not available, attempting to use '%s'"
-                  " instead", version, OS_LATEST)
-        return OS_LATEST
-
     def read_v2(self, version=None):
         """Reads a version 2 formatted location.
 
@@ -228,15 +228,18 @@
             path = self._path_join(self.base_path, path)
             data = None
             found = False
-            if self._path_exists(path):
-                try:
-                    data = self._path_read(path)
-                except IOError:
-                    raise NonReadable("Failed to read: %s" % path)
+            try:
+                data = self._path_read(path)
+            except IOError as e:
+                if not required:
+                    LOG.debug("Failed reading optional path %s due"
+                              " to: %s", path, e)
+                else:
+                    LOG.exception("Failed reading mandatory path %s", path)
+            else:
                 found = True
-            else:
-                if required:
-                    raise NonReadable("Missing mandatory path: %s" % path)
+            if required and not found:
+                raise NonReadable("Missing mandatory path: %s" % path)
             if found and translator:
                 try:
                     data = translator(data)
@@ -304,21 +307,35 @@
 class ConfigDriveReader(BaseReader):
     def __init__(self, base_path):
         super(ConfigDriveReader, self).__init__(base_path)
+        self._versions = None
 
     def _path_join(self, base, *add_ons):
         components = [base] + list(add_ons)
         return os.path.join(*components)
 
-    def _path_exists(self, path):
-        return os.path.exists(path)
-
     def _path_read(self, path):
         return util.load_file(path)
 
+    def _fetch_available_versions(self):
+        if self._versions is not None:
+            return self._versions
+        else:
+            versions_available = []
+            path = self._path_join(self.base_path, 'openstack')
+            try:
+                for child in os.listdir(path):
+                    child_path = os.path.join(path, child)
+                    if os.path.isdir(child_path):
+                        versions_available.append(child)
+            except (OSError, IOError):
+                pass
+            self._versions = tuple(versions_available)
+            return self._versions
+
     def _read_ec2_metadata(self):
         path = self._path_join(self.base_path,
                                'ec2', 'latest', 'meta-data.json')
-        if not self._path_exists(path):
+        if not os.path.exists(path):
             return {}
         else:
             try:
@@ -338,7 +355,7 @@
         found = {}
         for name in FILES_V1.keys():
             path = self._path_join(self.base_path, name)
-            if self._path_exists(path):
+            if os.path.exists(path):
                 found[name] = path
         if len(found) == 0:
             raise NonReadable("%s: no files found" % (self.base_path))
@@ -400,17 +417,31 @@
         self.ssl_details = ssl_details
         self.timeout = float(timeout)
         self.retries = int(retries)
+        self._versions = None
+
+    def _fetch_available_versions(self):
+        if self._versions is not None:
+            return self._versions
+        else:
+            path = self._path_join(self.base_path, "openstack")
+            versions_available = []
+            try:
+                versions = self._path_read(path)
+            except IOError as e:
+                LOG.warn("Unable to read openstack versions from %s due"
+                         " to: %s", path, e)
+            else:
+                for line in versions.splitlines():
+                    line = line.strip()
+                    if not line:
+                        continue
+                    versions_available.append(line)
+            self._versions = tuple(versions_available)
+            return self._versions
 
     def _path_read(self, path):
-        response = url_helper.readurl(path,
-                                      retries=self.retries,
-                                      ssl_details=self.ssl_details,
-                                      timeout=self.timeout)
-        return response.contents
-
-    def _path_exists(self, path):
-
-        def should_retry_cb(request, cause):
+
+        def should_retry_cb(_request_args, cause):
             try:
                 code = int(cause.code)
                 if code >= 400:
@@ -420,15 +451,12 @@
                 pass
             return True
 
-        try:
-            response = url_helper.readurl(path,
-                                          retries=self.retries,
-                                          ssl_details=self.ssl_details,
-                                          timeout=self.timeout,
-                                          exception_cb=should_retry_cb)
-            return response.ok()
-        except IOError:
-            return False
+        response = url_helper.readurl(path,
+                                      retries=self.retries,
+                                      ssl_details=self.ssl_details,
+                                      timeout=self.timeout,
+                                      exception_cb=should_retry_cb)
+        return response.contents
 
     def _path_join(self, base, *add_ons):
         return url_helper.combine_url(base, *add_ons)

=== modified file 'tests/unittests/test_datasource/test_openstack.py'
--- tests/unittests/test_datasource/test_openstack.py	2014-08-22 15:40:25 +0000
+++ tests/unittests/test_datasource/test_openstack.py	2014-08-27 19:59:20 +0000
@@ -67,8 +67,8 @@
 CONTENT_0 = 'This is contents of /etc/foo.cfg\n'
 CONTENT_1 = '# this is /etc/bar/bar.cfg\n'
 OS_FILES = {
-    'openstack/2012-08-10/meta_data.json': json.dumps(OSTACK_META),
-    'openstack/2012-08-10/user_data': USER_DATA,
+    'openstack/latest/meta_data.json': json.dumps(OSTACK_META),
+    'openstack/latest/user_data': USER_DATA,
     'openstack/content/0000': CONTENT_0,
     'openstack/content/0001': CONTENT_1,
     'openstack/latest/meta_data.json': json.dumps(OSTACK_META),
@@ -78,6 +78,9 @@
 EC2_FILES = {
     'latest/user-data': USER_DATA,
 }
+EC2_VERSIONS = [
+    'lastest',
+]
 
 
 def _register_uris(version, ec2_files, ec2_meta, os_files):
@@ -85,6 +88,9 @@
     same data returned by the openstack metadata service (and ec2 service)."""
 
     def match_ec2_url(uri, headers):
+        path = uri.path.strip("/")
+        if len(path) == 0:
+            return (200, headers, "\n".join(EC2_VERSIONS))
         path = uri.path.lstrip("/")
         if path in ec2_files:
             return (200, headers, ec2_files.get(path))
@@ -110,11 +116,20 @@
                 return (200, headers, str(value))
         return (404, headers, '')
 
+    def match_os_uri(uri, headers):
+        path = uri.path.strip("/")
+        if path == 'openstack':
+            return (200, headers, "\n".join([openstack.OS_LATEST]))
+        path = uri.path.lstrip("/")
+        if path in os_files:
+            return (200, headers, os_files.get(path))
+        return (404, headers, '')
+
     def get_request_callback(method, uri, headers):
         uri = urlparse(uri)
-        path = uri.path.lstrip("/")
-        if path in os_files:
-            return (200, headers, os_files.get(path))
+        path = uri.path.lstrip("/").split("/")
+        if path[0] == 'openstack':
+            return match_os_uri(uri, headers)
         return match_ec2_url(uri, headers)
 
     hp.register_uri(hp.GET, re.compile(r'http://169.254.169.254/.*'),