From 822000955100179f9f5944e0b2403c95fac39458 Mon Sep 17 00:00:00 2001 From: Jay Faulkner Date: Mon, 8 Sep 2014 21:32:21 -0700 Subject: Update read_config_drive to use OS_VERSIONS tuple for readers Updated read_config_drive: removed the unused version kwarg, used the OS_VERSIONS tuple from the openstack helper to avoid hardcoding versions. Added a comment to the tuple in helpers/openstack.py asking for it to be kept in chronological order. --- cloudinit/sources/helpers/openstack.py | 1 + 1 file changed, 1 insertion(+) (limited to 'cloudinit/sources/helpers/openstack.py') diff --git a/cloudinit/sources/helpers/openstack.py b/cloudinit/sources/helpers/openstack.py index 0fac0335..ed102c4c 100644 --- a/cloudinit/sources/helpers/openstack.py +++ b/cloudinit/sources/helpers/openstack.py @@ -48,6 +48,7 @@ OS_LATEST = 'latest' OS_FOLSOM = '2012-08-10' OS_GRIZZLY = '2013-04-04' OS_HAVANA = '2013-10-17' +# keep this in chronological order by time: add new entries to the end OS_VERSIONS = ( OS_FOLSOM, OS_GRIZZLY, -- cgit v1.2.3 From c4b09a27239bc88bcf6b4ec536410bcc02cdf11c Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Wed, 10 Sep 2014 15:37:09 -0400 Subject: make BaseReader select latest supported version --- cloudinit/sources/DataSourceConfigDrive.py | 16 ++++++---------- cloudinit/sources/DataSourceOpenStack.py | 3 +-- cloudinit/sources/helpers/openstack.py | 19 +++++++++++++------ 3 files changed, 20 insertions(+), 18 deletions(-) (limited to 'cloudinit/sources/helpers/openstack.py') diff --git a/cloudinit/sources/DataSourceConfigDrive.py b/cloudinit/sources/DataSourceConfigDrive.py index 83cc6b25..b8c16361 100644 --- a/cloudinit/sources/DataSourceConfigDrive.py +++ b/cloudinit/sources/DataSourceConfigDrive.py @@ -167,17 +167,13 @@ def get_ds_mode(cfgdrv_ver, ds_cfg=None, user=None): return "net" -def read_config_drive(source_dir): - excps = [] - finders = [] +def read_config_drive(source_dir, version=None): reader = openstack.ConfigDriveReader(source_dir) - - # openstack.OS_VERSIONS is stored in chronological order, so to check the - # newest first, use reversed() - for version in reversed(openstack.OS_VERSIONS): - finders.append((reader.read_v2, [], {'version': version})) - finders.append((reader.read_v1, [], {})) - + finders = [ + (reader.read_v2, [], {'version': version}), + (reader.read_v1, [], {}), + ] + excps = [] for (functor, args, kwargs) in finders: try: return functor(*args, **kwargs) diff --git a/cloudinit/sources/DataSourceOpenStack.py b/cloudinit/sources/DataSourceOpenStack.py index 0970d07b..ce8e8364 100644 --- a/cloudinit/sources/DataSourceOpenStack.py +++ b/cloudinit/sources/DataSourceOpenStack.py @@ -119,8 +119,7 @@ class DataSourceOpenStack(openstack.SourceMixin, sources.DataSource): 'Crawl of openstack metadata service', read_metadata_service, args=[self.metadata_address], - kwargs={'ssl_details': self.ssl_details, - 'version': openstack.OS_HAVANA}) + kwargs={'ssl_details': self.ssl_details}) except openstack.NonReadable: return False except (openstack.BrokenMetadata, IOError): diff --git a/cloudinit/sources/helpers/openstack.py b/cloudinit/sources/helpers/openstack.py index ed91a55c..ed9cdbaa 100644 --- a/cloudinit/sources/helpers/openstack.py +++ b/cloudinit/sources/helpers/openstack.py @@ -48,7 +48,7 @@ OS_LATEST = 'latest' OS_FOLSOM = '2012-08-10' OS_GRIZZLY = '2013-04-04' OS_HAVANA = '2013-10-17' -# keep this in chronological order by time: add new entries to the end +# keep this in chronological order. new supported versions go at the end. OS_VERSIONS = ( OS_FOLSOM, OS_GRIZZLY, @@ -162,7 +162,7 @@ class BaseReader(object): def _read_ec2_metadata(self): pass - def _find_working_version(self, version): + def _find_working_version(self, version=None): try: versions_available = self._fetch_available_versions() except Exception as e: @@ -170,7 +170,14 @@ class BaseReader(object): self.base_path, e) versions_available = [] - search_versions = [version] + list(OS_VERSIONS) + # openstack.OS_VERSIONS is stored in chronological order, so + # reverse it to check newest first. + supported = [v for v in reversed(list(OS_VERSIONS))] + if version is not None: + search_versions = [version] + supported + else: + search_versions = supported + selected_version = OS_LATEST for potential_version in search_versions: if potential_version not in versions_available: @@ -178,11 +185,12 @@ class BaseReader(object): selected_version = potential_version break - if selected_version != version: + if version is not None and selected_version != version: LOG.warn("Version '%s' not available, attempting to use " "version '%s' instead", version, selected_version) else: - LOG.debug("Version '%s' was available.", version) + LOG.debug("Selected version '%s' from %s", version, + versions_available) return selected_version def _read_content_path(self, item): @@ -434,7 +442,6 @@ class MetadataReader(BaseReader): self._versions = found return self._versions - def _path_read(self, path): def should_retry_cb(_request_args, cause): -- cgit v1.2.3 From c548fdf3201519c1c30c815ba9feec643b87e0bf Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Wed, 10 Sep 2014 15:40:30 -0400 Subject: fix log message --- cloudinit/sources/helpers/openstack.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'cloudinit/sources/helpers/openstack.py') diff --git a/cloudinit/sources/helpers/openstack.py b/cloudinit/sources/helpers/openstack.py index ed9cdbaa..3d903a3c 100644 --- a/cloudinit/sources/helpers/openstack.py +++ b/cloudinit/sources/helpers/openstack.py @@ -189,7 +189,7 @@ class BaseReader(object): LOG.warn("Version '%s' not available, attempting to use " "version '%s' instead", version, selected_version) else: - LOG.debug("Selected version '%s' from %s", version, + LOG.debug("Selected version '%s' from %s", selected_version, versions_available) return selected_version -- cgit v1.2.3 From 44113217719ebee756325b40a5d14045ba8f3a3a Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Wed, 10 Sep 2014 16:00:00 -0400 Subject: drop version= from readers instead of taking a version that they should look for, the readers now just select the highest supported version. definitely a use case later for having version= but nothing is using it now. --- cloudinit/sources/DataSourceConfigDrive.py | 4 ++-- cloudinit/sources/DataSourceOpenStack.py | 4 ++-- cloudinit/sources/helpers/openstack.py | 23 +++++++---------------- tests/unittests/test_datasource/test_openstack.py | 16 ++++++++-------- 4 files changed, 19 insertions(+), 28 deletions(-) (limited to 'cloudinit/sources/helpers/openstack.py') diff --git a/cloudinit/sources/DataSourceConfigDrive.py b/cloudinit/sources/DataSourceConfigDrive.py index b8c16361..a27d07fb 100644 --- a/cloudinit/sources/DataSourceConfigDrive.py +++ b/cloudinit/sources/DataSourceConfigDrive.py @@ -167,10 +167,10 @@ def get_ds_mode(cfgdrv_ver, ds_cfg=None, user=None): return "net" -def read_config_drive(source_dir, version=None): +def read_config_drive(source_dir): reader = openstack.ConfigDriveReader(source_dir) finders = [ - (reader.read_v2, [], {'version': version}), + (reader.read_v2, [], {}), (reader.read_v1, [], {}), ] excps = [] diff --git a/cloudinit/sources/DataSourceOpenStack.py b/cloudinit/sources/DataSourceOpenStack.py index ce8e8364..466de8f4 100644 --- a/cloudinit/sources/DataSourceOpenStack.py +++ b/cloudinit/sources/DataSourceOpenStack.py @@ -153,9 +153,9 @@ class DataSourceOpenStack(openstack.SourceMixin, sources.DataSource): return True -def read_metadata_service(base_url, version=None, ssl_details=None): +def read_metadata_service(base_url, ssl_details=None): reader = openstack.MetadataReader(base_url, ssl_details=ssl_details) - return reader.read_v2(version=version) + return reader.read_v2() # Used to match classes to dependencies diff --git a/cloudinit/sources/helpers/openstack.py b/cloudinit/sources/helpers/openstack.py index 3d903a3c..a7dd05df 100644 --- a/cloudinit/sources/helpers/openstack.py +++ b/cloudinit/sources/helpers/openstack.py @@ -162,7 +162,7 @@ class BaseReader(object): def _read_ec2_metadata(self): pass - def _find_working_version(self, version=None): + def _find_working_version(self): try: versions_available = self._fetch_available_versions() except Exception as e: @@ -173,24 +173,16 @@ class BaseReader(object): # openstack.OS_VERSIONS is stored in chronological order, so # reverse it to check newest first. supported = [v for v in reversed(list(OS_VERSIONS))] - if version is not None: - search_versions = [version] + supported - else: - search_versions = supported - selected_version = OS_LATEST - for potential_version in search_versions: + + for potential_version in supported: if potential_version not in versions_available: continue selected_version = potential_version break - if version is not None and selected_version != version: - LOG.warn("Version '%s' not available, attempting to use " - "version '%s' instead", version, selected_version) - else: - LOG.debug("Selected version '%s' from %s", selected_version, - versions_available) + LOG.debug("Selected version '%s' from %s", selected_version, + versions_available) return selected_version def _read_content_path(self, item): @@ -202,7 +194,7 @@ class BaseReader(object): path = self._path_join(self.base_path, "openstack", *path_pieces) return self._path_read(path) - def read_v2(self, version=None): + def read_v2(self): """Reads a version 2 formatted location. Return a dict with metadata, userdata, ec2-metadata, dsmode, @@ -233,12 +225,11 @@ class BaseReader(object): ) return files - version = self._find_working_version(version) results = { 'userdata': '', 'version': 2, } - data = datafiles(version) + data = datafiles(self._find_working_version()) for (name, (path, required, translator)) in data.iteritems(): path = self._path_join(self.base_path, path) data = None diff --git a/tests/unittests/test_datasource/test_openstack.py b/tests/unittests/test_datasource/test_openstack.py index 530fba20..6809823e 100644 --- a/tests/unittests/test_datasource/test_openstack.py +++ b/tests/unittests/test_datasource/test_openstack.py @@ -142,7 +142,7 @@ class TestOpenStackDataSource(test_helpers.HttprettyTestCase): @hp.activate def test_successful(self): _register_uris(self.VERSION, EC2_FILES, EC2_META, OS_FILES) - f = ds.read_metadata_service(BASE_URL, version=self.VERSION) + f = ds.read_metadata_service(BASE_URL) self.assertEquals(VENDOR_DATA, f.get('vendordata')) self.assertEquals(CONTENT_0, f['files']['/etc/foo.cfg']) self.assertEquals(CONTENT_1, f['files']['/etc/bar/bar.cfg']) @@ -164,7 +164,7 @@ class TestOpenStackDataSource(test_helpers.HttprettyTestCase): @hp.activate def test_no_ec2(self): _register_uris(self.VERSION, {}, {}, OS_FILES) - f = ds.read_metadata_service(BASE_URL, version=self.VERSION) + f = ds.read_metadata_service(BASE_URL) self.assertEquals(VENDOR_DATA, f.get('vendordata')) self.assertEquals(CONTENT_0, f['files']['/etc/foo.cfg']) self.assertEquals(CONTENT_1, f['files']['/etc/bar/bar.cfg']) @@ -180,7 +180,7 @@ class TestOpenStackDataSource(test_helpers.HttprettyTestCase): os_files.pop(k, None) _register_uris(self.VERSION, {}, {}, os_files) self.assertRaises(openstack.NonReadable, ds.read_metadata_service, - BASE_URL, version=self.VERSION) + BASE_URL) @hp.activate def test_bad_uuid(self): @@ -192,7 +192,7 @@ class TestOpenStackDataSource(test_helpers.HttprettyTestCase): os_files[k] = json.dumps(os_meta) _register_uris(self.VERSION, {}, {}, os_files) self.assertRaises(openstack.BrokenMetadata, ds.read_metadata_service, - BASE_URL, version=self.VERSION) + BASE_URL) @hp.activate def test_userdata_empty(self): @@ -201,7 +201,7 @@ class TestOpenStackDataSource(test_helpers.HttprettyTestCase): if k.endswith('user_data'): os_files.pop(k, None) _register_uris(self.VERSION, {}, {}, os_files) - f = ds.read_metadata_service(BASE_URL, version=self.VERSION) + f = ds.read_metadata_service(BASE_URL) self.assertEquals(VENDOR_DATA, f.get('vendordata')) self.assertEquals(CONTENT_0, f['files']['/etc/foo.cfg']) self.assertEquals(CONTENT_1, f['files']['/etc/bar/bar.cfg']) @@ -214,7 +214,7 @@ class TestOpenStackDataSource(test_helpers.HttprettyTestCase): if k.endswith('vendor_data.json'): os_files.pop(k, None) _register_uris(self.VERSION, {}, {}, os_files) - f = ds.read_metadata_service(BASE_URL, version=self.VERSION) + f = ds.read_metadata_service(BASE_URL) self.assertEquals(CONTENT_0, f['files']['/etc/foo.cfg']) self.assertEquals(CONTENT_1, f['files']['/etc/bar/bar.cfg']) self.assertFalse(f.get('vendordata')) @@ -227,7 +227,7 @@ class TestOpenStackDataSource(test_helpers.HttprettyTestCase): os_files[k] = '{' # some invalid json _register_uris(self.VERSION, {}, {}, os_files) self.assertRaises(openstack.BrokenMetadata, ds.read_metadata_service, - BASE_URL, version=self.VERSION) + BASE_URL) @hp.activate def test_metadata_invalid(self): @@ -237,7 +237,7 @@ class TestOpenStackDataSource(test_helpers.HttprettyTestCase): os_files[k] = '{' # some invalid json _register_uris(self.VERSION, {}, {}, os_files) self.assertRaises(openstack.BrokenMetadata, ds.read_metadata_service, - BASE_URL, version=self.VERSION) + BASE_URL) @hp.activate def test_datasource(self): -- cgit v1.2.3