openstack/nova
Code Issues Proposed changes

Add nova-status upgrade check for request spec migrations

Browse Source 
We can't easily add a blocker db sync migration to make
sure the migrate_instances_add_request_spec online data
migration has been run since we have to iterate both cells
(for instances) and the API DB (for request specs) and that's
not something we should do during a db sync call.

But we want to eventually drop the online data migration and
the accompanying compat code found in the api and conductor
services.

This adds a nova-status upgrade check for missing request specs
and fails if any existing non-deleted instances are found which
don't have a matching request spec.

Related to blueprint request-spec-use-by-compute

Change-Id: I1fb63765f0b0e8f35d6a66dccf9d12cc20e9c661
This commit is contained in:
Matt Riedemann 2018-07-11 12:51:11 -04:00
parent 90cdf80750
commit e73f828057
5 changed files with 170 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
doc/source/cli/nova-status.rst
View File

@ -115,6 +115,8 @@ Upgrade
across all cell mappings which might cause issues when querying instances
depending on how the **nova-api** service is configured.
See https://bugs.launchpad.net/nova/+bug/1759316 for details.
* Checks that existing instances have been migrated to have a matching
request spec in the API DB.
See Also
========

4
nova/cmd/manage.py
View File

@ -384,6 +384,10 @@ class DbCommands(object):
# finished.
online_migrations = (
# Added in Newton
# TODO(mriedem): Remove this in Stein along with the compatibility
# code in the api and conductor services; the nova-status upgrade check
# added in Rocky is the tool operators can use to make sure they have
# completed this migration.
request_spec.migrate_instances_add_request_spec,
# Added in Newton
keypair_obj.migrate_keypairs_to_api_db,

64
nova/cmd/status.py
View File

@ -513,6 +513,66 @@ class UpgradeCommands(object):
return UpgradeCheckResult(UpgradeCheckCode.WARNING, msg)
return UpgradeCheckResult(UpgradeCheckCode.SUCCESS)
def _check_request_spec_migration(self):
"""Checks to make sure request spec migrations are complete.
Iterates all cells checking to see that non-deleted instances have
a matching request spec in the API database. This is necessary in order
to drop the migrate_instances_add_request_spec online data migration
and accompanying compatibility code found through nova-api and
nova-conductor.
"""
meta = MetaData(bind=db_session.get_api_engine())
cell_mappings = Table('cell_mappings', meta, autoload=True)
mappings = cell_mappings.select().execute().fetchall()
if not mappings:
# There are no cell mappings so we can't determine this, just
# return a warning. The cellsv2 check would have already failed
# on this.
msg = (_('Unable to determine request spec migrations without '
'cell mappings.'))
return UpgradeCheckResult(UpgradeCheckCode.WARNING, msg)
request_specs = Table('request_specs', meta, autoload=True)
ctxt = nova_context.get_admin_context()
incomplete_cells = [] # list of cell mapping uuids
for mapping in mappings:
with nova_context.target_cell(ctxt, mapping) as cctxt:
# Get all instance uuids for non-deleted instances in this
# cell.
meta = MetaData(bind=db_session.get_engine(context=cctxt))
instances = Table('instances', meta, autoload=True)
instance_records = (
select([instances.c.uuid]).select_from(instances).where(
instances.c.deleted == 0
).execute().fetchall())
# For each instance in the list, verify that it has a matching
# request spec in the API DB.
for inst in instance_records:
spec_id = (
select([request_specs.c.id]).select_from(
request_specs).where(
request_specs.c.instance_uuid == inst['uuid']
).execute().scalar())
if spec_id is None:
# This cell does not have all of its instances
# migrated for request specs so track it and move on.
incomplete_cells.append(mapping['uuid'])
break
# It's a failure if there are any unmigrated instances at this point
# because we are planning to drop the online data migration routine and
# compatibility code in Stein.
if incomplete_cells:
msg = (_("The following cells have instances which do not have "
"matching request_specs in the API database: %s Run "
"'nova-manage db online_data_migrations' on each cell "
"to create the missing request specs.") %
', '.join(incomplete_cells))
return UpgradeCheckResult(UpgradeCheckCode.FAILURE, msg)
return UpgradeCheckResult(UpgradeCheckCode.SUCCESS)
# The format of the check functions is to return an UpgradeCheckResult
# object with the appropriate UpgradeCheckCode and details set. If the
# check hits warnings or failures then those should be stored in the
@ -530,7 +590,9 @@ class UpgradeCommands(object):
# Added in Rocky (but also useful going back to Pike)
(_('Ironic Flavor Migration'), _check_ironic_flavor_migration),
# Added in Rocky (but is backportable to Ocata)
(_('API Service Version'), _check_api_service_version)
(_('API Service Version'), _check_api_service_version),
# Added in Rocky
(_('Request Spec Migration'), _check_request_spec_migration),
)
def _get_details(self, upgrade_check_result):

95
nova/tests/unit/cmd/test_status.py
View File

@ -33,6 +33,7 @@ from nova import context
# NOTE(mriedem): We only use objects as a convenience to populate the database
# in the tests, we don't use them in the actual CLI.
from nova import objects
from nova.objects import request_spec as reqspec_obj
from nova import rc_fields as fields
from nova import test
from nova.tests import fixtures as nova_fixtures
@ -916,3 +917,97 @@ class TestUpgradeCheckAPIServiceVersion(test.NoDBTestCase):
result = self.cmd._check_api_service_version()
self.assertEqual(status.UpgradeCheckCode.SUCCESS, result.code)
class TestUpgradeCheckRequestSpecMigration(test.NoDBTestCase):
"""Tests for the nova-status upgrade check for request spec migration."""
# We'll setup the database ourselves because we need to use cells fixtures
# for multiple cell mappings.
USES_DB_SELF = True
# This will create three cell mappings: cell0, cell1 (default) and cell2
NUMBER_OF_CELLS = 2
def setUp(self):
super(TestUpgradeCheckRequestSpecMigration, self).setUp()
self.output = StringIO()
self.useFixture(fixtures.MonkeyPatch('sys.stdout', self.output))
# We always need the API DB to be setup.
self.useFixture(nova_fixtures.Database(database='api'))
self.cmd = status.UpgradeCommands()
@staticmethod
def _create_instance_in_cell(ctxt, cell, is_deleted=False,
create_request_spec=False):
with context.target_cell(ctxt, cell) as cctxt:
inst = objects.Instance(
context=cctxt,
uuid=uuidutils.generate_uuid())
inst.create()
if is_deleted:
inst.destroy()
if create_request_spec:
# Fake out some fields in the Instance so we don't lazy-load them.
inst.flavor = objects.Flavor()
inst.numa_topology = None
inst.system_metadata = {}
inst.pci_requests = None
inst.project_id = 'fake-project'
inst.user_id = 'fake-user'
reqspec_obj._create_minimal_request_spec(ctxt, inst)
return inst
def test_fresh_install_no_cell_mappings(self):
"""Tests the scenario where we don't have any cell mappings (no cells
v2 setup yet) so we don't know what state we're in and we return a
warning.
"""
result = self.cmd._check_request_spec_migration()
self.assertEqual(status.UpgradeCheckCode.WARNING, result.code)
self.assertIn('Unable to determine request spec migrations without '
'cell mappings.', result.details)
def test_deleted_instance_one_cell_migrated_other_success(self):
"""Tests the scenario that we have two cells, one has only a single
deleted instance in it and the other has a single already-migrated
instance in it, so the overall result is success.
"""
self._setup_cells()
ctxt = context.get_admin_context()
# Create a deleted instance in cell1.
self._create_instance_in_cell(
ctxt, self.cell_mappings['cell1'], is_deleted=True)
# Create a migrated instance in cell2.
self._create_instance_in_cell(
ctxt, self.cell_mappings['cell2'], create_request_spec=True)
result = self.cmd._check_request_spec_migration()
self.assertEqual(status.UpgradeCheckCode.SUCCESS, result.code)
def test_unmigrated_request_spec_instances(self):
"""Tests the scenario that we have a migrated instance in cell1 and
an unmigrated instance in cell2 so the check fails.
"""
self._setup_cells()
ctxt = context.get_admin_context()
# Create a migrated instance in cell1.
self._create_instance_in_cell(
ctxt, self.cell_mappings['cell1'], create_request_spec=True)
# Create an unmigrated instance in cell2.
self._create_instance_in_cell(ctxt, self.cell_mappings['cell2'])
result = self.cmd._check_request_spec_migration()
self.assertEqual(status.UpgradeCheckCode.FAILURE, result.code)
self.assertIn("The following cells have instances which do not have "
"matching request_specs in the API database: %s Run "
"'nova-manage db online_data_migrations' on each cell "
"to create the missing request specs." %
self.cell_mappings['cell2'].uuid, result.details)

6
releasenotes/notes/nova-status-check-requestspec-migration-2a3b50b98fff9324.yaml Normal file
View File

@ -0,0 +1,6 @@
---
upgrade:
- |
A new check is added to the ``nova-status upgrade check`` CLI to make sure
request spec online migrations have been run per-cell. Missing request spec
compatibility code is planned to be removed in the Stein release.