-
Notifications
You must be signed in to change notification settings - Fork 372
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
check for unexpected process in agent cgroups before cgroups enabled #3103
Merged
Merged
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
5558a78
check for unexpected process in cgroup before enable
nagworld9 ae32cf3
agent restart
nagworld9 e837868
move the process check
nagworld9 a1dbe21
fix unit tests
nagworld9 fd13729
address comments
nagworld9 27941cc
pylint
nagworld9 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -152,23 +152,9 @@ def initialize(self): | |
return | ||
# This check is to reset the quotas if agent goes from cgroup supported to unsupported distros later in time. | ||
if not CGroupsApi.cgroups_supported(): | ||
agent_drop_in_path = systemd.get_agent_drop_in_path() | ||
try: | ||
if os.path.exists(agent_drop_in_path) and os.path.isdir(agent_drop_in_path): | ||
files_to_cleanup = [] | ||
agent_drop_in_file_slice = os.path.join(agent_drop_in_path, _AGENT_DROP_IN_FILE_SLICE) | ||
agent_drop_in_file_cpu_accounting = os.path.join(agent_drop_in_path, | ||
_DROP_IN_FILE_CPU_ACCOUNTING) | ||
agent_drop_in_file_memory_accounting = os.path.join(agent_drop_in_path, | ||
_DROP_IN_FILE_MEMORY_ACCOUNTING) | ||
agent_drop_in_file_cpu_quota = os.path.join(agent_drop_in_path, _DROP_IN_FILE_CPU_QUOTA) | ||
files_to_cleanup.extend([agent_drop_in_file_slice, agent_drop_in_file_cpu_accounting, | ||
agent_drop_in_file_memory_accounting, agent_drop_in_file_cpu_quota]) | ||
self.__cleanup_all_files(files_to_cleanup) | ||
self.__reload_systemd_config() | ||
logger.info("Agent reset the quotas if distro: {0} goes from supported to unsupported list", get_distro()) | ||
except Exception as err: | ||
logger.warn("Unable to delete Agent drop-in files while resetting the quotas: {0}".format(err)) | ||
logger.info("Agent reset the quotas if distro: {0} goes from supported to unsupported list", | ||
get_distro()) | ||
self._reset_agent_cgroup_setup() | ||
|
||
# check whether cgroup monitoring is supported on the current distro | ||
self._cgroups_supported = CGroupsApi.cgroups_supported() | ||
|
@@ -187,15 +173,21 @@ def initialize(self): | |
if not self.__check_no_legacy_cgroups(): | ||
return | ||
|
||
cpu_controller_root, memory_controller_root = self.__get_cgroup_controllers() | ||
agent_unit_name = systemd.get_agent_unit_name() | ||
agent_slice = systemd.get_unit_property(agent_unit_name, "Slice") | ||
|
||
if conf.get_cgroup_disable_on_process_check_failure() and self._check_processes_in_agent_cgroup_before_enable(agent_slice, cpu_controller_root, memory_controller_root): | ||
reason = "Found unexpected processes in the agent cgroup before agent enable cgroups." | ||
self.disable(reason, DisableCgroups.ALL) | ||
return | ||
|
||
if agent_slice not in (AZURE_SLICE, "system.slice"): | ||
_log_cgroup_warning("The agent is within an unexpected slice: {0}", agent_slice) | ||
return | ||
|
||
self.__setup_azure_slice() | ||
|
||
cpu_controller_root, memory_controller_root = self.__get_cgroup_controllers() | ||
self._agent_cpu_cgroup_path, self._agent_memory_cgroup_path = self.__get_agent_cgroups(agent_slice, | ||
cpu_controller_root, | ||
memory_controller_root) | ||
|
@@ -341,6 +333,25 @@ def __setup_azure_slice(): | |
|
||
CGroupConfigurator._Impl.__reload_systemd_config() | ||
|
||
def _reset_agent_cgroup_setup(self): | ||
try: | ||
agent_drop_in_path = systemd.get_agent_drop_in_path() | ||
if os.path.exists(agent_drop_in_path) and os.path.isdir(agent_drop_in_path): | ||
files_to_cleanup = [] | ||
agent_drop_in_file_slice = os.path.join(agent_drop_in_path, _AGENT_DROP_IN_FILE_SLICE) | ||
agent_drop_in_file_cpu_accounting = os.path.join(agent_drop_in_path, | ||
_DROP_IN_FILE_CPU_ACCOUNTING) | ||
agent_drop_in_file_memory_accounting = os.path.join(agent_drop_in_path, | ||
_DROP_IN_FILE_MEMORY_ACCOUNTING) | ||
agent_drop_in_file_cpu_quota = os.path.join(agent_drop_in_path, _DROP_IN_FILE_CPU_QUOTA) | ||
files_to_cleanup.extend([agent_drop_in_file_slice, agent_drop_in_file_cpu_accounting, | ||
agent_drop_in_file_memory_accounting, agent_drop_in_file_cpu_quota]) | ||
self.__cleanup_all_files(files_to_cleanup) | ||
self.__reload_systemd_config() | ||
except Exception as err: | ||
logger.warn("Unable to delete Agent drop-in files while resetting the quotas: {0}".format(err)) | ||
|
||
|
||
@staticmethod | ||
def __reload_systemd_config(): | ||
# reload the systemd configuration; the new slices will be used once the agent's service restarts | ||
|
@@ -546,6 +557,35 @@ def __try_set_cpu_quota(quota): # pylint: disable=unused-private-member | |
return False | ||
return True | ||
|
||
def _check_processes_in_agent_cgroup_before_enable(self, agent_slice, cpu_root_cgroup_path, memory_root_cgroup_path): | ||
""" | ||
This check ensures that before we enable the agent's cgroups, there are no unexpected processes in the agent's cgroup already. | ||
|
||
The issue we observed that long running extension processes may be in agent cgroups if agent goes this cycle enabled(1)->disabled(2)->enabled(3). | ||
1. Agent cgroups enabled in some version | ||
2. Disabled agent cgroups due to check_cgroups regular check, now extension runs will be in agent cgroups. | ||
3. When ext_hanlder restart and enable the cgroups again, already running processes from step 2 still be in agent cgroups. This may cause the extensions run with agent limit. | ||
""" | ||
if agent_slice not in AZURE_SLICE: | ||
narrieta marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return False | ||
cpu_cgroup_relative_path, memory_cgroup_relative_path = self._cgroups_api.get_process_cgroup_paths("self") | ||
agent_cpu_cgroup_path = os.path.join(cpu_root_cgroup_path, cpu_cgroup_relative_path) | ||
agent_memory_cgroup_path = os.path.join(memory_root_cgroup_path, memory_cgroup_relative_path) | ||
|
||
check_cgroup_path = agent_cpu_cgroup_path if agent_cpu_cgroup_path is not None else agent_memory_cgroup_path | ||
|
||
if check_cgroup_path is None: | ||
return False | ||
|
||
try: | ||
_log_cgroup_info("Checking for unexpected processes in the agent's cgroup before enabling cgroups") | ||
self._check_processes_in_agent_cgroup(check_cgroup_path) | ||
except CGroupsException as exception: | ||
_log_cgroup_warning(ustr(exception)) | ||
return True | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It feels unintuitive that this function returns True when the check fails. Can we rename the function or switch the return value? |
||
|
||
return False | ||
|
||
def check_cgroups(self, cgroup_metrics): | ||
self._check_cgroups_lock.acquire() | ||
try: | ||
|
@@ -579,7 +619,7 @@ def check_cgroups(self, cgroup_metrics): | |
finally: | ||
self._check_cgroups_lock.release() | ||
|
||
def _check_processes_in_agent_cgroup(self): | ||
def _check_processes_in_agent_cgroup(self, cgroup_path=None): | ||
""" | ||
Verifies that the agent's cgroup includes only the current process, its parent, commands started using shellutil and instances of systemd-run | ||
(those processes correspond, respectively, to the extension handler, the daemon, commands started by the extension handler, and the systemd-run | ||
|
@@ -591,14 +631,15 @@ def _check_processes_in_agent_cgroup(self): | |
""" | ||
unexpected = [] | ||
agent_cgroup_proc_names = [] | ||
check_cgroup_path = cgroup_path if cgroup_path is not None else self._agent_cpu_cgroup_path | ||
try: | ||
daemon = os.getppid() | ||
extension_handler = os.getpid() | ||
agent_commands = set() | ||
agent_commands.update(shellutil.get_running_commands()) | ||
systemd_run_commands = set() | ||
systemd_run_commands.update(self._cgroups_api.get_systemd_run_commands()) | ||
agent_cgroup = CGroupsApi.get_processes_in_cgroup(self._agent_cpu_cgroup_path) | ||
agent_cgroup = CGroupsApi.get_processes_in_cgroup(check_cgroup_path) | ||
# get the running commands again in case new commands started or completed while we were fetching the processes in the cgroup; | ||
agent_commands.update(shellutil.get_running_commands()) | ||
systemd_run_commands.update(self._cgroups_api.get_systemd_run_commands()) | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -905,6 +905,24 @@ def test_check_cgroups_should_disable_cgroups_when_a_check_fails(self): | |||||
for p in patchers: | ||||||
p.stop() | ||||||
|
||||||
@patch('azurelinuxagent.ga.cgroupconfigurator.CGroupConfigurator._Impl._check_processes_in_agent_cgroup', side_effect=CGroupsException("Test")) | ||||||
@patch('azurelinuxagent.ga.cgroupconfigurator.add_event') | ||||||
def test_agent_not_enable_cgroups_if_unexpected_process_already_in_agent_cgroups(self, add_event, _): | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
command_mocks = [MockCommand(r"^systemctl show walinuxagent\.service --property Slice", | ||||||
'''Slice=azure.slice | ||||||
''')] | ||||||
with self._get_cgroup_configurator(mock_commands=command_mocks) as configurator: | ||||||
self.assertFalse(configurator.enabled(), "Cgroups should not be enabled") | ||||||
|
||||||
disable_events = [kwargs for _, kwargs in add_event.call_args_list if kwargs["op"] == WALAEventOperation.CGroupsDisabled] | ||||||
self.assertTrue( | ||||||
len(disable_events) == 1, | ||||||
"Exactly 1 event should have been emitted. Got: {0}".format(disable_events)) | ||||||
self.assertIn( | ||||||
"Found unexpected processes in the agent cgroup before agent enable cgroups", | ||||||
disable_events[0]["message"], | ||||||
"The error message is not correct when process check failed") | ||||||
|
||||||
def test_check_agent_memory_usage_should_raise_a_cgroups_exception_when_the_limit_is_exceeded(self): | ||||||
metrics = [MetricValue(MetricsCategory.MEMORY_CATEGORY, MetricsCounter.TOTAL_MEM_USAGE, AGENT_NAME_TELEMETRY, conf.get_agent_memory_quota() + 1), | ||||||
MetricValue(MetricsCategory.MEMORY_CATEGORY, MetricsCounter.SWAP_MEM_USAGE, AGENT_NAME_TELEMETRY, conf.get_agent_memory_quota() + 1)] | ||||||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,11 @@ | ||
# | ||
# The test suite verify the agent running in expected cgroups and also, checks agent tracking the cgroups for polling resource metrics. Also, it verifies the agent cpu quota is set as expected. | ||
# The test suite verify the agent running in expected cgroups and also, checks agent tracking the cgroups for polling resource metrics, | ||
# checks unexpected processes in the agent cgroups, and it verifies the agent cpu quota is set as expected. | ||
# | ||
name: "AgentCgroups" | ||
tests: | ||
- "agent_cgroups/agent_cgroups.py" | ||
- "agent_cgroups/agent_cpu_quota.py" | ||
- "agent_cgroups/agent_cgroups_process_check.py" | ||
images: "cgroups-endorsed" | ||
owns_vm: true |
77 changes: 77 additions & 0 deletions
77
tests_e2e/tests/agent_cgroups/agent_cgroups_process_check.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,77 @@ | ||
#!/usr/bin/env python3 | ||
|
||
# Microsoft Azure Linux Agent | ||
# | ||
# Copyright 2018 Microsoft Corporation | ||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||
# you may not use this file except in compliance with the License. | ||
# You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
# | ||
|
||
from typing import List, Dict, Any | ||
|
||
from tests_e2e.tests.lib.agent_test import AgentVmTest | ||
from tests_e2e.tests.lib.agent_test_context import AgentVmTestContext | ||
from tests_e2e.tests.lib.logging import log | ||
from tests_e2e.tests.lib.virtual_machine_extension_client import VirtualMachineExtensionClient | ||
from tests_e2e.tests.lib.vm_extension_identifier import VmExtensionIds | ||
|
||
|
||
class AgentCgroupsProcessCheck(AgentVmTest): | ||
""" | ||
Tests the agent's ability to detect processes that do not belong to the agent's cgroup | ||
""" | ||
def __init__(self, context: AgentVmTestContext): | ||
super().__init__(context) | ||
self._ssh_client = self._context.create_ssh_client() | ||
|
||
def run(self): | ||
""" | ||
Steps: | ||
1. Verify that agent detects processes that do not belong to the agent's cgroup and disable the cgroups | ||
2. Run the extension, so that they are run in the agent's cgroup | ||
3. Restart the ext_handler process to re-initialize the cgroups setup | ||
4. Verify that agent detects extension processes and will not enable the cgroups | ||
""" | ||
|
||
log.info("=====Validating agent cgroups process check") | ||
self._run_remote_test(self._ssh_client, "agent_cgroups_process_check-unknown_process_check.py", use_sudo=True) | ||
|
||
self._install_ama_extension() | ||
|
||
log.info("=====Validating agent cgroups not enabled") | ||
self._run_remote_test(self._ssh_client, "agent_cgroups_process_check-cgroups_not_enabled.py", use_sudo=True) | ||
|
||
def _install_ama_extension(self): | ||
ama_extension = VirtualMachineExtensionClient( | ||
self._context.vm, VmExtensionIds.AzureMonitorLinuxAgent, | ||
resource_name="AMAAgent") | ||
log.info("Installing %s", ama_extension) | ||
ama_extension.enable() | ||
ama_extension.assert_instance_view() | ||
|
||
def get_ignore_error_rules(self) -> List[Dict[str, Any]]: | ||
|
||
ignore_rules = [ | ||
# This is produced by the test, so it is expected | ||
# Examples: | ||
# 2024-04-01T19:16:11.929000Z INFO MonitorHandler ExtHandler [CGW] Disabling resource usage monitoring. Reason: Check on cgroups failed: | ||
# [CGroupsException] The agent's cgroup includes unexpected processes: ['[PID: 2957] dd\x00if=/dev/zero\x00of=/dev/null\x00 '] | ||
# 2024-04-01T19:17:04.995276Z WARNING ExtHandler ExtHandler [CGroupsException] The agent's cgroup includes unexpected processes: ['[PID: 3285] /usr/bin/python3\x00/var/lib/waagent/Microsoft.Azure.Monitor.AzureM', '[PID: 3286] /usr/bin/python3\x00/var/lib/waagent/Microsoft.Azure.Monitor.AzureM'] | ||
{'message': r"The agent's cgroup includes unexpected processes"}, | ||
{'message': r"Found unexpected processes in the agent cgroup before agent enable cgroups"} | ||
] | ||
return ignore_rules | ||
|
||
|
||
if __name__ == "__main__": | ||
AgentCgroupsProcessCheck.run_from_command_line() |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
60 changes: 60 additions & 0 deletions
60
tests_e2e/tests/scripts/agent_cgroups_process_check-cgroups_not_enabled.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
#!/usr/bin/env pypy3 | ||
|
||
# Microsoft Azure Linux Agent | ||
# | ||
# Copyright 2018 Microsoft Corporation | ||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||
# you may not use this file except in compliance with the License. | ||
# You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
# | ||
|
||
# This script verifies agent detected unexpected processes in the agent cgroup before cgroup initialization | ||
|
||
from assertpy import fail | ||
|
||
from azurelinuxagent.common.utils import shellutil | ||
from tests_e2e.tests.lib.cgroup_helpers import check_agent_quota_disabled, check_log_message | ||
from tests_e2e.tests.lib.logging import log | ||
from tests_e2e.tests.lib.retry import retry_if_false | ||
|
||
|
||
def restart_ext_handler(): | ||
log.info("Restarting the extension handler") | ||
shellutil.run_command(["pkill", "-f", "WALinuxAgent.*run-exthandler"]) | ||
|
||
|
||
def verify_agent_cgroups_not_enabled(): | ||
""" | ||
Verifies that the agent cgroups not enabled when ama extension(unexpected) processes are found in the agent cgroup | ||
""" | ||
log.info("Verifying agent cgroups are not enabled") | ||
|
||
ama_process_found: bool = retry_if_false(lambda: check_log_message("The agent's cgroup includes unexpected processes:.+/var/lib/waagent/Microsoft.Azure.Monitor")) | ||
if not ama_process_found: | ||
fail("Agent failed to found ama extension processes in the agent cgroup") | ||
|
||
found: bool = retry_if_false(lambda: check_log_message("Found unexpected processes in the agent cgroup before agent enable cgroups")) | ||
if not found: | ||
fail("Agent failed to found unknown processes in the agent cgroup") | ||
|
||
disabled: bool = retry_if_false(check_agent_quota_disabled) | ||
if not disabled: | ||
fail("The agent failed to disable its CPUQuota when cgroups were not enabled") | ||
|
||
|
||
def main(): | ||
restart_ext_handler() | ||
verify_agent_cgroups_not_enabled() | ||
|
||
|
||
if __name__ == "__main__": | ||
main() |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand #2.
If cgroups are disabled because some unexpected process is found in agent cgroup, then why would extension runs now be in agent cgroups?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once agent cgroups disabled (disable for both agent and extensions), we don't run the extensions in the extension slice. That means it will be in agent cgroups.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you please add those details to the comment