Merge "Fix nodes stuck at cleaning on Network Service issues"

3 files changed, 29 insertions, 7 deletions

diff --git a/ironic/conductor/cleaning.py b/ironic/conductor/cleaning.py
index e3151d4b8..53d66ddd8 100644
--- a/ironic/conductor/cleaning.py
+++ b/ironic/conductor/cleaning.py
@@ -69,7 +69,7 @@ def do_node_clean(task, clean_steps=None, disable_ramdisk=False):
         task.driver.power.validate(task)
         if not disable_ramdisk:
             task.driver.network.validate(task)
-    except exception.InvalidParameterValue as e:
+    except (exception.InvalidParameterValue, exception.NetworkError) as e:
         msg = (_('Validation of node %(node)s for cleaning failed: %(msg)s') %
                {'node': node.uuid, 'msg': e})
         return utils.cleaning_error_handler(task, msg)
diff --git a/ironic/tests/unit/conductor/test_cleaning.py b/ironic/tests/unit/conductor/test_cleaning.py
index 65261450a..a4c3d57b6 100644
--- a/ironic/tests/unit/conductor/test_cleaning.py
+++ b/ironic/tests/unit/conductor/test_cleaning.py
@@ -51,8 +51,6 @@ class DoNodeCleanTestCase(db_base.DbTestCase):
             'step': 'build_raid', 'priority': 0, 'interface': 'deploy'}
 
     def __do_node_clean_validate_fail(self, mock_validate, clean_steps=None):
-        # InvalidParameterValue should cause node to go to CLEANFAIL
-        mock_validate.side_effect = exception.InvalidParameterValue('error')
         tgt_prov_state = states.MANAGEABLE if clean_steps else states.AVAILABLE
         node = obj_utils.create_test_node(
             self.context, driver='fake-hardware',
@@ -68,26 +66,42 @@ class DoNodeCleanTestCase(db_base.DbTestCase):
         self.assertIsNone(node.fault)
         mock_validate.assert_called_once_with(mock.ANY, mock.ANY)
 
+    def __do_node_clean_validate_fail_invalid(self, mock_validate,
+                                              clean_steps=None):
+        # InvalidParameterValue should cause node to go to CLEANFAIL
+        mock_validate.side_effect = exception.InvalidParameterValue('error')
+        self.__do_node_clean_validate_fail(mock_validate,
+                                           clean_steps=clean_steps)
+
     @mock.patch('ironic.drivers.modules.fake.FakePower.validate',
                 autospec=True)
     def test__do_node_clean_automated_power_validate_fail(self, mock_validate):
-        self.__do_node_clean_validate_fail(mock_validate)
+        self.__do_node_clean_validate_fail_invalid(mock_validate)
 
     @mock.patch('ironic.drivers.modules.fake.FakePower.validate',
                 autospec=True)
     def test__do_node_clean_manual_power_validate_fail(self, mock_validate):
-        self.__do_node_clean_validate_fail(mock_validate, clean_steps=[])
+        self.__do_node_clean_validate_fail_invalid(mock_validate,
+                                                   clean_steps=[])
 
     @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.validate',
                 autospec=True)
     def test__do_node_clean_automated_network_validate_fail(self,
                                                             mock_validate):
-        self.__do_node_clean_validate_fail(mock_validate)
+        self.__do_node_clean_validate_fail_invalid(mock_validate)
 
     @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.validate',
                 autospec=True)
     def test__do_node_clean_manual_network_validate_fail(self, mock_validate):
-        self.__do_node_clean_validate_fail(mock_validate, clean_steps=[])
+        self.__do_node_clean_validate_fail_invalid(mock_validate,
+                                                   clean_steps=[])
+
+    @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.validate',
+                autospec=True)
+    def test__do_node_clean_network_error_fail(self, mock_validate):
+        # NetworkError should cause node to go to CLEANFAIL
+        mock_validate.side_effect = exception.NetworkError()
+        self.__do_node_clean_validate_fail(mock_validate)
 
     @mock.patch.object(conductor_utils, 'LOG', autospec=True)
     @mock.patch.object(conductor_steps, 'set_node_cleaning_steps',
diff --git a/releasenotes/notes/fix-cleaning-stuck-on-networkerror-4aedbf3673413af6.yaml b/releasenotes/notes/fix-cleaning-stuck-on-networkerror-4aedbf3673413af6.yaml
new file mode 100644
index 000000000..f7769afc1
--- /dev/null
+++ b/releasenotes/notes/fix-cleaning-stuck-on-networkerror-4aedbf3673413af6.yaml
@@ -0,0 +1,8 @@
+---
+fixes:
+  - |
+    Fixes an issue where cleaning operations could fail in such a way that was
+    not easily recoverable when pre-cleaning network interface configuration
+    was validated, yet contained invalid configuration.
+    Now Ironic properly captures the error and exits from cleaning in a
+    state which allows for cleaning to be retried.