Online migration for inspect_interface inspector->agent

Change-Id: If29bd3e3370c831d171c84846373c0bc374efc09

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'),
 )

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.

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)

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.