Skip to content

Commit

Permalink
Fix formatting non-templated cell URLs with no config
Browse files Browse the repository at this point in the history
If transport_url or connection are unset in config, we will fail to format
even non-templated cell mapping URLs due to an overly specific check in the
format routines. This fixes it by always bailing on templating if the config
is unset, and keeping the error message if we were unable to format a
template as a result.

Change-Id: I760580b8e6594be2bee99a5b825a690b07ab9deb
Closes-Bug: #1798158
  • Loading branch information
kk7ds authored and mriedem committed Oct 16, 2018
1 parent 6bf11e1 commit 133d6b3
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 6 deletions.
14 changes: 8 additions & 6 deletions nova/objects/cell_mapping.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,10 @@ def _format_url(url, default):

@staticmethod
def _format_db_url(url):
if CONF.database.connection is None and '{' in url:
LOG.error('Cell mapping database_connection is a template, but '
'[database]/connection is not set')
if CONF.database.connection is None:
if '{' in url:
LOG.error('Cell mapping database_connection is a template, '
'but [database]/connection is not set')
return url
try:
return CellMapping._format_url(url, CONF.database.connection)
Expand All @@ -142,9 +143,10 @@ def _format_db_url(url):

@staticmethod
def _format_mq_url(url):
if CONF.transport_url is None and '{' in url:
LOG.error('Cell mapping transport_url is a template, but '
'[DEFAULT]/transport_url is not set')
if CONF.transport_url is None:
if '{' in url:
LOG.error('Cell mapping transport_url is a template, but '
'[DEFAULT]/transport_url is not set')
return url
try:
return CellMapping._format_url(url, CONF.transport_url)
Expand Down
17 changes: 17 additions & 0 deletions nova/tests/unit/objects/test_cell_mapping.py
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,23 @@ def test_formatted_url_without_base_set(self, mock_get):
self.assertEqual(varurl, mapping_obj.database_connection)
self.assertEqual(varurl, mapping_obj.transport_url)

@mock.patch.object(cell_mapping.CellMapping, '_get_by_uuid_from_db')
@mock.patch.object(cell_mapping.CellMapping, '_format_url')
def test_non_formatted_url_with_no_base(self, mock_format, mock_get):
# Make sure we just pass through the template URL if the base
# URLs are not set, i.e. we don't try to format the URL to a template.
url = 'foo'
self.flags(transport_url=None)
self.flags(connection=None, group='database')
db_mapping = get_db_mapping(transport_url=url,
database_connection=url)
mock_get.return_value = db_mapping
mapping_obj = objects.CellMapping().get_by_uuid(self.context,
db_mapping['uuid'])
self.assertEqual(url, mapping_obj.database_connection)
self.assertEqual(url, mapping_obj.transport_url)
mock_format.assert_not_called()


class TestCellMappingObject(test_objects._LocalTest,
_TestCellMappingObject):
Expand Down

0 comments on commit 133d6b3

Please sign in to comment.