Repository: Azure/WALinuxAgent
Pull request #2178 (2021-02-16 21:58:16)

Clean up configuration parameters for cgroups
Added CGroups.Enabled to disable monitoring and cpu enforcement.

Removed unused CGroups.EnforceLimits and CGroups.Excluded.

codecov[bot]: # [Codecov](https://codecov.io/gh/Azure/WALinuxAgent/pull/2178?src=pr&el=h1) Report
> Merging [#2178](https://codecov.io/gh/Azure/WALinuxAgent/pull/2178?src=pr&el=desc) (f67507c) into [develop](https://codecov.io/gh/Azure/WALinuxAgent/commit/6f3a278e6b8d1f1ecb9069d7db6313fc2d9cf80a?el=desc) (6f3a278) will **decrease** coverage by `0.02%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/Azure/WALinuxAgent/pull/2178/graphs/tree.svg?width=650&height=150&src=pr&token=ae90E2KYCw)](https://codecov.io/gh/Azure/WALinuxAgent/pull/2178?src=pr&el=tree)

```diff
@@             Coverage Diff             @@
##           develop    #2178      +/-   ##
===========================================
- Coverage    70.47%   70.44%   -0.03%     
===========================================
  Files           96       96              
  Lines        13360    13360              
  Branches      1889     1890       +1     
===========================================
- Hits          9415     9411       -4     
- Misses        3534     3535       +1     
- Partials       411      414       +3     
```


| [Impacted Files](https://codecov.io/gh/Azure/WALinuxAgent/pull/2178?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [azurelinuxagent/common/cgroupconfigurator.py](https://codecov.io/gh/Azure/WALinuxAgent/pull/2178/diff?src=pr&el=tree#diff-YXp1cmVsaW51eGFnZW50L2NvbW1vbi9jZ3JvdXBjb25maWd1cmF0b3IucHk=) | `71.26% <100.00%> (+0.24%)` | :arrow_up: |
| [azurelinuxagent/common/conf.py](https://codecov.io/gh/Azure/WALinuxAgent/pull/2178/diff?src=pr&el=tree#diff-YXp1cmVsaW51eGFnZW50L2NvbW1vbi9jb25mLnB5) | `76.11% <100.00%> (+0.13%)` | :arrow_up: |
| [azurelinuxagent/ga/collect\_telemetry\_events.py](https://codecov.io/gh/Azure/WALinuxAgent/pull/2178/diff?src=pr&el=tree#diff-YXp1cmVsaW51eGFnZW50L2dhL2NvbGxlY3RfdGVsZW1ldHJ5X2V2ZW50cy5weQ==) | `89.65% <0.00%> (-1.73%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/Azure/WALinuxAgent/pull/2178?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/Azure/WALinuxAgent/pull/2178?src=pr&el=footer). Last update [6f3a278...f67507c](https://codecov.io/gh/Azure/WALinuxAgent/pull/2178?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/README.md b/README.md
index 24ec457d..646d6625 100644
--- a/README.md
+++ b/README.md
@@ -1,3 +1,4 @@
+
 # Microsoft Azure Linux Agent
 
 ## Develop branch status
@@ -219,8 +220,7 @@ OS.SshClientAliveInterval=180
 OS.SshDir=/etc/ssh
 HttpProxy.Host=None
 HttpProxy.Port=None
-CGroups.EnforceLimits=y
-CGroups.Excluded=customscript,runcommand
+CGroups.Enabled=y
 ```
 
 The various configuration options are described in detail below. Configuration
diff --git a/azurelinuxagent/common/cgroupconfigurator.py b/azurelinuxagent/common/cgroupconfigurator.py
index ce9b3cbe..067fab44 100644
--- a/azurelinuxagent/common/cgroupconfigurator.py
+++ b/azurelinuxagent/common/cgroupconfigurator.py
@@ -19,6 +19,7 @@ import os
 import re
 import subprocess
 
+from azurelinuxagent.common import conf
 from azurelinuxagent.common import logger
 from azurelinuxagent.common.cgroup import CpuCgroup, AGENT_NAME_TELEMETRY, MetricsCounter
 from azurelinuxagent.common.cgroupapi import CGroupsApi, SystemdCgroupsApi, SystemdRunError
@@ -105,6 +106,9 @@ class CGroupConfigurator(object):
                 if self._initialized:
                     return
 
+                if not conf.get_cgroups_enabled():
+                    return
+
                 # check whether cgroup monitoring is supported on the current distro
                 self._cgroups_supported = CGroupsApi.cgroups_supported()
                 if not self._cgroups_supported:
diff --git a/azurelinuxagent/common/conf.py b/azurelinuxagent/common/conf.py
index f5439ff8..b83f5ace 100644
--- a/azurelinuxagent/common/conf.py
+++ b/azurelinuxagent/common/conf.py
@@ -111,7 +111,7 @@ __SWITCH_OPTIONS__ = {
     "ResourceDisk.EnableSwapEncryption": False,
     "AutoUpdate.Enabled": True,
     "EnableOverProvisioning": True,
-    "CGroups.EnforceLimits": False,
+    "CGroups.Enabled": True,
 }
 
 
@@ -134,7 +134,6 @@ __STRING_OPTIONS__ = {
     "ResourceDisk.MountOptions": None,
     "ResourceDisk.Filesystem": "ext3",
     "AutoUpdate.GAFamily": "Prod",
-    "CGroups.Excluded": "customscript,runcommand",
 }
 
 
@@ -453,13 +452,9 @@ def get_disable_agent_file_path(conf=__conf__):
     return os.path.join(get_lib_dir(conf), DISABLE_AGENT_FILE)
 
 
-def get_cgroups_enforce_limits(conf=__conf__):
-    return conf.get_switch("CGroups.EnforceLimits", False)
+def get_cgroups_enabled(conf=__conf__):
+    return conf.get_switch("CGroups.Enabled", True)
 
 
-def get_cgroups_excluded(conf=__conf__):
-    excluded_value = conf.get("CGroups.Excluded", "customscript, runcommand")
-    return [s for s in [i.strip().lower() for i in excluded_value.split(',')] if len(s) > 0] if excluded_value else []
-
 def get_monitor_network_configuration_changes(conf=__conf__):
-    return conf.get_switch("Monitor.NetworkConfigurationChanges", False)
\ No newline at end of file
+    return conf.get_switch("Monitor.NetworkConfigurationChanges", False)
diff --git a/config/alpine/waagent.conf b/config/alpine/waagent.conf
index 4ab28363..d813ee5c 100644
--- a/config/alpine/waagent.conf
+++ b/config/alpine/waagent.conf
@@ -92,8 +92,3 @@ OS.SshDir=/etc/ssh
 # Add firewall rules to protect access to Azure host node services
 OS.EnableFirewall=y
 
-# Enforce control groups limits on the agent and extensions
-CGroups.EnforceLimits=n
-
-# CGroups which are excluded from limits, comma separated
-CGroups.Excluded=customscript,runcommand
diff --git a/config/bigip/waagent.conf b/config/bigip/waagent.conf
index 49acf9db..fe56c4d5 100644
--- a/config/bigip/waagent.conf
+++ b/config/bigip/waagent.conf
@@ -99,8 +99,3 @@ AutoUpdate.Enabled=y
 # Add firewall rules to protect access to Azure host node services
 OS.EnableFirewall=y
 
-# Enforce control groups limits on the agent and extensions
-CGroups.EnforceLimits=n
-
-# CGroups which are excluded from limits, comma separated
-CGroups.Excluded=customscript,runcommand
diff --git a/config/coreos/waagent.conf b/config/coreos/waagent.conf
index 1965b801..003482ab 100644
--- a/config/coreos/waagent.conf
+++ b/config/coreos/waagent.conf
@@ -121,8 +121,3 @@ OS.AllowHTTP=y
 # Add firewall rules to protect access to Azure host node services
 OS.EnableFirewall=y
 
-# Enforce control groups limits on the agent and extensions
-CGroups.EnforceLimits=n
-
-# CGroups which are excluded from limits, comma separated
-CGroups.Excluded=customscript,runcommand
diff --git a/config/debian/waagent.conf b/config/debian/waagent.conf
index 30647d50..74676419 100644
--- a/config/debian/waagent.conf
+++ b/config/debian/waagent.conf
@@ -129,8 +129,3 @@ OS.SshDir=/etc/ssh
 # - The default is false to protect the state of existing VMs
 OS.EnableFirewall=n
 
-# Enforce control groups limits on the agent and extensions
-CGroups.EnforceLimits=n
-
-# CGroups which are excluded from limits, comma separated
-CGroups.Excluded=customscript,runcommand
diff --git a/config/freebsd/waagent.conf b/config/freebsd/waagent.conf
index 571616d2..c917d16c 100644
--- a/config/freebsd/waagent.conf
+++ b/config/freebsd/waagent.conf
@@ -119,8 +119,3 @@ OS.SudoersDir=/usr/local/etc/sudoers.d
 # Add firewall rules to protect access to Azure host node services
 OS.EnableFirewall=y
 
-# Enforce control groups limits on the agent and extensions
-CGroups.EnforceLimits=n
-
-# CGroups which are excluded from limits, comma separated
-CGroups.Excluded=customscript,runcommand
diff --git a/config/ubuntu/waagent.conf b/config/ubuntu/waagent.conf
index 22d48a8e..81f79842 100644
--- a/config/ubuntu/waagent.conf
+++ b/config/ubuntu/waagent.conf
@@ -118,8 +118,3 @@ OS.SshDir=/etc/ssh
 # Add firewall rules to protect access to Azure host node services
 OS.EnableFirewall=y
 
-# Enforce control groups limits on the agent and extensions
-CGroups.EnforceLimits=n
-
-# CGroups which are excluded from limits, comma separated
-CGroups.Excluded=customscript,runcommand
diff --git a/config/waagent.conf b/config/waagent.conf
index 2118c73e..964532dd 100644
--- a/config/waagent.conf
+++ b/config/waagent.conf
@@ -151,8 +151,5 @@ OS.RemovePersistentNetRulesPeriod=30
 # How often (in seconds) to monitor for DHCP client restarts
 OS.MonitorDhcpClientRestartPeriod=30
 
-# Enforce control groups limits on the agent and extensions
-CGroups.EnforceLimits=n
-
-# CGroups which are excluded from limits, comma separated
-CGroups.Excluded=customscript,runcommand
+# Use cgroups to monitor the resource usage of the agent and extensions
+CGroups.Enabled=y


Test patch:
diff --git a/tests/common/test_cgroupconfigurator.py b/tests/common/test_cgroupconfigurator.py
index 43d9e1e9..bbafe824 100644
--- a/tests/common/test_cgroupconfigurator.py
+++ b/tests/common/test_cgroupconfigurator.py
@@ -69,6 +69,11 @@ class CGroupConfiguratorSystemdTestCase(AgentTestCase):
         with self._get_cgroup_configurator() as configurator:
             self.assertTrue(configurator.enabled(), "cgroups were not enabled")
 
+    def test_initialize_should_not_enable_cgroups_when_they_are_disabled_in_the_agent_configuration(self):
+        with patch('azurelinuxagent.common.conf.get_cgroups_enabled', return_value=False):
+            with self._get_cgroup_configurator() as configurator:
+                self.assertFalse(configurator.enabled(), "cgroups should not be enabled")
+
     def test_initialize_should_start_tracking_the_agent_cgroups(self):
         with self._get_cgroup_configurator() as configurator:
             tracked = CGroupsTelemetry._tracked
@@ -666,7 +671,6 @@ exit 0
                     self.assertTrue(
                         any("[PID: {0}]".format(pid) in reported_process for reported_process in reported),
                         "Process {0} was not reported. Got: {1}".format(format_processes([pid]), reported))
-
         finally:
             # create the file that stops the test processes and wait for them to complete
             open(stop_file, "w").close()
diff --git a/tests/common/test_conf.py b/tests/common/test_conf.py
index bb4e098b..8db5759f 100644
--- a/tests/common/test_conf.py
+++ b/tests/common/test_conf.py
@@ -19,7 +19,7 @@ import os.path
 
 import azurelinuxagent.common.conf as conf
 from azurelinuxagent.common.utils import fileutil
-from tests.tools import AgentTestCase, data_dir, patch
+from tests.tools import AgentTestCase, data_dir
 
 
 class TestConf(AgentTestCase):
@@ -66,8 +66,7 @@ class TestConf(AgentTestCase):
         "EnableOverProvisioning": True,
         "OS.AllowHTTP": False,
         "OS.EnableFirewall": False,
-        "CGroups.EnforceLimits": False,
-        "CGroups.Excluded": "customscript,runcommand",
+        "CGroups.Enabled": True,
     }
 
     def setUp(self):
@@ -135,36 +134,5 @@ class TestConf(AgentTestCase):
     def test_get_extensions_enabled(self):
         self.assertTrue(conf.get_extensions_enabled(self.conf))
 
-    @patch('azurelinuxagent.common.conf.ConfigurationProvider.get')
-    def assert_get_cgroups_excluded(self, patch_get, config, expected_value):
-        patch_get.return_value = config
-        self.assertEqual(expected_value, conf.get_cgroups_excluded(self.conf))
-
-    def test_get_cgroups_excluded(self):
-        self.assert_get_cgroups_excluded(config=None,  # pylint: disable=no-value-for-parameter
-                                         expected_value=[])
-
-        self.assert_get_cgroups_excluded(config='',  # pylint: disable=no-value-for-parameter
-                                         expected_value=[])
-
-        self.assert_get_cgroups_excluded(config='  ',  # pylint: disable=no-value-for-parameter
-                                         expected_value=[])
-
-        self.assert_get_cgroups_excluded(config='  ,  ,,  ,',  # pylint: disable=no-value-for-parameter
-                                         expected_value=[])
-
-        standard_values = ['customscript', 'runcommand']
-        self.assert_get_cgroups_excluded(config='CustomScript, RunCommand',  # pylint: disable=no-value-for-parameter
-                                         expected_value=standard_values)
-
-        self.assert_get_cgroups_excluded(config='customScript, runCommand  , , ,,',  # pylint: disable=no-value-for-parameter
-                                         expected_value=standard_values)
-
-        self.assert_get_cgroups_excluded(config='  customscript,runcommand  ',  # pylint: disable=no-value-for-parameter
-                                         expected_value=standard_values)
-
-        self.assert_get_cgroups_excluded(config='customscript,, runcommand',  # pylint: disable=no-value-for-parameter
-                                         expected_value=standard_values)
-
-        self.assert_get_cgroups_excluded(config=',,customscript ,runcommand',  # pylint: disable=no-value-for-parameter
-                                         expected_value=standard_values)
+    def test_get_cgroups_enabled(self):
+        self.assertTrue(conf.get_cgroups_enabled(self.conf))
diff --git a/tests/data/test_waagent.conf b/tests/data/test_waagent.conf
index 7b2f4fa7..dbdc017d 100644
--- a/tests/data/test_waagent.conf
+++ b/tests/data/test_waagent.conf
@@ -136,8 +136,3 @@ OS.SshDir=/notareal/path
 # - The default is false to protect the state of existing VMs
 OS.EnableFirewall=n
 
-# Enforce control groups limits on the agent and extensions
-CGroups.EnforceLimits=n
-
-# CGroups which are excluded from limits, comma separated
-CGroups.Excluded=customscript,runcommand
diff --git a/tests/test_agent.py b/tests/test_agent.py
index d74eaf7e..91b2dc69 100644
--- a/tests/test_agent.py
+++ b/tests/test_agent.py
@@ -26,8 +26,7 @@ EXPECTED_CONFIGURATION = \
 """AutoUpdate.Enabled = True
 AutoUpdate.GAFamily = Prod
 Autoupdate.Frequency = 3600
-CGroups.EnforceLimits = False
-CGroups.Excluded = customscript,runcommand
+CGroups.Enabled = True
 DVD.MountPoint = /mnt/cdrom/secure
 DetectScvmmEnv = False
 EnableOverProvisioning = True