From a0f5618ffa02cc8622669a5c31b98f9b4773ed60 Mon Sep 17 00:00:00 2001
From: Dan Smith <dansmith@redhat.com>
Date: Thu, 17 Jan 2019 13:27:25 -0800
Subject: [PATCH] Fix using template cell urls with nova-manage

When nova-manage went to validate the transport-url given in config or on the
command line, it was not doing the translation before passing the url to the
oslo.messaging parse routine to check it. This exposes the format functions
from the CellMapping object, and makes our _validate_transport_url() format
the url before passing it to parse.

This also adds a test that makes sure the template makes it into the database
(as a template) and that it gets loaded out in translated form with an
object load.

Change-Id: I40a435b8e97c8552c2f5f0ca3a24de2edd9f81bd
Closes-Bug: #1812196
(cherry picked from commit edd1cd9ee4d149b01a388f03fb423374020033d7)
---
 nova/cmd/manage.py                  |  4 +++-
 nova/objects/cell_mapping.py        |  8 ++++----
 nova/tests/unit/test_nova_manage.py | 25 +++++++++++++++++++++++++
 3 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/nova/cmd/manage.py b/nova/cmd/manage.py
index 94bde3c70a8e..fd402a175ef1 100644
--- a/nova/cmd/manage.py
+++ b/nova/cmd/manage.py
@@ -1053,7 +1053,9 @@ class CellV2Commands(object):
             return None
 
         try:
-            messaging.TransportURL.parse(conf=CONF, url=transport_url)
+            messaging.TransportURL.parse(conf=CONF,
+                                         url=objects.CellMapping.format_mq_url(
+                                             transport_url))
         except (messaging.InvalidTransportURL, ValueError) as e:
             print(_('Invalid transport URL: %s') % six.text_type(e))
             return None
diff --git a/nova/objects/cell_mapping.py b/nova/objects/cell_mapping.py
index 7739afaa5497..9fb777b96cfc 100644
--- a/nova/objects/cell_mapping.py
+++ b/nova/objects/cell_mapping.py
@@ -127,7 +127,7 @@ class CellMapping(base.NovaTimestampObject, base.NovaObject):
         return url.format(**subs)
 
     @staticmethod
-    def _format_db_url(url):
+    def format_db_url(url):
         if CONF.database.connection is None:
             if '{' in url:
                 LOG.error('Cell mapping database_connection is a template, '
@@ -141,7 +141,7 @@ class CellMapping(base.NovaTimestampObject, base.NovaObject):
             return url
 
     @staticmethod
-    def _format_mq_url(url):
+    def format_mq_url(url):
         if CONF.transport_url is None:
             if '{' in url:
                 LOG.error('Cell mapping transport_url is a template, but '
@@ -159,9 +159,9 @@ class CellMapping(base.NovaTimestampObject, base.NovaObject):
         for key in cell_mapping.fields:
             val = db_cell_mapping[key]
             if key == 'database_connection':
-                val = cell_mapping._format_db_url(val)
+                val = cell_mapping.format_db_url(val)
             elif key == 'transport_url':
-                val = cell_mapping._format_mq_url(val)
+                val = cell_mapping.format_mq_url(val)
             setattr(cell_mapping, key, val)
         cell_mapping.obj_reset_changes()
         cell_mapping._context = context
diff --git a/nova/tests/unit/test_nova_manage.py b/nova/tests/unit/test_nova_manage.py
index 0c809a2008af..6197705cbd6a 100644
--- a/nova/tests/unit/test_nova_manage.py
+++ b/nova/tests/unit/test_nova_manage.py
@@ -1846,6 +1846,31 @@ class CellV2CommandsTestCase(test.NoDBTestCase):
         self.assertEqual(kwargs['transport_url'], cell2.transport_url)
         self.assertIs(cell2.disabled, False)
 
+    def test_create_cell_use_params_with_template(self):
+        ctxt = context.get_context()
+        self.flags(transport_url='rabbit://host:1234')
+        kwargs = dict(
+            name='fake-name',
+            transport_url='{scheme}://other-{hostname}:{port}',
+            database_connection='fake-db-connection')
+        status = self.commands.create_cell(verbose=True, **kwargs)
+        self.assertEqual(0, status)
+        cell2_uuid = self.output.getvalue().strip()
+        self.commands.create_cell(**kwargs)
+
+        # Make sure it ended up as a template in the database
+        db_cm = objects.CellMapping._get_by_uuid_from_db(ctxt, cell2_uuid)
+        self.assertEqual('{scheme}://other-{hostname}:{port}',
+                         db_cm.transport_url)
+
+        # Make sure it gets translated if we load by object
+        cell2 = objects.CellMapping.get_by_uuid(ctxt, cell2_uuid)
+        self.assertEqual(kwargs['name'], cell2.name)
+        self.assertEqual(kwargs['database_connection'],
+                         cell2.database_connection)
+        self.assertEqual('rabbit://other-host:1234', cell2.transport_url)
+        self.assertIs(cell2.disabled, False)
+
     def test_create_cell_use_config_values(self):
         settings = dict(
             transport_url='http://fake-conf-transport-url',