Remove deprecated network options

The following were deprecated for several releases and can now be removed: * Remove ``port create|set`` options ``--device-id`` and ``--port-id`` * Remove ``router set`` option ``--clear-routes`` * Remove ``security group rule create`` options ``--src-group`` and ``--src-ip`` These are backwards incompatible changes and will require a major version bump after they are merged. Change-Id: Ieae74c14f6b3e263721a3146cf76f94a9ab792f6 Signed-off-by: Sean McGinnis <sean.mcginnis@gmail.com> Signed-off-by: Dean Troyer <dtroyer@gmail.com>
2018-10-26 11:50:32 -05:00 · 2018-10-26 11:50:32 -05:00 · 5a0fc68a87
commit 5a0fc68a87
parent 67dadda746
8 changed files with 56 additions and 252 deletions
--- a/doc/source/cli/backwards-incompatible.rst
+++ b/doc/source/cli/backwards-incompatible.rst
@ -55,6 +55,24 @@ Release 4.0
  * Removed in: 4.0
  * Commit: https://review.opendev.org/659431

+7. Remove ``port create|set`` options ``--device-id`` and ``--host-id``.
+   Use ``--device`` and ``--host`` instead.
+
+  * Removed in: 4.0
+  * Commit: https://review.opendev.org/613644
+
+8. Remove ``router set`` option ``--clear-routes``.
+   Use ``no-route`` option instead.
+
+  * Removed in: 4.0
+  * Commit: https://review.opendev.org/613644
+
+9. Remove ``security group rule create`` options ``--src-ip`` and ``--src-group``.
+   Use ``--remote-ip`` and ``--remote-group`` options instead.
+
+  * Removed in: 4.0
+  * Commit: https://review.opendev.org/613644
+
 .. 1. Change ``volume transfer request accept`` to use new option ``--auth-key``
 ..    rather than a second positional argument.

--- a/openstackclient/network/v2/port.py
+++ b/openstackclient/network/v2/port.py
@ -96,21 +96,6 @@ class JSONKeyValueAction(argparse.Action):
 def _get_attrs(client_manager, parsed_args):
    attrs = {}

-    # Handle deprecated options
-    # NOTE(dtroyer): --device-id and --host-id were deprecated in Mar 2016.
-    #                Do not remove before 3.x release or Mar 2017.
-    if parsed_args.device_id:
-        attrs['device_id'] = parsed_args.device_id
-        LOG.warning(_(
-            'The --device-id option is deprecated, '
-            'please use --device instead.'
-        ))
-    if parsed_args.host_id:
-        attrs['binding:host_id'] = parsed_args.host_id
-        LOG.warning(_(
-            'The --host-id option is deprecated, '
-            'please use --host instead.'
-        ))
    if parsed_args.description is not None:
        attrs['description'] = parsed_args.description
    if parsed_args.device:
@ -235,19 +220,11 @@ def _add_updatable_args(parser):
        metavar='<description>',
        help=_("Description of this port")
    )
-    # NOTE(dtroyer): --device-id is deprecated in Mar 2016.  Do not
-    #                remove before 3.x release or Mar 2017.
-    device_group = parser.add_mutually_exclusive_group()
-    device_group.add_argument(
+    parser.add_argument(
        '--device',
        metavar='<device-id>',
        help=_("Port device ID")
    )
-    device_group.add_argument(
-        '--device-id',
-        metavar='<device-id>',
-        help=argparse.SUPPRESS,
-    )
    parser.add_argument(
        '--mac-address',
        metavar='<mac-address>',
@ -268,19 +245,11 @@ def _add_updatable_args(parser):
               "macvtap | normal | baremetal | virtio-forwarder, "
               "default: normal)")
    )
-    # NOTE(dtroyer): --host-id is deprecated in Mar 2016.  Do not
-    #                remove before 3.x release or Mar 2017.
-    host_group = parser.add_mutually_exclusive_group()
-    host_group.add_argument(
+    parser.add_argument(
        '--host',
        metavar='<host-id>',
        help=_("Allocate port on host <host-id> (ID only)")
    )
-    host_group.add_argument(
-        '--host-id',
-        metavar='<host-id>',
-        help=argparse.SUPPRESS,
-    )
    parser.add_argument(
        '--dns-domain',
        metavar='dns-domain',
--- a/openstackclient/network/v2/router.py
+++ b/openstackclient/network/v2/router.py
@ -13,7 +13,6 @@

 """Router action implementations"""

-import argparse
 import copy
 import json
 import logging
@ -528,8 +527,6 @@ class SetRouter(command.Command):
            action='store_true',
            help=_("Set router to centralized mode (disabled router only)")
        )
-        routes_group = parser.add_mutually_exclusive_group()
-        # ToDo(Reedip):Remove mutual exclusiveness once clear-routes is removed
        parser.add_argument(
            '--route',
            metavar='destination=<subnet>,gateway=<ip-address>',
@ -542,18 +539,13 @@ class SetRouter(command.Command):
                   "gateway: nexthop IP address "
                   "(repeat option to set multiple routes)")
        )
-        routes_group.add_argument(
+        parser.add_argument(
            '--no-route',
            action='store_true',
            help=_("Clear routes associated with the router. "
                   "Specify both --route and --no-route to overwrite "
                   "current value of route.")
        )
-        routes_group.add_argument(
-            '--clear-routes',
-            action='store_true',
-            help=argparse.SUPPRESS,
-        )
        routes_ha = parser.add_mutually_exclusive_group()
        routes_ha.add_argument(
            '--ha',
@ -619,21 +611,16 @@ class SetRouter(command.Command):
            attrs['ha'] = True
        elif parsed_args.no_ha:
            attrs['ha'] = False
-        if parsed_args.clear_routes:
-            LOG.warning(_(
-                'The --clear-routes option is deprecated, '
-                'please use --no-route instead.'
-            ))

        if parsed_args.routes is not None:
            for route in parsed_args.routes:
                route['nexthop'] = route.pop('gateway')
            attrs['routes'] = parsed_args.routes
-            if not (parsed_args.no_route or parsed_args.clear_routes):
+            if not parsed_args.no_route:
                # Map the route keys and append to the current routes.
                # The REST API will handle route validation and duplicates.
                attrs['routes'] += obj.routes
-        elif parsed_args.no_route or parsed_args.clear_routes:
+        elif parsed_args.no_route:
            attrs['routes'] = []
        if (parsed_args.disable_snat or parsed_args.enable_snat or
                parsed_args.fixed_ip) and not parsed_args.external_gateway:
--- a/openstackclient/network/v2/security_group_rule.py
+++ b/openstackclient/network/v2/security_group_rule.py
@ -115,19 +115,6 @@ class CreateSecurityGroupRule(common.NetworkAndComputeShowOne):
            metavar="<group>",
            help=_("Remote security group (name or ID)"),
        )
-        # Handle deprecated options
-        # NOTE(dtroyer): --src-ip and --src-group were deprecated in Nov 2016.
-        #                Do not remove before 4.x release or Nov 2017.
-        remote_group.add_argument(
-            "--src-ip",
-            metavar="<ip-address>",
-            help=argparse.SUPPRESS,
-        )
-        remote_group.add_argument(
-            "--src-group",
-            metavar="<group>",
-            help=argparse.SUPPRESS,
-        )
        return parser

    def update_parser_network(self, parser):
@ -310,31 +297,13 @@ class CreateSecurityGroupRule(common.NetworkAndComputeShowOne):
        if parsed_args.icmp_code is not None and parsed_args.icmp_code >= 0:
            attrs['port_range_max'] = parsed_args.icmp_code

-        # NOTE(dtroyer): --src-ip and --src-group were deprecated in Nov 2016.
-        #                Do not remove before 4.x release or Nov 2017.
-        if not (parsed_args.remote_group is None and
-                parsed_args.src_group is None):
+        if parsed_args.remote_group is not None:
            attrs['remote_group_id'] = client.find_security_group(
-                parsed_args.remote_group or parsed_args.src_group,
+                parsed_args.remote_group,
                ignore_missing=False
            ).id
-            if parsed_args.src_group:
-                LOG.warning(
-                    _("The %(old)s option is deprecated, "
-                      "please use %(new)s instead."),
-                    {'old': '--src-group', 'new': '--remote-group'},
-                )
-        elif not (parsed_args.remote_ip is None and
-                  parsed_args.src_ip is None):
-            attrs['remote_ip_prefix'] = (
-                parsed_args.remote_ip or parsed_args.src_ip
-            )
-            if parsed_args.src_ip:
-                LOG.warning(
-                    _("The %(old)s option is deprecated, "
-                      "please use %(new)s instead."),
-                    {'old': '--src-ip', 'new': '--remote-ip'},
-                )
+        elif parsed_args.remote_ip is not None:
+            attrs['remote_ip_prefix'] = parsed_args.remote_ip
        elif attrs['ethertype'] == 'IPv4':
            attrs['remote_ip_prefix'] = '0.0.0.0/0'
        attrs['security_group_id'] = security_group_id
@ -361,29 +330,13 @@ class CreateSecurityGroupRule(common.NetworkAndComputeShowOne):
        else:
            from_port, to_port = parsed_args.dst_port

-        # NOTE(dtroyer): --src-ip and --src-group were deprecated in Nov 2016.
-        #                Do not remove before 4.x release or Nov 2017.
        remote_ip = None
-        if not (parsed_args.remote_group is None and
-                parsed_args.src_group is None):
+        if parsed_args.remote_group is not None:
            parsed_args.remote_group = client.api.security_group_find(
-                parsed_args.remote_group or parsed_args.src_group,
+                parsed_args.remote_group,
            )['id']
-            if parsed_args.src_group:
-                LOG.warning(
-                    _("The %(old)s option is deprecated, "
-                      "please use %(new)s instead."),
-                    {'old': '--src-group', 'new': '--remote-group'},
-                )
-        if not (parsed_args.remote_ip is None and
-                parsed_args.src_ip is None):
-            remote_ip = parsed_args.remote_ip or parsed_args.src_ip
-            if parsed_args.src_ip:
-                LOG.warning(
-                    _("The %(old)s option is deprecated, "
-                      "please use %(new)s instead."),
-                    {'old': '--src-ip', 'new': '--remote-ip'},
-                )
+        if parsed_args.remote_ip is not None:
+            remote_ip = parsed_args.remote_ip
        else:
            remote_ip = '0.0.0.0/0'

--- a/openstackclient/tests/unit/network/v2/test_router.py
+++ b/openstackclient/tests/unit/network/v2/test_router.py
@ -939,52 +939,6 @@ class TestSetRouter(TestRouter):
            _testrouter, **attrs)
        self.assertIsNone(result)

-    def test_set_clear_routes(self):
-        arglist = [
-            self._router.name,
-            '--clear-routes',
-        ]
-        verifylist = [
-            ('router', self._router.name),
-            ('clear_routes', True),
-        ]
-        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
-
-        result = self.cmd.take_action(parsed_args)
-
-        attrs = {
-            'routes': [],
-        }
-        self.network.update_router.assert_called_once_with(
-            self._router, **attrs)
-        self.assertIsNone(result)
-
-    def test_overwrite_route_clear_routes(self):
-        _testrouter = network_fakes.FakeRouter.create_one_router(
-            {'routes': [{"destination": "10.0.0.2",
-                         "nexthop": "1.1.1.1"}]})
-        self.network.find_router = mock.Mock(return_value=_testrouter)
-        arglist = [
-            _testrouter.name,
-            '--route', 'destination=10.20.30.0/24,gateway=10.20.30.1',
-            '--clear-routes',
-        ]
-        verifylist = [
-            ('router', _testrouter.name),
-            ('routes', [{'destination': '10.20.30.0/24',
-                         'gateway': '10.20.30.1'}]),
-            ('clear_routes', True),
-        ]
-        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
-        result = self.cmd.take_action(parsed_args)
-        attrs = {
-            'routes': [{'destination': '10.20.30.0/24',
-                        'nexthop': '10.20.30.1'}]
-        }
-        self.network.update_router.assert_called_once_with(
-            _testrouter, **attrs)
-        self.assertIsNone(result)
-
    def test_set_nothing(self):
        arglist = [
            self._router.name,
--- a/openstackclient/tests/unit/network/v2/test_security_group_rule_compute.py
+++ b/openstackclient/tests/unit/network/v2/test_security_group_rule_compute.py
@ -72,15 +72,6 @@ class TestCreateSecurityGroupRuleCompute(TestSecurityGroupRuleCompute):
        self.assertRaises(tests_utils.ParserException,
                          self.check_parser, self.cmd, [], [])

-    def test_security_group_rule_create_all_source_options(self, sgr_mock):
-        arglist = [
-            '--src-ip', '10.10.0.0/24',
-            '--src-group', self._security_group['id'],
-            self._security_group['id'],
-        ]
-        self.assertRaises(tests_utils.ParserException,
-                          self.check_parser, self.cmd, arglist, [])
-
    def test_security_group_rule_create_all_remote_options(self, sgr_mock):
        arglist = [
            '--remote-ip', '10.10.0.0/24',
@ -151,41 +142,6 @@ class TestCreateSecurityGroupRuleCompute(TestSecurityGroupRuleCompute):
        self.assertEqual(expected_columns, columns)
        self.assertEqual(expected_data, data)

-    def test_security_group_rule_create_source_group(self, sgr_mock):
-        expected_columns, expected_data = self._setup_security_group_rule({
-            'from_port': 22,
-            'to_port': 22,
-            'group': {'name': self._security_group['name']},
-        })
-        sgr_mock.return_value = self._security_group_rule
-        arglist = [
-            '--dst-port', str(self._security_group_rule['from_port']),
-            '--src-group', self._security_group['name'],
-            self._security_group['id'],
-        ]
-        verifylist = [
-            ('dst_port', (self._security_group_rule['from_port'],
-                          self._security_group_rule['to_port'])),
-            ('src_group', self._security_group['name']),
-            ('group', self._security_group['id']),
-        ]
-        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
-
-        columns, data = self.cmd.take_action(parsed_args)
-
-        # TODO(dtroyer): save this for the security group rule changes
-        # self.compute.api.security_group_rule_create.assert_called_once_with(
-        sgr_mock.assert_called_once_with(
-            security_group_id=self._security_group['id'],
-            ip_protocol=self._security_group_rule['ip_protocol'],
-            from_port=self._security_group_rule['from_port'],
-            to_port=self._security_group_rule['to_port'],
-            remote_ip=self._security_group_rule['ip_range']['cidr'],
-            remote_group=self._security_group['id'],
-        )
-        self.assertEqual(expected_columns, columns)
-        self.assertEqual(expected_data, data)
-
    def test_security_group_rule_create_remote_group(self, sgr_mock):
        expected_columns, expected_data = self._setup_security_group_rule({
            'from_port': 22,
@ -221,41 +177,6 @@ class TestCreateSecurityGroupRuleCompute(TestSecurityGroupRuleCompute):
        self.assertEqual(expected_columns, columns)
        self.assertEqual(expected_data, data)

-    def test_security_group_rule_create_source_ip(self, sgr_mock):
-        expected_columns, expected_data = self._setup_security_group_rule({
-            'ip_protocol': 'icmp',
-            'from_port': -1,
-            'to_port': -1,
-            'ip_range': {'cidr': '10.0.2.0/24'},
-        })
-        sgr_mock.return_value = self._security_group_rule
-        arglist = [
-            '--protocol', self._security_group_rule['ip_protocol'],
-            '--src-ip', self._security_group_rule['ip_range']['cidr'],
-            self._security_group['id'],
-        ]
-        verifylist = [
-            ('protocol', self._security_group_rule['ip_protocol']),
-            ('src_ip', self._security_group_rule['ip_range']['cidr']),
-            ('group', self._security_group['id']),
-        ]
-        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
-
-        columns, data = self.cmd.take_action(parsed_args)
-
-        # TODO(dtroyer): save this for the security group rule changes
-        # self.compute.api.security_group_rule_create.assert_called_once_with(
-        sgr_mock.assert_called_once_with(
-            security_group_id=self._security_group['id'],
-            ip_protocol=self._security_group_rule['ip_protocol'],
-            from_port=self._security_group_rule['from_port'],
-            to_port=self._security_group_rule['to_port'],
-            remote_ip=self._security_group_rule['ip_range']['cidr'],
-            remote_group=None,
-        )
-        self.assertEqual(expected_columns, columns)
-        self.assertEqual(expected_data, data)
-
    def test_security_group_rule_create_remote_ip(self, sgr_mock):
        expected_columns, expected_data = self._setup_security_group_rule({
            'ip_protocol': 'icmp',
@ -301,13 +222,13 @@ class TestCreateSecurityGroupRuleCompute(TestSecurityGroupRuleCompute):
        sgr_mock.return_value = self._security_group_rule
        arglist = [
            '--proto', self._security_group_rule['ip_protocol'],
-            '--src-ip', self._security_group_rule['ip_range']['cidr'],
+            '--remote-ip', self._security_group_rule['ip_range']['cidr'],
            self._security_group['id'],
        ]
        verifylist = [
            ('proto', self._security_group_rule['ip_protocol']),
            ('protocol', None),
-            ('src_ip', self._security_group_rule['ip_range']['cidr']),
+            ('remote_ip', self._security_group_rule['ip_range']['cidr']),
            ('group', self._security_group['id']),
        ]
        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
--- a/openstackclient/tests/unit/network/v2/test_security_group_rule_network.py
+++ b/openstackclient/tests/unit/network/v2/test_security_group_rule_network.py
@ -99,15 +99,6 @@ class TestCreateSecurityGroupRuleNetwork(TestSecurityGroupRuleNetwork):
        self.assertRaises(tests_utils.ParserException,
                          self.check_parser, self.cmd, [], [])

-    def test_create_all_source_options(self):
-        arglist = [
-            '--src-ip', '10.10.0.0/24',
-            '--src-group', self._security_group.id,
-            self._security_group.id,
-        ]
-        self.assertRaises(tests_utils.ParserException,
-                          self.check_parser, self.cmd, arglist, [])
-
    def test_create_all_remote_options(self):
        arglist = [
            '--remote-ip', '10.10.0.0/24',
@ -212,13 +203,13 @@ class TestCreateSecurityGroupRuleNetwork(TestSecurityGroupRuleNetwork):
        })
        arglist = [
            '--proto', self._security_group_rule.protocol,
-            '--src-ip', self._security_group_rule.remote_ip_prefix,
+            '--remote-ip', self._security_group_rule.remote_ip_prefix,
            self._security_group.id,
        ]
        verifylist = [
            ('proto', self._security_group_rule.protocol),
            ('protocol', None),
-            ('src_ip', self._security_group_rule.remote_ip_prefix),
+            ('remote_ip', self._security_group_rule.remote_ip_prefix),
            ('group', self._security_group.id),
        ]
        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@ -242,13 +233,13 @@ class TestCreateSecurityGroupRuleNetwork(TestSecurityGroupRuleNetwork):
        })
        arglist = [
            '--proto', 'any',
-            '--src-ip', self._security_group_rule.remote_ip_prefix,
+            '--remote-ip', self._security_group_rule.remote_ip_prefix,
            self._security_group.id,
        ]
        verifylist = [
            ('proto', 'any'),
            ('protocol', None),
-            ('src_ip', self._security_group_rule.remote_ip_prefix),
+            ('remote_ip', self._security_group_rule.remote_ip_prefix),
            ('group', self._security_group.id),
        ]
        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@ -269,19 +260,18 @@ class TestCreateSecurityGroupRuleNetwork(TestSecurityGroupRuleNetwork):
        self._setup_security_group_rule({
            'port_range_max': 22,
            'port_range_min': 22,
-            'remote_group_id': self._security_group.id,
        })
        arglist = [
            '--dst-port', str(self._security_group_rule.port_range_min),
            '--ingress',
-            '--src-group', self._security_group.name,
+            '--remote-group', self._security_group.name,
            self._security_group.id,
        ]
        verifylist = [
            ('dst_port', (self._security_group_rule.port_range_min,
                          self._security_group_rule.port_range_max)),
            ('ingress', True),
-            ('src_group', self._security_group.name),
+            ('remote_group', self._security_group.name),
            ('group', self._security_group.id),
        ]
        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@ -294,7 +284,7 @@ class TestCreateSecurityGroupRuleNetwork(TestSecurityGroupRuleNetwork):
            'port_range_max': self._security_group_rule.port_range_max,
            'port_range_min': self._security_group_rule.port_range_min,
            'protocol': self._security_group_rule.protocol,
-            'remote_group_id': self._security_group_rule.remote_group_id,
+            'remote_group_id': self._security_group.id,
            'security_group_id': self._security_group.id,
        })
        self.assertEqual(self.expected_columns, columns)
@ -306,12 +296,12 @@ class TestCreateSecurityGroupRuleNetwork(TestSecurityGroupRuleNetwork):
        })
        arglist = [
            '--ingress',
-            '--src-group', self._security_group.name,
+            '--remote-group', self._security_group.name,
            self._security_group.id,
        ]
        verifylist = [
            ('ingress', True),
-            ('src_group', self._security_group.name),
+            ('remote_group', self._security_group.name),
            ('group', self._security_group.id),
        ]
        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@ -335,12 +325,12 @@ class TestCreateSecurityGroupRuleNetwork(TestSecurityGroupRuleNetwork):
        })
        arglist = [
            '--protocol', self._security_group_rule.protocol,
-            '--src-ip', self._security_group_rule.remote_ip_prefix,
+            '--remote-ip', self._security_group_rule.remote_ip_prefix,
            self._security_group.id,
        ]
        verifylist = [
            ('protocol', self._security_group_rule.protocol),
-            ('src_ip', self._security_group_rule.remote_ip_prefix),
+            ('remote_ip', self._security_group_rule.remote_ip_prefix),
            ('group', self._security_group.id),
        ]
        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
--- a/releasenotes/notes/osc4-network-db2aed696d964ca6.yaml
+++ b/releasenotes/notes/osc4-network-db2aed696d964ca6.yaml
@ -0,0 +1,12 @@
+---
+upgrade:
+  - |
+    Remove deprecated ``port create|set`` options ``--device-id`` and ``--host-id``.
+    Use ``--device`` and ``--host`` options instead.
+  - |
+    Remove deprecated ``router set`` option ``--clear-routes``.
+    Use ``--no-route`` option instead.
+  - |
+    Remove deprecated ``security group rule create`` options ``--src-ip`` and
+    ``--src-group``.
+    Use ``--remote-ip`` and ``--remote-group`` options instead.