From 4be716eb27752d715ea1140b76e4a03907edd87f Mon Sep 17 00:00:00 2001
From: Tang Chen <tangchen@cn.fujitsu.com>
Date: Fri, 4 Dec 2015 16:30:10 +0800
Subject: [PATCH] Migrate network client to SDK.

The previous patches have migrate all network commands to
the new version using sdk. This patch will remove the
temporary method, and implement a new make_client() to
create sdk network client.

And also, find() in openstackclient/network/common.py must
support sdk. The logic of this function will become much
easier than before, so this patch also removes two useless
test cases of find().

This patch will also remove the patched methods in tests.

Change-Id: Ic2f7bca073beb9757172d16f95d9b82c48cbbc12
Implements: blueprint neutron-client
Co-Authored-By: Terry Howe <terrylhowe@gmail.com>
Co-Authored-By: Tang Chen <tangchen@cn.fujitsu.com>
---
 openstackclient/network/client.py             | 62 +++----------------
 openstackclient/network/common.py             | 24 +------
 openstackclient/network/v2/network.py         | 18 ------
 .../tests/compute/v2/test_server.py           | 12 ++++
 openstackclient/tests/network/test_common.py  | 33 +++-------
 .../tests/network/v2/test_network.py          | 62 +++++--------------
 6 files changed, 48 insertions(+), 163 deletions(-)

diff --git a/openstackclient/network/client.py b/openstackclient/network/client.py
index 69ed11feb8..7714c52504 100644
--- a/openstackclient/network/client.py
+++ b/openstackclient/network/client.py
@@ -13,6 +13,8 @@
 
 import logging
 
+from openstack import connection
+
 from openstackclient.common import utils
 
 
@@ -22,62 +24,18 @@ DEFAULT_API_VERSION = '2.0'
 API_VERSION_OPTION = 'os_network_api_version'
 API_NAME = "network"
 API_VERSIONS = {
-    "2.0": "neutronclient.v2_0.client.Client",
-    "2": "neutronclient.v2_0.client.Client",
-}
-# Translate our API version to auth plugin version prefix
-API_VERSION_MAP = {
-    '2.0': 'v2.0',
-    '2': 'v2.0',
-}
-
-NETWORK_API_TYPE = 'network'
-NETWORK_API_VERSIONS = {
-    '2.0': 'openstackclient.api.network_v2.APIv2',
-    '2': 'openstackclient.api.network_v2.APIv2',
+    "2.0": "openstack.connection.Connection",
+    "2": "openstack.connection.Connection",
 }
 
 
 def make_client(instance):
-    """Returns an network service client"""
-    network_client = utils.get_client_class(
-        API_NAME,
-        instance._api_version[API_NAME],
-        API_VERSIONS)
-    LOG.debug('Instantiating network client: %s', network_client)
-
-    endpoint = instance.get_endpoint_for_service_type(
-        API_NAME,
-        region_name=instance._region_name,
-        interface=instance._interface,
-    )
-
-    # Remember endpoint_type only if it is set
-    kwargs = utils.build_kwargs_dict('endpoint_type', instance._interface)
-
-    client = network_client(
-        session=instance.session,
-        region_name=instance._region_name,
-        **kwargs
-    )
-
-    network_api = utils.get_client_class(
-        API_NAME,
-        instance._api_version[API_NAME],
-        NETWORK_API_VERSIONS)
-    LOG.debug('Instantiating network api: %s', network_client)
-
-    # v2 is hard-coded until discovery is completed, neutron only has one atm
-    client.api = network_api(
-        session=instance.session,
-        service_type=NETWORK_API_TYPE,
-        endpoint='/'.join([
-            endpoint,
-            API_VERSION_MAP[instance._api_version[API_NAME]],
-        ])
-    )
-
-    return client
+    """Returns a network proxy"""
+    conn = connection.Connection(authenticator=instance.session.auth)
+    LOG.debug('Connection: %s', conn)
+    LOG.debug('Network client initialized using OpenStack SDK: %s',
+              conn.network)
+    return conn.network
 
 
 def build_option_parser(parser):
diff --git a/openstackclient/network/common.py b/openstackclient/network/common.py
index 31faef25f3..7b3f8a62e9 100644
--- a/openstackclient/network/common.py
+++ b/openstackclient/network/common.py
@@ -11,8 +11,6 @@
 #   under the License.
 #
 
-from openstackclient.common import exceptions
-
 
 def find(client, resource, resources, name_or_id, name_attr='name'):
     """Find a network resource
@@ -26,22 +24,6 @@ def find(client, resource, resources, name_or_id, name_attr='name'):
     For example:
         n = find(netclient, 'network', 'networks', 'matrix')
     """
-    list_method = getattr(client, "list_%s" % resources)
-
-    # Search by name
-    kwargs = {name_attr: name_or_id, 'fields': 'id'}
-    data = list_method(**kwargs)
-    info = data[resources]
-    if len(info) == 1:
-        return info[0]['id']
-    if len(info) > 1:
-        msg = "More than one %s exists with the name '%s'."
-        raise exceptions.CommandError(msg % (resource, name_or_id))
-
-    # Search by id
-    data = list_method(id=name_or_id, fields='id')
-    info = data[resources]
-    if len(info) == 1:
-        return info[0]['id']
-    msg = "No %s with a name or ID of '%s' exists." % (resource, name_or_id)
-    raise exceptions.CommandError(msg)
+    list_method = getattr(client, "find_%s" % resource)
+    data = list_method(name_or_id, ignore_missing=False)
+    return data.id
diff --git a/openstackclient/network/v2/network.py b/openstackclient/network/v2/network.py
index 4c94dc650e..64b98f5d85 100644
--- a/openstackclient/network/v2/network.py
+++ b/openstackclient/network/v2/network.py
@@ -19,8 +19,6 @@ from cliff import command
 from cliff import lister
 from cliff import show
 
-from openstack import connection
-
 from openstackclient.common import exceptions
 from openstackclient.common import utils
 from openstackclient.identity import common as identity_common
@@ -41,12 +39,6 @@ _formatters = {
 }
 
 
-def _make_client_sdk(instance):
-    """Return a network proxy"""
-    conn = connection.Connection(authenticator=instance.session.auth)
-    return conn.network
-
-
 class CreateNetwork(show.ShowOne):
     """Create new network"""
 
@@ -96,8 +88,6 @@ class CreateNetwork(show.ShowOne):
 
     def take_action(self, parsed_args):
         self.log.debug('take_action(%s)' % parsed_args)
-        self.app.client_manager.network = \
-            _make_client_sdk(self.app.client_manager)
         client = self.app.client_manager.network
         body = self.get_body(parsed_args)
         obj = client.create_network(**body)
@@ -138,8 +128,6 @@ class DeleteNetwork(command.Command):
 
     def take_action(self, parsed_args):
         self.log.debug('take_action(%s)' % parsed_args)
-        self.app.client_manager.network = \
-            _make_client_sdk(self.app.client_manager)
         client = self.app.client_manager.network
         for network in parsed_args.networks:
             obj = client.find_network(network)
@@ -170,8 +158,6 @@ class ListNetwork(lister.Lister):
 
     def take_action(self, parsed_args):
         self.log.debug('take_action(%s)' % parsed_args)
-        self.app.client_manager.network = \
-            _make_client_sdk(self.app.client_manager)
         client = self.app.client_manager.network
 
         if parsed_args.long:
@@ -270,8 +256,6 @@ class SetNetwork(command.Command):
 
     def take_action(self, parsed_args):
         self.log.debug('take_action(%s)' % parsed_args)
-        self.app.client_manager.network = \
-            _make_client_sdk(self.app.client_manager)
         client = self.app.client_manager.network
         obj = client.find_network(parsed_args.identifier, ignore_missing=False)
 
@@ -306,8 +290,6 @@ class ShowNetwork(show.ShowOne):
 
     def take_action(self, parsed_args):
         self.log.debug('take_action(%s)' % parsed_args)
-        self.app.client_manager.network = \
-            _make_client_sdk(self.app.client_manager)
         client = self.app.client_manager.network
         obj = client.find_network(parsed_args.identifier, ignore_missing=False)
         columns = sorted(obj.keys())
diff --git a/openstackclient/tests/compute/v2/test_server.py b/openstackclient/tests/compute/v2/test_server.py
index ce2dcdf7e8..01c945eecc 100644
--- a/openstackclient/tests/compute/v2/test_server.py
+++ b/openstackclient/tests/compute/v2/test_server.py
@@ -231,6 +231,18 @@ class TestServerCreate(TestServer):
         list_networks.return_value = {'networks': [{'id': 'net1_uuid'}]}
         list_ports.return_value = {'ports': [{'id': 'port1_uuid'}]}
 
+        # Mock sdk APIs.
+        _network = mock.Mock()
+        _network.id = 'net1_uuid'
+        _port = mock.Mock()
+        _port.id = 'port1_uuid'
+        find_network = mock.Mock()
+        find_port = mock.Mock()
+        find_network.return_value = _network
+        find_port.return_value = _port
+        self.app.client_manager.network.find_network = find_network
+        self.app.client_manager.network.find_port = find_port
+
         # DisplayCommandBase.take_action() returns two tuples
         columns, data = self.cmd.take_action(parsed_args)
 
diff --git a/openstackclient/tests/network/test_common.py b/openstackclient/tests/network/test_common.py
index b30fdfcb35..58586ac066 100644
--- a/openstackclient/tests/network/test_common.py
+++ b/openstackclient/tests/network/test_common.py
@@ -13,7 +13,6 @@
 
 import mock
 
-from openstackclient.common import exceptions
 from openstackclient.network import common
 from openstackclient.tests import utils
 
@@ -28,45 +27,31 @@ class TestFind(utils.TestCase):
         super(TestFind, self).setUp()
         self.mock_client = mock.Mock()
         self.list_resources = mock.Mock()
-        self.mock_client.list_resources = self.list_resources
-        self.matrix = {'id': ID}
+        self.mock_client.find_resource = self.list_resources
+        self.resource = mock.Mock()
+        self.resource.id = ID
 
     def test_name(self):
-        self.list_resources.return_value = {RESOURCES: [self.matrix]}
+        self.list_resources.return_value = self.resource
 
         result = common.find(self.mock_client, RESOURCE, RESOURCES, NAME)
 
         self.assertEqual(ID, result)
-        self.list_resources.assert_called_with(fields='id', name=NAME)
+        self.list_resources.assert_called_with(NAME, ignore_missing=False)
 
     def test_id(self):
-        self.list_resources.side_effect = [{RESOURCES: []},
-                                           {RESOURCES: [self.matrix]}]
+        self.list_resources.return_value = self.resource
 
         result = common.find(self.mock_client, RESOURCE, RESOURCES, NAME)
 
         self.assertEqual(ID, result)
-        self.list_resources.assert_called_with(fields='id', id=NAME)
+        self.list_resources.assert_called_with(NAME, ignore_missing=False)
 
     def test_nameo(self):
-        self.list_resources.return_value = {RESOURCES: [self.matrix]}
+        self.list_resources.return_value = self.resource
 
         result = common.find(self.mock_client, RESOURCE, RESOURCES, NAME,
                              name_attr='nameo')
 
         self.assertEqual(ID, result)
-        self.list_resources.assert_called_with(fields='id', nameo=NAME)
-
-    def test_dups(self):
-        dup = {'id': 'Larry'}
-        self.list_resources.return_value = {RESOURCES: [self.matrix, dup]}
-
-        self.assertRaises(exceptions.CommandError, common.find,
-                          self.mock_client, RESOURCE, RESOURCES, NAME)
-
-    def test_nada(self):
-        self.list_resources.side_effect = [{RESOURCES: []},
-                                           {RESOURCES: []}]
-
-        self.assertRaises(exceptions.CommandError, common.find,
-                          self.mock_client, RESOURCE, RESOURCES, NAME)
+        self.list_resources.assert_called_with(NAME, ignore_missing=False)
diff --git a/openstackclient/tests/network/v2/test_network.py b/openstackclient/tests/network/v2/test_network.py
index 67f446505b..3dcf381151 100644
--- a/openstackclient/tests/network/v2/test_network.py
+++ b/openstackclient/tests/network/v2/test_network.py
@@ -72,7 +72,6 @@ class TestNetwork(network_fakes.TestNetworkV2):
         self.api = self.app.client_manager.network.api
 
 
-@mock.patch('openstackclient.network.v2.network._make_client_sdk')
 class TestCreateNetworkIdentityV3(TestNetwork):
 
     # The new network created.
@@ -132,9 +131,7 @@ class TestCreateNetworkIdentityV3(TestNetwork):
             loaded=True,
         )
 
-    def test_create_no_options(self, _make_client_sdk):
-        _make_client_sdk.return_value = self.app.client_manager.network
-
+    def test_create_no_options(self):
         arglist = [
             self._network.name,
         ]
@@ -155,9 +152,7 @@ class TestCreateNetworkIdentityV3(TestNetwork):
         self.assertEqual(self.columns, columns)
         self.assertEqual(self.data, data)
 
-    def test_create_all_options(self, _make_client_sdk):
-        _make_client_sdk.return_value = self.app.client_manager.network
-
+    def test_create_all_options(self):
         arglist = [
             "--disable",
             "--share",
@@ -185,9 +180,7 @@ class TestCreateNetworkIdentityV3(TestNetwork):
         self.assertEqual(self.columns, columns)
         self.assertEqual(self.data, data)
 
-    def test_create_other_options(self, _make_client_sdk):
-        _make_client_sdk.return_value = self.app.client_manager.network
-
+    def test_create_other_options(self):
         arglist = [
             "--enable",
             "--no-share",
@@ -211,7 +204,6 @@ class TestCreateNetworkIdentityV3(TestNetwork):
         self.assertEqual(self.data, data)
 
 
-@mock.patch('openstackclient.network.v2.network._make_client_sdk')
 class TestCreateNetworkIdentityV2(TestNetwork):
 
     # The new network created.
@@ -265,9 +257,7 @@ class TestCreateNetworkIdentityV2(TestNetwork):
 
         # There is no DomainManager Mock in fake identity v2.
 
-    def test_create_with_project_identityv2(self, _make_client_sdk):
-        _make_client_sdk.return_value = self.app.client_manager.network
-
+    def test_create_with_project_identityv2(self):
         arglist = [
             "--project", identity_fakes_v2.project_name,
             self._network.name,
@@ -290,9 +280,7 @@ class TestCreateNetworkIdentityV2(TestNetwork):
         self.assertEqual(self.columns, columns)
         self.assertEqual(self.data, data)
 
-    def test_create_with_domain_identityv2(self, _make_client_sdk):
-        _make_client_sdk.return_value = self.app.client_manager.network
-
+    def test_create_with_domain_identityv2(self):
         arglist = [
             "--project", identity_fakes_v3.project_name,
             "--project-domain", identity_fakes_v3.domain_name,
@@ -315,7 +303,6 @@ class TestCreateNetworkIdentityV2(TestNetwork):
         )
 
 
-@mock.patch('openstackclient.network.v2.network._make_client_sdk')
 class TestDeleteNetwork(TestNetwork):
 
     # The network to delete.
@@ -331,9 +318,7 @@ class TestDeleteNetwork(TestNetwork):
         # Get the command object to test
         self.cmd = network.DeleteNetwork(self.app, self.namespace)
 
-    def test_delete(self, _make_client_sdk):
-        _make_client_sdk.return_value = self.app.client_manager.network
-
+    def test_delete(self):
         arglist = [
             self._network.name,
         ]
@@ -348,7 +333,6 @@ class TestDeleteNetwork(TestNetwork):
         self.assertEqual(None, result)
 
 
-@mock.patch('openstackclient.network.v2.network._make_client_sdk')
 class TestListNetwork(TestNetwork):
 
     # The networks going to be listed up.
@@ -401,9 +385,7 @@ class TestListNetwork(TestNetwork):
 
         self.network.networks = mock.Mock(return_value=self._network)
 
-    def test_network_list_no_options(self, _make_client_sdk):
-        _make_client_sdk.return_value = self.app.client_manager.network
-
+    def test_network_list_no_options(self):
         arglist = []
         verifylist = [
             ('external', False),
@@ -418,9 +400,7 @@ class TestListNetwork(TestNetwork):
         self.assertEqual(self.columns, columns)
         self.assertEqual(self.data, list(data))
 
-    def test_list_external(self, _make_client_sdk):
-        _make_client_sdk.return_value = self.app.client_manager.network
-
+    def test_list_external(self):
         arglist = [
             '--external',
         ]
@@ -439,9 +419,7 @@ class TestListNetwork(TestNetwork):
         self.assertEqual(self.columns, columns)
         self.assertEqual(self.data, list(data))
 
-    def test_network_list_long(self, _make_client_sdk):
-        _make_client_sdk.return_value = self.app.client_manager.network
-
+    def test_network_list_long(self):
         arglist = [
             '--long',
         ]
@@ -459,7 +437,6 @@ class TestListNetwork(TestNetwork):
         self.assertEqual(self.data_long, list(data))
 
 
-@mock.patch('openstackclient.network.v2.network._make_client_sdk')
 class TestSetNetwork(TestNetwork):
 
     # The network to set.
@@ -475,9 +452,7 @@ class TestSetNetwork(TestNetwork):
         # Get the command object to test
         self.cmd = network.SetNetwork(self.app, self.namespace)
 
-    def test_set_this(self, _make_client_sdk):
-        _make_client_sdk.return_value = self.app.client_manager.network
-
+    def test_set_this(self):
         self._network.is_dirty = True
 
         arglist = [
@@ -499,9 +474,7 @@ class TestSetNetwork(TestNetwork):
         self.network.update_network.assert_called_with(self._network)
         self.assertEqual(None, result)
 
-    def test_set_that(self, _make_client_sdk):
-        _make_client_sdk.return_value = self.app.client_manager.network
-
+    def test_set_that(self):
         self._network.is_dirty = True
 
         arglist = [
@@ -521,9 +494,7 @@ class TestSetNetwork(TestNetwork):
         self.network.update_network.assert_called_with(self._network)
         self.assertEqual(None, result)
 
-    def test_set_nothing(self, _make_client_sdk):
-        _make_client_sdk.return_value = self.app.client_manager.network
-
+    def test_set_nothing(self):
         self._network.is_dirty = False
 
         arglist = [self._network.name, ]
@@ -534,7 +505,6 @@ class TestSetNetwork(TestNetwork):
                           parsed_args)
 
 
-@mock.patch('openstackclient.network.v2.network._make_client_sdk')
 class TestShowNetwork(TestNetwork):
 
     # The network to set.
@@ -568,9 +538,7 @@ class TestShowNetwork(TestNetwork):
         # Get the command object to test
         self.cmd = network.ShowNetwork(self.app, self.namespace)
 
-    def test_show_no_options(self, _make_client_sdk):
-        _make_client_sdk.return_value = self.app.client_manager.network
-
+    def test_show_no_options(self):
         arglist = []
         verifylist = []
 
@@ -580,9 +548,7 @@ class TestShowNetwork(TestNetwork):
         except tests_utils.ParserException:
             pass
 
-    def test_show_all_options(self, _make_client_sdk):
-        _make_client_sdk.return_value = self.app.client_manager.network
-
+    def test_show_all_options(self):
         arglist = [
             self._network.name,
         ]