From 217e354a2dc0999d2b27130d0cc0abce9a097bf3 Mon Sep 17 00:00:00 2001
From: Dmitry Tantsur <dtantsur@protonmail.com>
Date: Tue, 7 Jul 2020 12:25:18 +0200
Subject: [PATCH] Do not validate driver on changing non-driver fields

This prevents setting maintenance on nodes with a broken driver.

Change-Id: Ibd94d9d8ad5467f8430f77673a34507feb83da84
---
 ironic/conductor/manager.py                   |  6 ++-
 ironic/tests/unit/conductor/test_manager.py   | 45 +++++++++++++++++++
 .../driver-maintenance-0945c2939fa4e917.yaml  |  5 +++
 3 files changed, 55 insertions(+), 1 deletion(-)
 create mode 100644 releasenotes/notes/driver-maintenance-0945c2939fa4e917.yaml

diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py
index 8f226f089f..7b9cbf7d0e 100644
--- a/ironic/conductor/manager.py
+++ b/ironic/conductor/manager.py
@@ -209,6 +209,7 @@ class ConductorManager(base_manager.BaseConductorManager):
                    "updated unless it is in one of allowed "
                    "(%(allowed)s) states or in maintenance mode.")
         updating_driver = 'driver' in delta
+        check_interfaces = updating_driver
         for iface in drivers_base.ALL_INTERFACES:
             interface_field = '%s_interface' % iface
             if interface_field not in delta:
@@ -224,7 +225,10 @@ class ConductorManager(base_manager.BaseConductorManager):
                               'allowed': ', '.join(allowed_update_states),
                               'field': interface_field})
 
-        driver_factory.check_and_update_node_interfaces(node_obj)
+            check_interfaces = True
+
+        if check_interfaces:
+            driver_factory.check_and_update_node_interfaces(node_obj)
 
         # NOTE(dtantsur): if we're updating the driver from an invalid value,
         # loading the old driver may be impossible. Since we only need to
diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py
index f0d25d6662..c488d3c2d6 100644
--- a/ironic/tests/unit/conductor/test_manager.py
+++ b/ironic/tests/unit/conductor/test_manager.py
@@ -960,6 +960,51 @@ class UpdateNodeTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
         self.assertIsNone(node.instance_uuid)
         self.assertIsNone(node.allocation_id)
 
+    def test_update_node_maintenance_with_broken_interface(self):
+        # Updates of non-driver fields are possible with a broken driver
+        node = obj_utils.create_test_node(self.context, driver='fake-hardware',
+                                          power_interface='foobar',
+                                          extra={'test': 'one'})
+
+        node.maintenance = True
+        res = self.service.update_node(self.context, node)
+        self.assertTrue(res.maintenance)
+
+        node.refresh()
+        self.assertTrue(node.maintenance)
+        self.assertEqual('foobar', node.power_interface)
+
+    def test_update_node_interface_field_with_broken_interface(self):
+        # Updates of driver fields are NOT possible with a broken driver,
+        # unless they're fixing the breakage.
+        node = obj_utils.create_test_node(self.context, driver='fake-hardware',
+                                          power_interface='foobar',
+                                          deploy_interface='fake',
+                                          extra={'test': 'one'})
+
+        node.deploy_interface = 'iscsi'
+        exc = self.assertRaises(messaging.rpc.ExpectedException,
+                                self.service.update_node,
+                                self.context, node)
+        self.assertEqual(exception.InterfaceNotFoundInEntrypoint,
+                         exc.exc_info[0])
+
+        node.refresh()
+        self.assertEqual('foobar', node.power_interface)
+        self.assertEqual('fake', node.deploy_interface)
+
+    def test_update_node_fix_broken_interface(self):
+        # Updates of non-driver fields are possible with a broken driver
+        node = obj_utils.create_test_node(self.context, driver='fake-hardware',
+                                          power_interface='foobar',
+                                          extra={'test': 'one'})
+
+        node.power_interface = 'fake'
+        self.service.update_node(self.context, node)
+
+        node.refresh()
+        self.assertEqual('fake', node.power_interface)
+
 
 @mgr_utils.mock_record_keepalive
 class VendorPassthruTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
diff --git a/releasenotes/notes/driver-maintenance-0945c2939fa4e917.yaml b/releasenotes/notes/driver-maintenance-0945c2939fa4e917.yaml
new file mode 100644
index 0000000000..0a7bf25e06
--- /dev/null
+++ b/releasenotes/notes/driver-maintenance-0945c2939fa4e917.yaml
@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    Fixes updating driver fields for nodes with a broken driver. This is
+    required to be able to set maintenance for such nodes.