Do not wipe instance_info on normal unprovisioning

Ironic uses information from instance_info on tear down and removes
it afterwards automatically.

Change-Id: I88ea06b7cb07d648cc68b677e905bf403ae8e1ed

metalsmith/_provisioner.py

@@ -491,7 +491,7 @@ class Provisioner(object):
                 for key, value in instance_info.items()
                 if key in _PRESERVE_INSTANCE_INFO_KEYS}
 
-    def _clean_up(self, node, nics=None):
+    def _clean_up(self, node, nics=None, remove_instance_info=True):
         if nics is None:
             created_ports = node.extra.get(_CREATED_PORTS, [])
             attached_ports = node.extra.get(_ATTACHED_PORTS, [])
@@ -521,14 +521,20 @@ class Provisioner(object):
             # Old-style reservations have to be cleared explicitly
             kwargs['instance_id'] = None
 
-        LOG.debug('Updating node %(node)s with empty instance info (was '
-                  '%(iinfo)s) and extras %(extra)s',
-                  {'node': _utils.log_res(node),
-                   'iinfo': node.instance_info,
-                   'extra': extra})
         try:
-            self.connection.baremetal.update_node(
-                node, instance_info={}, extra=extra, **kwargs)
+            if remove_instance_info:
+                LOG.debug('Updating node %(node)s with empty instance info '
+                          '(was %(iinfo)s) and extras %(extra)s',
+                          {'node': _utils.log_res(node),
+                           'iinfo': node.instance_info,
+                           'extra': extra})
+                self.connection.baremetal.update_node(
+                    node, instance_info={}, extra=extra, **kwargs)
+            else:
+                LOG.debug('Updating node %(node)s with extras %(extra)s',
+                          {'node': _utils.log_res(node), 'extra': extra})
+                self.connection.baremetal.update_node(
+                    node, extra=extra, **kwargs)
         except Exception as exc:
             LOG.debug('Failed to clear node %(node)s extra: %(exc)s',
                       {'node': _utils.log_res(node), 'exc': exc})
@@ -553,7 +559,7 @@ class Provisioner(object):
             LOG.warning("Dry run, not unprovisioning")
             return
 
-        self._clean_up(node)
+        self._clean_up(node, remove_instance_info=False)
         try:
             node = self.connection.baremetal.set_node_provision_state(
                 node, 'deleted', wait=False)

metalsmith/test/test_provisioner.py

@@ -1837,7 +1837,7 @@ class TestUnprovisionNode(Base):
         self.assertFalse(
             self.api.baremetal.wait_for_nodes_provision_state.called)
         self.api.baremetal.update_node.assert_called_once_with(
-            self.node, instance_info={}, extra={'foo': 'bar'})
+            self.node, extra={'foo': 'bar'})
         self.assertFalse(self.api.baremetal.delete_allocation.called)
         # We cannot delete an allocation for an active node, it will be deleted
         # automatically.
@@ -1859,7 +1859,7 @@ class TestUnprovisionNode(Base):
         self.assertFalse(
             self.api.baremetal.wait_for_nodes_provision_state.called)
         self.api.baremetal.update_node.assert_called_once_with(
-            self.node, instance_info={}, extra={'foo': 'bar'})
+            self.node, extra={'foo': 'bar'})
         self.api.baremetal.delete_allocation.assert_called_once_with('123')
 
     def test_with_attached(self):
@@ -1876,7 +1876,7 @@ class TestUnprovisionNode(Base):
         self.assertFalse(
             self.api.baremetal.wait_for_nodes_provision_state.called)
         self.api.baremetal.update_node.assert_called_once_with(
-            self.node, instance_info={}, extra={})
+            self.node, extra={})
 
     def test_with_wait(self):
         result = self.pr.unprovision_node(self.node, wait=3600)
@@ -1889,7 +1889,7 @@ class TestUnprovisionNode(Base):
         self.api.baremetal.set_node_provision_state.assert_called_once_with(
             self.node, 'deleted', wait=False)
         self.api.baremetal.update_node.assert_called_once_with(
-            self.node, instance_info={}, extra={})
+            self.node, extra={})
         wait_mock = self.api.baremetal.wait_for_nodes_provision_state
         wait_mock.assert_called_once_with([self.node], 'available',
                                           timeout=3600)
@@ -1920,7 +1920,7 @@ class TestUnprovisionNode(Base):
         self.assertFalse(
             self.api.baremetal.wait_for_nodes_provision_state.called)
         self.api.baremetal.update_node.assert_called_once_with(
-            self.node, instance_info={}, extra={'foo': 'bar'},
+            self.node, extra={'foo': 'bar'},
             instance_id=None)
         self.assertFalse(self.api.baremetal.delete_allocation.called)
 

releasenotes/notes/keep-instance-info-3e7bb09244d5aaa6.yaml

@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    No longer removes ``instance_info`` on normal unprovisioning, only on
+    failures during deployment.