Handle errors in individual controllers

azurelinuxagent/common/cgroupapi.py

@@ -117,15 +117,19 @@ def _get_controller_id(controller):
     @staticmethod
     def _foreach_controller(operation, message):
         """
-        Executes the given operation on all controllers that need to be tracked; outputs 'message' if the controller is not mounted.
+        Executes the given operation on all controllers that need to be tracked; outputs 'message' if the controller
+        is not mounted or if an error occurs in the operation
         """
         mounted_controllers = os.listdir(CGROUPS_FILE_SYSTEM_ROOT)
 
         for controller in CGROUP_CONTROLLERS:
-            if controller not in mounted_controllers:
-                logger.warn('Controller "{0}" is not mounted. {1}'.format(controller, message))
-            else:
-                operation(controller)
+            try:
+                if controller not in mounted_controllers:
+                    logger.warn('Cgroup controller "{0}" is not mounted. {1}'.format(controller, message))
+                else:
+                    operation(controller)
+            except Exception as e:
+                logger.warn('Error in cgroup controller "{0}": {1}. {2}'.format(controller, ustr(e), message))
 
 
 class FileSystemCgroupsApi(CGroupsApi):
@@ -188,21 +192,17 @@ def create_agent_cgroups(self):
         pid = int(os.getpid())
 
         def create_cgroup(controller):
-            try:
-                path = os.path.join(CGROUPS_FILE_SYSTEM_ROOT, controller, VM_AGENT_CGROUP_NAME)
+            path = os.path.join(CGROUPS_FILE_SYSTEM_ROOT, controller, VM_AGENT_CGROUP_NAME)
 
-                if not os.path.isdir(path):
-                    FileSystemCgroupsApi._try_mkdir(path)
-                    logger.info("Created cgroup {0}".format(path))
-
-                self._add_process_to_cgroup(pid, path)
+            if not os.path.isdir(path):
+                FileSystemCgroupsApi._try_mkdir(path)
+                logger.info("Created cgroup {0}".format(path))
 
-                cgroups.append(CGroup.create(path, controller, VM_AGENT_CGROUP_NAME))
+            self._add_process_to_cgroup(pid, path)
 
-            except Exception as e:
-                logger.warn('Cannot create "{0}" cgroup for the agent. Error: {1}'.format(controller, ustr(e)))
+            cgroups.append(CGroup.create(path, controller, VM_AGENT_CGROUP_NAME))
 
-        self._foreach_controller(create_cgroup, 'Will not create a cgroup for the VM Agent')
+        self._foreach_controller(create_cgroup, 'Failed to create a cgroup for the VM Agent; resource usage will not be tracked')
 
         if len(cgroups) == 0:
             raise CGroupsException("Failed to create any cgroup for the VM Agent")
@@ -220,7 +220,7 @@ def create_cgroup(controller):
                 FileSystemCgroupsApi._try_mkdir(path)
                 logger.info("Created {0}".format(path))
 
-        self._foreach_controller(create_cgroup, 'Will not create a root cgroup for extensions')
+        self._foreach_controller(create_cgroup, 'Failed to create a root cgroup for extensions')
 
     def create_extension_cgroups(self, extension_name):
         """
@@ -237,7 +237,7 @@ def create_cgroup(controller):
 
             cgroups.append(cgroup)
 
-        self._foreach_controller(create_cgroup, 'Will not create a cgroup for extension {0}'.format(extension_name))
+        self._foreach_controller(create_cgroup, 'Failed to create a cgroup for extension {0}'.format(extension_name))
 
         return cgroups
 
@@ -470,8 +470,6 @@ def create_cgroup(controller):
             cgroup_path = os.path.join(CGROUPS_FILE_SYSTEM_ROOT, controller, 'system.slice', scope_name + ".scope")
             cgroups.append(CGroup.create(cgroup_path, controller, extension_name))
 
-        self._foreach_controller(create_cgroup,
-                                 'Cannot create cgroup for extension {0}; resource usage will not be tracked.'.format(
-                                     extension_name))
+        self._foreach_controller(create_cgroup, 'Cannot create cgroup for extension {0}; resource usage will not be tracked.'.format(extension_name))
 
         return process, cgroups

tests/common/test_cgroupapi.py

@@ -30,6 +30,19 @@ def i_am_root():
 
 
 class CGroupsApiTestCase(AgentTestCase):
+    def setUp(self):
+        AgentTestCase.setUp(self)
+
+        self.cgroups_file_system_root = os.path.join(self.tmp_dir, "cgroup")
+        os.mkdir(self.cgroups_file_system_root)
+        os.mkdir(os.path.join(self.cgroups_file_system_root, "cpu"))
+        os.mkdir(os.path.join(self.cgroups_file_system_root, "memory"))
+
+        self.mock__base_cgroups = patch("azurelinuxagent.common.cgroupapi.CGROUPS_FILE_SYSTEM_ROOT", self.cgroups_file_system_root)
+        self.mock__base_cgroups.start()
+
+    def tearDown(self):
+        self.mock__base_cgroups.stop()
 
     def test_create_should_return_a_SystemdCgroupsApi_on_systemd_platforms(self):
         with patch("azurelinuxagent.common.cgroupapi.CGroupsApi._is_systemd", return_value=True):
@@ -121,6 +134,37 @@ def mock_read_file(filepath, asbin=False, remove_bom=False, encoding='utf-8'):
 
         self.assertFalse(is_systemd)
 
+    def test_foreach_controller_should_execute_operation_on_all_mounted_controllers(self):
+        executed_controllers = []
+
+        def controller_operation(controller):
+            executed_controllers.append(controller)
+
+        CGroupsApi._foreach_controller(controller_operation, 'A dummy message')
+
+        # The setUp method mocks azurelinuxagent.common.cgroupapi.CGROUPS_FILE_SYSTEM_ROOT to have the cpu and memory controllers mounted
+        self.assertIn('cpu', executed_controllers, 'The operation was not executed on the cpu controller')
+        self.assertIn('memory', executed_controllers, 'The operation was not executed on the memory controller')
+        self.assertEqual(len(executed_controllers), 2, 'The operation was not executed on unexpected controllers: {0}'.format(executed_controllers))
+
+    def test_foreach_controller_should_handle_errors_in_individual_controllers(self):
+        successful_controllers = []
+
+        def controller_operation(controller):
+            if controller == 'cpu':
+                raise Exception('A test exception')
+
+            successful_controllers.append(controller)
+
+        with patch("azurelinuxagent.common.cgroupapi.logger.warn") as mock_logger_warn:
+            CGroupsApi._foreach_controller(controller_operation, 'A dummy message')
+
+            self.assertIn('memory', successful_controllers, 'The operation was not executed on the memory controller')
+            self.assertEqual(len(successful_controllers), 1, 'The operation was not executed on unexpected controllers: {0}'.format(successful_controllers))
+
+            args, kwargs = mock_logger_warn.call_args
+            message = args[0]
+            self.assertIn('Error in cgroup controller "cpu": A test exception.', message)
 
 class FileSystemCgroupsApiTestCase(AgentTestCase):