Do not cache new data if command fails

The point of delaying the commit of data to the cache is that we want to
make sure the command succeeds before giving up on the data changes.
This will ensure that we keep trying the command with any given change
to the metadata until it succeeds.

Change-Id: Idf3a09686b4bbf0e16a9bc9f3359ee9937fcc627

os_collect_config/collect.py

@@ -127,7 +127,7 @@ def call_command(files, command):
     env = dict(os.environ)
     env["OS_CONFIG_FILES"] = ':'.join(files)
     logger.info("Executing %s" % command)
-    subprocess.call(CONF.command, env=env, shell=True)
+    subprocess.check_call(CONF.command, env=env, shell=True)
 
 
 def __main__(args=sys.argv, requests_impl_map=None):
@@ -152,9 +152,18 @@ def __main__(args=sys.argv, requests_impl_map=None):
             if any_changed:
                 # ignore HUP now since we will reexec after commit anyway
                 signal.signal(signal.SIGHUP, signal.SIG_IGN)
-                call_command(content, CONF.command)
-                for collector in cfg.CONF.collectors:
-                    cache.commit(collector)
+                try:
+                    call_command(content, CONF.command)
+                except subprocess.CalledProcessError as e:
+                    logger.error('Command failed, will not cache new data. %s'
+                                 % e)
+                    if not CONF.one_time:
+                        logger.warn('Sleeping %.2f seconds before re-exec.' %
+                                    CONF.polling_interval)
+                        time.sleep(CONF.polling_interval)
+                else:
+                    for collector in cfg.CONF.collectors:
+                        cache.commit(collector)
                 if not CONF.one_time:
                     reexec_self()
             else:

os_collect_config/tests/test_collect.py

@@ -106,6 +106,31 @@ class TestCollect(testtools.TestCase):
         self.assertIn("int1", keys_found)
         self.assertIn("map_ab", keys_found)
 
+    def test_main_command_failed_no_caching(self):
+        cache_dir = self.useFixture(fixtures.TempDir())
+        fake_metadata = _setup_local_metadata(self)
+        occ_args = [
+            'os-collect-config',
+            '--command',
+            'foo',
+            '--cachedir',
+            cache_dir.path,
+            '--config-file',
+            '/dev/null',
+            '--heat_local-path',
+            fake_metadata,
+        ]
+        calls = []
+
+        def capture_popen(proc_args):
+            calls.append(proc_args)
+            return dict(returncode=1)
+        self.useFixture(fixtures.FakePopen(capture_popen))
+        self._call_main(occ_args)
+        cache_contents = os.listdir(cache_dir.path)
+        last_files = [name for name in cache_contents if name.endswith('last')]
+        self.assertEqual([], last_files)
+
     def test_main_no_command(self):
         fake_args = [
             'os-collect-config',