Handle non-zero success codes from Salt

Salt does not always return 0 on success so verify actual
task results on non-zero return codes. Also add test for
salt config hook.

Change-Id: Ib848ef27ae29417ca38874d8310bdcd2bc35e2c3
Co-Authored-By: Chris Hultin <Chris.Hultin@rackspace.com>
Closes-Bug: #1483336

hot/software-config/elements/heat-config-salt/install.d/hook-salt.py

@@ -17,7 +17,7 @@ import logging
 import os
 import sys
 
-import salt.cli
+import salt.cli.caller
 import salt.config
 from salt.exceptions import SaltInvocationError
 import yaml
@@ -72,11 +72,12 @@ def main(argv=sys.argv):
     with os.fdopen(os.open(fn, os.O_CREAT | os.O_WRONLY, 0o700), 'w') as f:
         f.write(yaml_config.encode('utf-8'))
 
-    caller = salt.cli.caller.Caller(opts)
+    caller = salt.cli.caller.Caller.factory(opts)
 
     log.debug('Applying Salt state %s' % state_file)
 
     stdout, stderr = None, None
+    ret = {}
 
     try:
         ret = caller.call()
@@ -84,22 +85,37 @@ def main(argv=sys.argv):
         log.error(
             'Salt invocation error while applying Salt sate %s' % state_file)
         stderr = err
-    log.info('Return code %s' % ret['retcode'])
 
-    # returncode of 0 means there were successfull changes
-    if ret['retcode'] == 0:
-        log.info('Completed applying salt state %s' % state_file)
-        stdout = ret
-    else:
-        log.error('Error applying Salt state %s. [%s]\n'
-                  % (state_file, ret['retcode']))
-        stderr = ret
+    if ret:
+
+        log.info('Results: %s' % ret)
+        output = yaml.safe_dump(ret['return'])
+
+        # returncode of 0 means there were successfull changes
+        if ret['retcode'] == 0:
+            log.info('Completed applying salt state %s' % state_file)
+            stdout = output
+        else:
+            # Salt doesn't always return sane return codes so we have to check
+            # individual results
+            runfailed = False
+            for state, data in ret['return'].items():
+                if not data['result']:
+                    runfailed = True
+                    break
+            if runfailed:
+                log.error('Error applying Salt state %s. [%s]\n'
+                          % (state_file, ret['retcode']))
+                stderr = output
+            else:
+                ret['retcode'] = 0
+                stdout = output
 
     response = {}
 
-    for output in c.get('outputs') or []:
+    for output in c.get('outputs', []):
         output_name = output['name']
-        response[output_name] = ret[output_name]
+        response[output_name] = ret.get(output_name)
 
     response.update({
         'deploy_stdout': stdout,

test-requirements.txt

@@ -7,6 +7,7 @@ hacking>=0.10.0,<0.11
 mock>=1.0
 requests>=1.2.1,!=2.4.0
 requests-mock>=0.4.0  # Apache-2.0
+salt
 testrepository>=0.0.18
 testscenarios>=0.4
 testtools>=0.9.34

tests/software_config/test_hook_salt.py

@@ -0,0 +1,135 @@
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import fixtures
+import json
+import logging
+import os
+import yaml
+
+from tests.software_config import common
+
+log = logging.getLogger('test_hook_salt')
+
+slsok = """
+testit:
+  environ.setenv:
+  - name: does_not_matter
+  - value:
+      foo: {{ opts['fooval'] }}
+      bar: {{ opts['barval'] }}
+"""
+
+slsfail = """
+failure:
+  test.echo:
+  - text: I don't work
+"""
+
+slsnotallowed = """
+install_service:
+  pkg.installed:
+  - name: {{ opts['fooval'] }}
+"""
+
+
+class HookSaltTest(common.RunScriptTest):
+
+    data = {
+        'id': 'fake_stack',
+        'name': 'fake_resource_name',
+        'group': 'salt',
+        'inputs': [
+            {'name': 'fooval', 'value': 'bar'},
+            {'name': 'barval', 'value': 'foo'}
+        ],
+        'outputs': [
+            {'name': 'first_output'},
+            {'name': 'second_output'}
+        ],
+        'config': None
+    }
+
+    def setUp(self):
+        super(HookSaltTest, self).setUp()
+        self.hook_path = self.relative_path(
+            __file__,
+            '../..',
+            'hot/software-config/elements',
+            'heat-config-salt/install.d/hook-salt.py')
+
+        self.working_dir = self.useFixture(fixtures.TempDir())
+        self.minion_config_dir = self.useFixture(fixtures.TempDir())
+        self.minion_cach_dir = self.useFixture(fixtures.TempDir())
+
+        self.minion_conf = self.minion_config_dir.join("minion")
+
+        self.env = os.environ.copy()
+        self.env.update({
+            'HEAT_SALT_WORKING': self.working_dir.join(),
+            'SALT_MINION_CONFIG': self.minion_conf
+        })
+
+        with open(self.minion_conf, "w+") as conf_file:
+            conf_file.write("cachedir: %s\n" % self.minion_cach_dir.join())
+            conf_file.write("log_level: DEBUG\n")
+
+    def test_hook(self):
+
+        self.data['config'] = slsok
+
+        returncode, stdout, stderr = self.run_cmd(
+            [self.hook_path], self.env, json.dumps(self.data))
+
+        self.assertEqual(0, returncode, stderr)
+        ret = yaml.safe_load(stdout)
+        self.assertEqual(0, ret['deploy_status_code'])
+        self.assertIsNone(ret['deploy_stderr'])
+        self.assertIsNotNone(ret['deploy_stdout'])
+        resp = yaml.safe_load(ret['deploy_stdout'])
+        self.assertTrue(resp.values()[0]['result'])
+        self.assertEqual({'bar': 'foo', 'foo': 'bar'},
+                         resp.values()[0]['changes'])
+
+    def test_hook_salt_failed(self):
+
+        self.data['config'] = slsfail
+
+        returncode, stdout, stderr = self.run_cmd(
+            [self.hook_path], self.env, json.dumps(self.data))
+
+        self.assertEqual(0, returncode)
+        self.assertIsNotNone(stderr)
+        self.assertIsNotNone(stdout)
+        jsonout = json.loads(stdout)
+        self.assertIsNone(jsonout.get("deploy_stdout"),
+                          jsonout.get("deploy_stdout"))
+        self.assertEqual(2, jsonout.get("deploy_status_code"))
+        self.assertIsNotNone(jsonout.get("deploy_stderr"))
+        self.assertIn("was not found in SLS", jsonout.get("deploy_stderr"))
+
+    def test_hook_salt_retcode(self):
+
+        self.data['config'] = slsnotallowed
+
+        returncode, stdout, stderr = self.run_cmd(
+            [self.hook_path], self.env, json.dumps(self.data))
+
+        self.assertEqual(0, returncode, stderr)
+        self.assertIsNotNone(stdout)
+        self.assertIsNotNone(stderr)
+        ret = json.loads(stdout)
+        self.assertIsNone(ret['deploy_stdout'])
+        self.assertIsNotNone(ret['deploy_stderr'])
+        resp = yaml.safe_load(ret['deploy_stderr']).values()[0]
+        self.assertFalse(resp['result'])