Repository: Azure/WALinuxAgent
Pull request #2757 (2023-02-09 23:29:20)

Update log collector unit file to remove memorylimit
<!-- DO NOT DELETE THIS TEMPLATE -->

## Description
This previous PR (https://github.com/Azure/WALinuxAgent/pull/2637) removed memory limit from the log collector slice unit file, but cgroupconfigurator currently only updates the unit file if it does not already exist. This memory limit change is included in 2.9.0.4. For VMs which upgrade to 2.9.0.4 from earlier versions, the unit file is created with a memorylimit and never updated. 

This PR updates cgroupconfigurator to always update the log collector slice unit file to remove memorylimit. 


Issue # <!-- if any -->
<!--
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
- [ ] The title of the PR is clear and informative.
- [ ] 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).
- [ ] If applicable, the PR references the bug/issue that it fixes in the description.
- [ ] New Unit tests were added for the changes made

### Quality of Code and Contribution Guidelines
- [ ] I have read the [contribution guidelines](https://github.com/Azure/WALinuxAgent/blob/master/.github/CONTRIBUTING.md).

codecov[bot]: # [Codecov](https://codecov.io/gh/Azure/WALinuxAgent/pull/2757?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure) Report
> Merging [#2757](https://codecov.io/gh/Azure/WALinuxAgent/pull/2757?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure) (162ebbc) into [develop](https://codecov.io/gh/Azure/WALinuxAgent/commit/36a5ec1db1b6711898beb06fca43590745b1bea4?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure) (36a5ec1) will **decrease** coverage by `0.01%`.
> The diff coverage is `66.66%`.

> :exclamation: Current head 162ebbc differs from pull request most recent head c8dda53. Consider uploading reports for the commit c8dda53 to get more accurate results

```diff
@@             Coverage Diff             @@
##           develop    #2757      +/-   ##
===========================================
- Coverage    72.04%   72.03%   -0.01%     
===========================================
  Files          104      104              
  Lines        15832    15831       -1     
  Branches      2265     2264       -1     
===========================================
- Hits         11406    11404       -2     
+ Misses        3912     3909       -3     
- Partials       514      518       +4     
```


| [Impacted Files](https://codecov.io/gh/Azure/WALinuxAgent/pull/2757?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure) | Coverage Δ | |
|---|---|---|
| [azurelinuxagent/common/logcollector.py](https://codecov.io/gh/Azure/WALinuxAgent/pull/2757?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure#diff-YXp1cmVsaW51eGFnZW50L2NvbW1vbi9sb2djb2xsZWN0b3IucHk=) | `88.35% <0.00%> (ø)` | |
| [azurelinuxagent/common/cgroupconfigurator.py](https://codecov.io/gh/Azure/WALinuxAgent/pull/2757?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure#diff-YXp1cmVsaW51eGFnZW50L2NvbW1vbi9jZ3JvdXBjb25maWd1cmF0b3IucHk=) | `72.19% <100.00%> (-0.20%)` | :arrow_down: |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure)


Patch:
diff --git a/azurelinuxagent/common/cgroupconfigurator.py b/azurelinuxagent/common/cgroupconfigurator.py
index 840961e7..627567b0 100644
--- a/azurelinuxagent/common/cgroupconfigurator.py
+++ b/azurelinuxagent/common/cgroupconfigurator.py
@@ -368,10 +368,9 @@ class CGroupConfigurator(object):
             if not os.path.exists(vmextensions_slice):
                 files_to_create.append((vmextensions_slice, _VMEXTENSIONS_SLICE_CONTENTS))
 
-            if not os.path.exists(logcollector_slice):
-                slice_contents = _LOGCOLLECTOR_SLICE_CONTENTS_FMT.format(cpu_quota=_LOGCOLLECTOR_CPU_QUOTA)
-
-                files_to_create.append((logcollector_slice, slice_contents))
+            # Update log collector slice contents
+            slice_contents = _LOGCOLLECTOR_SLICE_CONTENTS_FMT.format(cpu_quota=_LOGCOLLECTOR_CPU_QUOTA)
+            files_to_create.append((logcollector_slice, slice_contents))
 
             if fileutil.findre_in_file(agent_unit_file, r"Slice=") is not None:
                 CGroupConfigurator._Impl.__cleanup_unit_file(agent_drop_in_file_slice)
diff --git a/azurelinuxagent/common/logcollector.py b/azurelinuxagent/common/logcollector.py
index 393333c9..fe62a7db 100644
--- a/azurelinuxagent/common/logcollector.py
+++ b/azurelinuxagent/common/logcollector.py
@@ -119,7 +119,7 @@ class LogCollector(object):
     @staticmethod
     def _initialize_telemetry():
         protocol = get_protocol_util().get_protocol(init_goal_state=False)
-        protocol.client.reset_goal_state(goalstate_properties=GoalStateProperties.RoleConfig | GoalStateProperties.HostingEnv)
+        protocol.client.reset_goal_state(goal_state_properties=GoalStateProperties.RoleConfig | GoalStateProperties.HostingEnv)
         # Initialize the common parameters for telemetry events
         initialize_event_logger_vminfo_common_parameters(protocol)
 


Test patch:
diff --git a/tests/common/mock_cgroup_environment.py b/tests/common/mock_cgroup_environment.py
index 10d49907..e3847106 100644
--- a/tests/common/mock_cgroup_environment.py
+++ b/tests/common/mock_cgroup_environment.py
@@ -91,6 +91,7 @@ _MOCKED_PATHS = [
 
 class UnitFilePaths:
     walinuxagent = "/lib/systemd/system/walinuxagent.service"
+    logcollector = "/lib/systemd/system/azure-walinuxagent-logcollector.slice"
     azure = "/lib/systemd/system/azure.slice"
     vmextensions = "/lib/systemd/system/azure-vmextensions.slice"
     extensionslice = "/lib/systemd/system/azure-vmextensions-Microsoft.CPlat.Extension.slice"
diff --git a/tests/common/test_cgroupconfigurator.py b/tests/common/test_cgroupconfigurator.py
index 60a7cfde..d3410cf5 100644
--- a/tests/common/test_cgroupconfigurator.py
+++ b/tests/common/test_cgroupconfigurator.py
@@ -188,6 +188,27 @@ cgroup on /sys/fs/cgroup/blkio type cgroup (rw,nosuid,nodev,noexec,relatime,blki
             self.assertTrue(os.path.exists(agent_drop_in_file_cpu_accounting), "{0} was not created".format(agent_drop_in_file_cpu_accounting))
             self.assertTrue(os.path.exists(agent_drop_in_file_memory_accounting), "{0} was not created".format(agent_drop_in_file_memory_accounting))
 
+    def test_initialize_should_update_logcollector_memorylimit(self):
+        with self._get_cgroup_configurator(initialize=False) as configurator:
+            log_collector_unit_file = configurator.mocks.get_mapped_path(UnitFilePaths.logcollector)
+            original_memory_limit = "MemoryLimit=30M"
+
+            # The mock creates the slice unit file with memory limit
+            configurator.mocks.add_data_file(os.path.join(data_dir, 'init', "azure-walinuxagent-logcollector.slice"),
+                                             UnitFilePaths.logcollector)
+            if not os.path.exists(log_collector_unit_file):
+                raise Exception("{0} should have been created during test setup".format(log_collector_unit_file))
+            if not fileutil.findre_in_file(log_collector_unit_file, original_memory_limit):
+                raise Exception("MemoryLimit was not set correctly. Expected: {0}. Got:\n{1}".format(
+                    original_memory_limit, fileutil.read_file(log_collector_unit_file)))
+
+            configurator.initialize()
+
+            # initialize() should update the unit file to remove the memory limit
+            self.assertFalse(fileutil.findre_in_file(log_collector_unit_file, original_memory_limit),
+                             "Log collector slice unit file was not updated correctly. Expected no memory limit. Got:\n{0}".format(
+                                 fileutil.read_file(log_collector_unit_file)))
+
     def test_setup_extension_slice_should_create_unit_files(self):
         with self._get_cgroup_configurator() as configurator:
             # get the paths to the mocked files
diff --git a/tests/data/init/azure-walinuxagent-logcollector.slice b/tests/data/init/azure-walinuxagent-logcollector.slice
new file mode 100644
index 00000000..63c09d43
--- /dev/null
+++ b/tests/data/init/azure-walinuxagent-logcollector.slice
@@ -0,0 +1,9 @@
+[Unit]
+Description=Slice for Azure VM Agent Periodic Log Collector
+DefaultDependencies=no
+Before=slices.target
+[Slice]
+CPUAccounting=yes
+CPUQuota=5%
+MemoryAccounting=yes
+MemoryLimit=30M
\ No newline at end of file