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

6 files changed, 187 insertions, 10 deletions

diff --git a/novaclient/tests/unit/v2/fakes.py b/novaclient/tests/unit/v2/fakes.py
index 59e5e858..15c44783 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 90469ea1..821f9108 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 044653f5..c6d985e2 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 22f3d537..4f545403 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 f3a4fcb2..971e9879 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 00000000..21007cd9
--- /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.