From 8717436db99a84588532e792f8c9ea909f3e3628 Mon Sep 17 00:00:00 2001 From: Steve Baker Date: Thu, 1 Sep 2016 11:57:17 +1200 Subject: [PATCH] Treat ec2 collector data as immutable Currently the ec2 collector polls the nova metadata service every $polling_period even though the data is not expected to change and no known config actions have been written to respond to changes in these values. With larger overclouds, this metadata polling will cause noticeable load on the undercloud, especially nova-api and neutron (for port lookups). This change allows collect calls to raise a SourceAlreadyCollected exception if the data for this collector never changes. The ec2 collector raises this if its data has already been cached to /var/lib/os-collect-config/ec2.json Change-Id: Ib7df590601132857690c8ab64fe32098a81752d8 Closes-Bug: #1619072 --- os_collect_config/collect.py | 4 ++++ os_collect_config/ec2.py | 6 ++++++ os_collect_config/exc.py | 8 ++++++++ os_collect_config/tests/test_collect.py | 8 ++++++++ os_collect_config/tests/test_ec2.py | 15 +++++++++++++++ 5 files changed, 41 insertions(+) diff --git a/os_collect_config/collect.py b/os_collect_config/collect.py index f519dfb..de19a7a 100644 --- a/os_collect_config/collect.py +++ b/os_collect_config/collect.py @@ -170,6 +170,10 @@ def collect_all(collectors, store=False, collector_kwargs_map=None): except exc.SourceNotConfigured: logger.debug('Source [%s] Not configured.' % collector) continue + except exc.SourceAlreadyCollected: + logger.debug('Source [%s] Already collected and cached.' + % collector) + continue if store: for output_key, output_content in content: diff --git a/os_collect_config/ec2.py b/os_collect_config/ec2.py index d652922..03a0ce6 100644 --- a/os_collect_config/ec2.py +++ b/os_collect_config/ec2.py @@ -13,9 +13,12 @@ # See the License for the specific language governing permissions and # limitations under the License. +import os + from oslo_config import cfg from oslo_log import log +from os_collect_config import cache from os_collect_config import common from os_collect_config import exc @@ -60,5 +63,8 @@ class Collector(object): return content def collect(self): + cache_path = cache.get_path('ec2') + if os.path.exists(cache_path): + raise exc.Ec2MetadataAlreadyCollected() root_url = '%s/' % (CONF.ec2.metadata_url) return [('ec2', self._fetch_metadata(root_url, CONF.ec2.timeout))] diff --git a/os_collect_config/exc.py b/os_collect_config/exc.py index 73a9687..37ab039 100644 --- a/os_collect_config/exc.py +++ b/os_collect_config/exc.py @@ -22,10 +22,18 @@ class SourceNotConfigured(RuntimeError): """The requested data source is not configured.""" +class SourceAlreadyCollected(RuntimeError): + """The requested data source is immutable and already cached.""" + + class Ec2MetadataNotAvailable(SourceNotAvailable): """The EC2 metadata service is not available.""" +class Ec2MetadataAlreadyCollected(SourceAlreadyCollected): + """The EC2 metadata has already been fetched and cached.""" + + class CfnMetadataNotAvailable(SourceNotAvailable): """The cfn metadata service is not available.""" diff --git a/os_collect_config/tests/test_collect.py b/os_collect_config/tests/test_collect.py index 5f4ef3f..05f8f32 100644 --- a/os_collect_config/tests/test_collect.py +++ b/os_collect_config/tests/test_collect.py @@ -447,6 +447,10 @@ class TestCollectAll(testtools.TestCase): cache.commit(changed) (changed_keys, paths2) = self._call_collect_all(store=True) self.assertEqual(set(), changed_keys) + + # check the second collect skips ec2, it has already been cached. + ec2_path = os.path.join(self.cache_dir.path, 'ec2.json') + paths.remove(ec2_path) self.assertEqual(paths, paths2) def test_collect_all_no_change_softwareconfig(self): @@ -477,6 +481,10 @@ class TestCollectAll(testtools.TestCase): (changed_keys, paths2) = self._call_collect_all( store=True, collector_kwargs_map=soft_config_map) self.assertEqual(set(), changed_keys) + + # check the second collect skips ec2, it has already been cached. + ec2_path = os.path.join(self.cache_dir.path, 'ec2.json') + paths.remove(ec2_path) self.assertEqual(paths, paths2) def test_collect_all_nostore(self): diff --git a/os_collect_config/tests/test_ec2.py b/os_collect_config/tests/test_ec2.py index 7b27d96..e2a0e04 100644 --- a/os_collect_config/tests/test_ec2.py +++ b/os_collect_config/tests/test_ec2.py @@ -13,9 +13,12 @@ # See the License for the specific language governing permissions and # limitations under the License. +import json +import os import uuid import fixtures +from oslo_config import cfg import requests import six.moves.urllib.parse as urlparse import testtools @@ -114,3 +117,15 @@ class TestEc2(testtools.TestCase): collect_ec2 = ec2.Collector(requests_impl=FakeFailRequests) self.assertRaises(exc.Ec2MetadataNotAvailable, collect_ec2.collect) self.assertIn('Forbidden', self.log.output) + + def test_collect_ec2_collected(self): + collect.setup_conf() + cache_dir = self.useFixture(fixtures.TempDir()) + self.addCleanup(cfg.CONF.reset) + cfg.CONF.set_override('cachedir', cache_dir.path) + ec2_path = os.path.join(cache_dir.path, 'ec2.json') + with open(ec2_path, 'w') as f: + json.dump(META_DATA, f) + + collect_ec2 = ec2.Collector(requests_impl=FakeFailRequests) + self.assertRaises(exc.Ec2MetadataAlreadyCollected, collect_ec2.collect)