Commit changes to exploded deployments to cache
Before this, the exploded deployments that the cfn collector produced would not ever be committed, and thus would always appear to have been changed. This resulted in os-collect-config running the command endlessly. This requires some refactoring so that we commit changes to the cache based on what was actually written, rather than just the static list of collectors. Closes-Bug: #1307153 Change-Id: I618ef5d752ed6519e8b7bfc090de03f2f24e73ce
This commit is contained in:
parent
77fb0651bc
commit
6198acbfcc
|
@ -103,7 +103,8 @@ def setup_conf():
|
|||
|
||||
|
||||
def collect_all(collectors, store=False, requests_impl_map=None):
|
||||
any_changed = False
|
||||
changed_keys = set()
|
||||
all_keys = list()
|
||||
if store:
|
||||
paths_or_content = []
|
||||
else:
|
||||
|
@ -125,19 +126,21 @@ def collect_all(collectors, store=False, requests_impl_map=None):
|
|||
|
||||
if store:
|
||||
for output_key, output_content in content:
|
||||
all_keys.append(output_key)
|
||||
(changed, path) = cache.store(output_key, output_content)
|
||||
any_changed |= changed
|
||||
if changed:
|
||||
changed_keys.add(output_key)
|
||||
paths_or_content.append(path)
|
||||
else:
|
||||
paths_or_content.update(content)
|
||||
|
||||
if any_changed:
|
||||
cache.store_meta_list('os_config_files', collectors)
|
||||
if changed_keys:
|
||||
cache.store_meta_list('os_config_files', all_keys)
|
||||
if os.path.exists(CONF.backup_cachedir):
|
||||
shutil.rmtree(CONF.backup_cachedir)
|
||||
if os.path.exists(CONF.cachedir):
|
||||
shutil.copytree(CONF.cachedir, CONF.backup_cachedir)
|
||||
return (any_changed, paths_or_content)
|
||||
return (changed_keys, paths_or_content)
|
||||
|
||||
|
||||
def reexec_self(signal=None, frame=None):
|
||||
|
@ -204,12 +207,12 @@ def __main__(args=sys.argv, requests_impl_map=None):
|
|||
config_hash = getfilehash(config_files)
|
||||
while True:
|
||||
store_and_run = bool(CONF.command and not CONF.print_only)
|
||||
(any_changed, content) = collect_all(
|
||||
(changed_keys, content) = collect_all(
|
||||
cfg.CONF.collectors,
|
||||
store=store_and_run,
|
||||
requests_impl_map=requests_impl_map)
|
||||
if store_and_run:
|
||||
if any_changed or CONF.force:
|
||||
if changed_keys or CONF.force:
|
||||
# ignore HUP now since we will reexec after commit anyway
|
||||
signal.signal(signal.SIGHUP, signal.SIG_IGN)
|
||||
try:
|
||||
|
@ -232,8 +235,8 @@ def __main__(args=sys.argv, requests_impl_map=None):
|
|||
logger.warn('Config changed, re-execing now')
|
||||
config_hash = new_config_hash
|
||||
else:
|
||||
for collector in cfg.CONF.collectors:
|
||||
cache.commit(collector)
|
||||
for changed in changed_keys:
|
||||
cache.commit(changed)
|
||||
if not CONF.one_time:
|
||||
reexec_self()
|
||||
else:
|
||||
|
|
|
@ -310,10 +310,13 @@ class TestCollectAll(testtools.TestCase):
|
|||
store=store,
|
||||
requests_impl_map=requests_impl_map)
|
||||
|
||||
def _test_collect_all_store(self, requests_impl_map=None):
|
||||
(any_changed, paths) = self._call_collect_all(
|
||||
def _test_collect_all_store(self, requests_impl_map=None,
|
||||
expected_changed=None):
|
||||
(changed_keys, paths) = self._call_collect_all(
|
||||
store=True, requests_impl_map=requests_impl_map)
|
||||
self.assertTrue(any_changed)
|
||||
if expected_changed is None:
|
||||
expected_changed = set(['heat_local', 'cfn', 'ec2'])
|
||||
self.assertEqual(expected_changed, changed_keys)
|
||||
self.assertThat(paths, matchers.IsInstance(list))
|
||||
for path in paths:
|
||||
self.assertTrue(os.path.exists(path))
|
||||
|
@ -325,14 +328,16 @@ class TestCollectAll(testtools.TestCase):
|
|||
def test_collect_all_store_softwareconfig(self):
|
||||
soft_config_map = {'ec2': test_ec2.FakeRequests,
|
||||
'cfn': test_cfn.FakeRequestsSoftwareConfig(self)}
|
||||
self._test_collect_all_store(requests_impl_map=soft_config_map)
|
||||
expected_changed = set(('heat_local', 'ec2', 'cfn', 'dep-name1'))
|
||||
self._test_collect_all_store(requests_impl_map=soft_config_map,
|
||||
expected_changed=expected_changed)
|
||||
|
||||
def test_collect_all_store_alt_order(self):
|
||||
# Ensure different than default
|
||||
new_list = list(reversed(cfg.CONF.collectors))
|
||||
(any_changed, paths) = self._call_collect_all(store=True,
|
||||
collectors=new_list)
|
||||
self.assertTrue(any_changed)
|
||||
(changed_keys, paths) = self._call_collect_all(
|
||||
store=True, collectors=new_list)
|
||||
self.assertEqual(set(cfg.CONF.collectors), changed_keys)
|
||||
self.assertThat(paths, matchers.IsInstance(list))
|
||||
expected_paths = [
|
||||
os.path.join(self.cache_dir.path, '%s.json' % collector)
|
||||
|
@ -340,18 +345,34 @@ class TestCollectAll(testtools.TestCase):
|
|||
self.assertEqual(expected_paths, paths)
|
||||
|
||||
def test_collect_all_no_change(self):
|
||||
(any_changed, paths) = self._call_collect_all(store=True)
|
||||
self.assertTrue(any_changed)
|
||||
(changed_keys, paths) = self._call_collect_all(store=True)
|
||||
self.assertEqual(set(cfg.CONF.collectors), changed_keys)
|
||||
# Commit
|
||||
for collector in cfg.CONF.collectors:
|
||||
cache.commit(collector)
|
||||
(any_changed, paths2) = self._call_collect_all(store=True)
|
||||
self.assertFalse(any_changed)
|
||||
for changed in changed_keys:
|
||||
cache.commit(changed)
|
||||
(changed_keys, paths2) = self._call_collect_all(store=True)
|
||||
self.assertEqual(set(), changed_keys)
|
||||
self.assertEqual(paths, paths2)
|
||||
|
||||
def test_collect_all_no_change_softwareconfig(self):
|
||||
soft_config_map = {'ec2': test_ec2.FakeRequests,
|
||||
'cfn': test_cfn.FakeRequestsSoftwareConfig(self)}
|
||||
(changed_keys, paths) = self._call_collect_all(
|
||||
store=True, requests_impl_map=soft_config_map)
|
||||
expected_changed = set(cfg.CONF.collectors)
|
||||
expected_changed.add('dep-name1')
|
||||
self.assertEqual(expected_changed, changed_keys)
|
||||
# Commit
|
||||
for changed in changed_keys:
|
||||
cache.commit(changed)
|
||||
(changed_keys, paths2) = self._call_collect_all(
|
||||
store=True, requests_impl_map=soft_config_map)
|
||||
self.assertEqual(set(), changed_keys)
|
||||
self.assertEqual(paths, paths2)
|
||||
|
||||
def test_collect_all_nostore(self):
|
||||
(any_changed, content) = self._call_collect_all(store=False)
|
||||
self.assertFalse(any_changed)
|
||||
(changed_keys, content) = self._call_collect_all(store=False)
|
||||
self.assertEqual(set(), changed_keys)
|
||||
self.assertThat(content, matchers.IsInstance(dict))
|
||||
for collector in cfg.CONF.collectors:
|
||||
self.assertIn(collector, content)
|
||||
|
@ -360,9 +381,9 @@ class TestCollectAll(testtools.TestCase):
|
|||
def test_collect_all_ec2_unavailable(self):
|
||||
requests_impl_map = {'ec2': test_ec2.FakeFailRequests,
|
||||
'cfn': test_cfn.FakeRequests(self)}
|
||||
(any_changed, content) = self._call_collect_all(
|
||||
(changed_keys, content) = self._call_collect_all(
|
||||
store=False, requests_impl_map=requests_impl_map)
|
||||
self.assertFalse(any_changed)
|
||||
self.assertEqual(set(), changed_keys)
|
||||
self.assertThat(content, matchers.IsInstance(dict))
|
||||
self.assertNotIn('ec2', content)
|
||||
|
||||
|
|
Loading…
Reference in New Issue