From 1ad6f695933b51b5dbd89ae6e03faa7fa071799c Mon Sep 17 00:00:00 2001
From: Kaifeng Wang <kaifeng.w@gmail.com>
Date: Wed, 18 Jul 2018 16:51:53 +0800
Subject: [PATCH] Stop introspection if set boot device failed

At the beginning of introspection, ironic inspector sets boot device and
reboot node through ironic, currently it only stops when reboot the node
is failed. Actually the node is not guaranteed to reboot with pxe in either
case.

This patch eliminates the descripency, so that if anything goes wrong, it
will be exposed early.

Change-Id: I416e42137e59e04f7fd282aa309f2f89cf574209
Story: #2002977
Task: #22985
---
 ironic_inspector/introspect.py                |  8 +++----
 ironic_inspector/test/unit/test_introspect.py | 23 ++++++++++++++++++-
 ...en-setbootdev-failed-68d84fec0843bdc8.yaml |  5 ++++
 3 files changed, 31 insertions(+), 5 deletions(-)
 create mode 100644 releasenotes/notes/stop-when-setbootdev-failed-68d84fec0843bdc8.yaml

diff --git a/ironic_inspector/introspect.py b/ironic_inspector/introspect.py
index 94f5f1225..8190d072d 100644
--- a/ironic_inspector/introspect.py
+++ b/ironic_inspector/introspect.py
@@ -106,15 +106,15 @@ def _background_introspect_locked(node_info, ironic):
             ironic.node.set_boot_device(node_info.uuid, 'pxe',
                                         persistent=False)
         except Exception as exc:
-            LOG.warning('Failed to set boot device to PXE: %s',
-                        exc, node_info=node_info)
+            raise utils.Error(_('Failed to set boot device to PXE: %s') % exc,
+                              node_info=node_info)
 
         try:
             ironic.node.set_power_state(node_info.uuid, 'reboot')
         except Exception as exc:
             raise utils.Error(_('Failed to power on the node, check it\'s '
-                                'power management configuration: %s'),
-                              exc, node_info=node_info)
+                                'power management configuration: %s') % exc,
+                              node_info=node_info)
         LOG.info('Introspection started successfully',
                  node_info=node_info)
     else:
diff --git a/ironic_inspector/test/unit/test_introspect.py b/ironic_inspector/test/unit/test_introspect.py
index c46f8ab19..b93086fc6 100644
--- a/ironic_inspector/test/unit/test_introspect.py
+++ b/ironic_inspector/test/unit/test_introspect.py
@@ -122,7 +122,6 @@ class TestIntrospect(BaseTest):
 
     def test_power_failure(self, client_mock, start_mock):
         cli = self._prepare(client_mock)
-        cli.node.set_boot_device.side_effect = exceptions.BadRequest()
         cli.node.set_power_state.side_effect = exceptions.BadRequest()
         start_mock.return_value = self.node_info
 
@@ -163,6 +162,28 @@ class TestIntrospect(BaseTest):
         self.node_info.acquire_lock.assert_called_once_with()
         self.node_info.release_lock.assert_called_once_with()
 
+    def test_set_boot_device_failure(self, client_mock, start_mock):
+        cli = self._prepare(client_mock)
+        cli.node.set_boot_device.side_effect = exceptions.BadRequest()
+        start_mock.return_value = self.node_info
+
+        introspect.introspect(self.node.uuid)
+
+        cli.node.get.assert_called_once_with(self.uuid)
+
+        start_mock.assert_called_once_with(self.uuid,
+                                           bmc_address=self.bmc_address,
+                                           manage_boot=True,
+                                           ironic=cli)
+        cli.node.set_boot_device.assert_called_once_with(self.uuid,
+                                                         'pxe',
+                                                         persistent=False)
+        cli.node.set_power_state.assert_not_called()
+        start_mock.return_value.finished.assert_called_once_with(
+            introspect.istate.Events.error, error=mock.ANY)
+        self.node_info.acquire_lock.assert_called_once_with()
+        self.node_info.release_lock.assert_called_once_with()
+
     def test_no_macs(self, client_mock, start_mock):
         cli = self._prepare(client_mock)
         self.node_info.ports.return_value = []
diff --git a/releasenotes/notes/stop-when-setbootdev-failed-68d84fec0843bdc8.yaml b/releasenotes/notes/stop-when-setbootdev-failed-68d84fec0843bdc8.yaml
new file mode 100644
index 000000000..832987ee0
--- /dev/null
+++ b/releasenotes/notes/stop-when-setbootdev-failed-68d84fec0843bdc8.yaml
@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    Stops introspection when setting boot device is failed, as the node is
+    not guarenteed to perform a PXE boot in this case.