From 0cde82dcd86205f9e190b1a4f02136e1d83797b9 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Tue, 14 Dec 2021 15:14:49 +0000 Subject: [PATCH] compute: Return information about fixed IP The compute API provides this information to us. We might as well use it. Change-Id: I5608fa80745975ce49712718452cfe296c0f64d2 Signed-off-by: Stephen Finucane --- openstackclient/compute/v2/server.py | 33 ++++- .../tests/unit/compute/v2/fakes.py | 28 +++++ .../tests/unit/compute/v2/test_server.py | 117 +++++++++++++----- 3 files changed, 145 insertions(+), 33 deletions(-) diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index 0931a6ca01..2c1e42cc79 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -204,7 +204,7 @@ def boolenv(*vars, default=False): return default -class AddFixedIP(command.Command): +class AddFixedIP(command.ShowOne): _description = _("Add fixed IP address to server") def get_parser(self, prog_name): @@ -231,7 +231,7 @@ class AddFixedIP(command.Command): metavar='', help=_( 'Tag for the attached interface. ' - '(supported by --os-compute-api-version 2.52 or above)' + '(supported by --os-compute-api-version 2.49 or above)' ) ) return parser @@ -265,16 +265,39 @@ class AddFixedIP(command.Command): server.id, net_id ) - return + return ((), ()) kwargs = { 'net_id': net_id, 'fixed_ip': parsed_args.fixed_ip_address, } - if parsed_args.tag: kwargs['tag'] = parsed_args.tag - compute_client.create_server_interface(server.id, **kwargs) + + interface = compute_client.create_server_interface(server.id, **kwargs) + + columns = ( + 'port_id', 'server_id', 'net_id', 'mac_addr', 'port_state', + 'fixed_ips', + ) + column_headers = ( + 'Port ID', 'Server ID', 'Network ID', 'MAC Address', 'Port State', + 'Fixed IPs', + ) + if sdk_utils.supports_microversion(compute_client, '2.49'): + columns += ('tag',) + column_headers += ('Tag',) + + return ( + column_headers, + utils.get_item_properties( + interface, + columns, + formatters={ + 'fixed_ips': format_columns.ListDictColumn, + }, + ), + ) class AddFloatingIP(network_common.NetworkAndComputeCommand): diff --git a/openstackclient/tests/unit/compute/v2/fakes.py b/openstackclient/tests/unit/compute/v2/fakes.py index 7618c229c5..2a53164735 100644 --- a/openstackclient/tests/unit/compute/v2/fakes.py +++ b/openstackclient/tests/unit/compute/v2/fakes.py @@ -21,6 +21,7 @@ import uuid from novaclient import api_versions from openstack.compute.v2 import flavor as _flavor from openstack.compute.v2 import server +from openstack.compute.v2 import server_interface as _server_interface from openstack.compute.v2 import volume_attachment from openstackclient.api import compute_v2 @@ -1859,3 +1860,30 @@ class FakeVolumeAttachment(object): attrs, methods)) return volume_attachments + + +def create_one_server_interface(attrs=None): + """Create a fake SDK ServerInterface. + + :param dict attrs: A dictionary with all attributes + :param dict methods: A dictionary with all methods + :return: A fake ServerInterface object with various attributes set + """ + attrs = attrs or {} + + # Set default attributes. + server_interface_info = { + "fixed_ips": uuid.uuid4().hex, + "mac_addr": "aa:aa:aa:aa:aa:aa", + "net_id": uuid.uuid4().hex, + "port_id": uuid.uuid4().hex, + "port_state": "ACTIVE", + "server_id": uuid.uuid4().hex, + # introduced in API microversion 2.70 + "tag": "foo", + } + + # Overwrite default attributes. + server_interface_info.update(attrs) + + return _server_interface.ServerInterface(**server_interface_info) diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py index 5d47b68784..6553f90465 100644 --- a/openstackclient/tests/unit/compute/v2/test_server.py +++ b/openstackclient/tests/unit/compute/v2/test_server.py @@ -212,7 +212,7 @@ class TestServer(compute_fakes.TestComputev2): class TestServerAddFixedIP(TestServer): def setUp(self): - super(TestServerAddFixedIP, self).setUp() + super().setUp() # Get the command object to test self.cmd = server.AddFixedIP(self.app, None) @@ -221,13 +221,8 @@ class TestServerAddFixedIP(TestServer): self.find_network = mock.Mock() self.app.client_manager.network.find_network = self.find_network - # Set add_fixed_ip method to be tested. - self.methods = { - 'interface_attach': None, - } - @mock.patch.object(sdk_utils, 'supports_microversion') - def test_server_add_fixed_ip_pre_2_44(self, sm_mock): + def test_server_add_fixed_ip_pre_v244(self, sm_mock): sm_mock.return_value = False servers = self.setup_sdk_servers_mock(count=1) @@ -255,10 +250,11 @@ class TestServerAddFixedIP(TestServer): servers[0].id, network['id'] ) - self.assertIsNone(result) + # the legacy API operates asynchronously + self.assertEqual(((), ()), result) @mock.patch.object(sdk_utils, 'supports_microversion') - def test_server_add_fixed_ip_pre_2_44_with_fixed_ip(self, sm_mock): + def test_server_add_fixed_ip_pre_v244_with_fixed_ip(self, sm_mock): sm_mock.return_value = False servers = self.setup_sdk_servers_mock(count=1) @@ -287,10 +283,11 @@ class TestServerAddFixedIP(TestServer): servers[0].id, network['id'] ) - self.assertIsNone(result) + # the legacy API operates asynchronously + self.assertEqual(((), ()), result) @mock.patch.object(sdk_utils, 'supports_microversion') - def test_server_add_fixed_ip_pre_2_44_with_tag(self, sm_mock): + def test_server_add_fixed_ip_pre_v244_with_tag(self, sm_mock): sm_mock.return_value = False servers = self.setup_sdk_servers_mock(count=1) @@ -324,7 +321,7 @@ class TestServerAddFixedIP(TestServer): str(ex)) @mock.patch.object(sdk_utils, 'supports_microversion') - def test_server_add_fixed_ip_pre_2_49_with_tag(self, sm_mock): + def test_server_add_fixed_ip_pre_v249_with_tag(self, sm_mock): sm_mock.side_effect = [False, True] servers = self.setup_sdk_servers_mock(count=1) @@ -358,11 +355,13 @@ class TestServerAddFixedIP(TestServer): str(ex)) @mock.patch.object(sdk_utils, 'supports_microversion') - def test_server_add_fixed_ip_post_2_49(self, sm_mock): - sm_mock.side_effect = [True, True] + def test_server_add_fixed_ip(self, sm_mock): + sm_mock.side_effect = [True, False] servers = self.setup_sdk_servers_mock(count=1) network = compute_fakes.FakeNetwork.create_one_network() + interface = compute_fakes.create_one_server_interface() + self.sdk_client.create_server_interface.return_value = interface with mock.patch.object( self.app.client_manager, @@ -379,21 +378,41 @@ class TestServerAddFixedIP(TestServer): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - result = self.cmd.take_action(parsed_args) + expected_columns = ( + 'Port ID', + 'Server ID', + 'Network ID', + 'MAC Address', + 'Port State', + 'Fixed IPs', + ) + expected_data = ( + interface.port_id, + interface.server_id, + interface.net_id, + interface.mac_addr, + interface.port_state, + format_columns.ListDictColumn(interface.fixed_ips), + ) + columns, data = self.cmd.take_action(parsed_args) + + self.assertEqual(expected_columns, columns) + self.assertEqual(expected_data, tuple(data)) self.sdk_client.create_server_interface.assert_called_once_with( servers[0].id, net_id=network['id'], fixed_ip=None ) - self.assertIsNone(result) @mock.patch.object(sdk_utils, 'supports_microversion') - def test_server_add_fixed_ip_post_2_49_with_fixedip(self, sm_mock): + def test_server_add_fixed_ip_with_fixed_ip(self, sm_mock): sm_mock.side_effect = [True, True] servers = self.setup_sdk_servers_mock(count=1) network = compute_fakes.FakeNetwork.create_one_network() + interface = compute_fakes.create_one_server_interface() + self.sdk_client.create_server_interface.return_value = interface with mock.patch.object( self.app.client_manager, @@ -412,21 +431,43 @@ class TestServerAddFixedIP(TestServer): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - result = self.cmd.take_action(parsed_args) + expected_columns = ( + 'Port ID', + 'Server ID', + 'Network ID', + 'MAC Address', + 'Port State', + 'Fixed IPs', + 'Tag', + ) + expected_data = ( + interface.port_id, + interface.server_id, + interface.net_id, + interface.mac_addr, + interface.port_state, + format_columns.ListDictColumn(interface.fixed_ips), + interface.tag, + ) + columns, data = self.cmd.take_action(parsed_args) + + self.assertEqual(expected_columns, columns) + self.assertEqual(expected_data, tuple(data)) self.sdk_client.create_server_interface.assert_called_once_with( servers[0].id, net_id=network['id'], fixed_ip='5.6.7.8' ) - self.assertIsNone(result) @mock.patch.object(sdk_utils, 'supports_microversion') - def test_server_add_fixed_ip_post_2_49_with_tag(self, sm_mock): - sm_mock.side_effect = [True, True] + def test_server_add_fixed_ip_with_tag(self, sm_mock): + sm_mock.side_effect = [True, True, True] servers = self.setup_sdk_servers_mock(count=1) network = compute_fakes.FakeNetwork.create_one_network() + interface = compute_fakes.create_one_server_interface() + self.sdk_client.create_server_interface.return_value = interface with mock.patch.object( self.app.client_manager, @@ -447,15 +488,35 @@ class TestServerAddFixedIP(TestServer): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - result = self.cmd.take_action(parsed_args) + expected_columns = ( + 'Port ID', + 'Server ID', + 'Network ID', + 'MAC Address', + 'Port State', + 'Fixed IPs', + 'Tag', + ) + expected_data = ( + interface.port_id, + interface.server_id, + interface.net_id, + interface.mac_addr, + interface.port_state, + format_columns.ListDictColumn(interface.fixed_ips), + interface.tag, + ) + columns, data = self.cmd.take_action(parsed_args) + + self.assertEqual(expected_columns, columns) + self.assertEqual(expected_data, tuple(data)) self.sdk_client.create_server_interface.assert_called_once_with( servers[0].id, net_id=network['id'], fixed_ip='5.6.7.8', - tag='tag1' + tag='tag1', ) - self.assertIsNone(result) @mock.patch( @@ -5265,7 +5326,7 @@ class TestServerMigrate(TestServer): self.assertNotCalled(self.servers_mock.live_migrate) self.assertNotCalled(self.servers_mock.migrate) - def test_server_migrate_with_host_pre_2_56(self): + def test_server_migrate_with_host_pre_v256(self): # Tests that --host is not allowed for a cold migration # before microversion 2.56 (the test defaults to 2.1). arglist = [ @@ -6682,7 +6743,7 @@ class TestServerRebuild(TestServer): self.image, None, userdata=mock_file,) - def test_rebuild_with_user_data_pre_257(self): + def test_rebuild_with_user_data_pre_v257(self): self.app.client_manager.compute.api_version = \ api_versions.APIVersion('2.56') @@ -6722,7 +6783,7 @@ class TestServerRebuild(TestServer): self.server.rebuild.assert_called_with( self.image, None, userdata=None) - def test_rebuild_with_no_user_data_pre_254(self): + def test_rebuild_with_no_user_data_pre_v254(self): self.app.client_manager.compute.api_version = \ api_versions.APIVersion('2.53') @@ -6814,7 +6875,7 @@ class TestServerRebuild(TestServer): self.server.rebuild.assert_called_with( self.image, None, trusted_image_certificates=None) - def test_rebuild_with_no_trusted_image_cert_pre_257(self): + def test_rebuild_with_no_trusted_image_cert_pre_v263(self): self.app.client_manager.compute.api_version = \ api_versions.APIVersion('2.62')