From 86101b022fc61c58a77cf975da528c5e78d8eeb4 Mon Sep 17 00:00:00 2001
From: Hiroaki Kobayashi <kobayashi.hiroaki@lab.ntt.co.jp>
Date: Fri, 6 Oct 2017 10:23:18 +0900
Subject: [PATCH] Check if a host is reserved before deleting it

Currently, if you try to delete a host which is reserved, it is removed
from the freepool but remains in the computehosts table. This patch
checks if the host is reserved before removing it.

Change-Id: I3daaba338809375e16018ae5e03107d938e89b9c
Closes-Bug: #1721456
---
 blazar/manager/exceptions.py                  |  5 ++++
 blazar/plugins/oshosts/host_plugin.py         | 20 ++++++++-----
 .../plugins/test_physical_host_plugin.py      | 29 ++++++++++++++++++-
 3 files changed, 46 insertions(+), 8 deletions(-)

diff --git a/blazar/manager/exceptions.py b/blazar/manager/exceptions.py
index 97ec0a5e..51575aeb 100644
--- a/blazar/manager/exceptions.py
+++ b/blazar/manager/exceptions.py
@@ -30,6 +30,11 @@ class CantRemoveHost(exceptions.BlazarException):
     msg_fmt = _("Can't remove host(s) %(host)s from Aggregate %(pool)s")
 
 
+class CantDeleteHost(exceptions.BlazarException):
+    code = 409
+    msg_fmt = _("Can't delete host %(host)s. %(msg)s")
+
+
 class CantAddHost(exceptions.BlazarException):
     code = 409
     msg_fmt = _("Can't add host(s) %(host)s to Aggregate %(pool)s")
diff --git a/blazar/plugins/oshosts/host_plugin.py b/blazar/plugins/oshosts/host_plugin.py
index 4aebb052..1e80070c 100644
--- a/blazar/plugins/oshosts/host_plugin.py
+++ b/blazar/plugins/oshosts/host_plugin.py
@@ -321,8 +321,13 @@ class PhysicalHostPlugin(base.BasePlugin, nova.NovaClientWrapper):
             raise manager_ex.HostNotFound(host=host_id)
 
         with trusts.create_ctx_from_trust(host['trust_id']):
-            # TODO(sbauza):
-            #  - Check if no leases having this host scheduled
+            if db_api.host_allocation_get_all_by_values(
+                    compute_host_id=host_id):
+                raise manager_ex.CantDeleteHost(
+                    host=host_id,
+                    msg='The host is reserved.'
+                )
+
             inventory = nova.NovaInventory()
             servers = inventory.get_servers_per_host(
                 host['hypervisor_hostname'])
@@ -337,11 +342,12 @@ class PhysicalHostPlugin(base.BasePlugin, nova.NovaClientWrapper):
                 # NOTE(sbauza): Extracapabilities will be destroyed thanks to
                 #  the DB FK.
                 db_api.host_destroy(host_id)
-            except db_ex.BlazarDBException:
-                # Nothing so bad, but we need to advert the admin
-                # he has to rerun
-                raise manager_ex.CantRemoveHost(host=host_id,
-                                                pool=self.freepool_name)
+            except db_ex.BlazarDBException as e:
+                # Nothing so bad, but we need to alert admins
+                # they have to rerun
+                raise manager_ex.CantDeleteHost(
+                    host=host_id,
+                    msg=e.message)
 
     def _matching_hosts(self, hypervisor_properties, resource_properties,
                         count_range, start_date, end_date):
diff --git a/blazar/tests/plugins/test_physical_host_plugin.py b/blazar/tests/plugins/test_physical_host_plugin.py
index a3d87759..f9ca0ade 100644
--- a/blazar/tests/plugins/test_physical_host_plugin.py
+++ b/blazar/tests/plugins/test_physical_host_plugin.py
@@ -280,13 +280,36 @@ class PhysicalHostPluginTestCase(tests.TestCase):
         })
 
     def test_delete_host(self):
+        host_allocation_get_all = self.patch(
+            self.db_api,
+            'host_allocation_get_all_by_values')
+        host_allocation_get_all.return_value = []
         self.fake_phys_plugin.delete_computehost(self.fake_host_id)
 
         self.db_host_destroy.assert_called_once_with(self.fake_host_id)
         self.get_servers_per_host.assert_called_once_with(
             self.fake_host["hypervisor_hostname"])
 
+    def test_delete_host_reserved(self):
+        host_allocation_get_all = self.patch(
+            self.db_api,
+            'host_allocation_get_all_by_values')
+        host_allocation_get_all.return_value = [
+            {
+                'id': u'dd305477-4df8-4547-87f6-69069ee546a6',
+                'compute_host_id': self.fake_host_id
+            }
+        ]
+
+        self.assertRaises(manager_exceptions.CantDeleteHost,
+                          self.fake_phys_plugin.delete_computehost,
+                          self.fake_host_id)
+
     def test_delete_host_having_vms(self):
+        host_allocation_get_all = self.patch(
+            self.db_api,
+            'host_allocation_get_all_by_values')
+        host_allocation_get_all.return_value = []
         self.get_servers_per_host.return_value = ['server1', 'server2']
         self.assertRaises(manager_exceptions.HostHavingServers,
                           self.fake_phys_plugin.delete_computehost,
@@ -303,8 +326,12 @@ class PhysicalHostPluginTestCase(tests.TestCase):
     def test_delete_host_issuing_rollback(self):
         def fake_db_host_destroy(*args, **kwargs):
             raise db_exceptions.BlazarDBException
+        host_allocation_get_all = self.patch(
+            self.db_api,
+            'host_allocation_get_all_by_values')
+        host_allocation_get_all.return_value = []
         self.db_host_destroy.side_effect = fake_db_host_destroy
-        self.assertRaises(manager_exceptions.CantRemoveHost,
+        self.assertRaises(manager_exceptions.CantDeleteHost,
                           self.fake_phys_plugin.delete_computehost,
                           self.fake_host_id)