Add extra dhcp option to 'port create/set/unset'

This patch adds '--dhcp-options' and '--no-dhcp-options' to
'port create', 'port set', 'port unset' commands.

Partial-Bug: #1612136
Partially-Implements: blueprint network-commands-options
Depends-On:I412b2aabf9f1eeabfc0ef19f23cc17ba22c71ad7
Implements: blueprint allow-overwrite-set-options
Co-Authored-By: Reedip Banerjee <reedip14@gmail.com>
Change-Id: I21f0bdc71217757e8995504b91149c51222663eb

doc/source/cli/command-objects/port.rst

@@ -34,6 +34,7 @@ Create new port
         [--project <project> [--project-domain <project-domain>]]
         [--enable-port-security | --disable-port-security]
         [--tag <tag> | --no-tag]
+        [--dhcp-option <dhcp-option> [,<dhcp-option>]]
         <name>
 
 .. option:: --network <network>
@@ -139,6 +140,12 @@ Create new port
 
     No tags associated with the port
 
+.. option:: --dhcp-option <dhcp-option>
+
+    Extra DHCP option to be assigned to this port: name=<dhcp-option-name>,
+    value=<value>,ip-version={4,6} default 4
+    (repeat option to set multiple extra DHCP options)
+
 .. _port_create-name:
 .. describe:: <name>
 
@@ -265,6 +272,8 @@ Set port properties
         [--no-allowed-address]
         [--data-plane-status <status>]
         [--tag <tag>] [--no-tag]
+        [--dhcp-option <dhcp-option> [,<dhcp-option>]]
+        [--no-dhcp-option]
         <port>
 
 .. option:: --description <description>
@@ -383,6 +392,16 @@ Set port properties
     Clear tags associated with the port. Specify both --tag
     and --no-tag to overwrite current tags
 
+.. option:: --dhcp-option <dhcp-option>
+
+    Extra DHCP option to be assigned to this port: name=<dhcp-option-name>,
+    value=<value>,ip-version={4,6} default 4
+    (repeat option to set multiple extra DHCP options)
+
+.. option:: --no-dhcp-option
+
+    Clear existing extra DHCP options associated with this port
+
 .. _port_set-port:
 .. describe:: <port>
 
@@ -420,6 +439,7 @@ Unset port properties
         [--qos-policy]
         [--data-plane-status]
         [--tag <tag> | --all-tag]
+        [--dhcp-option <dhcp-option> [...]]
         <port>
 
 .. option:: --fixed-ip subnet=<subnet>,ip-address=<ip-address>
@@ -461,6 +481,12 @@ Unset port properties
 
     Clear all tags associated with the port
 
+.. option:: --dhcp-option <dhcp-option>
+
+    Extra DHCP option which should be removed from this port:
+    name=<dhcp-option-name>, value=<value>,ip-version={4,6}
+    (repeat option to unset multiple extra DHCP options)
+
 .. _port_unset-port:
 .. describe:: <port>
 

openstackclient/network/v2/port.py

@@ -276,6 +276,25 @@ def _add_updatable_args(parser):
     )
 
 
+def _convert_dhcp_options(parsed_args):
+    ops = []
+    for opt in parsed_args.dhcp_option:
+        opt_ele = {}
+        opt_ele['opt_name'] = opt['name']
+        if not opt.get('value', None):
+            opt_ele['opt_value'] = None
+        else:
+            opt_ele['opt_value'] = opt['value']
+        if not opt.get('ip-version', None):
+            opt_ele['ip_version'] = 4
+        elif opt['ip-version'] in ['4', '6']:
+            opt_ele['ip_version'] = int(opt['ip-version'])
+        else:
+            raise(exceptions.BadRequest(message='IP Version should be 4/6'))
+        ops.append(opt_ele)
+    return ops
+
+
 # TODO(abhiraut): Use the SDK resource mapped attribute names once the
 # OSC minimum requirements include SDK 1.0.
 def _convert_address_pairs(parsed_args):
@@ -348,8 +367,6 @@ class CreatePort(command.ShowOne):
             metavar='<name>',
             help=_("Name of this port")
         )
-        # TODO(singhj): Add support for extended options:
-        # dhcp
         secgroups = parser.add_mutually_exclusive_group()
         secgroups.add_argument(
             '--security-group',
@@ -393,6 +410,16 @@ class CreatePort(command.ShowOne):
                    "(repeat option to set multiple allowed-address pairs)")
         )
         _tag.add_tag_option_to_parser_for_create(parser, _('port'))
+        parser.add_argument(
+            '--dhcp-option',
+            metavar='<dhcp-option>',
+            action=parseractions.MultiKeyValueCommaAction,
+            required_keys=['name'],
+            optional_keys=['value', 'ip-version'],
+            help=_('Extra DHCP options to be assigned to this port: '
+                   'name=<dhcp-option-name>,value=<value>,ip-version={4,6} '
+                   'default 4 (repeat option to set multiple extra '
+                   'DHCP options)'))
         return parser
 
     def take_action(self, parsed_args):
@@ -426,6 +453,9 @@ class CreatePort(command.ShowOne):
         if parsed_args.qos_policy:
             attrs['qos_policy_id'] = client.find_qos_policy(
                 parsed_args.qos_policy, ignore_missing=False).id
+        if parsed_args.dhcp_option:
+            attrs['extra_dhcp_opts'] = _convert_dhcp_options(parsed_args)
+
         obj = client.create_port(**attrs)
         # tags cannot be set when created, so tags need to be set later.
         _tag.update_tags_for_set(client, obj, parsed_args)
@@ -711,7 +741,22 @@ class SetPort(command.Command):
                    "(requires data plane status extension)")
         )
         _tag.add_tag_option_to_parser_for_set(parser, _('port'))
-
+        parser.add_argument(
+            '--dhcp-option',
+            metavar='<dhcp-option>',
+            action=parseractions.MultiKeyValueCommaAction,
+            required_keys=['name'],
+            optional_keys=['value', 'ip-version'],
+            help=_('Extra DHCP options to be assigned to this port: '
+                   'name=<dhcp-option-name>,value=<value>,ip-version={4,6} '
+                   'default 4 '
+                   '(repeat option to set multiple extra DHCP options)')
+        )
+        parser.add_argument(
+            '--no-dhcp-option',
+            action='store_true',
+            help=_('Clear existing extra DHCP options associated with this '
+                   'port'))
         return parser
 
     def take_action(self, parsed_args):
@@ -766,12 +811,21 @@ class SetPort(command.Command):
             )
         if parsed_args.data_plane_status:
             attrs['data_plane_status'] = parsed_args.data_plane_status
-
         if attrs:
             client.update_port(obj, **attrs)
 
         # tags is a subresource and it needs to be updated separately.
         _tag.update_tags_for_set(client, obj, parsed_args)
+        if parsed_args.dhcp_option and parsed_args.no_dhcp_option:
+            attrs['extra_dhcp_opts'] = _convert_dhcp_options(parsed_args)
+        elif parsed_args.dhcp_option:
+            attrs['extra_dhcp_opts'] = (
+                [opt for opt in obj.extra_dhcp_opts if opt] +
+                _convert_dhcp_options(parsed_args))
+        elif parsed_args.no_dhcp_option:
+            attrs['extra_dhcp_opts'] = []
+
+        client.update_port(obj, **attrs)
 
 
 class ShowPort(command.ShowOne):
@@ -809,14 +863,15 @@ class UnsetPort(command.Command):
             help=_("Desired IP and/or subnet which should be "
                    "removed from this port (name or ID): subnet=<subnet>,"
                    "ip-address=<ip-address> (repeat option to unset multiple "
-                   "fixed IP addresses)"))
-
+                   "fixed IP addresses)")
+        )
         parser.add_argument(
             '--binding-profile',
             metavar='<binding-profile-key>',
             action='append',
             help=_("Desired key which should be removed from binding:profile"
-                   "(repeat option to unset multiple binding:profile data)"))
+                   "(repeat option to unset multiple binding:profile data)")
+        )
         parser.add_argument(
             '--security-group',
             metavar='<security-group>',
@@ -825,7 +880,16 @@ class UnsetPort(command.Command):
             help=_("Security group which should be removed this port (name "
                    "or ID) (repeat option to unset multiple security groups)")
         )
-
+        parser.add_argument(
+            '--dhcp-option',
+            metavar='<dhcp-option>',
+            action=parseractions.MultiKeyValueCommaAction,
+            required_keys=['name'],
+            optional_keys=['value', 'ip-version'],
+            help=_('Extra DHCP options which should be removed from this '
+                   'port: name=<dhcp-option-name>,value=<value>,ip-version='
+                   '{4,6} (repeat option to set multiple extra DHCP options)')
+        )
         parser.add_argument(
             'port',
             metavar="<port>",
@@ -869,6 +933,7 @@ class UnsetPort(command.Command):
         tmp_binding_profile = copy.deepcopy(obj.binding_profile)
         tmp_secgroups = copy.deepcopy(obj.security_group_ids)
         tmp_addr_pairs = copy.deepcopy(obj.allowed_address_pairs)
+        tmp_dhcpoptions = copy.deepcopy(obj.extra_dhcp_opts)
         _prepare_fixed_ips(self.app.client_manager, parsed_args)
         attrs = {}
         if parsed_args.fixed_ip:
@@ -910,6 +975,17 @@ class UnsetPort(command.Command):
         if parsed_args.data_plane_status:
             attrs['data_plane_status'] = None
 
+        if parsed_args.dhcp_option:
+            try:
+                for dhcp_opt in _convert_dhcp_options(parsed_args):
+                    tmp_dhcpoptions[dhcp_opt]['opt_value'] = None
+            except ValueError:
+                msg = _(
+                    "Port does not contain dhcp "
+                    "option %s") % parsed_args.dhcp_option
+                raise exceptions.CommandError(msg)
+            attrs['extra_dhcp_opts'] = tmp_dhcpoptions
+
         if attrs:
             client.update_port(obj, **attrs)
 

openstackclient/tests/functional/network/v2/test_port.py

@@ -261,3 +261,39 @@ class PortTests(common.NetworkTagTests):
             '{} create -f json --network {} {} {}'
             .format(self.base_command, self.NETWORK_NAME, args, name)
         ))
+
+    def test_port_dhcp_set(self):
+        name = uuid.uuid4().hex
+        json_output = json.loads(self.openstack(
+            "port create -f json " +
+            "--network " + self.NETWORK_NAME + " "
+            "--description dhcp " +
+            "--dhcp-option name=test,value='10.0.1.1/24' " +
+            name
+        ))
+        id1 = json_output.get('id')
+        self.addCleanup(self.openstack, 'port delete ' + id1)
+        self.assertEqual(name, json_output.get('name'))
+        self.assertEqual('dhcp', json_output.get('description'))
+        json_output = json.loads(self.openstack(
+            'port show -f json ' + name
+        ))
+        self.assertEqual(
+            "ip_version='4', opt_name='test', opt_value='10.0.1.1/24'",
+            json_output.get('extra_dhcp_opts'))
+        self.openstack(
+            "port unset --dhcp-option name=test,value='10.0.1.1/24' " + name)
+        output = json.loads(self.openstack(
+            'port show -f json ' + name
+        ))
+        self.assertEqual('', output.get('extra_dhcp_opts'))
+        raw_output = self.openstack(
+            'port set ' +
+            '--dhcp-option name=test,value=10.0.0.1/24,ip-version=6 ' +
+            name
+        )
+        self.assertOutput('', raw_output)
+        output = json.loads(self.openstack('port show -f json ' + name))
+        self.assertEqual("ip_version='6', opt_name='test', "
+                         "opt_value='10.0.0.1/24'",
+                         output.get('extra_dhcp_opts'),)

openstackclient/tests/unit/network/v2/test_port.py

@@ -566,6 +566,103 @@ class TestCreatePort(TestPort):
     def test_create_with_no_tag(self):
         self._test_create_with_tag(add_tags=False)
 
+    def test_create_port_with_extradhcp_opts(self):
+        dhcp_opts = [{'name': 'server-ip-addr',
+                      'value': '123.123.123.45'},
+                     {'name': "classless-static-route",
+                      'value': "169.254.169.254/32,40.40.40.5"}]
+        arglist = [
+            '--network', self._port.network_id,
+            '--dhcp-option',
+            'name=server-ip-addr,value=123.123.123.45',
+            '--dhcp-option',
+            'name=classless-static-route,value=169.254.169.254/32,40.40.40.5',
+            'test-port',
+        ]
+        verifylist = [
+            ('network', self._port.network_id,),
+            ('enable', True),
+            ('dhcp_option', dhcp_opts),
+            ('name', 'test-port'),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        self.cmd.take_action(parsed_args)
+
+        extra_dhcp_opts = [{'opt_name': 'server-ip-addr',
+                            'opt_value': '123.123.123.45',
+                            'ip_version': 4},
+                           {'opt_name': 'classless-static-route',
+                            'opt_value': '169.254.169.254/32,40.40.40.5',
+                            'ip_version': 4}]
+
+        self.network.create_port.assert_called_once_with(**{
+            'admin_state_up': True,
+            'network_id': self._port.network_id,
+            'extra_dhcp_opts': extra_dhcp_opts,
+            'name': 'test-port',
+        })
+
+    def test_create_port_with_extra_dhcp_fail(self):
+        arglist = [
+            '--network', self._port.network_id,
+            '--dhcp-option',
+            'name,value=123.123.123.45',
+            '--dhcp-option',
+            'name=classless-static-route,value=169.254.169.254/32,40.40.40.5',
+            'test-port',
+        ]
+        self.assertRaises(argparse.ArgumentTypeError,
+                          self.check_parser,
+                          self.cmd,
+                          arglist,
+                          None)
+
+    def test_create_port_with_extradhcp_opts_ip_version(self):
+        dhcp_opts = [{'name': 'bootfile-name',
+                      'value': 'pxelinux.0'},
+                     {'name': 'tftp-server',
+                      'value': '123.123.123.123',
+                      'ip-version': '6'},
+                     {'name': 'server-ip-addr',
+                      'value': '123.123.123.45',
+                      'ip-version': '4'}]
+        arglist = [
+            '--network', self._port.network_id,
+            '--dhcp-option',
+            ('name=bootfile-name,value=pxelinux.0'),
+            '--dhcp-option',
+            ('name=tftp-server,value=123.123.123.123,ip-version=6'),
+            '--dhcp-option',
+            ('name=server-ip-addr,value=123.123.123.45,ip-version=4'),
+            'test-port',
+        ]
+        verifylist = [
+            ('network', self._port.network_id,),
+            ('enable', True),
+            ('dhcp_option', dhcp_opts),
+            ('name', 'test-port'),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        self.cmd.take_action(parsed_args)
+
+        extra_dhcp_opts = [{'opt_name': 'bootfile-name',
+                            'opt_value': 'pxelinux.0',
+                            'ip_version': 4},
+                           {'opt_name': 'tftp-server',
+                            'opt_value': '123.123.123.123',
+                            'ip_version': 6},
+                           {'opt_name': 'server-ip-addr',
+                            'opt_value': '123.123.123.45',
+                            'ip_version': 4}]
+        self.network.create_port.assert_called_once_with(**{
+            'admin_state_up': True,
+            'network_id': self._port.network_id,
+            'extra_dhcp_opts': extra_dhcp_opts,
+            'name': 'test-port',
+        })
+
 
 class TestDeletePort(TestPort):
 
@@ -1440,6 +1537,124 @@ class TestSetPort(TestPort):
             'port_security_enabled': False,
         })
 
+    def test_set_extra_dhcp_opt(self):
+        arglist = [
+            '--dhcp-option',
+            "name=server-ip-addr,value=123.123.123.45,ip-version=6",
+            self._port.name,
+        ]
+        verifylist = [
+            ('dhcp_option', [{'name': 'server-ip-addr',
+                              'value': '123.123.123.45',
+                              'ip-version': '6'}]),
+            ('port', self._port.name),
+        ]
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        result = self.cmd.take_action(parsed_args)
+
+        attrs = {
+            'extra_dhcp_opts': [{'opt_name': 'server-ip-addr',
+                                 'opt_value': '123.123.123.45',
+                                 'ip_version': 6}]
+
+        }
+        self.network.update_port.assert_called_once_with(self._port, **attrs)
+        self.assertIsNone(result)
+
+    def test_append_extra_dhcp_opts(self):
+        _testport = network_fakes.FakePort.create_one_port(
+            {'extra_dhcp_opts': [{'opt_name': 'bootfile-name',
+                                  'opt_value': 'pxelinux.0',
+                                  'ip_version': 4},
+                                 {'opt_name': 'tftp-server',
+                                  'opt_value': '123.123.123.123',
+                                  'ip_version': 4}]})
+        self.network.find_port = mock.Mock(return_value=_testport)
+        arglist = [
+            '--dhcp-option',
+            "name=server-ip-addr,value=123.123.123.45,ip-version=6",
+            _testport.name,
+        ]
+        verifylist = [
+            ('dhcp_option', [{'name': 'server-ip-addr',
+                              'value': '123.123.123.45',
+                              'ip-version': '6'}]),
+            ('port', _testport.name),
+        ]
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        result = self.cmd.take_action(parsed_args)
+
+        attrs = {
+            'extra_dhcp_opts': [{'opt_name': 'bootfile-name',
+                                 'opt_value': 'pxelinux.0',
+                                 'ip_version': 4},
+                                {'opt_name': 'tftp-server',
+                                 'opt_value': '123.123.123.123',
+                                 'ip_version': 4},
+                                {'opt_name': 'server-ip-addr',
+                                 'opt_value': '123.123.123.45',
+                                 'ip_version': 6}]
+        }
+        self.network.update_port.assert_called_once_with(_testport, **attrs)
+        self.assertIsNone(result)
+
+    def test_overwrite_extra_dhcp_opt(self):
+        _testport = network_fakes.FakePort.create_one_port(
+            {'extra_dhcp_opts': [{'opt_name': 'bootfile-name',
+                                  'opt_value': 'pxelinux.0',
+                                  'ip_version': 4},
+                                 {'opt_name': 'tftp-server',
+                                  'opt_value': '123.123.123.123',
+                                  'ip_version': 4}]})
+        self.network.find_port = mock.Mock(return_value=_testport)
+        arglist = [
+            '--dhcp-option',
+            "name=server-ip-addr,value=123.123.123.45,ip-version=6",
+            '--no-dhcp-option',
+            _testport.name,
+        ]
+        verifylist = [
+            ('dhcp_option', [{'name': 'server-ip-addr',
+                              'value': '123.123.123.45',
+                              'ip-version': '6'}]),
+            ('no_dhcp_option', True),
+            ('port', _testport.name),
+        ]
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        result = self.cmd.take_action(parsed_args)
+
+        attrs = {
+            'extra_dhcp_opts': [{'opt_name': 'server-ip-addr',
+                                 'opt_value': '123.123.123.45',
+                                 'ip_version': 6}]
+
+        }
+        self.network.update_port.assert_called_once_with(_testport, **attrs)
+        self.assertIsNone(result)
+
+    def test_set_no_extra_dhcp_opts(self):
+        arglist = [
+            '--no-dhcp-option',
+            self._port.name,
+        ]
+
+        verifylist = [
+            ('no_dhcp_option', True),
+            ('port', self._port.name),
+        ]
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        result = self.cmd.take_action(parsed_args)
+
+        attrs = {
+            'extra_dhcp_opts': [],
+        }
+        self.network.update_port.assert_called_once_with(self._port, **attrs)
+        self.assertIsNone(result)
+
     def test_set_port_with_qos(self):
         qos_policy = network_fakes.FakeNetworkQosPolicy.create_one_qos_policy()
         self.network.find_qos_policy = mock.Mock(return_value=qos_policy)
@@ -1754,6 +1969,37 @@ class TestUnsetPort(TestPort):
         self.network.update_port.assert_called_once_with(_fake_port, **attrs)
         self.assertIsNone(result)
 
+    def test_unset_port_extra_dhcp_opts(self):
+        _fake_port = network_fakes.FakePort.create_one_port(
+            {'extra_dhcp_opts': [{'opt_name': 'bootfile-name',
+                                  'opt_value': 'pxelinux.0',
+                                  'ip_version': 4},
+                                 {'opt_name': 'tftp-server',
+                                  'opt_value': '123.123.123.45',
+                                  'ip_version': 6}]})
+        self.network.find_port = mock.Mock(return_value=_fake_port)
+        arglist = [
+            '--dhcp-option',
+            "name=tftp-server,value=123.123.123.45,ip-version=6",
+            _fake_port.name,
+        ]
+        verifylist = [
+            ('dhcp_option', [{'name': 'tftp-server',
+                              'value': '123.123.123.45',
+                              'ip-version': '6'}])
+        ]
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        result = self.cmd.take_action(parsed_args)
+
+        attrs = {
+            'extra_dhcp_opts': [{'opt_name': 'bootfile-name',
+                                 'opt_value': 'pxelinux.0',
+                                 'ip_version': 4}],
+        }
+        self.network.update_port.assert_called_once_with(_fake_port, **attrs)
+        self.assertIsNone(result)
+
     def _test_unset_tags(self, with_tags=True):
         if with_tags:
             arglist = ['--tag', 'red', '--tag', 'blue']
@@ -1781,3 +2027,27 @@ class TestUnsetPort(TestPort):
 
     def test_unset_with_all_tag(self):
         self._test_unset_tags(with_tags=False)
+
+    def test_unset_port_extra_dhcp_opts_not_existent(self):
+        _fake_port = network_fakes.FakePort.create_one_port(
+            {'extra_dhcp_opts': [{'opt_name': 'bootfile-name',
+                                  'opt_value': 'pxelinux.0',
+                                  'ip_version': 4},
+                                 {'opt_name': 'tftp-server',
+                                  'opt_value': '123.123.123.123',
+                                  'ip_version': 4}]})
+        self.network.find_port = mock.Mock(return_value=_fake_port)
+        arglist = [
+            '--dhcp-option',
+            "name=bootfile-name,value=10.10.10.1",
+            self._testport.name,
+        ]
+        verifylist = [
+            ('dhcp_option', [{'name': 'bootfile-name',
+                              'value': '10.10.10.1'}]),
+        ]
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        self.assertRaises(exceptions.CommandError,
+                          self.cmd.take_action,
+                          parsed_args)

releasenotes/notes/bug-1612136-a1e714c826eef040.yaml

@@ -0,0 +1,6 @@
+---
+features:
+  - |
+    Add ``--dhcp-option`` and ``--no-dhcp-option`` to ``port create``,
+    ``port set`` and ``port unset`` command.
+    [Bug `1612136 <https://bugs.launchpad.net/python-openstackclient/+bug/1612136>`_]