From 92eefc9d63b5f9a5092820f679b4c2d961743a1a Mon Sep 17 00:00:00 2001
From: Julia Kreger <juliaashleykreger@gmail.com>
Date: Fri, 3 Feb 2023 08:32:48 -0800
Subject: fix inspectwait logic

The tl;dr is that we changed ``inspecting`` to include a
``inspect wait`` state. Unfortunately we never spotted the logic
inside of the db API. We never spotted it because our testing in
inspection code uses a mocked task manager... and we *really* don't
have intense db testing because we expect the objects and higher
level interactions to validate the lowest db level.

Unfortunately, because of the out of band inspection workflow,
we have to cover both cases in terms of what the starting state
and ending state could be, but we've added tests to
validate this is handled as we expect.

Change-Id: Icccbc6d65531e460c55555e021bf81d362f5fc8b
---
 ironic/db/sqlalchemy/api.py                        |  6 ++-
 ironic/tests/unit/db/test_nodes.py                 | 47 ++++++++++++++++++++++
 ...x-inspectwait-finished-at-4b817af4bf4c30c2.yaml |  5 +++
 3 files changed, 56 insertions(+), 2 deletions(-)
 create mode 100644 releasenotes/notes/fix-inspectwait-finished-at-4b817af4bf4c30c2.yaml

diff --git a/ironic/db/sqlalchemy/api.py b/ironic/db/sqlalchemy/api.py
index de3934318..a3d671f50 100644
--- a/ironic/db/sqlalchemy/api.py
+++ b/ironic/db/sqlalchemy/api.py
@@ -904,11 +904,13 @@ class Connection(api.Connection):
                 if values['provision_state'] == states.INSPECTING:
                     values['inspection_started_at'] = timeutils.utcnow()
                     values['inspection_finished_at'] = None
-                elif (ref.provision_state == states.INSPECTING
+                elif ((ref.provision_state == states.INSPECTING
+                       or ref.provision_state == states.INSPECTWAIT)
                       and values['provision_state'] == states.MANAGEABLE):
                     values['inspection_finished_at'] = timeutils.utcnow()
                     values['inspection_started_at'] = None
-                elif (ref.provision_state == states.INSPECTING
+                elif ((ref.provision_state == states.INSPECTING
+                       or ref.provision_state == states.INSPECTWAIT)
                       and values['provision_state'] == states.INSPECTFAIL):
                     values['inspection_started_at'] = None
 
diff --git a/ironic/tests/unit/db/test_nodes.py b/ironic/tests/unit/db/test_nodes.py
index 78bba0a35..e234fc634 100644
--- a/ironic/tests/unit/db/test_nodes.py
+++ b/ironic/tests/unit/db/test_nodes.py
@@ -884,6 +884,53 @@ class DbNodeTestCase(base.DbTestCase):
                          timeutils.normalize_time(result))
         self.assertIsNone(res['inspection_started_at'])
 
+    @mock.patch.object(timeutils, 'utcnow', autospec=True)
+    def test_update_node_inspection_finished_at_inspecting(self, mock_utcnow):
+        mocked_time = datetime.datetime(2000, 1, 1, 0, 0)
+        mock_utcnow.return_value = mocked_time
+        node = utils.create_test_node(uuid=uuidutils.generate_uuid(),
+                                      inspection_finished_at=mocked_time,
+                                      provision_state=states.INSPECTING)
+        res = self.dbapi.update_node(node.id,
+                                     {'provision_state': states.MANAGEABLE})
+        result = res['inspection_finished_at']
+        self.assertEqual(mocked_time,
+                         timeutils.normalize_time(result))
+        self.assertIsNone(res['inspection_started_at'])
+
+    @mock.patch.object(timeutils, 'utcnow', autospec=True)
+    def test_update_node_inspection_finished_at_inspectwait(self,
+                                                            mock_utcnow):
+        mocked_time = datetime.datetime(2000, 1, 1, 0, 0)
+        mock_utcnow.return_value = mocked_time
+        node = utils.create_test_node(uuid=uuidutils.generate_uuid(),
+                                      inspection_finished_at=mocked_time,
+                                      provision_state=states.INSPECTWAIT)
+        res = self.dbapi.update_node(node.id,
+                                     {'provision_state': states.MANAGEABLE})
+        result = res['inspection_finished_at']
+        self.assertEqual(mocked_time,
+                         timeutils.normalize_time(result))
+        self.assertIsNone(res['inspection_started_at'])
+
+    def test_update_node_inspection_started_at_inspecting(self):
+        mocked_time = datetime.datetime(2000, 1, 1, 0, 0)
+        node = utils.create_test_node(uuid=uuidutils.generate_uuid(),
+                                      inspection_started_at=mocked_time,
+                                      provision_state=states.INSPECTING)
+        res = self.dbapi.update_node(node.id,
+                                     {'provision_state': states.INSPECTFAIL})
+        self.assertIsNone(res['inspection_started_at'])
+
+    def test_update_node_inspection_started_at_inspectwait(self):
+        mocked_time = datetime.datetime(2000, 1, 1, 0, 0)
+        node = utils.create_test_node(uuid=uuidutils.generate_uuid(),
+                                      inspection_started_at=mocked_time,
+                                      provision_state=states.INSPECTWAIT)
+        res = self.dbapi.update_node(node.id,
+                                     {'provision_state': states.INSPECTFAIL})
+        self.assertIsNone(res['inspection_started_at'])
+
     def test_reserve_node(self):
         node = utils.create_test_node()
         self.dbapi.set_node_tags(node.id, ['tag1', 'tag2'])
diff --git a/releasenotes/notes/fix-inspectwait-finished-at-4b817af4bf4c30c2.yaml b/releasenotes/notes/fix-inspectwait-finished-at-4b817af4bf4c30c2.yaml
new file mode 100644
index 000000000..167a7f4a5
--- /dev/null
+++ b/releasenotes/notes/fix-inspectwait-finished-at-4b817af4bf4c30c2.yaml
@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    Fixes a database API internal check to update the
+    ``inspection_finished_at`` field upon the completion of inspection.
-- 
cgit v1.2.1