From 0134e90bf556cf7cc162fbf4d2ffda8d1c3c44b5 Mon Sep 17 00:00:00 2001 From: Nguyen Phuong An Date: Fri, 20 Jan 2017 03:18:12 -0500 Subject: [PATCH] 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 Change-Id: I21f0bdc71217757e8995504b91149c51222663eb --- doc/source/cli/command-objects/port.rst | 26 ++ openstackclient/network/v2/port.py | 92 +++++- .../tests/functional/network/v2/test_port.py | 36 +++ .../tests/unit/network/v2/test_port.py | 270 ++++++++++++++++++ .../notes/bug-1612136-a1e714c826eef040.yaml | 6 + 5 files changed, 422 insertions(+), 8 deletions(-) create mode 100644 releasenotes/notes/bug-1612136-a1e714c826eef040.yaml diff --git a/doc/source/cli/command-objects/port.rst b/doc/source/cli/command-objects/port.rst index cf29bb2cbd..83cc03068c 100644 --- a/doc/source/cli/command-objects/port.rst +++ b/doc/source/cli/command-objects/port.rst @@ -34,6 +34,7 @@ Create new port [--project [--project-domain ]] [--enable-port-security | --disable-port-security] [--tag | --no-tag] + [--dhcp-option [,]] .. option:: --network @@ -139,6 +140,12 @@ Create new port No tags associated with the port +.. option:: --dhcp-option + + Extra DHCP option to be assigned to this port: name=, + value=,ip-version={4,6} default 4 + (repeat option to set multiple extra DHCP options) + .. _port_create-name: .. describe:: @@ -265,6 +272,8 @@ Set port properties [--no-allowed-address] [--data-plane-status ] [--tag ] [--no-tag] + [--dhcp-option [,]] + [--no-dhcp-option] .. option:: --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 + + Extra DHCP option to be assigned to this port: name=, + 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:: @@ -420,6 +439,7 @@ Unset port properties [--qos-policy] [--data-plane-status] [--tag | --all-tag] + [--dhcp-option [...]] .. option:: --fixed-ip subnet=,ip-address= @@ -461,6 +481,12 @@ Unset port properties Clear all tags associated with the port +.. option:: --dhcp-option + + Extra DHCP option which should be removed from this port: + name=, value=,ip-version={4,6} + (repeat option to unset multiple extra DHCP options) + .. _port_unset-port: .. describe:: diff --git a/openstackclient/network/v2/port.py b/openstackclient/network/v2/port.py index 4b23b339c9..4fe5d0c5c1 100644 --- a/openstackclient/network/v2/port.py +++ b/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='', 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='', + action=parseractions.MultiKeyValueCommaAction, + required_keys=['name'], + optional_keys=['value', 'ip-version'], + help=_('Extra DHCP options to be assigned to this port: ' + 'name=,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='', + action=parseractions.MultiKeyValueCommaAction, + required_keys=['name'], + optional_keys=['value', 'ip-version'], + help=_('Extra DHCP options to be assigned to this port: ' + 'name=,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=," "ip-address= (repeat option to unset multiple " - "fixed IP addresses)")) - + "fixed IP addresses)") + ) parser.add_argument( '--binding-profile', metavar='', 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='', @@ -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='', + action=parseractions.MultiKeyValueCommaAction, + required_keys=['name'], + optional_keys=['value', 'ip-version'], + help=_('Extra DHCP options which should be removed from this ' + 'port: name=,value=,ip-version=' + '{4,6} (repeat option to set multiple extra DHCP options)') + ) parser.add_argument( 'port', metavar="", @@ -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) diff --git a/openstackclient/tests/functional/network/v2/test_port.py b/openstackclient/tests/functional/network/v2/test_port.py index a705979028..fe8027823f 100644 --- a/openstackclient/tests/functional/network/v2/test_port.py +++ b/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'),) diff --git a/openstackclient/tests/unit/network/v2/test_port.py b/openstackclient/tests/unit/network/v2/test_port.py index 3f751818b9..b89c2f5bf1 100644 --- a/openstackclient/tests/unit/network/v2/test_port.py +++ b/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) diff --git a/releasenotes/notes/bug-1612136-a1e714c826eef040.yaml b/releasenotes/notes/bug-1612136-a1e714c826eef040.yaml new file mode 100644 index 0000000000..ee9edb5386 --- /dev/null +++ b/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 `_]