From f899bbc4f29d41439187d9353d6c3426b0d58742 Mon Sep 17 00:00:00 2001 From: Matthew Jones Date: Mon, 14 Feb 2022 15:14:27 +0000 Subject: [PATCH 1/2] Purge deprecated resource profiling capability --- ansible_runner/__main__.py | 54 --------------------------------- ansible_runner/config/runner.py | 53 +------------------------------- ansible_runner/interface.py | 7 ----- ansible_runner/runner.py | 31 ------------------- docs/intro.rst | 23 -------------- test/integration/test_events.py | 45 --------------------------- test/unit/config/test_runner.py | 50 ------------------------------ 7 files changed, 1 insertion(+), 262 deletions(-) diff --git a/ansible_runner/__main__.py b/ansible_runner/__main__.py index 0413f9f50..df95162fb 100644 --- a/ansible_runner/__main__.py +++ b/ansible_runner/__main__.py @@ -320,54 +320,6 @@ "to prevent multiple simultaneous executions from conflicting " "(default=None)" ) - ), - ( - ("--resource-profiling",), - dict( - dest='resource_profiling', - action="store_true", - help="Records resource utilization during playbook execution" - ) - ), - ( - ("--resource-profiling-base-cgroup",), - dict( - dest='resource_profiling_base_cgroup', - default="ansible-runner", - help="Top-level cgroup used to collect information on resource utilization. Defaults to ansible-runner" - ) - ), - ( - ("--resource-profiling-cpu-poll-interval",), - dict( - dest='resource_profiling_cpu_poll_interval', - default=0.25, - help="Interval (in seconds) between CPU polling for determining CPU usage. Defaults to 0.25" - ) - ), - ( - ("--resource-profiling-memory-poll-interval",), - dict( - dest='resource_profiling_memory_poll_interval', - default=0.25, - help="Interval (in seconds) between memory polling for determining memory usage. Defaults to 0.25" - ) - ), - ( - ("--resource-profiling-pid-poll-interval",), - dict( - dest='resource_profiling_pid_poll_interval', - default=0.25, - help="Interval (in seconds) between polling PID count for determining number of processes used. Defaults to 0.25" - ) - ), - ( - ("--resource-profiling-results-dir",), - dict( - dest='resource_profiling_results_dir', - help="Directory where profiling data files should be saved. " - "Defaults to None (profiling_data folder under private data dir is used in this case)." - ) ) ), "modules_group": ( @@ -895,12 +847,6 @@ def main(sys_args=None): container_volume_mounts=vargs.get('container_volume_mounts'), container_options=vargs.get('container_options'), directory_isolation_base_path=vargs.get('directory_isolation_base_path'), - resource_profiling=vargs.get('resource_profiling'), - resource_profiling_base_cgroup=vargs.get('resource_profiling_base_cgroup'), - resource_profiling_cpu_poll_interval=vargs.get('resource_profiling_cpu_poll_interval'), - resource_profiling_memory_poll_interval=vargs.get('resource_profiling_memory_poll_interval'), - resource_profiling_pid_poll_interval=vargs.get('resource_profiling_pid_poll_interval'), - resource_profiling_results_dir=vargs.get('resource_profiling_results_dir'), cmdline=vargs.get('cmdline'), limit=vargs.get('limit'), streamer=streamer diff --git a/ansible_runner/config/runner.py b/ansible_runner/config/runner.py index 9fb96bb20..11f6d353b 100644 --- a/ansible_runner/config/runner.py +++ b/ansible_runner/config/runner.py @@ -67,10 +67,7 @@ def __init__(self, module=None, module_args=None, verbosity=None, host_pattern=None, binary=None, extravars=None, suppress_output_file=False, suppress_ansible_output=False, process_isolation_path=None, process_isolation_hide_paths=None, process_isolation_show_paths=None, - process_isolation_ro_paths=None, resource_profiling=False, - resource_profiling_base_cgroup='ansible-runner', resource_profiling_cpu_poll_interval=0.25, - resource_profiling_memory_poll_interval=0.25, resource_profiling_pid_poll_interval=0.25, - resource_profiling_results_dir=None, tags=None, skip_tags=None, + process_isolation_ro_paths=None, tags=None, skip_tags=None, directory_isolation_base_path=None, forks=None, cmdline=None, omit_event_data=False, only_failed_event_data=False, **kwargs): @@ -92,13 +89,6 @@ def __init__(self, self.process_isolation_hide_paths = process_isolation_hide_paths self.process_isolation_show_paths = process_isolation_show_paths self.process_isolation_ro_paths = process_isolation_ro_paths - self.resource_profiling = resource_profiling - self.resource_profiling_base_cgroup = resource_profiling_base_cgroup - self.resource_profiling_cpu_poll_interval = resource_profiling_cpu_poll_interval - self.resource_profiling_memory_poll_interval = resource_profiling_memory_poll_interval - self.resource_profiling_pid_poll_interval = resource_profiling_pid_poll_interval - self.resource_profiling_results_dir = resource_profiling_results_dir - self.directory_isolation_path = directory_isolation_base_path self.verbosity = verbosity self.suppress_output_file = suppress_output_file @@ -194,14 +184,6 @@ def prepare_env(self): self.directory_isolation_path = self.settings.get('directory_isolation_base_path', self.directory_isolation_path) self.directory_isolation_cleanup = bool(self.settings.get('directory_isolation_cleanup', True)) - self.resource_profiling = self.settings.get('resource_profiling', self.resource_profiling) - self.resource_profiling_base_cgroup = self.settings.get('resource_profiling_base_cgroup', self.resource_profiling_base_cgroup) - self.resource_profiling_cpu_poll_interval = self.settings.get('resource_profiling_cpu_poll_interval', self.resource_profiling_cpu_poll_interval) - self.resource_profiling_memory_poll_interval = self.settings.get('resource_profiling_memory_poll_interval', - self.resource_profiling_memory_poll_interval) - self.resource_profiling_pid_poll_interval = self.settings.get('resource_profiling_pid_poll_interval', self.resource_profiling_pid_poll_interval) - self.resource_profiling_results_dir = self.settings.get('resource_profiling_results_dir', self.resource_profiling_results_dir) - if 'AD_HOC_COMMAND_ID' in self.env or not os.path.exists(self.project_dir): self.cwd = self.private_data_dir else: @@ -217,28 +199,6 @@ def prepare_env(self): else: self.fact_cache = os.path.join(self.artifact_dir, self.settings['fact_cache']) - if self.resource_profiling: - callback_whitelist = os.environ.get('ANSIBLE_CALLBACK_WHITELIST', '').strip() - self.env['ANSIBLE_CALLBACK_WHITELIST'] = ','.join(filter(None, [callback_whitelist, 'cgroup_perf_recap'])) - self.env['CGROUP_CONTROL_GROUP'] = '{}/{}'.format(self.resource_profiling_base_cgroup, self.ident) - if self.resource_profiling_results_dir: - cgroup_output_dir = self.resource_profiling_results_dir - else: - cgroup_output_dir = os.path.normpath(os.path.join(self.private_data_dir, 'profiling_data')) - - # Create results directory if it does not exist - if not os.path.isdir(cgroup_output_dir): - os.mkdir(cgroup_output_dir, stat.S_IREAD | stat.S_IWRITE | stat.S_IEXEC) - - self.env['CGROUP_OUTPUT_DIR'] = cgroup_output_dir - self.env['CGROUP_OUTPUT_FORMAT'] = 'json' - self.env['CGROUP_CPU_POLL_INTERVAL'] = str(self.resource_profiling_cpu_poll_interval) - self.env['CGROUP_MEMORY_POLL_INTERVAL'] = str(self.resource_profiling_memory_poll_interval) - self.env['CGROUP_PID_POLL_INTERVAL'] = str(self.resource_profiling_pid_poll_interval) - self.env['CGROUP_FILE_PER_TASK'] = 'True' - self.env['CGROUP_WRITE_FILES'] = 'True' - self.env['CGROUP_DISPLAY_RECAP'] = 'False' - if self.roles_path: if isinstance(self.roles_path, list): self.env['ANSIBLE_ROLES_PATH'] = ':'.join(self.roles_path) @@ -367,14 +327,6 @@ def build_process_isolation_temp_dir(self): return path - def wrap_args_with_cgexec(self, args): - ''' - Wrap existing command line with cgexec in order to profile resource usage - ''' - new_args = ['cgexec', '--sticky', '-g', 'cpuacct,memory,pids:{}/{}'.format(self.resource_profiling_base_cgroup, self.ident)] - new_args.extend(args) - return new_args - def wrap_args_for_sandbox(self, args): ''' Wrap existing command line with bwrap to restrict access to: @@ -443,9 +395,6 @@ def _handle_command_wrap(self): else: debug('sandbox disabled') - if self.resource_profiling and self.execution_mode == ExecutionMode.ANSIBLE_PLAYBOOK: - self.command = self.wrap_args_with_cgexec(self.command) - if self.containerized: debug('containerization enabled') # conatiner volume mount is handled explicitly for run API's diff --git a/ansible_runner/interface.py b/ansible_runner/interface.py index 1c2e01b39..b936b563d 100644 --- a/ansible_runner/interface.py +++ b/ansible_runner/interface.py @@ -192,13 +192,6 @@ def run(**kwargs): :param str container_image: Container image to use when running an ansible task (default: quay.io/ansible/ansible-runner:devel) :param list container_volume_mounts: List of bind mounts in the form 'host_dir:/container_dir. (default: None) :param list container_options: List of container options to pass to execution engine. - :param bool resource_profiling: Enable collection of resource utilization data during playbook execution. - :param str resource_profiling_base_cgroup: Name of existing cgroup which will be sub-grouped in order to measure - resource utilization (default: ansible-runner) - :param float resource_profiling_cpu_poll_interval: Interval (in seconds) between CPU polling for determining CPU usage (default: 0.25) - :param float resource_profiling_memory_poll_interval: Interval (in seconds) between memory polling for determining memory usage (default: 0.25) - :param float resource_profiling_pid_poll_interval: Interval (in seconds) between polling PID count for determining number of processes used (default: 0.25) - :param str resource_profiling_results_dir: Directory where profiling data files should be saved (defaults to profiling_data folder inside private data dir) :param str directory_isolation_base_path: An optional path will be used as the base path to create a temp directory, the project contents will be copied to this location which will then be used as the working directory during playbook execution. :param str fact_cache: A string that will be used as the name for the subdirectory of the fact cache in artifacts directory. diff --git a/ansible_runner/runner.py b/ansible_runner/runner.py index e635923c2..b91f0469d 100644 --- a/ansible_runner/runner.py +++ b/ansible_runner/runner.py @@ -43,7 +43,6 @@ def __init__(self, config, cancel_callback=None, remove_partials=True, event_han # default runner mode to pexpect self.runner_mode = self.config.runner_mode if hasattr(self.config, 'runner_mode') else 'pexpect' - self.resource_profiling = self.config.resource_profiling if hasattr(self.config, 'resource_profiling') else False self.directory_isolation_path = self.config.directory_isolation_path if hasattr(self.config, 'directory_isolation_path') else None self.directory_isolation_cleanup = self.config.directory_isolation_cleanup if hasattr(self.config, 'directory_isolation_cleanup') else None self.process_isolation = self.config.process_isolation if hasattr(self.config, 'process_isolation') else None @@ -200,29 +199,6 @@ def run(self): for k, v in pexpect_env.items() } - # Prepare to collect performance data - if self.resource_profiling: - cgroup_path = '{0}/{1}'.format(self.config.resource_profiling_base_cgroup, self.config.ident) - - import getpass - import grp - user = getpass.getuser() - group = grp.getgrgid(os.getgid()).gr_name - - cmd = ['cgcreate', - '-a', f'{user}:{group}', - '-t', f'{user}:{group}', - '-g', f'cpuacct,memory,pids:{cgroup_path}', - ] - proc = Popen(cmd, stdout=PIPE, stderr=PIPE) - _, stderr = proc.communicate() - if proc.returncode: - # Unable to create cgroup - logger.error('Unable to create cgroup: {}'.format(stderr)) - raise RuntimeError('Unable to create cgroup: {}'.format(stderr)) - else: - logger.info("Created cgroup '{}'".format(cgroup_path)) - self.status_callback('running') self.last_stdout_update = time.time() @@ -398,13 +374,6 @@ def _delete(retries=15): raise return True _delete() - if self.resource_profiling: - cmd = ['cgdelete', '-g', f'cpuacct,memory,pids:{cgroup_path}'] - proc = Popen(cmd, stdout=PIPE, stderr=PIPE) - _, stderr = proc.communicate() - if proc.returncode: - logger.error('Failed to delete cgroup: {}'.format(stderr)) - raise RuntimeError('Failed to delete cgroup: {}'.format(stderr)) if self.artifacts_handler is not None: try: diff --git a/docs/intro.rst b/docs/intro.rst index b8ee5b8ec..76bd6efbe 100644 --- a/docs/intro.rst +++ b/docs/intro.rst @@ -175,29 +175,6 @@ To run Ansible Runner with your custom container: See ``ansible-runner -h`` for other container-related options. -Performance Data Collection Settings for Runner -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -**Runner** is capable of collecting performance data (namely cpu usage, memory usage, and pid count) during the execution of a playbook run. - -Resource profiling is made possible by the use of control groups (often referred to simply as cgroups). When a process runs inside of a cgroup, the resources used by that specific process can be measured. - -Before enabling Runner's resource profiling feature, users must create a cgroup that **Runner** can use. It is worth noting that only privileged users can create cgroups. The new cgroup should be associated with the same user (and related group) that will be invoking **Runner**. The following command accomplishes this on a RHEL system:: - - sudo yum install libcgroup-tools - sudo cgcreate -a `whoami` -t `whoami` -g cpuacct,memory,pids:ansible-runner - -In the above command, ``cpuacct``, ``memory``, and ``pids`` refer to kernel resource controllers, while ``ansible-runner`` refers to the name of the cgroup being created. More detailed information on the structure of cgroups can be found in the RHEL guide on `Managing, monitoring, and updating the kernel `_ - -After a cgroup has been created, the following settings can be used to configure resource profiling. Note that ``resource_profiling_base_cgroup`` must match the name of the cgroup you create. - -* ``resource_profiling``: ``False`` Enable performance data collection. -* ``resource_profiling_base_cgroup``: ``ansible-runner`` Top-level cgroup used to measure playbook resource utilization. -* ``resource_profiling_cpu_poll_interval``: ``0.25`` Polling interval in seconds for collecting cpu usage. -* ``resource_profiling_memory_poll_interval``: ``0.25`` Polling interval in seconds for collecting memory usage. -* ``resource_profiling_pid_poll_interval``: ``0.25`` Polling interval in seconds for measuring PID count. -* ``resource_profiling_results_dir``: ``None`` Directory where resource utilization data will be written (if not specified, will be placed in the ``profiling_data`` folder under the private data directory). - Inventory --------- diff --git a/test/integration/test_events.py b/test/integration/test_events.py index e91fc35ab..ebd0bafb5 100644 --- a/test/integration/test_events.py +++ b/test/integration/test_events.py @@ -142,48 +142,3 @@ def test_include_role_events(project_fixtures): assert not event_data.get('warning', False) # role use should not contain warnings if event['event'] == 'runner_on_ok': assert event_data['res']['msg'] == 'Hello world!' - - -@pytest.mark.skipif(shutil.which('cgexec') is None, - reason="cgexec not available") -def test_profile_data(tmp_path): - try: - r = run(private_data_dir=str(tmp_path), - inventory='localhost ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}"', - resource_profiling=True, - resource_profiling_base_cgroup='ansible-runner', - playbook=[{'hosts': 'all', 'gather_facts': False, 'tasks': [{'debug': {'msg': "test"}}]}]) - assert r.config.env['ANSIBLE_CALLBACK_WHITELIST'] == 'cgroup_perf_recap' - assert r.config.env['CGROUP_CONTROL_GROUP'].startswith('ansible-runner/') - expected_datadir = os.path.join(str(tmp_path), 'profiling_data') - assert r.config.env['CGROUP_OUTPUT_DIR'] == expected_datadir - assert r.config.env['CGROUP_OUTPUT_FORMAT'] == 'json' - assert r.config.env['CGROUP_CPU_POLL_INTERVAL'] == '0.25' - assert r.config.env['CGROUP_MEMORY_POLL_INTERVAL'] == '0.25' - assert r.config.env['CGROUP_PID_POLL_INTERVAL'] == '0.25' - assert r.config.env['CGROUP_FILE_PER_TASK'] == 'True' - assert r.config.env['CGROUP_WRITE_FILES'] == 'True' - assert r.config.env['CGROUP_DISPLAY_RECAP'] == 'False' - - data_files = [f for f in os.listdir(expected_datadir) - if os.path.isfile(os.path.join(expected_datadir, f))] - # Ensure each type of metric is represented in the results - for metric in ('cpu', 'memory', 'pids'): - assert len([f for f in data_files if '{}.json'.format(metric) in f]) == 1 - - # Ensure each file consists of a list of json dicts - for file in data_files: - with open(os.path.join(expected_datadir, file)) as f: - for line in f: - line = line[1:-1] # strip RS and LF (see https://tools.ietf.org/html/rfc7464#section-2.2) - try: - json.loads(line) - except json.JSONDecodeError as e: - pytest.fail("Failed to parse {}: '{}'" - .format(os.path.join(expected_datadir, file), e)) - - except RuntimeError: - pytest.skip( - 'this test requires a cgroup to run e.g., ' - 'sudo cgcreate -a `whoami` -t `whoami` -g cpuacct,memory,pids:ansible-runner' - ) # noqa diff --git a/test/unit/config/test_runner.py b/test/unit/config/test_runner.py index e9cee80cb..4886d2956 100644 --- a/test/unit/config/test_runner.py +++ b/test/unit/config/test_runner.py @@ -669,56 +669,6 @@ def test_process_isolation_settings(mocker, tmp_path): assert rc.command[23:] == ['--chdir', '/project', 'ansible-playbook', '-i', '/inventory', 'main.yaml'] -def test_profiling_plugin_settings(mocker): - mocker.patch('os.mkdir', return_value=True) - - rc = RunnerConfig('/') - rc.playbook = 'main.yaml' - rc.command = 'ansible-playbook' - rc.resource_profiling = True - rc.resource_profiling_base_cgroup = 'ansible-runner' - rc.prepare() - - expected_command_start = [ - 'cgexec', - '--sticky', - '-g', - 'cpuacct,memory,pids:ansible-runner/{}'.format(rc.ident), - 'ansible-playbook', - 'main.yaml', - ] - - assert expected_command_start == rc.command - assert 'main.yaml' in rc.command - assert rc.env['ANSIBLE_CALLBACK_WHITELIST'] == 'cgroup_perf_recap' - assert rc.env['CGROUP_CONTROL_GROUP'] == 'ansible-runner/{}'.format(rc.ident) - assert rc.env['CGROUP_OUTPUT_DIR'] == os.path.normpath(os.path.join(rc.private_data_dir, 'profiling_data')) - assert rc.env['CGROUP_OUTPUT_FORMAT'] == 'json' - assert rc.env['CGROUP_CPU_POLL_INTERVAL'] == '0.25' - assert rc.env['CGROUP_MEMORY_POLL_INTERVAL'] == '0.25' - assert rc.env['CGROUP_PID_POLL_INTERVAL'] == '0.25' - assert rc.env['CGROUP_FILE_PER_TASK'] == 'True' - assert rc.env['CGROUP_WRITE_FILES'] == 'True' - assert rc.env['CGROUP_DISPLAY_RECAP'] == 'False' - - -def test_profiling_plugin_settings_with_custom_intervals(mocker): - mocker.patch('os.mkdir', return_value=True) - - rc = RunnerConfig('/') - rc.playbook = 'main.yaml' - rc.command = 'ansible-playbook' - rc.resource_profiling = True - rc.resource_profiling_base_cgroup = 'ansible-runner' - rc.resource_profiling_cpu_poll_interval = '.5' - rc.resource_profiling_memory_poll_interval = '.75' - rc.resource_profiling_pid_poll_interval = '1.5' - rc.prepare() - assert rc.env['CGROUP_CPU_POLL_INTERVAL'] == '.5' - assert rc.env['CGROUP_MEMORY_POLL_INTERVAL'] == '.75' - assert rc.env['CGROUP_PID_POLL_INTERVAL'] == '1.5' - - def test_container_volume_mounting_with_Z(mocker, tmp_path): mocker.patch('os.path.isdir', return_value=True) mocker.patch('os.path.exists', return_value=True) From 50ffe72f48cc1276966650b79e5e17232ca09f85 Mon Sep 17 00:00:00 2001 From: Matthew Jones Date: Mon, 14 Feb 2022 16:33:26 +0000 Subject: [PATCH 2/2] Removing unused imports --- test/integration/test_events.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/integration/test_events.py b/test/integration/test_events.py index ebd0bafb5..0df8e88a3 100644 --- a/test/integration/test_events.py +++ b/test/integration/test_events.py @@ -1,7 +1,4 @@ -import os import json - -import shutil import pytest from ansible_runner import defaults, run, run_async