summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authormelanie witt <melwittt@gmail.com>2020-10-12 22:27:52 +0000
committermelanie witt <melwittt@gmail.com>2020-11-02 18:13:38 +0000
commitda91b19d8be3b9cad8f713a3218a08e2d50238c8 (patch)
tree789bb0433cec93ede61aac0c485b193063b917b9
parent7383d25327fc47a19a664af520d9b12f9e0bd6e4 (diff)
downloadnova-da91b19d8be3b9cad8f713a3218a08e2d50238c8.tar.gz
Prevent archiving of pci_devices records because of 'instance_uuid'
Currently in the archive_deleted_rows code, we will attempt to clean up "residue" of deleted instance records by assuming any table with a 'instance_uuid' column represents data tied to an instance's lifecycle and delete such records. This behavior poses a problem in the case where an instance has a PCI device allocated and someone deletes the instance. The 'instance_uuid' column in the pci_devices table is used to track the allocation association of a PCI with an instance. There is a small time window during which the instance record has been deleted but the PCI device has not yet been freed from a database record perspective as PCI devices are freed during the _complete_deletion method in the compute manager as part of the resource tracker update call. Records in the pci_devices table are anyway not related to the lifecycle of instances so they should not be considered residue to clean up if an instance is deleted. This adds a condition to avoid archiving pci_devices on the basis of an instance association. Closes-Bug: #1899541 Conflicts: nova/db/sqlalchemy/api.py nova/tests/functional/db/test_archive.py NOTE(melwitt): The conflicts are because change I9725f752f8aef8066f7c9705e87610cad887bf8e (refactor nova-manage archive_deleted_rows) and change Id16c3d91d9ce5db9ffd125b59fffbfedf4a6843d (nova-manage db archive_deleted_rows is not multi-cell aware) are not in Stein. Change-Id: Ie62d3566230aa3e2786d129adbb2e3570b06e4c6 (cherry picked from commit 1c256cf774693e2395ae8fe4a7a2f416a7aeb03a) (cherry picked from commit 09784db62fcd01124a101c4c69cab6e71e1ac781) (cherry picked from commit 79df36fecf8c8be5ae9d59397882ac844852043e) (cherry picked from commit e3bb6119cf2d0a503768979312aea4d10cf85cda)
-rw-r--r--nova/db/sqlalchemy/api.py8
-rw-r--r--nova/tests/functional/db/test_archive.py15
2 files changed, 21 insertions, 2 deletions
diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py
index a0535aa2a5..126b40d37f 100644
--- a/nova/db/sqlalchemy/api.py
+++ b/nova/db/sqlalchemy/api.py
@@ -5625,8 +5625,12 @@ def _archive_deleted_rows_for_table(tablename, max_rows):
"%(tablename)s: %(error)s",
{'tablename': tablename, 'error': six.text_type(ex)})
- if ((max_rows is None or rows_archived < max_rows)
- and 'instance_uuid' in columns):
+ if ((max_rows is None or rows_archived < max_rows) and
+ # NOTE(melwitt): The pci_devices table uses the 'instance_uuid'
+ # column to track the allocated association of a PCI device and its
+ # records are not tied to the lifecycles of instance records.
+ (tablename != 'pci_devices' and
+ 'instance_uuid' in columns)):
instances = models.BASE.metadata.tables['instances']
limit = max_rows - rows_archived if max_rows is not None else None
extra = _archive_if_instance_deleted(table, shadow_table, instances,
diff --git a/nova/tests/functional/db/test_archive.py b/nova/tests/functional/db/test_archive.py
index 432d814d91..a8b4df6f16 100644
--- a/nova/tests/functional/db/test_archive.py
+++ b/nova/tests/functional/db/test_archive.py
@@ -135,6 +135,19 @@ class TestDatabaseArchive(test_servers.ServersTestBase):
# Verify we have some system_metadata since we'll check that later.
self.assertTrue(len(instance.system_metadata),
'No system_metadata for instance: %s' % server_id)
+ # Create a pci_devices record to simulate an instance that had a PCI
+ # device allocated at the time it was deleted. There is a window of
+ # time between deletion of the instance record and freeing of the PCI
+ # device in nova-compute's _complete_deletion method during RT update.
+ db.pci_device_update(admin_context, 1, 'fake-address',
+ {'compute_node_id': 1,
+ 'address': 'fake-address',
+ 'vendor_id': 'fake',
+ 'product_id': 'fake',
+ 'dev_type': 'fake',
+ 'label': 'fake',
+ 'status': 'allocated',
+ 'instance_uuid': instance.uuid})
# Now try and archive the soft deleted records.
results, deleted_instance_uuids = db.archive_deleted_rows(max_rows=100)
# verify system_metadata was dropped
@@ -147,6 +160,8 @@ class TestDatabaseArchive(test_servers.ServersTestBase):
# by the archive
self.assertIn('instance_actions', results)
self.assertIn('instance_actions_events', results)
+ # Verify that the pci_devices record has not been dropped
+ self.assertNotIn('pci_devices', results)
def _get_table_counts(self):
engine = sqlalchemy_api.get_engine()