Repository: Azure/WALinuxAgent
Pull request #1484 (2019-03-08 02:42:21)

Make launch_command resilient to cgroups failures
<!-- DO NOT DELETE THIS TEMPLATE -->

## Description

Catch all exceptions thrown by cgroups methods. We don't want cgroups failures to interfere with the extension lifecycle.

<!--
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.
-->

---

### 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/.flake8 b/.flake8
new file mode 100644
index 00000000..63303c3b
--- /dev/null
+++ b/.flake8
@@ -0,0 +1,32 @@
+#
+# The project did not use flake8 since inception so there are a number
+# of time-consuming flake8-identified improvements that are just a lot
+# of busy work.  Each of these should be disabled and code cleaned up.
+#
+# W503: Line break occurred before a binary operator
+# W504: Line break occurred after a binary operator
+# E126: Continuation line over-indented for hanging indent
+# E127: Continuation line over-indented for visual indent
+# E128: Continuation line under-indented for visual indent
+# E201: Whitespace after '('
+# E202: Whitespace before ')'
+# E203: Whitespace before ':'
+# E221: Multiple spaces before operator
+# E225: Missing whitespace around operator
+# E226: Missing whitespace around arithmetic operator
+# E231: Missing whitespace after ',', ';', or ':'
+# E261: At least two spaces before inline comment
+# E265: Block comment should start with '# '
+# E302: Expected 2 blank lines, found 0
+# E501: Line too long (xx > yy characters)
+# E502: The backslash is redundant between brackets
+# F401: Module imported but unused
+# F403: 'from module import *' used; unable to detect undefined names
+# F405: Name may be undefined, or defined from star imports: module
+#
+
+[flake8]
+ignore = W503,W504,E126,E127,E128,E201,E202,E203,E221,E225,E226,E231,E261,E265,E302,E501,E502,F401,F403,F405
+exclude = .git,__pycache__,docs/source/conf.py,old,build,dist,tests
+max-complexity = 30
+max-line-length = 120
\ No newline at end of file
diff --git a/.travis.yml b/.travis.yml
index 853d5457..4b27c5d7 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,14 +1,46 @@
+---
+os: linux
+dist: xenial
 language: python
-python:
-  - "2.6"
-  - "2.7"
-  #- "3.2"
-  #- "3.3"
-  - "3.4"
-# command to install dependencies
+env:
+  - NOSEOPTS="--no-skip" SETUPOPTS=""
+  # Add SETUPOPTS="check flake8" to enable flake8 checks
+
+matrix:
+  allow_failures:
+    - python: nightly
+
+  # exclude the default "python" build - we're being specific here...
+  exclude:
+    - python:
+      env:
+        - NOSEOPTS="--no-skip" SETUPOPTS="check flake8"
+
+  include:
+    - python: 2.6
+      dist: trusty
+      env:
+        - NOSEOPTS="--no-skip" SETUPOPTS=""
+    - python: 2.7
+    - python: 3.4
+    - python: 3.6
+    - python: 3.7
+      env:
+        - >-
+          NOSEOPTS="--no-skip --with-coverage --cover-inclusive
+          --cover-min-percentage=60 --cover-branches
+          --cover-package=azurelinuxagent --cover-xml"
+          SETUPOPTS=""
+    - python: nightly
+
 install:
-  #- pip install .
-  #- pip install -r requirements.txt
-  - pip install pyasn1
-# command to run tests
-script: nosetests tests
+  - pip install -r requirements.txt
+  - pip install -r test-requirements.txt
+
+script:
+  # future: - pylint setup.py makepkg.py azurelinuxagent/
+  - nosetests $NOSEOPTS tests
+  - if [ ! -z "$SETUPOPTS" ]; then /usr/bin/env python setup.py $SETUPOPTS; fi
+
+after_success:
+  - if [[ $TRAVIS_PYTHON_VERSION == 3.7 ]]; then codecov; fi
\ No newline at end of file
diff --git a/README.md b/README.md
index ccb3074f..99b7b628 100644
--- a/README.md
+++ b/README.md
@@ -2,6 +2,9 @@
 
 ## Master branch status
 
+[![Travis CI](https://travis-ci.org/Azure/WALinuxAgent.svg?branch=develop)](https://travis-ci.org/Azure/WALinuxAgent/branches)
+[![CodeCov](https://codecov.io/gh/Azure/WALinusAgent/branch/develop/graph/badge.svg)](https://codecov.io/gh/Azure/WALinuxAgent/branch/develop)
+
 Each badge below represents our basic validation tests for an image, which are executed several times each day. These include provisioning, user account, disk, extension and networking scenarios.
 
 Image | Status |
diff --git a/azurelinuxagent/common/exception.py b/azurelinuxagent/common/exception.py
index e713c177..3711cdc3 100644
--- a/azurelinuxagent/common/exception.py
+++ b/azurelinuxagent/common/exception.py
@@ -74,6 +74,7 @@ class ExtensionOperationError(ExtensionError):
     """
     When failed to execute an extension
     """
+
     def __init__(self, msg=None, inner=None, code=-1):
         super(ExtensionOperationError, self).__init__(msg, inner, code)
 
@@ -186,3 +187,55 @@ class RemoteAccessError(AgentError):
 
     def __init__(self, msg=None, inner=None):
         super(RemoteAccessError, self).__init__(msg, inner)
+
+
+class ExtensionErrorCodes(object):
+    """
+    Common Error codes used across by Compute RP for better understanding
+    the cause and clarify common occurring errors
+    """
+
+    # Unknown Failures
+    PluginUnknownFailure = -1
+
+    # Success
+    PluginSuccess = 0
+
+    # Catch all error code.
+    PluginProcessingError = 1000
+
+    # Plugin failed to download
+    PluginManifestDownloadError = 1001
+
+    # Cannot find or load successfully the HandlerManifest.json
+    PluginHandlerManifestNotFound = 1002
+
+    # Cannot successfully serialize the HandlerManifest.json
+    PluginHandlerManifestDeserializationError = 1003
+
+    # Cannot download the plugin package
+    PluginPackageDownloadFailed = 1004
+
+    # Cannot extract the plugin form package
+    PluginPackageExtractionFailed = 1005
+
+    # Install failed
+    PluginInstallProcessingFailed = 1007
+
+    # Update failed
+    PluginUpdateProcessingFailed = 1008
+
+    # Enable failed
+    PluginEnableProcessingFailed = 1009
+
+    # Disable failed
+    PluginDisableProcessingFailed = 1010
+
+    # Extension script timed out
+    PluginHandlerScriptTimedout = 1011
+
+    # Invalid status file of the extension.
+    PluginSettingsStatusInvalid = 1012
+
+    def __init__(self):
+        pass
diff --git a/azurelinuxagent/ga/exthandlers.py b/azurelinuxagent/ga/exthandlers.py
index 12363b6b..d7503935 100644
--- a/azurelinuxagent/ga/exthandlers.py
+++ b/azurelinuxagent/ga/exthandlers.py
@@ -42,7 +42,7 @@ from azurelinuxagent.common.cgroups import CGroups, CGroupsTelemetry
 from azurelinuxagent.common.errorstate import ErrorState, ERROR_STATE_DELTA_INSTALL
 from azurelinuxagent.common.event import add_event, WALAEventOperation, elapsed_milliseconds, report_event
 from azurelinuxagent.common.exception import ExtensionError, ProtocolError, ProtocolNotFoundError, \
-    ExtensionDownloadError, ExtensionOperationError
+    ExtensionDownloadError, ExtensionOperationError, ExtensionErrorCodes
 from azurelinuxagent.common.future import ustr
 from azurelinuxagent.common.protocol import get_protocol_util
 from azurelinuxagent.common.protocol.restapi import ExtHandlerStatus, \
@@ -812,7 +812,8 @@ class ExtHandlerInstance(object):
                 i += 1
 
             if not downloaded:
-                raise ExtensionDownloadError("Failed to download extension", code=1001)
+                raise ExtensionDownloadError("Failed to download extension",
+                                             code=ExtensionErrorCodes.PluginManifestDownloadError)
 
         self.pkg_file = destination
 
@@ -874,7 +875,8 @@ class ExtHandlerInstance(object):
         man = self.load_manifest()
         enable_cmd = man.get_enable_command()
         self.logger.info("Enable extension [{0}]".format(enable_cmd))
-        self.launch_command(enable_cmd, timeout=300, extension_error_code=1009)
+        self.launch_command(enable_cmd, timeout=300,
+                            extension_error_code=ExtensionErrorCodes.PluginEnableProcessingFailed)
         self.set_handler_state(ExtHandlerState.Enabled)
         self.set_handler_status(status="Ready", message="Plugin enabled")
 
@@ -884,7 +886,7 @@ class ExtHandlerInstance(object):
         disable_cmd = man.get_disable_command()
         self.logger.info("Disable extension [{0}]".format(disable_cmd))
         self.launch_command(disable_cmd, timeout=900,
-                            extension_error_code=1010)
+                            extension_error_code=ExtensionErrorCodes.PluginDisableProcessingFailed)
         self.set_handler_state(ExtHandlerState.Installed)
         self.set_handler_status(status="NotReady", message="Plugin disabled")
 
@@ -893,7 +895,8 @@ class ExtHandlerInstance(object):
         install_cmd = man.get_install_command()
         self.logger.info("Install extension [{0}]".format(install_cmd))
         self.set_operation(WALAEventOperation.Install)
-        self.launch_command(install_cmd, timeout=900, extension_error_code=1007)
+        self.launch_command(install_cmd, timeout=900,
+                            extension_error_code=ExtensionErrorCodes.PluginInstallProcessingFailed)
         self.set_handler_state(ExtHandlerState.Installed)
 
     def uninstall(self):
@@ -935,7 +938,7 @@ class ExtHandlerInstance(object):
             self.logger.info("Update extension [{0}]".format(update_cmd))
             self.launch_command(update_cmd,
                                 timeout=900,
-                                extension_error_code=1008,
+                                extension_error_code=ExtensionErrorCodes.PluginUpdateProcessingFailed,
                                 env={'VERSION': version})
         except ExtensionError:
             # prevent the handler update from being retried
@@ -1014,7 +1017,7 @@ class ExtHandlerInstance(object):
             ext_status.status = "error"
         except ExtensionError as e:
             ext_status.message = u"Malformed status file {0}".format(e)
-            ext_status.code = e.code
+            ext_status.code = ExtensionErrorCodes.PluginSettingsStatusInvalid
             ext_status.status = "error"
         except ValueError as e:
             ext_status.message = u"Malformed status file {0}".format(e)
@@ -1102,7 +1105,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=1000, env=None):
+    def launch_command(self, cmd, timeout=300, extension_error_code=ExtensionErrorCodes.PluginProcessingError, env=None):
         begin_utc = datetime.datetime.utcnow()
         self.logger.verbose("Launch command: [{0}]", cmd)
 
@@ -1138,8 +1141,12 @@ class ExtHandlerInstance(object):
                     raise ExtensionOperationError("Failed to launch '{0}': {1}".format(full_path, e.strerror),
                                                   code=extension_error_code)
 
-                cg = CGroups.for_extension(self.ext_handler.name)
-                CGroupsTelemetry.track_extension(self.ext_handler.name, cg)
+                try:
+                    cg = CGroups.for_extension(self.ext_handler.name)
+                    CGroupsTelemetry.track_extension(self.ext_handler.name, cg)
+                except Exception as e:
+                    self.logger.warn("Unable to setup cgroup {0}: {1}".format(self.ext_handler.name, e))
+
                 msg = ExtHandlerInstance._capture_process_output(process, stdout, stderr, cmd, timeout, extension_error_code)
 
                 duration = elapsed_milliseconds(begin_utc)
@@ -1150,7 +1157,8 @@ class ExtHandlerInstance(object):
                 return msg
 
     @staticmethod
-    def _capture_process_output(process, stdout_file, stderr_file, cmd, timeout, code=-1):
+    def _capture_process_output(process, stdout_file, stderr_file, cmd, timeout,
+                                code=ExtensionErrorCodes.PluginUnknownFailure):
         retry = timeout
         while retry > 0 and process.poll() is None:
             time.sleep(1)
@@ -1171,7 +1179,8 @@ class ExtHandlerInstance(object):
         # timeout expired
         if retry == 0:
             os.killpg(os.getpgid(process.pid), signal.SIGKILL)
-            raise ExtensionError("Timeout({0}): {1}\n{2}".format(timeout, cmd, read_output()), code=code)
+            raise ExtensionError("Timeout({0}): {1}\n{2}".format(timeout, cmd, read_output()),
+                                 code=ExtensionErrorCodes.PluginHandlerScriptTimedout)
 
         # process completed or forked; sleep 1 sec to give the child process (if any) a chance to start
         time.sleep(1)
@@ -1187,9 +1196,11 @@ 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), code=1002)
+            raise ExtensionError('Failed to load manifest file ({0}): {1}'.format(man_file, e.strerror),
+                                 code=ExtensionErrorCodes.PluginHandlerManifestNotFound)
         except ValueError:
-            raise ExtensionError('Malformed manifest file ({0}).'.format(man_file), code=1003)
+            raise ExtensionError('Malformed manifest file ({0}).'.format(man_file),
+                                 code=ExtensionErrorCodes.PluginHandlerManifestDeserializationError)
 
         return HandlerManifest(data[0])
 
diff --git a/requirements.txt b/requirements.txt
new file mode 100644
index 00000000..b0b7c874
--- /dev/null
+++ b/requirements.txt
@@ -0,0 +1,2 @@
+distro; python_version >= '3.8'
+pyasn1
\ No newline at end of file


Test patch:
diff --git a/test-requirements.txt b/test-requirements.txt
new file mode 100644
index 00000000..ad91e71c
--- /dev/null
+++ b/test-requirements.txt
@@ -0,0 +1,5 @@
+codecov
+coverage
+flake8; python_version >= '2.7'
+mock
+nose
\ No newline at end of file
diff --git a/tests/ga/test_exthandlers.py b/tests/ga/test_exthandlers.py
index 2877283c..c93e04e8 100644
--- a/tests/ga/test_exthandlers.py
+++ b/tests/ga/test_exthandlers.py
@@ -5,11 +5,12 @@ import stat
 
 from azurelinuxagent.common.protocol.restapi import ExtensionStatus, Extension, ExtHandler, ExtHandlerProperties
 from azurelinuxagent.ga.exthandlers import parse_ext_status, ExtHandlerInstance, get_exthandlers_handler
-from azurelinuxagent.common.exception import ProtocolError, ExtensionError
+from azurelinuxagent.common.exception import ProtocolError, ExtensionError, ExtensionErrorCodes
 from azurelinuxagent.common.event import WALAEventOperation
 from azurelinuxagent.common.utils.processutil import TELEMETRY_MESSAGE_MAX_LEN, format_stdout_stderr
 from tests.tools import *
 
+
 class TestExtHandlers(AgentTestCase):
     def test_parse_extension_status00(self):
         """
@@ -209,25 +210,11 @@ class TestExtHandlers(AgentTestCase):
         self.assertEquals(second_call_args['is_success'], False)
         self.assertIn(test_message, second_call_args['message'])
 
+
 class LaunchCommandTestCase(AgentTestCase):
     """
     Test cases for launch_command
     """
-    @classmethod
-    def setUpClass(cls):
-        AgentTestCase.setUpClass()
-        cls.mock_cgroups = patch("azurelinuxagent.ga.exthandlers.CGroups")
-        cls.mock_cgroups.start()
-
-        cls.mock_cgroups_telemetry = patch("azurelinuxagent.ga.exthandlers.CGroupsTelemetry")
-        cls.mock_cgroups_telemetry.start()
-
-    @classmethod
-    def tearDownClass(cls):
-        cls.mock_cgroups_telemetry.stop()
-        cls.mock_cgroups.stop()
-
-        AgentTestCase.tearDownClass()
 
     def setUp(self):
         AgentTestCase.setUp(self)
@@ -312,7 +299,7 @@ sys.stderr.write("{1}")
         self.assertListEqual(files_before, files_after, "Not all temporary files were deleted. File list: {0}".format(files_after))
 
     def test_it_should_raise_an_exception_when_the_command_times_out(self):
-        extension_error_code = 1234
+        extension_error_code = ExtensionErrorCodes.PluginHandlerScriptTimedout
         stdout = "stdout" * 7
         stderr = "stderr" * 7
 
@@ -599,3 +586,16 @@ sys.stderr.write("STDERR")
 
         self.assertIn("[stderr]\nCannot read stdout/stderr:", output)
 
+    def test_it_should_handle_exceptions_from_cgroups_and_run_command(self):
+        # file used to verify the command completed successfully
+        signal_file = os.path.join(self.tmp_dir, "signal_file.txt")
+
+        command = self._create_script("create_file.py", '''
+open("{0}", "w").close()
+
+'''.format(signal_file))
+
+        with patch('azurelinuxagent.common.cgroups.CGroups.for_extension', side_effect=Exception):
+            self.ext_handler_instance.launch_command(command)
+
+        self.assertTrue(os.path.exists(signal_file))
diff --git a/tests/ga/test_exthandlers_download_extension.py b/tests/ga/test_exthandlers_download_extension.py
index 725649ae..12f7c981 100644
--- a/tests/ga/test_exthandlers_download_extension.py
+++ b/tests/ga/test_exthandlers_download_extension.py
@@ -6,7 +6,7 @@ import zipfile, time
 from azurelinuxagent.common.protocol.restapi import ExtHandler, ExtHandlerProperties, ExtHandlerPackage, ExtHandlerVersionUri
 from azurelinuxagent.common.protocol.wire import WireProtocol
 from azurelinuxagent.ga.exthandlers import ExtHandlerInstance, NUMBER_OF_DOWNLOAD_RETRIES
-from azurelinuxagent.common.exception import ExtensionDownloadError
+from azurelinuxagent.common.exception import ExtensionDownloadError, ExtensionErrorCodes
 from tests.tools import *
 
 class DownloadExtensionTestCase(AgentTestCase):
@@ -203,7 +203,7 @@ class DownloadExtensionTestCase(AgentTestCase):
         self.assertEquals(mock_download_ext_handler_pkg.call_count, NUMBER_OF_DOWNLOAD_RETRIES * len(self.pkg.uris))
 
         self.assertRegex(str(context_manager.exception), "Failed to download extension")
-        self.assertEquals(context_manager.exception.code, 1001)
+        self.assertEquals(context_manager.exception.code, ExtensionErrorCodes.PluginManifestDownloadError)
 
         self.assertFalse(os.path.exists(self.extension_dir), "The extension directory was not removed")
         self.assertFalse(os.path.exists(self._get_extension_package_file()), "The extension package was not removed")