From 2183a611475090347863917f6c90f0f38cd80893 Mon Sep 17 00:00:00 2001 From: Diwei Zhu Date: Thu, 28 Oct 2021 02:16:23 +0000 Subject: [PATCH] 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 --- openstackclient/compute/v2/server.py | 23 ++++---- .../functional/compute/v2/test_server.py | 48 +++++++++++++++ .../tests/unit/compute/v2/test_server.py | 58 +++++++++++++------ ...work-add-port-to-sdk-7d81b25f59cfbec9.yaml | 4 ++ 4 files changed, 104 insertions(+), 29 deletions(-) create mode 100644 releasenotes/notes/migrate-server-add-network-add-port-to-sdk-7d81b25f59cfbec9.yaml diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index c11f4b5781..fcd7d69cdc 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 9cf2fc7f0f..59b1fad5f9 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 3d8c17fdeb..705a96b79c 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 = [ diff --git a/releasenotes/notes/migrate-server-add-network-add-port-to-sdk-7d81b25f59cfbec9.yaml b/releasenotes/notes/migrate-server-add-network-add-port-to-sdk-7d81b25f59cfbec9.yaml new file mode 100644 index 0000000000..930d6bc597 --- /dev/null +++ b/releasenotes/notes/migrate-server-add-network-add-port-to-sdk-7d81b25f59cfbec9.yaml @@ -0,0 +1,4 @@ +--- +features: + - | + Migrate server add network/port from novaclient to openstacksdk.