From f19d58a728ffa5f6b982f5373bfdb1a503006edf Mon Sep 17 00:00:00 2001
From: Dmitry Tantsur <divius.inside@gmail.com>
Date: Fri, 18 May 2018 12:11:06 +0200
Subject: [PATCH] baremetal: refuse to inspect associated machines

Automatically inspecting "available" machines is a controversial feature,
since it enables "stealing" a machine that Nova already picked for deployment.
To reduce this probability, refuse to inspect nodes with instance_uuid set.

Also finish the incomplete comment.

Change-Id: I6cde6a6f9303f2a21efcfce979ffc0c1fea4bdb3
---
 openstack/cloud/openstackcloud.py             | 12 +++++++++--
 .../tests/unit/cloud/test_baremetal_node.py   | 20 +++++++++++++++++++
 ...o-inspect-associated-563e272785bb6016.yaml |  5 +++++
 3 files changed, 35 insertions(+), 2 deletions(-)
 create mode 100644 releasenotes/notes/no-inspect-associated-563e272785bb6016.yaml

diff --git a/openstack/cloud/openstackcloud.py b/openstack/cloud/openstackcloud.py
index 0c4557ca2..f2ea4a90d 100644
--- a/openstack/cloud/openstackcloud.py
+++ b/openstack/cloud/openstackcloud.py
@@ -8912,9 +8912,17 @@ class OpenStackCloud(_normalize.Normalizer):
             raise exc.OpenStackCloudException(
                 "Machine inspection failed to find: %s." % name_or_id)
 
-        # NOTE(TheJulia): If in available state, we can do this, however
-        # We need to to move the host back to m
+        # NOTE(TheJulia): If in available state, we can do this. However,
+        # we need to to move the machine back to manageable first.
         if "available" in machine['provision_state']:
+            if machine['instance_uuid']:
+                raise exc.OpenStackCloudException(
+                    "Refusing to inspect available machine %(node)s "
+                    "which is associated with an instance "
+                    "(instance_uuid %(inst)s)" %
+                    {'node': machine['uuid'],
+                     'inst': machine['instance_uuid']})
+
             return_to_available = True
             # NOTE(TheJulia): Changing available machine to managedable state
             # and due to state transitions we need to until that transition has
diff --git a/openstack/tests/unit/cloud/test_baremetal_node.py b/openstack/tests/unit/cloud/test_baremetal_node.py
index fe842202b..a433f3385 100644
--- a/openstack/tests/unit/cloud/test_baremetal_node.py
+++ b/openstack/tests/unit/cloud/test_baremetal_node.py
@@ -222,6 +222,26 @@ class TestBaremetalNode(base.IronicTestCase):
 
         self.assert_calls()
 
+    def test_inspect_machine_fail_associated(self):
+        self.fake_baremetal_node['provision_state'] = 'available'
+        self.fake_baremetal_node['instance_uuid'] = '1234'
+        self.register_uris([
+            dict(method='GET',
+                 uri=self.get_mock_url(
+                     resource='nodes',
+                     append=[self.fake_baremetal_node['uuid']]),
+                 json=self.fake_baremetal_node),
+        ])
+        self.assertRaisesRegex(
+            exc.OpenStackCloudException,
+            'associated with an instance',
+            self.cloud.inspect_machine,
+            self.fake_baremetal_node['uuid'],
+            wait=True,
+            timeout=1)
+
+        self.assert_calls()
+
     def test_inspect_machine_failed(self):
         inspecting_node = self.fake_baremetal_node.copy()
         self.fake_baremetal_node['provision_state'] = 'inspect failed'
diff --git a/releasenotes/notes/no-inspect-associated-563e272785bb6016.yaml b/releasenotes/notes/no-inspect-associated-563e272785bb6016.yaml
new file mode 100644
index 000000000..c2faab6a9
--- /dev/null
+++ b/releasenotes/notes/no-inspect-associated-563e272785bb6016.yaml
@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    Machine inspection is now blocked for machines associated with an instance.
+    This is to avoid "stealing" a machine from under a provisioner (e.g. Nova).