From 8dd09d396281d4990abe56a1be6f4f20cefcd4bf Mon Sep 17 00:00:00 2001
From: Dmitry Tantsur <dtantsur@protonmail.com>
Date: Thu, 1 Feb 2024 10:38:11 +0100
Subject: [PATCH] Online migration for inspect_interface inspector->agent
Change-Id: If29bd3e3370c831d171c84846373c0bc374efc09
---
ironic/cmd/dbsync.py | 1 +
ironic/db/sqlalchemy/api.py | 62 ++++++++++++++++++
ironic/tests/unit/db/test_api.py | 63 +++++++++++++++++++
.../migrate-inspector-48de1216ef81f43a.yaml | 13 ++++
4 files changed, 139 insertions(+)
create mode 100644 releasenotes/notes/migrate-inspector-48de1216ef81f43a.yaml
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
+ <https://docs.openstack.org/ironic/latest/admin/inspection/index.html>`_.
+ 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.