Repository: Azure/WALinuxAgent
Pull request #1295 (2018-08-09 22:08:18)

Fix ExtensionError error code usage
<!-- DO NOT DELETE THIS TEMPLATE -->

## Description

<!--
Please add an informative description that covers that changes made by the pull request. 
This checklist is used to make sure that common issues in a pull request are addressed.
This will expedite the process of getting your pull request merged and avoid extra work on your part to fix issues discovered during the review process.
-->
Fix regression found during integration testing where an `ExtensionError` in `launch_command` does not contain the appropriate error code.

Process util unit tests have been modified to verify the presence of the error code.
I am not sure how to unit test the changes in `exthandlers.py`.

---

### PR information
- [x] The title of the PR is clear and informative.
- [x] There are a small number of commits, each of which has an informative message. This means that previously merged commits do not appear in the history of the PR. For information on cleaning up the commits in your pull request, [see this page](https://github.com/Azure/azure-powershell/blob/master/documentation/development-docs/cleaning-up-commits.md).
- [x] Except for special cases involving multiple contributors, the PR is started from a fork of the main repository, not a branch.
- [x] If applicable, the PR references the bug/issue that it fixes in the description.
- [x] New Unit tests were added for the changes made and Travis.CI is passing.

### Quality of Code and Contribution Guidelines
- [x] I have read the [contribution guidelines](https://github.com/Azure/WALinuxAgent/blob/master/.github/CONTRIBUTING.md).


Patch:
diff --git a/azurelinuxagent/common/utils/processutil.py b/azurelinuxagent/common/utils/processutil.py
index 9c0eb24b..b6f30ee7 100644
--- a/azurelinuxagent/common/utils/processutil.py
+++ b/azurelinuxagent/common/utils/processutil.py
@@ -100,7 +100,7 @@ def _destroy_process(process, signal_to_send=signal.SIGKILL):
         pass    # If the process is already gone, that's fine
 
 
-def capture_from_process_poll(process, cmd, timeout):
+def capture_from_process_poll(process, cmd, timeout, code):
     """
     If the process forks, we cannot capture anything we block until the process tree completes
     """
@@ -114,12 +114,12 @@ def capture_from_process_poll(process, cmd, timeout):
         os.killpg(os.getpgid(process.pid), signal.SIGKILL)
         stdout, stderr = process.communicate()
         msg = format_stdout_stderr(sanitize(stdout), sanitize(stderr))
-        raise ExtensionError("Timeout({0}): {1}\n{2}".format(timeout, cmd, msg))
+        raise ExtensionError("Timeout({0}): {1}\n{2}".format(timeout, cmd, msg), code=code)
 
     # process completed or forked
     return_code = process.wait()
     if return_code != 0:
-        raise ExtensionError("Non-zero exit code: {0}, {1}".format(return_code, cmd))
+        raise ExtensionError("Non-zero exit code: {0}, {1}".format(return_code, cmd), code=code)
 
     stderr = b''
     stdout = b'cannot collect stdout'
@@ -155,20 +155,20 @@ def capture_from_process_poll(process, cmd, timeout):
     return stdout, stderr
 
 
-def capture_from_process_no_timeout(process, cmd):
+def capture_from_process_no_timeout(process, cmd, code):
     try:
         stdout, stderr = process.communicate()
     except OSError as e:
         _destroy_process(process, signal.SIGKILL)
-        raise ExtensionError("Error while running '{0}': {1}".format(cmd, e.strerror))
+        raise ExtensionError("Error while running '{0}': {1}".format(cmd, e.strerror), code=code)
     except Exception as e:
         _destroy_process(process, signal.SIGKILL)
-        raise ExtensionError("Exception while running '{0}': {1}".format(cmd, e))
+        raise ExtensionError("Exception while running '{0}': {1}".format(cmd, e), code=code)
 
     return stdout, stderr
 
 
-def capture_from_process_raw(process, cmd, timeout):
+def capture_from_process_raw(process, cmd, timeout, code):
     """
     Captures stdout and stderr from an already-created process.
 
@@ -180,18 +180,18 @@ def capture_from_process_raw(process, cmd, timeout):
     :raises ExtensionError: if a timeout occurred or if anything was raised by Popen.communicate()
     """
     if not timeout:
-        stdout, stderr = capture_from_process_no_timeout(process, cmd)
+        stdout, stderr = capture_from_process_no_timeout(process, cmd, code)
     else:
         if os.getpgid(process.pid) != process.pid:
             _destroy_process(process, signal.SIGKILL)
-            raise ExtensionError("Subprocess was not root of its own process group")
+            raise ExtensionError("Subprocess was not root of its own process group", code=code)
 
-        stdout, stderr = capture_from_process_poll(process, cmd, timeout)
+        stdout, stderr = capture_from_process_poll(process, cmd, timeout, code)
 
     return stdout, stderr
 
 
-def capture_from_process(process, cmd, timeout=0):
+def capture_from_process(process, cmd, timeout=0, code=-1):
     """
     Captures stdout and stderr from an already-created process. The output is "cooked"
     into a string of reasonable length.
@@ -203,5 +203,5 @@ def capture_from_process(process, cmd, timeout=0):
     :rtype: (str, str)
     :raises ExtensionError: if a timeout occurred or if anything was raised by Popen.communicate()
     """
-    stdout, stderr = capture_from_process_raw(process, cmd, timeout)
+    stdout, stderr = capture_from_process_raw(process, cmd, timeout, code)
     return format_stdout_stderr(sanitize(stdout), sanitize(stderr))
diff --git a/azurelinuxagent/ga/exthandlers.py b/azurelinuxagent/ga/exthandlers.py
index cdfa825a..add289e5 100644
--- a/azurelinuxagent/ga/exthandlers.py
+++ b/azurelinuxagent/ga/exthandlers.py
@@ -483,14 +483,14 @@ class ExtHandlersHandler(object):
                 active_exts = ext_handler_i.report_ext_status()
                 handler_status.extensions.extend(active_exts)
             except ExtensionError as e:
-                ext_handler_i.set_handler_status(message=ustr(e), code=-1)
+                ext_handler_i.set_handler_status(message=ustr(e), code=e.code)
 
             try:
                 heartbeat = ext_handler_i.collect_heartbeat()
                 if heartbeat is not None:
                     handler_status.status = heartbeat.get('status')
             except ExtensionError as e:
-                ext_handler_i.set_handler_status(message=ustr(e), code=-1)
+                ext_handler_i.set_handler_status(message=ustr(e), code=e.code)
 
         vm_status.vmAgent.extensionHandlers.append(handler_status)
 
@@ -664,7 +664,7 @@ class ExtHandlerInstance(object):
                 logger.warn("Error while downloading extension: {0}", e)
         
         if package is None or chosen_uri is None:
-            raise ExtensionError("Failed to download extension")
+            raise ExtensionError("Failed to download extension", code=1001)
 
         self.logger.verbose("Unpack extension package")
         self.pkg_file = os.path.join(conf.get_lib_dir(), os.path.basename(chosen_uri.uri) + ".zip")
@@ -675,7 +675,7 @@ class ExtHandlerInstance(object):
         except IOError as e:
             fileutil.clean_ioerror(e,
                 paths=[self.get_base_dir(), self.pkg_file])
-            raise ExtensionError(u"Failed to write and unzip plugin", e)
+            raise ExtensionError(u"Failed to write and unzip plugin", e, code=1001)
 
         # Add user execute permission to all files under the base dir
         for file in fileutil.get_all_files(self.get_base_dir()):
@@ -847,7 +847,11 @@ class ExtHandlerInstance(object):
             ext_status.message = u"Failed to get status file {0}".format(e)
             ext_status.code = -1
             ext_status.status = "error"
-        except (ExtensionError, ValueError) as e:
+        except ExtensionError as e:
+            ext_status.message = u"Malformed status file {0}".format(e)
+            ext_status.code = e.code
+            ext_status.status = "error"
+        except ValueError as e:
             ext_status.message = u"Malformed status file {0}".format(e)
             ext_status.code = -1
             ext_status.status = "error"
@@ -904,7 +908,7 @@ class ExtHandlerInstance(object):
         last_update = int(time.time() - os.stat(heartbeat_file).st_mtime)
         return last_update <= 600
 
-    def launch_command(self, cmd, timeout=300, extension_error_code=-1, env=None):
+    def launch_command(self, cmd, timeout=300, extension_error_code=1000, env=None):
         begin_utc = datetime.datetime.utcnow()
         self.logger.verbose("Launch command: [{0}]", cmd)
         base_dir = self.get_base_dir()
@@ -942,7 +946,7 @@ class ExtHandlerInstance(object):
 
         cg = CGroups.for_extension(self.ext_handler.name)
         CGroupsTelemetry.track_extension(self.ext_handler.name, cg)
-        msg = capture_from_process(process, cmd, timeout)
+        msg = capture_from_process(process, cmd, timeout, extension_error_code)
 
         ret = process.poll()
         if ret is None:
@@ -960,9 +964,9 @@ class ExtHandlerInstance(object):
         try:
             data = json.loads(fileutil.read_file(man_file))
         except (IOError, OSError) as e:
-            raise ExtensionError('Failed to load manifest file ({0}): {1}'.format(man_file, e.strerror))
+            raise ExtensionError('Failed to load manifest file ({0}): {1}'.format(man_file, e.strerror), code=1002)
         except ValueError:
-            raise ExtensionError('Malformed manifest file ({0}).'.format(man_file))
+            raise ExtensionError('Malformed manifest file ({0}).'.format(man_file), code=1003)
 
         return HandlerManifest(data[0])
 


Test patch:
diff --git a/tests/utils/test_process_util.py b/tests/utils/test_process_util.py
index a950e556..55d65440 100644
--- a/tests/utils/test_process_util.py
+++ b/tests/utils/test_process_util.py
@@ -26,6 +26,7 @@ import sys
 process_target = "{0}/process_target.sh".format(os.path.abspath(os.path.join(__file__, os.pardir)))
 process_cmd_template = "{0} -o '{1}' -e '{2}'"
 
+EXTENSION_ERROR_CODE = 1000
 
 class TestProcessUtils(AgentTestCase):
     def test_format_stdout_stderr00(self):
@@ -138,13 +139,14 @@ class TestProcessUtils(AgentTestCase):
                                    preexec_fn=os.setsid)
 
         try:
-            capture_from_process(process, 'sleep 20', 10)
+            capture_from_process(process, 'sleep 20', 10, EXTENSION_ERROR_CODE)
             self.fail('Timeout exception was expected')
         except ExtensionError as e:
             body = str(e)
             self.assertTrue('Timeout(10)' in body)
             self.assertTrue('Iteration 9' in body)
             self.assertFalse('Iteration 11' in body)
+            self.assertEqual(EXTENSION_ERROR_CODE, e.code)
         except Exception as gen_ex:
             self.fail('Unexpected exception: {0}'.format(gen_ex))
 
@@ -240,10 +242,10 @@ class TestProcessUtils(AgentTestCase):
                                    env=os.environ)
 
         if sys.version_info < (2, 7):
-            self.assertRaises(ExtensionError, capture_from_process, process, cmd, 10)
+            self.assertRaises(ExtensionError, capture_from_process, process, cmd, 10, EXTENSION_ERROR_CODE)
         else:
             with self.assertRaises(ExtensionError) as ee:
-                capture_from_process(process, cmd, 10)
+                capture_from_process(process, cmd, 10, EXTENSION_ERROR_CODE)
 
             body = str(ee.exception)
             if sys.version_info >= (3, 2):
@@ -251,6 +253,8 @@ class TestProcessUtils(AgentTestCase):
             else:
                 self.assertRegexpMatches(body, "process group")
 
+            self.assertEqual(EXTENSION_ERROR_CODE, ee.exception.code)
+
 
 if __name__ == '__main__':
     unittest.main()