Switch openstack server add port/network to using sdk.

The old novaclient.v2.server.Server.interface_attach() method is
replaced with proxy.create_server_interface().
In swargs, 'net_id' and 'port_id' are mutual-exclusive, if one of
them is given with value, the other one cannot be None, as the
API would responde with 400 (None is not string).
In unit test, temporary method 'setup_sdk_servers_mock' is added,
because other tests are still using the old 'setup_servers_mock'.
Functional tests are added. Releasenote is generated.

Change-Id: I9899f0509febc5143560a1859ae6344d0a6d1427

3 files changed, 100 insertions, 29 deletions

diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py
index c11f4b57..fcd7d69c 100644
--- a/openstackclient/compute/v2/server.py
+++ b/openstackclient/compute/v2/server.py
@@ -27,6 +27,7 @@ import iso8601
 from novaclient import api_versions
 from novaclient.v2 import servers
 from openstack import exceptions as sdk_exceptions
+from openstack import utils as sdk_utils
 from osc_lib.cli import format_columns
 from osc_lib.cli import parseractions
 from osc_lib.command import command
@@ -378,10 +379,10 @@ class AddPort(command.Command):
         return parser
 
     def take_action(self, parsed_args):
-        compute_client = self.app.client_manager.compute
+        compute_client = self.app.client_manager.sdk_connection.compute
 
-        server = utils.find_resource(
-            compute_client.servers, parsed_args.server)
+        server = compute_client.find_server(
+            parsed_args.server, ignore_missing=False)
 
         if self.app.client_manager.is_network_endpoint_enabled():
             network_client = self.app.client_manager.network
@@ -392,12 +393,11 @@ class AddPort(command.Command):
 
         kwargs = {
             'port_id': port_id,
-            'net_id': None,
             'fixed_ip': None,
         }
 
         if parsed_args.tag:
-            if compute_client.api_version < api_versions.APIVersion("2.49"):
+            if not sdk_utils.supports_microversion(compute_client, '2.49'):
                 msg = _(
                     '--os-compute-api-version 2.49 or greater is required to '
                     'support the --tag option'
@@ -405,7 +405,7 @@ class AddPort(command.Command):
                 raise exceptions.CommandError(msg)
             kwargs['tag'] = parsed_args.tag
 
-        server.interface_attach(**kwargs)
+        compute_client.create_server_interface(server, **kwargs)
 
 
 class AddNetwork(command.Command):
@@ -434,10 +434,10 @@ class AddNetwork(command.Command):
         return parser
 
     def take_action(self, parsed_args):
-        compute_client = self.app.client_manager.compute
+        compute_client = self.app.client_manager.sdk_connection.compute
 
-        server = utils.find_resource(
-            compute_client.servers, parsed_args.server)
+        server = compute_client.find_server(
+            parsed_args.server, ignore_missing=False)
 
         if self.app.client_manager.is_network_endpoint_enabled():
             network_client = self.app.client_manager.network
@@ -447,13 +447,12 @@ class AddNetwork(command.Command):
             net_id = parsed_args.network
 
         kwargs = {
-            'port_id': None,
             'net_id': net_id,
             'fixed_ip': None,
         }
 
         if parsed_args.tag:
-            if compute_client.api_version < api_versions.APIVersion('2.49'):
+            if not sdk_utils.supports_microversion(compute_client, '2.49'):
                 msg = _(
                     '--os-compute-api-version 2.49 or greater is required to '
                     'support the --tag option'
@@ -462,7 +461,7 @@ class AddNetwork(command.Command):
 
             kwargs['tag'] = parsed_args.tag
 
-        server.interface_attach(**kwargs)
+        compute_client.create_server_interface(server, **kwargs)
 
 
 class AddServerSecurityGroup(command.Command):
diff --git a/openstackclient/tests/functional/compute/v2/test_server.py b/openstackclient/tests/functional/compute/v2/test_server.py
index 9cf2fc7f..59b1fad5 100644
--- a/openstackclient/tests/functional/compute/v2/test_server.py
+++ b/openstackclient/tests/functional/compute/v2/test_server.py
@@ -1071,3 +1071,51 @@ class ServerTests(common.ComputeTestCase):
             # networks and the test didn't specify a specific network.
             self.assertNotIn('nics are required after microversion 2.36',
                              e.stderr)
+
+    def test_server_add_remove_network_port(self):
+        name = uuid.uuid4().hex
+        cmd_output = json.loads(self.openstack(
+            'server create -f json ' +
+            '--network private ' +
+            '--flavor ' + self.flavor_name + ' ' +
+            '--image ' + self.image_name + ' ' +
+            '--wait ' +
+            name
+        ))
+
+        self.assertIsNotNone(cmd_output['id'])
+        self.assertEqual(name, cmd_output['name'])
+
+        self.openstack(
+            'server add network ' + name + ' public')
+
+        cmd_output = json.loads(self.openstack(
+            'server show -f json ' + name
+        ))
+
+        addresses = cmd_output['addresses']
+        self.assertIn('public', addresses)
+
+        port_name = 'test-port'
+
+        cmd_output = json.loads(self.openstack(
+            'port list -f json'
+        ))
+        self.assertNotIn(port_name, cmd_output)
+
+        cmd_output = json.loads(self.openstack(
+            'port create -f json ' +
+            '--network private ' + port_name
+        ))
+        self.assertIsNotNone(cmd_output['id'])
+
+        self.openstack('server add port ' + name + ' ' + port_name)
+
+        cmd_output = json.loads(self.openstack(
+            'server show -f json ' + name
+        ))
+
+        # TODO(diwei): test remove network/port after the commands are switched
+
+        self.openstack('server delete ' + name)
+        self.openstack('port delete ' + port_name)
diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py
index 3d8c17fd..705a96b7 100644
--- a/openstackclient/tests/unit/compute/v2/test_server.py
+++ b/openstackclient/tests/unit/compute/v2/test_server.py
@@ -24,6 +24,7 @@ from unittest.mock import call
 import iso8601
 from novaclient import api_versions
 from openstack import exceptions as sdk_exceptions
+from openstack import utils as sdk_utils
 from osc_lib.cli import format_columns
 from osc_lib import exceptions
 from osc_lib import utils as common_utils
@@ -69,6 +70,10 @@ class TestServer(compute_fakes.TestComputev2):
         self.servers_mock = self.app.client_manager.compute.servers
         self.servers_mock.reset_mock()
 
+        self.app.client_manager.sdk_connection = mock.Mock()
+        self.app.client_manager.sdk_connection.compute = mock.Mock()
+        self.sdk_client = self.app.client_manager.sdk_connection.compute
+
         # Get a shortcut to the compute client ServerMigrationsManager Mock
         self.server_migrations_mock = \
             self.app.client_manager.compute.server_migrations
@@ -133,6 +138,21 @@ class TestServer(compute_fakes.TestComputev2):
                                                                      0)
         return servers
 
+    def setup_sdk_servers_mock(self, count):
+        servers = compute_fakes.FakeServer.create_sdk_servers(
+            attrs=self.attrs,
+            methods=self.methods,
+            count=count,
+        )
+
+        # This is the return value for compute_client.find_server()
+        self.sdk_client.find_server = compute_fakes.FakeServer.get_servers(
+            servers,
+            0,
+        )
+
+        return servers
+
     def run_method_with_servers(self, method_name, server_count):
         servers = self.setup_servers_mock(server_count)
 
@@ -570,7 +590,7 @@ class TestServerAddPort(TestServer):
         self.app.client_manager.network.find_port = self.find_port
 
     def _test_server_add_port(self, port_id):
-        servers = self.setup_servers_mock(count=1)
+        servers = self.setup_sdk_servers_mock(count=1)
         port = 'fake-port'
 
         arglist = [
@@ -585,8 +605,8 @@ class TestServerAddPort(TestServer):
 
         result = self.cmd.take_action(parsed_args)
 
-        servers[0].interface_attach.assert_called_once_with(
-            port_id=port_id, net_id=None, fixed_ip=None)
+        self.sdk_client.create_server_interface.assert_called_once_with(
+            servers[0], port_id=port_id, fixed_ip=None)
         self.assertIsNone(result)
 
     def test_server_add_port(self):
@@ -599,11 +619,12 @@ class TestServerAddPort(TestServer):
         self._test_server_add_port('fake-port')
         self.find_port.assert_not_called()
 
-    def test_server_add_port_with_tag(self):
+    @mock.patch.object(sdk_utils, 'supports_microversion', return_value=True)
+    def test_server_add_port_with_tag(self, sm_mock):
         self.app.client_manager.compute.api_version = api_versions.APIVersion(
             '2.49')
 
-        servers = self.setup_servers_mock(count=1)
+        servers = self.setup_sdk_servers_mock(count=1)
         self.find_port.return_value.id = 'fake-port'
         arglist = [
             servers[0].id,
@@ -620,13 +641,14 @@ class TestServerAddPort(TestServer):
         result = self.cmd.take_action(parsed_args)
         self.assertIsNone(result)
 
-        servers[0].interface_attach.assert_called_once_with(
+        self.sdk_client.create_server_interface.assert_called_once_with(
+            servers[0],
             port_id='fake-port',
-            net_id=None,
             fixed_ip=None,
             tag='tag1')
 
-    def test_server_add_port_with_tag_pre_v249(self):
+    @mock.patch.object(sdk_utils, 'supports_microversion', return_value=False)
+    def test_server_add_port_with_tag_pre_v249(self, sm_mock):
         self.app.client_manager.compute.api_version = api_versions.APIVersion(
             '2.48')
 
@@ -891,7 +913,7 @@ class TestServerAddNetwork(TestServer):
         self.app.client_manager.network.find_network = self.find_network
 
     def _test_server_add_network(self, net_id):
-        servers = self.setup_servers_mock(count=1)
+        servers = self.setup_sdk_servers_mock(count=1)
         network = 'fake-network'
 
         arglist = [
@@ -906,8 +928,8 @@ class TestServerAddNetwork(TestServer):
 
         result = self.cmd.take_action(parsed_args)
 
-        servers[0].interface_attach.assert_called_once_with(
-            port_id=None, net_id=net_id, fixed_ip=None)
+        self.sdk_client.create_server_interface.assert_called_once_with(
+            servers[0], net_id=net_id, fixed_ip=None)
         self.assertIsNone(result)
 
     def test_server_add_network(self):
@@ -920,11 +942,12 @@ class TestServerAddNetwork(TestServer):
         self._test_server_add_network('fake-network')
         self.find_network.assert_not_called()
 
-    def test_server_add_network_with_tag(self):
+    @mock.patch.object(sdk_utils, 'supports_microversion', return_value=True)
+    def test_server_add_network_with_tag(self, sm_mock):
         self.app.client_manager.compute.api_version = api_versions.APIVersion(
             '2.49')
 
-        servers = self.setup_servers_mock(count=1)
+        servers = self.setup_sdk_servers_mock(count=1)
         self.find_network.return_value.id = 'fake-network'
 
         arglist = [
@@ -942,18 +965,19 @@ class TestServerAddNetwork(TestServer):
         result = self.cmd.take_action(parsed_args)
         self.assertIsNone(result)
 
-        servers[0].interface_attach.assert_called_once_with(
-            port_id=None,
+        self.sdk_client.create_server_interface.assert_called_once_with(
+            servers[0],
             net_id='fake-network',
             fixed_ip=None,
             tag='tag1'
         )
 
-    def test_server_add_network_with_tag_pre_v249(self):
+    @mock.patch.object(sdk_utils, 'supports_microversion', return_value=False)
+    def test_server_add_network_with_tag_pre_v249(self, sm_mock):
         self.app.client_manager.compute.api_version = api_versions.APIVersion(
             '2.48')
 
-        servers = self.setup_servers_mock(count=1)
+        servers = self.setup_sdk_servers_mock(count=1)
         self.find_network.return_value.id = 'fake-network'
 
         arglist = [