From 2c1e1341214356808936c4a812c89d4008cdb284 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Mon, 13 Feb 2017 15:48:43 -0500 Subject: [PATCH] Cleanup some issues with CONF.placement.os_interface This change fixes a few things with the recently added "os_interface" option in the [placement] config group. 1. It adds tests for the scheduler report client that were missing in the original change that added the config option. 2. It uses the option in the "nova-status upgrade check" command so it is consistent with how the scheduler report client uses it. 3. It removes the restrictive choices list from the config option definition. keystoneauth1 allows an "auth" value for the endpoint interface which means don't use the service catalog to find the endpoint but instead just read it from the "auth_url" config option. Also, the Keystone v3 API performs strict validation of the endpoint interface when creating an endpoint record. The list of supported interfaces may change over time, so we shouldn't encode that list within Nova. 4. As part of removing the choices, the release note associated with the new option is updated and changed from a 'feature' release note to simply 'other' since it's not really a feature as much as it is a bug fix. Change-Id: Ia5af05cc4d8155349bab942280c83e7318749959 Closes-Bug: #1664334 --- nova/cmd/status.py | 3 +- nova/conf/placement.py | 2 -- nova/scheduler/client/report.py | 7 ++--- .../openstack/placement/test_report_client.py | 4 +-- nova/tests/unit/cmd/test_status.py | 31 +++++++++++++++++++ .../unit/scheduler/client/test_report.py | 13 +++++++- ...dpoint-interface-set-29af8b9400ce7775.yaml | 10 +++--- 7 files changed, 55 insertions(+), 15 deletions(-) diff --git a/nova/cmd/status.py b/nova/cmd/status.py index a1160e7b7d63..0eee9e532c67 100644 --- a/nova/cmd/status.py +++ b/nova/cmd/status.py @@ -180,7 +180,8 @@ class UpgradeCommands(object): """ ks_filter = {'service_type': 'placement', - 'region_name': CONF.placement.os_region_name} + 'region_name': CONF.placement.os_region_name, + 'interface': CONF.placement.os_interface} auth = keystone.load_auth_from_conf_options( CONF, 'placement') client = session.Session(auth=auth) diff --git a/nova/conf/placement.py b/nova/conf/placement.py index b11f873d2bb3..14754bac5691 100644 --- a/nova/conf/placement.py +++ b/nova/conf/placement.py @@ -29,8 +29,6 @@ Possible values: * Any string representing region name """), cfg.StrOpt('os_interface', - default="public", - choices=["public", "admin", "internal"], help=""" Endpoint interface for this node. This is used when picking the URL in the service catalog. diff --git a/nova/scheduler/client/report.py b/nova/scheduler/client/report.py index cb0472451acf..88764445de4d 100644 --- a/nova/scheduler/client/report.py +++ b/nova/scheduler/client/report.py @@ -185,10 +185,6 @@ def get_placement_request_id(response): class SchedulerReportClient(object): """Client class for updating the scheduler.""" - ks_filter = {'service_type': 'placement', - 'region_name': CONF.placement.os_region_name, - 'interface': CONF.placement.os_interface} - def __init__(self): # A dict, keyed by the resource provider UUID, of ResourceProvider # objects that will have their inventories and allocations tracked by @@ -202,6 +198,9 @@ class SchedulerReportClient(object): self._client = session.Session(auth=auth_plugin) # NOTE(danms): Keep track of how naggy we've been self._warn_count = 0 + self.ks_filter = {'service_type': 'placement', + 'region_name': CONF.placement.os_region_name, + 'interface': CONF.placement.os_interface} def get(self, url, version=None): kwargs = {} diff --git a/nova/tests/functional/api/openstack/placement/test_report_client.py b/nova/tests/functional/api/openstack/placement/test_report_client.py index dcce084af4e2..258f8474d170 100644 --- a/nova/tests/functional/api/openstack/placement/test_report_client.py +++ b/nova/tests/functional/api/openstack/placement/test_report_client.py @@ -31,9 +31,7 @@ class NoAuthReportClient(report.SchedulerReportClient): """A SchedulerReportClient that avoids keystone.""" def __init__(self): - self._resource_providers = {} - self._provider_aggregate_map = {} - self._disabled = False + super(NoAuthReportClient, self).__init__() # Supply our own session so the wsgi-intercept can intercept # the right thing. Another option would be to use the direct # urllib3 interceptor. diff --git a/nova/tests/unit/cmd/test_status.py b/nova/tests/unit/cmd/test_status.py index c4e5164bcb71..ce742f4ea947 100644 --- a/nova/tests/unit/cmd/test_status.py +++ b/nova/tests/unit/cmd/test_status.py @@ -22,6 +22,7 @@ from six.moves import StringIO from keystoneauth1 import exceptions as ks_exc from keystoneauth1 import loading as keystone +from keystoneauth1 import session from oslo_utils import uuidutils from nova.cmd import status @@ -120,6 +121,36 @@ class TestPlacementCheck(test.NoDBTestCase): self.assertEqual(status.UpgradeCheckCode.FAILURE, res.code) self.assertIn('No credentials specified', res.details) + @mock.patch.object(keystone, "load_auth_from_conf_options") + @mock.patch.object(session.Session, 'get') + def _test_placement_get_interface( + self, expected_interface, mock_get, mock_auth): + + def fake_get(path, *a, **kw): + self.assertEqual(mock.sentinel.path, path) + self.assertIn('endpoint_filter', kw) + self.assertEqual(expected_interface, + kw['endpoint_filter']['interface']) + return mock.Mock(autospec='requests.models.Response') + + mock_get.side_effect = fake_get + self.cmd._placement_get(mock.sentinel.path) + mock_auth.assert_called_once_with(status.CONF, 'placement') + self.assertTrue(mock_get.called) + + @mock.patch.object(keystone, "load_auth_from_conf_options") + @mock.patch.object(session.Session, 'get') + def test_placement_get_interface_default(self, mock_get, mock_auth): + """Tests that None is specified for interface by default.""" + self._test_placement_get_interface(None) + + @mock.patch.object(keystone, "load_auth_from_conf_options") + @mock.patch.object(session.Session, 'get') + def test_placement_get_interface_internal(self, mock_get, mock_auth): + """Tests that "internal" is specified for interface when configured.""" + self.flags(os_interface='internal', group='placement') + self._test_placement_get_interface('internal') + @mock.patch.object(status.UpgradeCommands, "_placement_get") def test_invalid_auth(self, get): """Test failure when wrong credentials are specified or service user diff --git a/nova/tests/unit/scheduler/client/test_report.py b/nova/tests/unit/scheduler/client/test_report.py index b4cd0f31d75d..402bca7ae49a 100644 --- a/nova/tests/unit/scheduler/client/test_report.py +++ b/nova/tests/unit/scheduler/client/test_report.py @@ -135,10 +135,21 @@ class TestConstructor(test.NoDBTestCase): @mock.patch('keystoneauth1.session.Session') @mock.patch('keystoneauth1.loading.load_auth_from_conf_options') def test_constructor(self, load_auth_mock, ks_sess_mock): - report.SchedulerReportClient() + client = report.SchedulerReportClient() load_auth_mock.assert_called_once_with(CONF, 'placement') ks_sess_mock.assert_called_once_with(auth=load_auth_mock.return_value) + self.assertIsNone(client.ks_filter['interface']) + + @mock.patch('keystoneauth1.session.Session') + @mock.patch('keystoneauth1.loading.load_auth_from_conf_options') + def test_constructor_admin_interface(self, load_auth_mock, ks_sess_mock): + self.flags(os_interface='admin', group='placement') + client = report.SchedulerReportClient() + + load_auth_mock.assert_called_once_with(CONF, 'placement') + ks_sess_mock.assert_called_once_with(auth=load_auth_mock.return_value) + self.assertEqual('admin', client.ks_filter['interface']) class SchedulerReportClientTestCase(test.NoDBTestCase): diff --git a/releasenotes/notes/placement-api-endpoint-interface-set-29af8b9400ce7775.yaml b/releasenotes/notes/placement-api-endpoint-interface-set-29af8b9400ce7775.yaml index 96c14e1cc24e..8f80b68eeaaf 100644 --- a/releasenotes/notes/placement-api-endpoint-interface-set-29af8b9400ce7775.yaml +++ b/releasenotes/notes/placement-api-endpoint-interface-set-29af8b9400ce7775.yaml @@ -1,7 +1,9 @@ --- -features: - - The placement API can be set to connect to a specific +other: + - The Placement API can be set to connect to a specific keystone endpoint interface using the ``os_interface`` option in the ``[placement]`` section inside ``nova.conf``. - This value is not required and will default to ``public``. - Other acceptable options are ``admin`` or ``internal``. + This value is not required but can be used if a non-default + endpoint interface is desired for connecting to the Placement + service. By default, keystoneauth will connect to the "public" + endpoint.