mirror of
https://github.com/ansible/awx.git
synced 2026-06-23 07:37:50 -02:30
[AAP-74343] Decouple installed_collections/ansible_version from indirect node counting flag (#16453)
* [AAP-74343] Decouple installed_collections and ansible_version from indirect node counting flag The indirect_instance_count callback plugin and its artifact processing were entirely gated behind FEATURE_INDIRECT_NODE_COUNTING_ENABLED. This caused installed_collections and ansible_version to remain unpopulated when the flag was off, even though these are baseline analytics fields unrelated to indirect host counting. Always run the callback plugin and persist installed_collections and ansible_version to the database. Only the indirect-counting-specific parts (EventQuery creation, event_queries_processed flag, and vendor collections) remain gated behind the feature flag. * [AAP-74343] Read callbacks_enabled from ansible.cfg so user-configured callbacks are preserved The check for 'callbacks_enabled' in config_values was dead code because read_ansible_config was never asked to read that setting. Now that the callback registration runs unconditionally, fix this by including 'callbacks_enabled' in the variables of interest. * [AAP-74343] Use comma delimiter for ANSIBLE_CALLBACKS_ENABLED Ansible's CALLBACKS_ENABLED config is type list and splits on commas. The colon delimiter would cause combined callback names to be treated as a single invalid name. * [AAP-74343] Add tests for ANSIBLE_CALLBACKS_ENABLED configuration Verify that indirect_instance_count is always set, user-configured callbacks from ansible.cfg are preserved, and the comma delimiter is used as ansible-core expects. * [AAP-74343] Use public API for namespace package path access Replace library.__path__._path[0] with library.__path__[0] to avoid relying on a private CPython implementation detail of _NamespacePath. * [AAP-74343] Skip host query scanning when indirect counting flag is off The indirect_instance_count callback plugin now checks AWX_COLLECT_HOST_QUERIES to decide whether to scan for host query files. When the feature flag is off, the plugin only collects collection metadata (name + version) and ansible_version, skipping the expensive embedded/external query file discovery. * [AAP-74343] Set AWX_COLLECT_HOST_QUERIES in query discovery tests The TestExternalQueryDiscovery tests exercise the host query scanning path, which now requires AWX_COLLECT_HOST_QUERIES=1 in the environment. * [AAP-74343] Use Ansible plugin config system for collect_host_queries Declare collect_host_queries as a formal plugin option in DOCUMENTATION with env var AWX_COLLECT_HOST_QUERIES, replacing the raw os.getenv() call with self.get_option(). This follows the standard Ansible plugin configuration pattern. * [AAP-74343] Add test for disabled collect_host_queries path Verify that when collect_host_queries is false, the plugin still enumerates collections for metadata but skips host query file scanning. --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Dirk Julich
GitHub
@@ -1,4 +1,9 @@
|
||||
from awx.main.tasks.callback import RunnerCallback
|
||||
import json
|
||||
import os
|
||||
import tempfile
|
||||
from unittest import mock
|
||||
|
||||
from awx.main.tasks.callback import RunnerCallback, try_load_query_file
|
||||
from awx.main.constants import ANSIBLE_RUNNER_NEEDS_UPDATE_MESSAGE
|
||||
|
||||
from django.utils.translation import gettext_lazy as _
|
||||
@@ -50,3 +55,102 @@ def test_special_ansible_runner_message(mock_me):
|
||||
'Traceback:\ngot an unexpected keyword argument\nFile: bar.py\n'
|
||||
f'{ANSIBLE_RUNNER_NEEDS_UPDATE_MESSAGE}'
|
||||
)
|
||||
|
||||
|
||||
SAMPLE_ANSIBLE_DATA = {
|
||||
'installed_collections': {
|
||||
'ansible.builtin': {'version': '2.16.0'},
|
||||
'community.general': {'version': '8.0.0', 'host_query': 'SELECT * FROM hosts'},
|
||||
},
|
||||
'ansible_version': '2.16.0',
|
||||
}
|
||||
|
||||
|
||||
class TestTryLoadQueryFile:
|
||||
def test_loads_file_without_feature_flag(self):
|
||||
with tempfile.TemporaryDirectory() as tmpdir:
|
||||
path = os.path.join(tmpdir, 'ansible_data.json')
|
||||
with open(path, 'w') as f:
|
||||
json.dump(SAMPLE_ANSIBLE_DATA, f)
|
||||
|
||||
with mock.patch('awx.main.tasks.callback.flag_enabled', return_value=False):
|
||||
success, data = try_load_query_file(tmpdir)
|
||||
|
||||
assert success is True
|
||||
assert data['ansible_version'] == '2.16.0'
|
||||
assert 'ansible.builtin' in data['installed_collections']
|
||||
|
||||
def test_loads_file_with_feature_flag(self):
|
||||
with tempfile.TemporaryDirectory() as tmpdir:
|
||||
path = os.path.join(tmpdir, 'ansible_data.json')
|
||||
with open(path, 'w') as f:
|
||||
json.dump(SAMPLE_ANSIBLE_DATA, f)
|
||||
|
||||
with mock.patch('awx.main.tasks.callback.flag_enabled', return_value=True):
|
||||
success, data = try_load_query_file(tmpdir)
|
||||
|
||||
assert success is True
|
||||
assert data == SAMPLE_ANSIBLE_DATA
|
||||
|
||||
def test_returns_false_when_file_missing(self):
|
||||
with tempfile.TemporaryDirectory() as tmpdir:
|
||||
success, data = try_load_query_file(tmpdir)
|
||||
|
||||
assert success is False
|
||||
assert data is None
|
||||
|
||||
|
||||
class TestArtifactsHandler:
|
||||
def test_always_persists_metadata_when_flag_off(self, mock_me):
|
||||
rc = RunnerCallback()
|
||||
with tempfile.TemporaryDirectory() as tmpdir:
|
||||
path = os.path.join(tmpdir, 'ansible_data.json')
|
||||
with open(path, 'w') as f:
|
||||
json.dump(SAMPLE_ANSIBLE_DATA, f)
|
||||
|
||||
with mock.patch('awx.main.tasks.callback.flag_enabled', return_value=False):
|
||||
rc.artifacts_handler(tmpdir)
|
||||
|
||||
assert rc.extra_update_fields['installed_collections'] == SAMPLE_ANSIBLE_DATA['installed_collections']
|
||||
assert rc.extra_update_fields['ansible_version'] == '2.16.0'
|
||||
assert 'event_queries_processed' not in rc.extra_update_fields
|
||||
assert rc.artifacts_processed is True
|
||||
|
||||
@mock.patch('awx.main.tasks.callback.EventQuery')
|
||||
def test_creates_event_queries_when_flag_on(self, mock_event_query, mock_me):
|
||||
rc = RunnerCallback()
|
||||
with tempfile.TemporaryDirectory() as tmpdir:
|
||||
path = os.path.join(tmpdir, 'ansible_data.json')
|
||||
with open(path, 'w') as f:
|
||||
json.dump(SAMPLE_ANSIBLE_DATA, f)
|
||||
|
||||
with mock.patch('awx.main.tasks.callback.flag_enabled', return_value=True):
|
||||
rc.artifacts_handler(tmpdir)
|
||||
|
||||
assert rc.extra_update_fields['installed_collections'] == SAMPLE_ANSIBLE_DATA['installed_collections']
|
||||
assert rc.extra_update_fields['ansible_version'] == '2.16.0'
|
||||
assert rc.extra_update_fields['event_queries_processed'] is False
|
||||
mock_event_query.assert_called_once()
|
||||
|
||||
@mock.patch('awx.main.tasks.callback.EventQuery')
|
||||
def test_no_event_queries_when_flag_off(self, mock_event_query, mock_me):
|
||||
rc = RunnerCallback()
|
||||
with tempfile.TemporaryDirectory() as tmpdir:
|
||||
path = os.path.join(tmpdir, 'ansible_data.json')
|
||||
with open(path, 'w') as f:
|
||||
json.dump(SAMPLE_ANSIBLE_DATA, f)
|
||||
|
||||
with mock.patch('awx.main.tasks.callback.flag_enabled', return_value=False):
|
||||
rc.artifacts_handler(tmpdir)
|
||||
|
||||
mock_event_query.assert_not_called()
|
||||
|
||||
def test_handles_missing_artifact_file(self, mock_me):
|
||||
rc = RunnerCallback()
|
||||
with tempfile.TemporaryDirectory() as tmpdir:
|
||||
with mock.patch('awx.main.tasks.callback.flag_enabled', return_value=False):
|
||||
rc.artifacts_handler(tmpdir)
|
||||
|
||||
assert 'installed_collections' not in rc.extra_update_fields
|
||||
assert 'ansible_version' not in rc.extra_update_fields
|
||||
assert rc.artifacts_processed is True
|
||||
|
||||
@@ -39,6 +39,13 @@ def create_queries_dir_mock(file_lookup_func):
|
||||
class MockCallbackBase:
|
||||
def __init__(self):
|
||||
self._display = mock.MagicMock()
|
||||
self._plugin_options = {}
|
||||
|
||||
def get_option(self, key):
|
||||
return self._plugin_options.get(key)
|
||||
|
||||
def set_option(self, key, value):
|
||||
self._plugin_options[key] = value
|
||||
|
||||
def v2_playbook_on_stats(self, stats):
|
||||
pass
|
||||
@@ -289,6 +296,7 @@ class TestExternalQueryDiscovery:
|
||||
|
||||
callback = CallbackModule()
|
||||
callback._display = mock.Mock()
|
||||
callback.set_option('collect_host_queries', True)
|
||||
|
||||
with mock.patch('builtins.open', mock.mock_open()):
|
||||
with mock.patch('json.dumps', return_value='{}'):
|
||||
@@ -318,6 +326,7 @@ class TestExternalQueryDiscovery:
|
||||
|
||||
callback = CallbackModule()
|
||||
callback._display = mock.Mock()
|
||||
callback.set_option('collect_host_queries', True)
|
||||
|
||||
with mock.patch('builtins.open', mock.mock_open()):
|
||||
with mock.patch('json.dumps', return_value='{}'):
|
||||
@@ -342,6 +351,7 @@ class TestExternalQueryDiscovery:
|
||||
|
||||
callback = CallbackModule()
|
||||
callback._display = mock.Mock()
|
||||
callback.set_option('collect_host_queries', True)
|
||||
|
||||
with mock.patch('builtins.open', mock.mock_open()):
|
||||
with mock.patch('json.dumps', return_value='{}'):
|
||||
@@ -372,6 +382,7 @@ class TestExternalQueryDiscovery:
|
||||
|
||||
callback = CallbackModule()
|
||||
callback._display = mock.Mock()
|
||||
callback.set_option('collect_host_queries', True)
|
||||
|
||||
with mock.patch('builtins.open', mock.mock_open()):
|
||||
with mock.patch('json.dumps', return_value='{}'):
|
||||
@@ -382,6 +393,28 @@ class TestExternalQueryDiscovery:
|
||||
assert '4.1.0' in call_args
|
||||
assert 'community.vmware' in call_args
|
||||
|
||||
@mock.patch('awx.playbooks.library.indirect_instance_count.list_collections')
|
||||
@mock.patch('awx.playbooks.library.indirect_instance_count.files')
|
||||
@mock.patch('awx.playbooks.library.indirect_instance_count.find_external_query_with_fallback')
|
||||
@mock.patch.dict('os.environ', {'AWX_ISOLATED_DATA_DIR': '/tmp/artifacts'})
|
||||
def test_queries_not_collected_when_option_disabled(self, mock_fallback, mock_files, mock_list_collections):
|
||||
"""Host query scanning is skipped when collect_host_queries is disabled."""
|
||||
from awx.playbooks.library.indirect_instance_count import CallbackModule
|
||||
|
||||
mock_list_collections.return_value = [mock.Mock(namespace='demo', name='query', ver='1.0.0', fqcn='demo.query')]
|
||||
|
||||
callback = CallbackModule()
|
||||
callback._display = mock.Mock()
|
||||
callback.set_option('collect_host_queries', False)
|
||||
|
||||
with mock.patch('builtins.open', mock.mock_open()):
|
||||
with mock.patch('json.dumps', return_value='{}'):
|
||||
callback.v2_playbook_on_stats(mock.Mock())
|
||||
|
||||
mock_list_collections.assert_called_once()
|
||||
mock_files.assert_not_called()
|
||||
mock_fallback.assert_not_called()
|
||||
|
||||
|
||||
class TestPrivateDataDirIntegration:
|
||||
"""Tests for vendor collection copying (AC7.10-AC7.11)."""
|
||||
|
||||
@@ -918,6 +918,81 @@ class TestJobCredentials(TestJobExecution):
|
||||
assert env['FOO'] == 'BAR'
|
||||
|
||||
|
||||
class TestCallbacksEnabled(TestJobExecution):
|
||||
@pytest.fixture(autouse=True)
|
||||
def mock_flag_enabled(self):
|
||||
with mock.patch('awx.main.tasks.jobs.flag_enabled', return_value=False):
|
||||
yield
|
||||
|
||||
def test_callbacks_enabled_default(self, patch_Job, private_data_dir, execution_environment, mock_me):
|
||||
job = Job(project=Project(), inventory=Inventory())
|
||||
job.execution_environment = execution_environment
|
||||
|
||||
task = jobs.RunJob()
|
||||
task.instance = job
|
||||
task._write_extra_vars_file = mock.Mock()
|
||||
|
||||
with mock.patch.object(task, 'build_credentials_list', return_value=[], autospec=True):
|
||||
env = task.build_env(job, private_data_dir)
|
||||
|
||||
assert env['ANSIBLE_CALLBACKS_ENABLED'] == 'indirect_instance_count'
|
||||
|
||||
def test_callbacks_enabled_preserves_user_config(self, patch_Job, private_data_dir, execution_environment, mock_me):
|
||||
job = Job(project=Project(), inventory=Inventory())
|
||||
job.execution_environment = execution_environment
|
||||
|
||||
task = jobs.RunJob()
|
||||
task.instance = job
|
||||
task._write_extra_vars_file = mock.Mock()
|
||||
|
||||
with mock.patch.object(task, 'build_credentials_list', return_value=[], autospec=True):
|
||||
with mock.patch('awx.main.tasks.jobs.read_ansible_config', return_value={'callbacks_enabled': 'custom_callback,another_callback'}):
|
||||
env = task.build_env(job, private_data_dir)
|
||||
|
||||
assert env['ANSIBLE_CALLBACKS_ENABLED'] == 'indirect_instance_count,custom_callback,another_callback'
|
||||
|
||||
def test_callbacks_enabled_uses_comma_delimiter(self, patch_Job, private_data_dir, execution_environment, mock_me):
|
||||
job = Job(project=Project(), inventory=Inventory())
|
||||
job.execution_environment = execution_environment
|
||||
|
||||
task = jobs.RunJob()
|
||||
task.instance = job
|
||||
task._write_extra_vars_file = mock.Mock()
|
||||
|
||||
with mock.patch.object(task, 'build_credentials_list', return_value=[], autospec=True):
|
||||
with mock.patch('awx.main.tasks.jobs.read_ansible_config', return_value={'callbacks_enabled': 'my_callback'}):
|
||||
env = task.build_env(job, private_data_dir)
|
||||
|
||||
assert env['ANSIBLE_CALLBACKS_ENABLED'] == 'indirect_instance_count,my_callback'
|
||||
|
||||
def test_collect_host_queries_set_when_flag_on(self, patch_Job, private_data_dir, execution_environment, mock_me):
|
||||
job = Job(project=Project(), inventory=Inventory())
|
||||
job.execution_environment = execution_environment
|
||||
|
||||
task = jobs.RunJob()
|
||||
task.instance = job
|
||||
task._write_extra_vars_file = mock.Mock()
|
||||
|
||||
with mock.patch.object(task, 'build_credentials_list', return_value=[], autospec=True):
|
||||
with mock.patch('awx.main.tasks.jobs.flag_enabled', return_value=True):
|
||||
env = task.build_env(job, private_data_dir)
|
||||
|
||||
assert env['AWX_COLLECT_HOST_QUERIES'] == '1'
|
||||
|
||||
def test_collect_host_queries_not_set_when_flag_off(self, patch_Job, private_data_dir, execution_environment, mock_me):
|
||||
job = Job(project=Project(), inventory=Inventory())
|
||||
job.execution_environment = execution_environment
|
||||
|
||||
task = jobs.RunJob()
|
||||
task.instance = job
|
||||
task._write_extra_vars_file = mock.Mock()
|
||||
|
||||
with mock.patch.object(task, 'build_credentials_list', return_value=[], autospec=True):
|
||||
env = task.build_env(job, private_data_dir)
|
||||
|
||||
assert 'AWX_COLLECT_HOST_QUERIES' not in env
|
||||
|
||||
|
||||
@pytest.mark.usefixtures("patch_Organization")
|
||||
class TestProjectUpdateGalaxyCredentials(TestJobExecution):
|
||||
@pytest.fixture
|
||||
|
||||
Reference in New Issue
Block a user