diff --git a/awx/conf/settings.py b/awx/conf/settings.py index 7d9ca68b3739..dfe2e50602cc 100644 --- a/awx/conf/settings.py +++ b/awx/conf/settings.py @@ -1,6 +1,7 @@ # Python import contextlib import logging +import psycopg import threading import time import os @@ -13,7 +14,7 @@ from django.core.cache import cache as django_cache from django.core.exceptions import ImproperlyConfigured, SynchronousOnlyOperation from django.db import transaction, connection -from django.db.utils import Error as DBError, ProgrammingError +from django.db.utils import DatabaseError, ProgrammingError from django.utils.functional import cached_property # Django REST Framework @@ -80,18 +81,26 @@ def _ctit_db_wrapper(trans_safe=False): logger.debug('Obtaining database settings in spite of broken transaction.') transaction.set_rollback(False) yield - except DBError as exc: + except ProgrammingError as e: + # Exception raised for programming errors + # Examples may be table not found or already exists, + # this generally means we can't fetch Tower configuration + # because the database hasn't actually finished migrating yet; + # this is usually a sign that a service in a container (such as ws_broadcast) + # has come up *before* the database has finished migrating, and + # especially that the conf.settings table doesn't exist yet + # syntax error in the SQL statement, wrong number of parameters specified, etc. if trans_safe: - level = logger.warning - if isinstance(exc, ProgrammingError): - if 'relation' in str(exc) and 'does not exist' in str(exc): - # this generally means we can't fetch Tower configuration - # because the database hasn't actually finished migrating yet; - # this is usually a sign that a service in a container (such as ws_broadcast) - # has come up *before* the database has finished migrating, and - # especially that the conf.settings table doesn't exist yet - level = logger.debug - level(f'Database settings are not available, using defaults. error: {str(exc)}') + logger.debug(f'Database settings are not available, using defaults. error: {str(e)}') + else: + logger.exception('Error modifying something related to database settings.') + except DatabaseError as e: + if trans_safe: + cause = e.__cause__ + if cause and hasattr(cause, 'sqlstate'): + sqlstate = cause.sqlstate + sqlstate_str = psycopg.errors.lookup(sqlstate) + logger.error('SQL Error state: {} - {}'.format(sqlstate, sqlstate_str)) else: logger.exception('Error modifying something related to database settings.') finally: diff --git a/awx/main/tasks/system.py b/awx/main/tasks/system.py index 4ebe1d576612..e7c2e472e4b7 100644 --- a/awx/main/tasks/system.py +++ b/awx/main/tasks/system.py @@ -6,6 +6,7 @@ import json import logging import os +import psycopg from io import StringIO from contextlib import redirect_stdout import shutil @@ -630,10 +631,18 @@ def cluster_node_heartbeat(dispatch_time=None, worker_tasks=None): logger.error("Host {} last checked in at {}, marked as lost.".format(other_inst.hostname, other_inst.last_seen)) except DatabaseError as e: - if 'did not affect any rows' in str(e): - logger.debug('Another instance has marked {} as lost'.format(other_inst.hostname)) + cause = e.__cause__ + if cause and hasattr(cause, 'sqlstate'): + sqlstate = cause.sqlstate + sqlstate_str = psycopg.errors.lookup(sqlstate) + logger.debug('SQL Error state: {} - {}'.format(sqlstate, sqlstate_str)) + + if sqlstate == psycopg.errors.NoData: + logger.debug('Another instance has marked {} as lost'.format(other_inst.hostname)) + else: + logger.exception("Error marking {} as lost.".format(other_inst.hostname)) else: - logger.exception('Error marking {} as lost'.format(other_inst.hostname)) + logger.exception('No SQL state available. Error marking {} as lost'.format(other_inst.hostname)) # Run local reaper if worker_tasks is not None: @@ -788,10 +797,17 @@ def update_inventory_computed_fields(inventory_id): try: i.update_computed_fields() except DatabaseError as e: - if 'did not affect any rows' in str(e): - logger.debug('Exiting duplicate update_inventory_computed_fields task.') - return - raise + cause = e.__cause__ + if cause and hasattr(cause, 'sqlstate'): + sqlstate = cause.sqlstate + sqlstate_str = psycopg.errors.lookup(sqlstate) + logger.error('SQL Error state: {} - {}'.format(sqlstate, sqlstate_str)) + raise + # To my knowledge no database or library raises an exception for "did not affect any rows". + # xxx.rowcount on update/delete shoud be used to determine affected rows. + # Not sure how an exception is thrown from update_computed_fields() + else: + logger.debug('Exiting duplicate update_inventory_computed_fields task. {}'.format(str(e))) def update_smart_memberships_for_inventory(smart_inventory): diff --git a/awx/main/tests/unit/tasks/test_system.py b/awx/main/tests/unit/tasks/test_system.py new file mode 100644 index 000000000000..9e17d1726e15 --- /dev/null +++ b/awx/main/tests/unit/tasks/test_system.py @@ -0,0 +1,66 @@ +import pytest +from unittest.mock import MagicMock, patch +from awx.main.tasks.system import update_inventory_computed_fields + +# Assuming your_module is the module where the function is defined +from awx.main.models import Inventory +from django.db import DatabaseError + + +@pytest.fixture +def mock_logger(): + with patch("awx.main.tasks.system.logger") as logger: + yield logger + + +@pytest.fixture +def mock_inventory(): + return MagicMock(spec=Inventory) + + +def test_update_inventory_computed_fields_existing_inventory(mock_logger, mock_inventory): + # Mocking the Inventory.objects.filter method to return a non-empty queryset + with patch("awx.main.tasks.system.Inventory.objects.filter") as mock_filter: + mock_filter.return_value.exists.return_value = True + mock_filter.return_value.__getitem__.return_value = mock_inventory + + # Mocking the update_computed_fields method + with patch.object(mock_inventory, "update_computed_fields") as mock_update_computed_fields: + update_inventory_computed_fields(1) + + # Assertions + mock_filter.assert_called_once_with(id=1) + mock_update_computed_fields.assert_called_once() + + # You can add more assertions based on your specific requirements + + +def test_update_inventory_computed_fields_missing_inventory(mock_logger): + # Mocking the Inventory.objects.filter method to return an empty queryset + with patch("awx.main.tasks.system.Inventory.objects.filter") as mock_filter: + mock_filter.return_value.exists.return_value = False + + update_inventory_computed_fields(1) + + # Assertions + mock_filter.assert_called_once_with(id=1) + mock_logger.error.assert_called_once_with("Update Inventory Computed Fields failed due to missing inventory: 1") + + +def test_update_inventory_computed_fields_database_error(mock_logger, mock_inventory): + # Mocking the Inventory.objects.filter method to return a non-empty queryset + with patch("awx.main.tasks.system.Inventory.objects.filter") as mock_filter: + mock_filter.return_value.exists.return_value = True + mock_filter.return_value.__getitem__.return_value = mock_inventory + + # Mocking the update_computed_fields method to raise a DatabaseError + with patch("awx.main.tasks.system.Inventory.update_computed_fields") as mock_update_computed_fields: + mock_update_computed_fields.side_effect = DatabaseError("Some error") + + with pytest.raises(DatabaseError): + update_inventory_computed_fields(1) + + # Assertions + mock_filter.assert_called_once_with(id=1) + mock_inventory.update_computed_fields.assert_called_once() + mock_logger.error.assert_called_once() diff --git a/awx/main/utils/common.py b/awx/main/utils/common.py index 7ab98425847c..6d0fc741db96 100644 --- a/awx/main/utils/common.py +++ b/awx/main/utils/common.py @@ -7,6 +7,7 @@ import yaml import logging import time +import psycopg import os import subprocess import re @@ -23,7 +24,7 @@ from django.utils.dateparse import parse_datetime from django.utils.translation import gettext_lazy as _ from django.utils.functional import cached_property -from django.db import connection, transaction, ProgrammingError, IntegrityError +from django.db import connection, DatabaseError, transaction, ProgrammingError, IntegrityError from django.db.models.fields.related import ForeignObjectRel, ManyToManyField from django.db.models.fields.related_descriptors import ForwardManyToOneDescriptor, ManyToManyDescriptor from django.db.models.query import QuerySet @@ -1155,10 +1156,25 @@ def create_partition(tblname, start=None): f'ALTER TABLE {tblname} ATTACH PARTITION {tblname}_{partition_label} ' f'FOR VALUES FROM (\'{start_timestamp}\') TO (\'{end_timestamp}\');' ) + except (ProgrammingError, IntegrityError) as e: - if 'already exists' in str(e): - logger.info(f'Caught known error due to partition creation race: {e}') - else: + cause = e.__cause__ + if cause and hasattr(cause, 'sqlstate'): + # 42P07 = DuplicateTable + sqlstate = cause.sqlstate + sqlstate_str = psycopg.errors.lookup(sqlstate) + + if psycopg.errors.DuplicateTable == sqlstate: + logger.info(f'Caught known error due to partition creation race: {e}') + else: + logger.error('SQL Error state: {} - {}'.format(sqlstate, sqlstate_str)) + raise + except DatabaseError as e: + cause = e.__cause__ + if cause and hasattr(cause, 'sqlstate'): + sqlstate = cause.sqlstate + sqlstate_str = psycopg.errors.lookup(sqlstate) + logger.error('SQL Error state: {} - {}'.format(sqlstate, sqlstate_str)) raise