diff --git a/ironic/cmd/dbsync.py b/ironic/cmd/dbsync.py index 9cdfa2f4f1..d3d87e3072 100644 --- a/ironic/cmd/dbsync.py +++ b/ironic/cmd/dbsync.py @@ -64,6 +64,7 @@ dbapi = db_api.get_instance() # object, in case it is lazy loaded. The attribute will be accessed when needed # by doing getattr on the object ONLINE_MIGRATIONS = ( + (dbapi, 'migrate_to_builtin_inspection'), # NOTE(rloo): Don't remove this; it should always be last (dbapi, 'update_to_latest_versions'), ) diff --git a/ironic/db/sqlalchemy/api.py b/ironic/db/sqlalchemy/api.py index b3bda9802c..89d597e5c1 100644 --- a/ironic/db/sqlalchemy/api.py +++ b/ironic/db/sqlalchemy/api.py @@ -1992,6 +1992,68 @@ class Connection(api.Connection): return total_to_migrate, total_migrated + @oslo_db_api.retry_on_deadlock + def migrate_to_builtin_inspection(self, context, max_count): + """Handle the migration from "inspector" to "agent" inspection. + + :param context: the admin context + :param max_count: The maximum number of objects to migrate. Must be + >= 0. If zero, all the objects will be migrated. + :returns: A 2-tuple, 1. the total number of objects that need to be + migrated (at the beginning of this call) and 2. the number + of migrated objects. + """ + # TODO(dtantsur): remove this check when removing inspector and just + # unconditionally migrate everything. + if ('agent' not in CONF.enabled_inspect_interfaces + or 'inspector' in CONF.enabled_inspect_interfaces): + return 0, 0 + + model = models.Node + + with _session_for_read() as session: + # NOTE(dtantsur): this is the total number of objects, including + # the ones that are on inspection and cannot be migrated. + total_to_migrate = session.query(model).filter( + model.inspect_interface == 'inspector').count() + + if not total_to_migrate: + return 0, 0 + + no_states = [states.INSPECTING, states.INSPECTWAIT, states.INSPECTFAIL] + + with _session_for_write() as session: + query = session.query(model).filter( + sql.and_(model.inspect_interface == 'inspector', + model.provision_state.not_in(no_states))) + # NOTE(rloo) Caution here; after doing query.count(), it is + # possible that the value is different in the + # next invocation of the query. + total_count = query.count() + if not total_count: + return 0, 0 + elif max_count and max_count < total_count: + # Only want to update max_count objects; cannot use + # sql's limit(), so we generate a new query with + # max_count objects. + ids = [obj['id'] for obj in query.slice(0, max_count)] + num_migrated = ( + session.query(model). + filter(sql.and_(model.id.in_(ids), + model.inspect_interface == 'inspector', + model.provision_state.not_in(no_states))). + update({model.inspect_interface: 'agent'}, + synchronize_session=False)) + else: + num_migrated = ( + session.query(model). + filter(sql.and_(model.inspect_interface == 'inspector', + model.provision_state.not_in(no_states))). + update({model.inspect_interface: 'agent'}, + synchronize_session=False)) + + return total_to_migrate, num_migrated + @staticmethod def _verify_max_traits_per_node(node_id, num_traits): """Verify that an operation would not exceed the per-node trait limit. diff --git a/ironic/tests/unit/db/test_api.py b/ironic/tests/unit/db/test_api.py index d69414112a..01d99c4321 100644 --- a/ironic/tests/unit/db/test_api.py +++ b/ironic/tests/unit/db/test_api.py @@ -13,6 +13,7 @@ import random from unittest import mock +from oslo_config import cfg from oslo_db.sqlalchemy import utils as db_utils from oslo_utils import uuidutils import sqlalchemy as sa @@ -20,11 +21,15 @@ import sqlalchemy as sa from ironic.common import context from ironic.common import exception from ironic.common import release_mappings +from ironic.common import states from ironic.db import api as db_api from ironic.tests.unit.db import base from ironic.tests.unit.db import utils +CONF = cfg.CONF + + class UpgradingTestCase(base.DbTestCase): def setUp(self): @@ -236,3 +241,61 @@ class UpdateToLatestVersionsTestCase(base.DbTestCase): for uuid in nodes: node = self.dbapi.get_node_by_uuid(uuid) self.assertEqual(self.node_ver, node.version) + + +class MigrateToBuiltinInspectionTestCase(base.DbTestCase): + + def setUp(self): + super().setUp() + self.context = context.get_admin_context() + self.dbapi = db_api.get_instance() + CONF.set_override('enabled_inspect_interfaces', 'agent,no-inspect') + + for _ in range(3): + utils.create_test_node(uuid=uuidutils.generate_uuid(), + inspect_interface='inspector') + for _ in range(2): + utils.create_test_node(uuid=uuidutils.generate_uuid(), + inspect_interface='agent') + utils.create_test_node(uuid=uuidutils.generate_uuid(), + inspect_interface='no-inspect') + + def _check(self, migrated, left): + current = sorted(x[0] for x in self.dbapi.get_nodeinfo_list( + columns=['inspect_interface'])) + self.assertEqual(['agent'] * migrated + ['inspector'] * left + + ['no-inspect'], current) + + def test_migrate_all(self): + total, migrated = self.dbapi.migrate_to_builtin_inspection( + self.context, 0) + self.assertEqual(3, total) + self.assertEqual(3, migrated) + self._check(5, 0) + + def test_cannot_migrate(self): + CONF.set_override('enabled_inspect_interfaces', 'inspector,no-inspect') + total, migrated = self.dbapi.migrate_to_builtin_inspection( + self.context, 0) + self.assertEqual(0, total) + self.assertEqual(0, migrated) + self._check(2, 3) + + def test_cannot_migrate_some(self): + for state in [states.INSPECTING, states.INSPECTWAIT, + states.INSPECTFAIL]: + utils.create_test_node(uuid=uuidutils.generate_uuid(), + inspect_interface='inspector', + provision_state=state) + total, migrated = self.dbapi.migrate_to_builtin_inspection( + self.context, 0) + self.assertEqual(6, total) + self.assertEqual(3, migrated) + self._check(5, 3) + + def test_migrate_with_limit(self): + total, migrated = self.dbapi.migrate_to_builtin_inspection( + self.context, 2) + self.assertEqual(3, total) + self.assertEqual(2, migrated) + self._check(4, 1) diff --git a/releasenotes/notes/migrate-inspector-48de1216ef81f43a.yaml b/releasenotes/notes/migrate-inspector-48de1216ef81f43a.yaml new file mode 100644 index 0000000000..533f99ef7f --- /dev/null +++ b/releasenotes/notes/migrate-inspector-48de1216ef81f43a.yaml @@ -0,0 +1,13 @@ +--- +upgrade: + - | + Adds an online migration to the `new inspection interface + `_. + If the ``agent`` inspection is enabled and the ``inspector`` inspection is + disabled, the ``inspect_interface`` field will be updated for all nodes + that use ``inspector`` and are currently not on inspection (i.e. not in the + ``inspect wait`` or ``inspecting`` states). + + If some nodes may be inspecting during the upgrade, you may want to run + the online migrations several times with a delay to finish migrating all + nodes.