Compare commits

..

4 Commits

Author SHA1 Message Date
Dirk Julich
1a205af41f AAP-57614 fix: remove early dispatch, rely on events_processed_hook
Dispatching save_indirect_host_entries from artifacts_handler was
fundamentally flawed: it ran before job events were written to the DB
by the callback receiver, so the task found no events to process, set
event_queries_processed=True, and blocked all future processing.

Remove the dispatch and the now-unused import.  The existing
events_processed_hook (called from both the task runner after the
final save and the callback receiver after the wrapup event) handles
dispatching at the right time — after events are in the DB.

The direct DB write of event_queries_processed=False and
installed_collections (added in the previous commit) remains: it
ensures events_processed_hook sees the correct values regardless of
which call site runs first.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-03-22 21:22:26 +01:00
Dirk Julich
96bd35bfb4 AAP-57614 fix: also write installed_collections directly to DB
save_indirect_host_entries calls fetch_job_event_query which reads
job.installed_collections from the DB. When dispatched from
artifacts_handler, installed_collections was still only in
delay_update (not yet flushed to DB), so the task found no matching
EventQuery records and created no IndirectManagedNodeAudit entries.

Write both event_queries_processed and installed_collections directly
to the DB before dispatching, so save_indirect_host_entries has all
the data it needs immediately.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-03-22 21:22:26 +01:00
Dirk Julich
21e73cb065 AAP-57614 fix: write event_queries_processed directly to DB
The previous commit dispatched save_indirect_host_entries from
artifacts_handler, but used delay_update to set event_queries_processed
to False. delay_update only queues the write for the final job status
save, so save_indirect_host_entries would read the default (True) from
the DB and bail out before processing.

Replace delay_update(event_queries_processed=False) with a direct
Job.objects.filter().update() call so the value is visible in the DB
before save_indirect_host_entries runs.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-03-22 21:22:26 +01:00
Dirk Julich
53be3d16bd AAP-57614 fix: dispatch save_indirect_host_entries from artifacts_handler
The artifacts_handler and handle_success_and_failure_notifications can
run in either order after job completion. Since event_queries_processed
defaults to True on the Job model, when the notification handler runs
first it sees True (the default) and skips dispatching
save_indirect_host_entries. When artifacts_handler runs later and sets
event_queries_processed to False, no task is dispatched to process the
EventQuery records, leaving event_queries_processed stuck at False and
no IndirectManagedNodeAudit records created.

Fix by also dispatching save_indirect_host_entries from
artifacts_handler after EventQuery records are created. The task's
select_for_update lock prevents duplicate processing if both code
paths dispatch.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-03-22 21:22:26 +01:00
6 changed files with 31 additions and 125 deletions

View File

@@ -409,11 +409,9 @@ class Command(BaseCommand):
del_child_group_pks = list(set(db_children_name_pk_map.values())) del_child_group_pks = list(set(db_children_name_pk_map.values()))
for offset in range(0, len(del_child_group_pks), self._batch_size): for offset in range(0, len(del_child_group_pks), self._batch_size):
child_group_pks = del_child_group_pks[offset : (offset + self._batch_size)] child_group_pks = del_child_group_pks[offset : (offset + self._batch_size)]
children_to_remove = list(db_children.filter(pk__in=child_group_pks)) for db_child in db_children.filter(pk__in=child_group_pks):
if children_to_remove: group_group_count += 1
group_group_count += len(children_to_remove) db_group.children.remove(db_child)
db_group.children.remove(*children_to_remove)
for db_child in children_to_remove:
logger.debug('Group "%s" removed from group "%s"', db_child.name, db_group.name) logger.debug('Group "%s" removed from group "%s"', db_child.name, db_group.name)
# FIXME: Inventory source group relationships # FIXME: Inventory source group relationships
# Delete group/host relationships not present in imported data. # Delete group/host relationships not present in imported data.
@@ -443,11 +441,11 @@ class Command(BaseCommand):
del_host_pks = list(del_host_pks) del_host_pks = list(del_host_pks)
for offset in range(0, len(del_host_pks), self._batch_size): for offset in range(0, len(del_host_pks), self._batch_size):
del_pks = del_host_pks[offset : (offset + self._batch_size)] del_pks = del_host_pks[offset : (offset + self._batch_size)]
hosts_to_remove = list(db_hosts.filter(pk__in=del_pks)) for db_host in db_hosts.filter(pk__in=del_pks):
if hosts_to_remove: group_host_count += 1
group_host_count += len(hosts_to_remove) if db_host not in db_group.hosts.all():
db_group.hosts.remove(*hosts_to_remove) continue
for db_host in hosts_to_remove: db_group.hosts.remove(db_host)
logger.debug('Host "%s" removed from group "%s"', db_host.name, db_group.name) logger.debug('Host "%s" removed from group "%s"', db_host.name, db_group.name)
if settings.SQL_DEBUG: if settings.SQL_DEBUG:
logger.warning( logger.warning(

View File

@@ -531,7 +531,6 @@ class CredentialType(CommonModelNameNotUnique):
existing = ct_class.objects.filter(name=default.name, kind=default.kind).first() existing = ct_class.objects.filter(name=default.name, kind=default.kind).first()
if existing is not None: if existing is not None:
existing.namespace = default.namespace existing.namespace = default.namespace
existing.description = getattr(default, 'description', '')
existing.inputs = {} existing.inputs = {}
existing.injectors = {} existing.injectors = {}
existing.save() existing.save()
@@ -571,14 +570,7 @@ class CredentialType(CommonModelNameNotUnique):
@classmethod @classmethod
def load_plugin(cls, ns, plugin): def load_plugin(cls, ns, plugin):
# TODO: User "side-loaded" credential custom_injectors isn't supported # TODO: User "side-loaded" credential custom_injectors isn't supported
ManagedCredentialType.registry[ns] = SimpleNamespace( ManagedCredentialType.registry[ns] = SimpleNamespace(namespace=ns, name=plugin.name, kind='external', inputs=plugin.inputs, backend=plugin.backend)
namespace=ns,
name=plugin.name,
kind='external',
inputs=plugin.inputs,
backend=plugin.backend,
description=getattr(plugin, 'plugin_description', ''),
)
def inject_credential(self, credential, env, safe_env, args, private_data_dir, container_root=None): def inject_credential(self, credential, env, safe_env, args, private_data_dir, container_root=None):
from awx_plugins.interfaces._temporary_private_inject_api import inject_credential from awx_plugins.interfaces._temporary_private_inject_api import inject_credential
@@ -590,13 +582,7 @@ class CredentialTypeHelper:
@classmethod @classmethod
def get_creation_params(cls, cred_type): def get_creation_params(cls, cred_type):
if cred_type.kind == 'external': if cred_type.kind == 'external':
return { return dict(namespace=cred_type.namespace, kind=cred_type.kind, name=cred_type.name, managed=True)
'namespace': cred_type.namespace,
'kind': cred_type.kind,
'name': cred_type.name,
'managed': True,
'description': getattr(cred_type, 'description', ''),
}
return dict( return dict(
namespace=cred_type.namespace, namespace=cred_type.namespace,
kind=cred_type.kind, kind=cred_type.kind,

View File

@@ -277,7 +277,6 @@ class RunnerCallback:
def artifacts_handler(self, artifact_dir): def artifacts_handler(self, artifact_dir):
success, query_file_contents = try_load_query_file(artifact_dir) success, query_file_contents = try_load_query_file(artifact_dir)
if success: if success:
self.delay_update(event_queries_processed=False)
collections_info = collect_queries(query_file_contents) collections_info = collect_queries(query_file_contents)
for collection, data in collections_info.items(): for collection, data in collections_info.items():
version = data['version'] version = data['version']
@@ -301,6 +300,24 @@ class RunnerCallback:
else: else:
logger.warning(f'The file {COLLECTION_FILENAME} unexpectedly did not contain ansible_version') logger.warning(f'The file {COLLECTION_FILENAME} unexpectedly did not contain ansible_version')
# Write event_queries_processed and installed_collections directly
# to the DB instead of using delay_update. delay_update defers
# writes until the final job status save, but
# events_processed_hook (called from both the task runner after
# the final save and the callback receiver after the wrapup
# event) needs event_queries_processed=False visible in the DB
# to dispatch save_indirect_host_entries. The field defaults to
# True, so without a direct write the hook would see True and
# skip the dispatch. installed_collections is also written
# directly so it is available if the callback receiver
# dispatches before the final save.
from awx.main.models import Job
db_updates = {'event_queries_processed': False}
if 'installed_collections' in query_file_contents:
db_updates['installed_collections'] = query_file_contents['installed_collections']
Job.objects.filter(id=self.instance.id).update(**db_updates)
self.artifacts_processed = True self.artifacts_processed = True

View File

@@ -305,47 +305,6 @@ class TestINIImports:
has_host_group = inventory.groups.get(name='has_a_host') has_host_group = inventory.groups.get(name='has_a_host')
assert has_host_group.hosts.count() == 1 assert has_host_group.hosts.count() == 1
@mock.patch.object(inventory_import, 'AnsibleInventoryLoader', MockLoader)
def test_overwrite_removes_stale_memberships(self, inventory):
"""When overwrite is enabled, host-group and group-group memberships
that are no longer in the imported data should be removed."""
# First import: parent_group has two children, host_group has two hosts
inventory_import.AnsibleInventoryLoader._data = {
"_meta": {"hostvars": {"host1": {}, "host2": {}}},
"all": {"children": ["ungrouped", "parent_group", "child_a", "child_b", "host_group"]},
"parent_group": {"children": ["child_a", "child_b"]},
"host_group": {"hosts": ["host1", "host2"]},
"ungrouped": {"hosts": []},
}
cmd = inventory_import.Command()
cmd.handle(inventory_id=inventory.pk, source=__file__, overwrite=True)
parent = inventory.groups.get(name='parent_group')
assert set(parent.children.values_list('name', flat=True)) == {'child_a', 'child_b'}
host_grp = inventory.groups.get(name='host_group')
assert set(host_grp.hosts.values_list('name', flat=True)) == {'host1', 'host2'}
# Second import: child_b removed from parent_group, host2 moved out of host_group
inventory_import.AnsibleInventoryLoader._data = {
"_meta": {"hostvars": {"host1": {}, "host2": {}}},
"all": {"children": ["ungrouped", "parent_group", "child_a", "child_b", "host_group"]},
"parent_group": {"children": ["child_a"]},
"host_group": {"hosts": ["host1"]},
"ungrouped": {"hosts": ["host2"]},
}
cmd = inventory_import.Command()
cmd.handle(inventory_id=inventory.pk, source=__file__, overwrite=True)
parent.refresh_from_db()
host_grp.refresh_from_db()
# child_b should be removed from parent_group
assert set(parent.children.values_list('name', flat=True)) == {'child_a'}
# host2 should be removed from host_group
assert set(host_grp.hosts.values_list('name', flat=True)) == {'host1'}
# host2 and child_b should still exist in the inventory, just not in those groups
assert inventory.hosts.filter(name='host2').exists()
assert inventory.groups.filter(name='child_b').exists()
@mock.patch.object(inventory_import, 'AnsibleInventoryLoader', MockLoader) @mock.patch.object(inventory_import, 'AnsibleInventoryLoader', MockLoader)
def test_recursive_group_error(self, inventory): def test_recursive_group_error(self, inventory):
inventory_import.AnsibleInventoryLoader._data = { inventory_import.AnsibleInventoryLoader._data = {

View File

@@ -2,11 +2,7 @@
import pytest import pytest
from types import SimpleNamespace
from unittest import mock
from awx.main.models import Credential, CredentialType from awx.main.models import Credential, CredentialType
from awx.main.models.credential import CredentialTypeHelper, ManagedCredentialType
from django.apps import apps from django.apps import apps
@@ -82,53 +78,3 @@ def test_credential_context_property_independent_instances():
assert cred1.context == {'key1': 'value1'} assert cred1.context == {'key1': 'value1'}
assert cred2.context == {'key2': 'value2'} assert cred2.context == {'key2': 'value2'}
assert cred1.context is not cred2.context assert cred1.context is not cred2.context
def test_load_plugin_passes_description():
plugin = SimpleNamespace(name='test_plugin', inputs={'fields': []}, backend=None, plugin_description='A test plugin')
CredentialType.load_plugin('test_ns', plugin)
entry = ManagedCredentialType.registry['test_ns']
assert entry.description == 'A test plugin'
del ManagedCredentialType.registry['test_ns']
def test_load_plugin_missing_description():
plugin = SimpleNamespace(name='test_plugin', inputs={'fields': []}, backend=None)
CredentialType.load_plugin('test_ns', plugin)
entry = ManagedCredentialType.registry['test_ns']
assert entry.description == ''
del ManagedCredentialType.registry['test_ns']
def test_get_creation_params_external_includes_description():
cred_type = SimpleNamespace(namespace='test_ns', kind='external', name='Test', description='My description')
params = CredentialTypeHelper.get_creation_params(cred_type)
assert params['description'] == 'My description'
def test_get_creation_params_external_missing_description():
cred_type = SimpleNamespace(namespace='test_ns', kind='external', name='Test')
params = CredentialTypeHelper.get_creation_params(cred_type)
assert params['description'] == ''
@pytest.mark.django_db
def test_setup_tower_managed_defaults_updates_description():
registry_entry = SimpleNamespace(
namespace='test_ns',
kind='external',
name='Test Plugin',
inputs={'fields': []},
backend=None,
description='Updated description',
)
# Create an existing credential type with no description
ct = CredentialType.objects.create(name='Test Plugin', kind='external', namespace='old_ns')
assert ct.description == ''
with mock.patch.dict(ManagedCredentialType.registry, {'test_ns': registry_entry}, clear=True):
CredentialType._setup_tower_managed_defaults()
ct.refresh_from_db()
assert ct.description == 'Updated description'
assert ct.namespace == 'test_ns'

View File

@@ -116,7 +116,7 @@ cython==3.1.3
# via -r /awx_devel/requirements/requirements.in # via -r /awx_devel/requirements/requirements.in
daphne==4.2.1 daphne==4.2.1
# via -r /awx_devel/requirements/requirements.in # via -r /awx_devel/requirements/requirements.in
dispatcherd[pg-notify]==2026.3.25 dispatcherd[pg-notify]==2026.02.26
# via -r /awx_devel/requirements/requirements.in # via -r /awx_devel/requirements/requirements.in
distro==1.9.0 distro==1.9.0
# via -r /awx_devel/requirements/requirements.in # via -r /awx_devel/requirements/requirements.in