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

@@ -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)

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.

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 = {}

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.

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

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):

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.