Browse Source

Expose error when a hook script raises an exception

Currently this can result in errors being ignored so it's not clear
to the user that something failed.

Change-Id: Idf6badecbfa72150f3506a485eed9ae2cb5858f7
Story: 2002084
Task: 19753
Steven Hardy 11 months ago
parent
commit
e2f3df268d

+ 7
- 2
heat-config/os-refresh-config/configure.d/55-heat-config View File

@@ -155,6 +155,11 @@ def invoke_hook(c, log):
155 155
     if subproc.returncode:
156 156
         log.error("Error running %s. [%s]\n" % (
157 157
             hook_path, subproc.returncode))
158
+        signal_data = {
159
+            'deploy_stdout': stdout.decode("utf-8", "replace"),
160
+            'deploy_stderr': stderr.decode("utf-8", "replace"),
161
+            'deploy_status_code': subproc.returncode,
162
+        }
158 163
     else:
159 164
         log.info('Completed %s' % hook_path)
160 165
 
@@ -163,8 +168,8 @@ def invoke_hook(c, log):
163 168
             signal_data = json.loads(stdout.decode('utf-8', 'replace'))
164 169
     except ValueError:
165 170
         signal_data = {
166
-            'deploy_stdout': stdout,
167
-            'deploy_stderr': stderr,
171
+            'deploy_stdout': stdout.decode("utf-8", "replace"),
172
+            'deploy_stderr': stderr.decode("utf-8", "replace"),
168 173
             'deploy_status_code': subproc.returncode,
169 174
         }
170 175
 

+ 41
- 0
tests/hook-fake-raises.py View File

@@ -0,0 +1,41 @@
1
+#!/usr/bin/env python
2
+#
3
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
4
+#    not use this file except in compliance with the License. You may obtain
5
+#    a copy of the License at
6
+#
7
+#         http://www.apache.org/licenses/LICENSE-2.0
8
+#
9
+#    Unless required by applicable law or agreed to in writing, software
10
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
11
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
12
+#    License for the specific language governing permissions and limitations
13
+#    under the License.
14
+'''
15
+A fake heat-config hook for unit testing the 55-heat-config
16
+os-refresh-config script, this version raises an error.
17
+'''
18
+
19
+import json
20
+import os
21
+import sys
22
+
23
+
24
+def main(argv=sys.argv):
25
+    c = json.load(sys.stdin)
26
+
27
+    inputs = {}
28
+    for input in c['inputs']:
29
+        inputs[input['name']] = input.get('value', '')
30
+
31
+    # write out stdin json for test asserts
32
+    stdin_path = os.path.join(
33
+        os.path.dirname(os.path.realpath(__file__)), '%s.stdin' % c['group'])
34
+
35
+    with open(stdin_path, 'w') as f:
36
+        json.dump(c, f)
37
+        f.flush()
38
+    raise OSError("Something bad happened!")
39
+
40
+if __name__ == '__main__':
41
+    sys.exit(main(sys.argv))

+ 42
- 15
tests/test_heat_config.py View File

@@ -15,6 +15,7 @@ import copy
15 15
 import json
16 16
 import os
17 17
 import shutil
18
+import six
18 19
 import tempfile
19 20
 
20 21
 import fixtures
@@ -26,7 +27,7 @@ from tests import common
26 27
 class HeatConfigTest(common.RunScriptTest):
27 28
 
28 29
     fake_hooks = ['cfn-init', 'chef', 'puppet', 'salt', 'script',
29
-                  'apply-config', 'hiera', 'json-file']
30
+                  'apply-config', 'hiera', 'json-file', 'hook-raises']
30 31
 
31 32
     data = [
32 33
         {
@@ -85,6 +86,11 @@ class HeatConfigTest(common.RunScriptTest):
85 86
             'group': 'no-such-hook',
86 87
             'inputs': [],
87 88
             'config': 'nine'
89
+        }, {
90
+            'id': '0123',
91
+            'group': 'hook-raises',
92
+            'inputs': [],
93
+            'config': 'ten'
88 94
         }]
89 95
 
90 96
     outputs = {
@@ -128,6 +134,11 @@ class HeatConfigTest(common.RunScriptTest):
128 134
             'deploy_status_code': '0',
129 135
             'deploy_stderr': 'stderr',
130 136
             'deploy_stdout': 'stdout'
137
+        },
138
+        'hook-raises': {
139
+            'deploy_status_code': '1',
140
+            'deploy_stderr': 'OSError: Something bad happened!',
141
+            'deploy_stdout': ''
131 142
         }
132 143
     }
133 144
 
@@ -135,7 +146,8 @@ class HeatConfigTest(common.RunScriptTest):
135 146
         super(HeatConfigTest, self).setUp()
136 147
 
137 148
         self.fake_hook_path = self.relative_path(__file__, 'hook-fake.py')
138
-
149
+        self.fake_hook_raises_path = self.relative_path(__file__,
150
+                                                        'hook-fake-raises.py')
139 151
         self.heat_config_path = self.relative_path(
140 152
             __file__,
141 153
             '..',
@@ -147,11 +159,17 @@ class HeatConfigTest(common.RunScriptTest):
147 159
         with open(self.fake_hook_path) as f:
148 160
             fake_hook = f.read()
149 161
 
162
+        with open(self.fake_hook_raises_path) as f:
163
+            fake_hook_raises = f.read()
164
+
150 165
         for hook in self.fake_hooks:
151 166
             hook_name = self.hooks_dir.join(hook)
152 167
             with open(hook_name, 'w') as f:
153 168
                 os.utime(hook_name, None)
154
-                f.write(fake_hook)
169
+                if hook == 'hook-raises':
170
+                    f.write(fake_hook_raises)
171
+                else:
172
+                    f.write(fake_hook)
155 173
                 f.flush()
156 174
             os.chmod(hook_name, 0o755)
157 175
         self.env = os.environ.copy()
@@ -170,10 +188,7 @@ class HeatConfigTest(common.RunScriptTest):
170 188
                 'HEAT_CONFIG_DEPLOYED': self.deployed_dir.join(),
171 189
                 'HEAT_SHELL_CONFIG': config_file.name
172 190
             })
173
-            returncode, stdout, stderr = self.run_cmd(
174
-                [self.heat_config_path], self.env)
175
-
176
-            self.assertEqual(0, returncode, stderr)
191
+            return self.run_cmd([self.heat_config_path], self.env)
177 192
 
178 193
     def test_hooks_exist(self):
179 194
         self.assertThat(
@@ -186,13 +201,15 @@ class HeatConfigTest(common.RunScriptTest):
186 201
 
187 202
     def test_run_heat_config(self):
188 203
 
189
-        self.run_heat_config(self.data)
204
+        returncode, stdout, stderr = self.run_heat_config(self.data)
190 205
 
191 206
         for config in self.data:
192 207
             hook = config['group']
193 208
             stdin_path = self.hooks_dir.join('%s.stdin' % hook)
194 209
             stdout_path = self.hooks_dir.join('%s.stdout' % hook)
195 210
             deployed_file = self.deployed_dir.join('%s.json' % config['id'])
211
+            notify_file = self.deployed_dir.join('%s.notify.json' %
212
+                                                 config['id'])
196 213
 
197 214
             if hook == 'no-such-hook':
198 215
                 self.assertThat(
@@ -201,10 +218,8 @@ class HeatConfigTest(common.RunScriptTest):
201 218
                     stdout_path, matchers.Not(matchers.FileExists()))
202 219
                 continue
203 220
 
204
-            self.assertThat(stdin_path, matchers.FileExists())
205
-            self.assertThat(stdout_path, matchers.FileExists())
206
-
207 221
             # parsed stdin should match the config item
222
+            self.assertThat(stdin_path, matchers.FileExists())
208 223
             self.assertEqual(config,
209 224
                              self.json_from_file(stdin_path))
210 225
 
@@ -212,12 +227,24 @@ class HeatConfigTest(common.RunScriptTest):
212 227
             self.assertEqual(config,
213 228
                              self.json_from_file(deployed_file))
214 229
 
215
-            self.assertEqual(self.outputs[hook],
216
-                             self.json_from_file(stdout_path))
230
+            if hook != 'hook-raises':
231
+                self.assertEqual(self.outputs[hook],
232
+                                 self.json_from_file(notify_file))
233
+                self.assertEqual(self.outputs[hook],
234
+                                 self.json_from_file(stdout_path))
235
+                self.assertThat(stdout_path, matchers.FileExists())
236
+                os.remove(stdout_path)
237
+            else:
238
+                notify_data = self.json_from_file(notify_file)
239
+                self.assertEqual(
240
+                    self.outputs[hook]['deploy_status_code'],
241
+                    six.text_type(notify_data['deploy_status_code']))
242
+                self.assertIn(
243
+                    self.outputs[hook]['deploy_stderr'],
244
+                    notify_data['deploy_stderr'])
217 245
 
218 246
             # clean up files in preparation for second run
219 247
             os.remove(stdin_path)
220
-            os.remove(stdout_path)
221 248
 
222 249
         # run again with no changes, assert no new files
223 250
         self.run_heat_config(self.data)
@@ -261,7 +288,7 @@ class HeatConfigTest(common.RunScriptTest):
261 288
         self.run_heat_config(data)
262 289
         for config in self.data:
263 290
             hook = config['group']
264
-            if hook == 'no-such-hook':
291
+            if hook in ('no-such-hook', 'hook-raises'):
265 292
                 continue
266 293
             deployed_file = self.deployed_dir.join('%s.json' % config['id'])
267 294
             old_deployed_file = old_deployed_dir.join('%s.json' % config['id'])

Loading…
Cancel
Save