From 0346b02ccea75b992d0b72a6dbaa1f05864450e0 Mon Sep 17 00:00:00 2001
From: Jason Anderson <jasonanderson@uchicago.edu>
Date: Fri, 11 Oct 2019 16:51:28 -0500
Subject: [PATCH] Fix issue moving hosts back to freepool

If an aggregate fails to be created for some reason, for example because
the requested node is not in the freepool, some cleanup occurs. Part of
the cleanup is ensuring the target node is moved _back_ to the freepool.
This should theoretically "heal" cases where the node wasn't in the
freepool but also wasn't in some lease aggregate.

However, this call to Nova's API refers to the freepool aggregate by its
name, which is not supported here: the ID must be used[1]. This caused
this operation to fail and raise a NotFound, confusingly (because Nova
couldn't find an aggregate with ID='freepool' for example.)

[1]:
https://docs.openstack.org/api-ref/compute/?expanded=add-host-detail#add-host

Closes-Bug: #1847821
Change-Id: I7af4d407b183578617131f0de42becb3dc2bc415
---
 blazar/tests/utils/openstack/test_nova.py            | 4 ++--
 blazar/utils/openstack/nova.py                       | 2 +-
 releasenotes/notes/bug-1847821-2f9d6e61f438dc4f.yaml | 8 ++++++++
 3 files changed, 11 insertions(+), 3 deletions(-)
 create mode 100644 releasenotes/notes/bug-1847821-2f9d6e61f438dc4f.yaml

diff --git a/blazar/tests/utils/openstack/test_nova.py b/blazar/tests/utils/openstack/test_nova.py
index 49a1184f..088606a2 100644
--- a/blazar/tests/utils/openstack/test_nova.py
+++ b/blazar/tests/utils/openstack/test_nova.py
@@ -347,8 +347,8 @@ class ReservationPoolTestCase(tests.TestCase):
         check0 = self.nova.aggregates.add_host
         check0.assert_has_calls([mock.call(self.fake_aggregate.id, 'host1'),
                                  mock.call(self.fake_aggregate.id, 'host2'),
-                                 mock.call('freepool', 'host1'),
-                                 mock.call('freepool', 'host2')])
+                                 mock.call(self.fake_freepool.id, 'host1'),
+                                 mock.call(self.fake_freepool.id, 'host2')])
         check1 = self.nova.aggregates.remove_host
         check1.assert_has_calls([mock.call(self.fake_freepool.id, 'host1'),
                                  mock.call(self.fake_freepool.id, 'host2'),
diff --git a/blazar/utils/openstack/nova.py b/blazar/utils/openstack/nova.py
index 32c30b43..9d9d6d99 100644
--- a/blazar/utils/openstack/nova.py
+++ b/blazar/utils/openstack/nova.py
@@ -387,7 +387,7 @@ class ReservationPool(NovaClientWrapper):
             if removed_hosts:
                 LOG.warn('Adding hosts back to freepool: %s', removed_hosts)
                 for host in removed_hosts:
-                    self.nova.aggregates.add_host(freepool_agg.name, host)
+                    self.nova.aggregates.add_host(freepool_agg.id, host)
             raise e
 
         return self.get_aggregate_from_name_or_id(pool)
diff --git a/releasenotes/notes/bug-1847821-2f9d6e61f438dc4f.yaml b/releasenotes/notes/bug-1847821-2f9d6e61f438dc4f.yaml
new file mode 100644
index 00000000..4d62a638
--- /dev/null
+++ b/releasenotes/notes/bug-1847821-2f9d6e61f438dc4f.yaml
@@ -0,0 +1,8 @@
+---
+fixes:
+  - |
+    Updates the on_start failure handlers to look up the freepool aggregate by
+    ID, not by name, when moving hosts back to the freepool. Fixes issue where
+    hosts wind up without any aggregate during lease start failure.  For more
+    details, see `bug 1847821
+    <https://bugs.launchpad.net/blazar/+bug/1847821>`_.