Merge "Dropping the use of 'addFixedIp' server action"

Zuul · openstack-gerrit · commit 90fe18e51bce · 2023-05-18T11:52:56.000Z
diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py
@@ -345,10 +345,6 @@ def take_action(self, parsed_args):
         else:
             net_id = parsed_args.network
 
-        if not sdk_utils.supports_microversion(compute_client, '2.44'):
-            compute_client.add_fixed_ip_to_server(server.id, net_id)
-            return ((), ())
-
         kwargs = {'net_id': net_id}
         if parsed_args.fixed_ip_address:
             kwargs['fixed_ips'] = [
@@ -375,9 +371,11 @@ def take_action(self, parsed_args):
             'Port State',
             'Fixed IPs',
         )
-        if sdk_utils.supports_microversion(compute_client, '2.49'):
-            columns += ('tag',)
-            column_headers += ('Tag',)
+
+        if parsed_args.tag:
+            if sdk_utils.supports_microversion(compute_client, '2.49'):
+                columns += ('tag',)
+                column_headers += ('Tag',)
 
         return (
             column_headers,
diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py
@@ -206,72 +206,8 @@ def setUp(self):
         self.app.client_manager.network.find_network = self.find_network
 
     @mock.patch.object(sdk_utils, 'supports_microversion')
-    def test_server_add_fixed_ip_pre_v244(self, sm_mock):
-        sm_mock.return_value = False
-
-        servers = self.setup_sdk_servers_mock(count=1)
-        network = compute_fakes.create_one_network()
-
-        with mock.patch.object(
-            self.app.client_manager,
-            'is_network_endpoint_enabled',
-            return_value=False,
-        ):
-            arglist = [
-                servers[0].id,
-                network['id'],
-            ]
-            verifylist = [
-                ('server', servers[0].id),
-                ('network', network['id']),
-                ('fixed_ip_address', None),
-            ]
-            parsed_args = self.check_parser(self.cmd, arglist, verifylist)
-
-            result = self.cmd.take_action(parsed_args)
-
-            self.sdk_client.add_fixed_ip_to_server.assert_called_once_with(
-                servers[0].id, network['id']
-            )
-            # the legacy API operates asynchronously
-            self.assertEqual(((), ()), result)
-
-    @mock.patch.object(sdk_utils, 'supports_microversion')
-    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)
-        network = compute_fakes.create_one_network()
-
-        with mock.patch.object(
-            self.app.client_manager,
-            'is_network_endpoint_enabled',
-            return_value=False,
-        ):
-            arglist = [
-                servers[0].id,
-                network['id'],
-                '--fixed-ip-address',
-                '5.6.7.8',
-            ]
-            verifylist = [
-                ('server', servers[0].id),
-                ('network', network['id']),
-                ('fixed_ip_address', '5.6.7.8'),
-            ]
-            parsed_args = self.check_parser(self.cmd, arglist, verifylist)
-
-            result = self.cmd.take_action(parsed_args)
-
-            self.sdk_client.add_fixed_ip_to_server.assert_called_once_with(
-                servers[0].id, network['id']
-            )
-            # the legacy API operates asynchronously
-            self.assertEqual(((), ()), result)
-
-    @mock.patch.object(sdk_utils, 'supports_microversion')
-    def test_server_add_fixed_ip_pre_v244_with_tag(self, sm_mock):
-        sm_mock.return_value = False
+    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)
         network = compute_fakes.create_one_network()
@@ -305,43 +241,54 @@ def test_server_add_fixed_ip_pre_v244_with_tag(self, sm_mock):
             )
 
     @mock.patch.object(sdk_utils, 'supports_microversion')
-    def test_server_add_fixed_ip_pre_v249_with_tag(self, sm_mock):
-        sm_mock.side_effect = [False, 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.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,
             'is_network_endpoint_enabled',
             return_value=False,
         ):
-            arglist = [
-                servers[0].id,
-                network['id'],
-                '--fixed-ip-address',
-                '5.6.7.8',
-                '--tag',
-                'tag1',
-            ]
+            arglist = [servers[0].id, network['id']]
             verifylist = [
                 ('server', servers[0].id),
                 ('network', network['id']),
-                ('fixed_ip_address', '5.6.7.8'),
-                ('tag', 'tag1'),
             ]
             parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
-            ex = self.assertRaises(
-                exceptions.CommandError, self.cmd.take_action, parsed_args
+            expected_columns = (
+                'Port ID',
+                'Server ID',
+                'Network ID',
+                'MAC Address',
+                'Port State',
+                'Fixed IPs',
             )
-            self.assertIn(
-                '--os-compute-api-version 2.49 or greater is required', str(ex)
+            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']
             )
 
     @mock.patch.object(sdk_utils, 'supports_microversion')
-    def test_server_add_fixed_ip(self, sm_mock):
-        sm_mock.side_effect = [True, False]
+    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.create_one_network()
@@ -353,10 +300,16 @@ def test_server_add_fixed_ip(self, sm_mock):
             'is_network_endpoint_enabled',
             return_value=False,
         ):
-            arglist = [servers[0].id, network['id']]
+            arglist = [
+                servers[0].id,
+                network['id'],
+                '--fixed-ip-address',
+                '5.6.7.8',
+            ]
             verifylist = [
                 ('server', servers[0].id),
                 ('network', network['id']),
+                ('fixed_ip_address', '5.6.7.8'),
             ]
             parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
@@ -382,12 +335,14 @@ def test_server_add_fixed_ip(self, sm_mock):
             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']
+                servers[0].id,
+                net_id=network['id'],
+                fixed_ips=[{'ip_address': '5.6.7.8'}],
             )
 
     @mock.patch.object(sdk_utils, 'supports_microversion')
-    def test_server_add_fixed_ip_with_fixed_ip(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.create_one_network()
@@ -404,11 +359,14 @@ def test_server_add_fixed_ip_with_fixed_ip(self, sm_mock):
                 network['id'],
                 '--fixed-ip-address',
                 '5.6.7.8',
+                '--tag',
+                'tag1',
             ]
             verifylist = [
                 ('server', servers[0].id),
                 ('network', network['id']),
                 ('fixed_ip_address', '5.6.7.8'),
+                ('tag', 'tag1'),
             ]
             parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
@@ -439,11 +397,12 @@ def test_server_add_fixed_ip_with_fixed_ip(self, sm_mock):
                 servers[0].id,
                 net_id=network['id'],
                 fixed_ips=[{'ip_address': '5.6.7.8'}],
+                tag='tag1',
             )
 
     @mock.patch.object(sdk_utils, 'supports_microversion')
-    def test_server_add_fixed_ip_with_tag(self, sm_mock):
-        sm_mock.side_effect = [True, True, True]
+    def test_server_add_fixed_ip_with_fixed_ip_with_tag(self, sm_mock):
+        sm_mock.side_effect = [True, True]
 
         servers = self.setup_sdk_servers_mock(count=1)
         network = compute_fakes.create_one_network()
