From f5b19b37382ff5c5f54abf480349c069ef285674 Mon Sep 17 00:00:00 2001 From: Sean Dague Date: Fri, 12 Aug 2016 15:02:36 -0400 Subject: [PATCH] Use neutron for network name -> id resolution The 2.36 microversion deprecates the nova proxy APIs for looking up a network from neutron. The CLI will lookup a network by name for booting a server as a convenience. Over 90% of clouds now have neutron. We use a service catalog lookup to assume that neutron is available, and if not fall back to our proxy. This should mostly shield people from the 2.36 transition. To test this we add support in our fake client for the neutron network lookup. Comments were also left about some cleanups we should do in other parts of the shell testing for net lookup. Related to blueprint deprecate-api-proxies Change-Id: I4f809de4f7efb913c814f132f5b3482731c588c5 --- novaclient/tests/unit/v2/fakes.py | 46 +++++++++++++- novaclient/tests/unit/v2/test_shell.py | 62 ++++++++++++++++--- novaclient/v2/client.py | 20 ++++++ novaclient/v2/networks.py | 33 ++++++++++ novaclient/v2/shell.py | 24 +++++++ .../no-neutron-proxy-18fd54febe939a6b.yaml | 12 ++++ 6 files changed, 187 insertions(+), 10 deletions(-) create mode 100644 releasenotes/notes/no-neutron-proxy-18fd54febe939a6b.yaml diff --git a/novaclient/tests/unit/v2/fakes.py b/novaclient/tests/unit/v2/fakes.py index 59e5e8581..15c44783e 100644 --- a/novaclient/tests/unit/v2/fakes.py +++ b/novaclient/tests/unit/v2/fakes.py @@ -158,7 +158,7 @@ class FakeHTTPClient(base_client.HTTPClient): }) return r, body - def get_endpoint(self): + def get_endpoint(self, **kwargs): # check if endpoint matches expected format (eg, v2.1) if (hasattr(self, 'endpoint_type') and ENDPOINT_TYPE_RE.search(self.endpoint_type)): @@ -1909,6 +1909,50 @@ class FakeHTTPClient(base_client.HTTPClient): 'hypervisor_hostname': "hyper1", 'uptime': "fake uptime"}}) + def get_v2_0_networks(self, **kw): + """Return neutron proxied networks. + + We establish a few different possible networks that we can get + by name, which we can then call in tests. The only usage of + this API should be for name -> id translation, however a full + valid neutron block is provided for the private network to see + the kinds of things that will be in that payload. + """ + + name = kw.get('name', "blank") + + networks_by_name = { + 'private': [ + {"status": "ACTIVE", + "router:external": False, + "availability_zone_hints": [], + "availability_zones": ["nova"], + "description": "", + "name": "private", + "subnets": ["64706c26-336c-4048-ab3c-23e3283bca2c", + "18512740-c760-4d5f-921f-668105c9ee44"], + "shared": False, + "tenant_id": "abd42f270bca43ea80fe4a166bc3536c", + "created_at": "2016-08-15T17:34:49", + "tags": [], + "ipv6_address_scope": None, + "updated_at": "2016-08-15T17:34:49", + "admin_state_up": True, + "ipv4_address_scope": None, + "port_security_enabled": True, + "mtu": 1450, + "id": "e43a56c7-11d4-45c9-8681-ddc8171b5850", + "revision": 2}], + 'duplicate': [ + {"status": "ACTIVE", + "id": "e43a56c7-11d4-45c9-8681-ddc8171b5850"}, + {"status": "ACTIVE", + "id": "f43a56c7-11d4-45c9-8681-ddc8171b5850"}], + 'blank': [] + } + + return (200, {}, {"networks": networks_by_name[name]}) + def get_os_networks(self, **kw): return (200, {}, {'networks': [{"label": "1", "cidr": "10.0.0.0/24", 'project_id': diff --git a/novaclient/tests/unit/v2/test_shell.py b/novaclient/tests/unit/v2/test_shell.py index 90469ea1c..821f91081 100644 --- a/novaclient/tests/unit/v2/test_shell.py +++ b/novaclient/tests/unit/v2/test_shell.py @@ -26,6 +26,7 @@ import mock from oslo_utils import timeutils import six from six.moves import builtins +import testtools import novaclient from novaclient import api_versions @@ -671,14 +672,10 @@ class ShellTest(utils.TestCase): '--nic net-id=net-id1,net-id=net-id2 some-server' % FAKE_UUID_1) self.assertRaises(exceptions.CommandError, self.run_command, cmd) - @mock.patch( - 'novaclient.tests.unit.v2.fakes.FakeHTTPClient.get_os_networks') - def test_boot_nics_net_name(self, mock_networks_list): - mock_networks_list.return_value = (200, {}, { - 'networks': [{"label": "some-net", 'id': '1'}]}) - + @mock.patch('novaclient.v2.client.Client.has_neutron', return_value=False) + def test_boot_nics_net_name(self, has_neutron): cmd = ('boot --image %s --flavor 1 ' - '--nic net-name=some-net some-server' % FAKE_UUID_1) + '--nic net-name=1 some-server' % FAKE_UUID_1) self.run_command(cmd) self.assert_called_anytime( 'POST', '/servers', @@ -696,14 +693,61 @@ class ShellTest(utils.TestCase): }, ) - def test_boot_nics_net_name_not_found(self): + @mock.patch('novaclient.v2.client.Client.has_neutron', return_value=True) + def test_boot_nics_net_name_neutron(self, has_neutron): + cmd = ('boot --image %s --flavor 1 ' + '--nic net-name=private some-server' % FAKE_UUID_1) + self.run_command(cmd) + self.assert_called_anytime( + 'POST', '/servers', + { + 'server': { + 'flavorRef': '1', + 'name': 'some-server', + 'imageRef': FAKE_UUID_1, + 'min_count': 1, + 'max_count': 1, + 'networks': [ + {'uuid': 'e43a56c7-11d4-45c9-8681-ddc8171b5850'}, + ], + }, + }, + ) + + @mock.patch('novaclient.v2.client.Client.has_neutron', return_value=True) + def test_boot_nics_net_name_neutron_dup(self, has_neutron): + cmd = ('boot --image %s --flavor 1 ' + '--nic net-name=duplicate some-server' % FAKE_UUID_1) + # this should raise a multiple matches error + msg = ("Multiple network matches found for 'duplicate', " + "use an ID to be more specific.") + with testtools.ExpectedException(exceptions.CommandError, msg): + self.run_command(cmd) + + @mock.patch('novaclient.v2.client.Client.has_neutron', return_value=True) + def test_boot_nics_net_name_neutron_blank(self, has_neutron): + cmd = ('boot --image %s --flavor 1 ' + '--nic net-name=blank some-server' % FAKE_UUID_1) + # this should raise a multiple matches error + msg = 'No Network matching blank\..*' + with testtools.ExpectedException(exceptions.CommandError, msg): + self.run_command(cmd) + + # TODO(sdague): the following tests should really avoid mocking + # out other tests, and they should check the string in the + # CommandError, because it's not really enough to distinguish + # between various errors. + @mock.patch('novaclient.v2.client.Client.has_neutron', return_value=False) + def test_boot_nics_net_name_not_found(self, has_neutron): cmd = ('boot --image %s --flavor 1 ' '--nic net-name=some-net some-server' % FAKE_UUID_1) self.assertRaises(exceptions.ResourceNotFound, self.run_command, cmd) + @mock.patch('novaclient.v2.client.Client.has_neutron', return_value=False) @mock.patch( 'novaclient.tests.unit.v2.fakes.FakeHTTPClient.get_os_networks') - def test_boot_nics_net_name_multiple_matches(self, mock_networks_list): + def test_boot_nics_net_name_multiple_matches(self, mock_networks_list, + has_neutron): mock_networks_list.return_value = (200, {}, { 'networks': [{"label": "some-net", 'id': '1'}, {"label": "some-net", 'id': '2'}]}) diff --git a/novaclient/v2/client.py b/novaclient/v2/client.py index 044653f52..c6d985e25 100644 --- a/novaclient/v2/client.py +++ b/novaclient/v2/client.py @@ -15,6 +15,8 @@ import logging +from keystoneauth1.exceptions import catalog as key_ex + from novaclient import api_versions from novaclient import client from novaclient import exceptions @@ -149,6 +151,7 @@ class Client(object): self.volumes = volumes.VolumeManager(self) self.keypairs = keypairs.KeypairManager(self) self.networks = networks.NetworkManager(self) + self.neutron = networks.NeutronManager(self) self.quota_classes = quota_classes.QuotaClassSetManager(self) self.quotas = quotas.QuotaSetManager(self) self.security_groups = security_groups.SecurityGroupManager(self) @@ -233,6 +236,23 @@ class Client(object): def reset_timings(self): self.client.reset_timings() + def has_neutron(self): + """Check the service catalog to figure out if we have neutron. + + This is an intermediary solution for the window of time where + we still have nova-network support in the client, but we + expect most people have neutron. This ensures that if they + have neutron we understand, we talk to it, if they don't, we + fail back to nova proxies. + """ + try: + endpoint = self.client.get_endpoint(service_type='network') + if endpoint: + return True + return False + except key_ex.EndpointNotFound: + return False + @client._original_only def authenticate(self): """Authenticate against the server. diff --git a/novaclient/v2/networks.py b/novaclient/v2/networks.py index 22f3d537c..4f545403f 100644 --- a/novaclient/v2/networks.py +++ b/novaclient/v2/networks.py @@ -41,6 +41,39 @@ class Network(base.Resource): return self.manager.delete(self) +class NeutronManager(base.Manager): + """A manager for name -> id lookups for neutron networks. + + This uses neutron directly from service catalog. Do not use it + for anything else besides that. You have been warned. + """ + + resource_class = Network + + def find_network(self, name): + """Find a network by name (user provided input).""" + + with self.alternate_service_type( + 'network', allowed_types=('network',)): + + matches = self._list('/v2.0/networks?name=%s' % name, 'networks') + + num_matches = len(matches) + if num_matches == 0: + msg = "No %s matching %s." % ( + self.resource_class.__name__, name) + raise exceptions.NotFound(404, msg) + elif num_matches > 1: + msg = (_("Multiple %(class)s matches found for " + "'%(name)s', use an ID to be more specific.") % + {'class': self.resource_class.__name__.lower(), + 'name': name}) + raise exceptions.NoUniqueMatch(msg) + else: + matches[0].append_request_ids(matches.request_ids) + return matches[0] + + class NetworkManager(base.ManagerWithFind): """ Manage :class:`Network` resources. diff --git a/novaclient/v2/shell.py b/novaclient/v2/shell.py index f3a4fcb2f..971e9879d 100644 --- a/novaclient/v2/shell.py +++ b/novaclient/v2/shell.py @@ -2270,7 +2270,31 @@ def _find_flavor(cs, flavor): return cs.flavors.find(ram=flavor) +def _find_network_id_neutron(cs, net_name): + """Get unique network ID from network name from neutron""" + try: + return cs.neutron.find_network(net_name).id + except (exceptions.NotFound, exceptions.NoUniqueMatch) as e: + raise exceptions.CommandError(six.text_type(e)) + + def _find_network_id(cs, net_name): + """Find the network id for a network name. + + If we have access to neutron in the service catalog, use neutron + for this lookup, otherwise use nova. This ensures that we do the + right thing in the future. + + Once nova network support is deleted, we can delete this check and + the has_neutron function. + """ + if cs.has_neutron(): + return _find_network_id_neutron(cs, net_name) + else: + return _find_network_id_novanet(cs, net_name) + + +def _find_network_id_novanet(cs, net_name): """Get unique network ID from network name.""" network_id = None for net_info in cs.networks.list(): diff --git a/releasenotes/notes/no-neutron-proxy-18fd54febe939a6b.yaml b/releasenotes/notes/no-neutron-proxy-18fd54febe939a6b.yaml new file mode 100644 index 000000000..21007cd93 --- /dev/null +++ b/releasenotes/notes/no-neutron-proxy-18fd54febe939a6b.yaml @@ -0,0 +1,12 @@ +--- +upgrade: + - | + The 2.36 microversion deprecated the network proxy APIs in + Nova. Because of this we now go directly to neutron for name to + net-id lookups. For nova-net deployements the old proxies will + continue to be used. + + To do this the following is assumed: + + #. There is a **network** entry in the service catalog. + #. The network v2 API is available.