diff --git a/openstackclient/network/v2/port.py b/openstackclient/network/v2/port.py index 814e82abc8..6ca069bb17 100644 --- a/openstackclient/network/v2/port.py +++ b/openstackclient/network/v2/port.py @@ -322,6 +322,22 @@ def _add_updatable_args(parser): action='store_true', help=_("NUMA affinity policy using legacy mode to schedule this port"), ) + parser.add_argument( + '--hint', + metavar='', + action=JSONKeyValueAction, + default={}, + help=_( + 'Port hints as ALIAS=VALUE or as JSON. ' + 'Valid hint aliases/values: ' + 'ovs-tx-steering=thread, ovs-tx-steering=hash. ' + 'Valid JSON values are as specified by the Neutron API. ' + '(requires port-hints extension) ' + '(requires port-hint-ovs-tx-steering extension for alias: ' + 'ovs-tx-steering) ' + '(repeat option to set multiple hints)' + ), + ) # TODO(abhiraut): Use the SDK resource mapped attribute names once the @@ -350,6 +366,34 @@ def _convert_extra_dhcp_options(parsed_args): return dhcp_options +# When we have multiple hints, we'll need to refactor this to allow +# arbitrary combinations. But until then let's have it as simple as possible. +def _validate_port_hints(hints): + if hints not in ( + {}, + # by hint alias + {'ovs-tx-steering': 'thread'}, + {'ovs-tx-steering': 'hash'}, + # by fully specified value of the port's hints field + {'openvswitch': {'other_config': {'tx-steering': 'thread'}}}, + {'openvswitch': {'other_config': {'tx-steering': 'hash'}}}, + ): + msg = _("Invalid value to --hints, see --help for valid values.") + raise argparse.ArgumentTypeError(msg) + + +# When we have multiple hints, we'll need to refactor this to expand aliases +# without losing other hints. But until then let's have it as simple as +# possible. +def _expand_port_hint_aliases(hints): + if hints == {'ovs-tx-steering': 'thread'}: + return {'openvswitch': {'other_config': {'tx-steering': 'thread'}}} + elif hints == {'ovs-tx-steering': 'hash'}: + return {'openvswitch': {'other_config': {'tx-steering': 'hash'}}} + else: + return hints + + class CreatePort(command.ShowOne, common.NeutronCommandWithExtraArgs): _description = _("Create a new port") @@ -527,6 +571,29 @@ class CreatePort(command.ShowOne, common.NeutronCommandWithExtraArgs): parsed_args.qos_policy, ignore_missing=False ).id + if parsed_args.hint: + _validate_port_hints(parsed_args.hint) + expanded_hints = _expand_port_hint_aliases(parsed_args.hint) + try: + client.find_extension('port-hints', ignore_missing=False) + except Exception as e: + msg = _('Not supported by Network API: %(e)s') % {'e': e} + raise exceptions.CommandError(msg) + if ( + 'openvswitch' in expanded_hints + and 'other_config' in expanded_hints['openvswitch'] + and 'tx-steering' + in expanded_hints['openvswitch']['other_config'] + ): + try: + client.find_extension( + 'port-hint-ovs-tx-steering', ignore_missing=False + ) + except Exception as e: + msg = _('Not supported by Network API: %(e)s') % {'e': e} + raise exceptions.CommandError(msg) + attrs['hints'] = expanded_hints + set_tags_in_post = bool( client.find_extension('tag-ports-during-bulk-creation') ) @@ -972,6 +1039,29 @@ class SetPort(common.NeutronCommandWithExtraArgs): if parsed_args.data_plane_status: attrs['data_plane_status'] = parsed_args.data_plane_status + if parsed_args.hint: + _validate_port_hints(parsed_args.hint) + expanded_hints = _expand_port_hint_aliases(parsed_args.hint) + try: + client.find_extension('port-hints', ignore_missing=False) + except Exception as e: + msg = _('Not supported by Network API: %(e)s') % {'e': e} + raise exceptions.CommandError(msg) + if ( + 'openvswitch' in expanded_hints + and 'other_config' in expanded_hints['openvswitch'] + and 'tx-steering' + in expanded_hints['openvswitch']['other_config'] + ): + try: + client.find_extension( + 'port-hint-ovs-tx-steering', ignore_missing=False + ) + except Exception as e: + msg = _('Not supported by Network API: %(e)s') % {'e': e} + raise exceptions.CommandError(msg) + attrs['hints'] = expanded_hints + attrs.update( self._parse_extra_properties(parsed_args.extra_properties) ) @@ -1083,6 +1173,12 @@ class UnsetPort(common.NeutronUnsetCommandWithExtraArgs): default=False, help=_("Clear host binding for the port."), ) + parser.add_argument( + '--hints', + action='store_true', + default=False, + help=_("Clear hints for the port."), + ) _tag.add_tag_option_to_parser_for_unset(parser, _('port')) @@ -1143,6 +1239,8 @@ class UnsetPort(common.NeutronUnsetCommandWithExtraArgs): attrs['numa_affinity_policy'] = None if parsed_args.host: attrs['binding:host_id'] = None + if parsed_args.hints: + attrs['hints'] = None attrs.update( self._parse_extra_properties(parsed_args.extra_properties) diff --git a/openstackclient/tests/unit/network/v2/fakes.py b/openstackclient/tests/unit/network/v2/fakes.py index ce7ec79830..06867707b0 100644 --- a/openstackclient/tests/unit/network/v2/fakes.py +++ b/openstackclient/tests/unit/network/v2/fakes.py @@ -1778,6 +1778,7 @@ def create_one_port(attrs=None): 'subnet_id': 'subnet-id-' + uuid.uuid4().hex, } ], + 'hints': {}, 'id': 'port-id-' + uuid.uuid4().hex, 'mac_address': 'fa:16:3e:a9:4e:72', 'name': 'port-name-' + uuid.uuid4().hex, diff --git a/openstackclient/tests/unit/network/v2/test_port.py b/openstackclient/tests/unit/network/v2/test_port.py index 78ac6635df..b38d1783c3 100644 --- a/openstackclient/tests/unit/network/v2/test_port.py +++ b/openstackclient/tests/unit/network/v2/test_port.py @@ -60,6 +60,7 @@ class TestPort(network_fakes.TestNetworkV2): 'dns_name', 'extra_dhcp_opts', 'fixed_ips', + 'hints', 'id', 'ip_allocation', 'mac_address', @@ -99,6 +100,7 @@ class TestPort(network_fakes.TestNetworkV2): fake_port.dns_name, format_columns.ListDictColumn(fake_port.extra_dhcp_opts), format_columns.ListDictColumn(fake_port.fixed_ips), + fake_port.hints, fake_port.id, fake_port.ip_allocation, fake_port.mac_address, @@ -931,6 +933,133 @@ class TestCreatePort(TestPort): self.assertEqual(set(self.columns), set(columns)) self.assertCountEqual(self.data, data) + def test_create_hints_invalid_json(self): + arglist = [ + '--network', + self._port.network_id, + '--hint', + 'invalid json', + 'test-port', + ] + self.assertRaises( + argparse.ArgumentTypeError, + self.check_parser, + self.cmd, + arglist, + None, + ) + + def test_create_hints_invalid_alias(self): + arglist = [ + '--network', + self._port.network_id, + '--hint', + 'invalid-alias=value', + 'test-port', + ] + verifylist = [ + ('network', self._port.network_id), + ('enable', True), + ('hint', {'invalid-alias': 'value'}), + ('name', 'test-port'), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + self.assertRaises( + argparse.ArgumentTypeError, + self.cmd.take_action, + parsed_args, + ) + + def test_create_hints_invalid_value(self): + arglist = [ + '--network', + self._port.network_id, + '--hint', + 'ovs-tx-steering=invalid-value', + 'test-port', + ] + verifylist = [ + ('network', self._port.network_id), + ('enable', True), + ('hint', {'ovs-tx-steering': 'invalid-value'}), + ('name', 'test-port'), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + self.assertRaises( + argparse.ArgumentTypeError, + self.cmd.take_action, + parsed_args, + ) + + def test_create_hints_valid_alias_value(self): + arglist = [ + '--network', + self._port.network_id, + '--hint', + 'ovs-tx-steering=hash', + 'test-port', + ] + verifylist = [ + ('network', self._port.network_id), + ('enable', True), + ('hint', {'ovs-tx-steering': 'hash'}), + ('name', 'test-port'), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + columns, data = self.cmd.take_action(parsed_args) + + self.network.create_port.assert_called_once_with( + **{ + 'admin_state_up': True, + 'network_id': self._port.network_id, + 'hints': { + 'openvswitch': {'other_config': {'tx-steering': 'hash'}} + }, + 'name': 'test-port', + } + ) + + self.assertEqual(set(self.columns), set(columns)) + self.assertCountEqual(self.data, data) + + def test_create_hints_valid_json(self): + arglist = [ + '--network', + self._port.network_id, + '--hint', + '{"openvswitch": {"other_config": {"tx-steering": "hash"}}}', + 'test-port', + ] + verifylist = [ + ('network', self._port.network_id), + ('enable', True), + ( + 'hint', + {"openvswitch": {"other_config": {"tx-steering": "hash"}}}, + ), + ('name', 'test-port'), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + columns, data = self.cmd.take_action(parsed_args) + + self.network.create_port.assert_called_once_with( + **{ + 'admin_state_up': True, + 'network_id': self._port.network_id, + 'hints': { + 'openvswitch': {'other_config': {'tx-steering': 'hash'}} + }, + 'name': 'test-port', + } + ) + + self.assertEqual(set(self.columns), set(columns)) + self.assertCountEqual(self.data, data) + class TestDeletePort(TestPort): # Ports to delete. @@ -2010,7 +2139,7 @@ class TestSetPort(TestPort): self._port, **{ 'port_security_enabled': True, - } + }, ) def test_set_port_security_disabled(self): @@ -2034,7 +2163,7 @@ class TestSetPort(TestPort): self._port, **{ 'port_security_enabled': False, - } + }, ) def test_set_port_with_qos(self): @@ -2155,6 +2284,115 @@ class TestSetPort(TestPort): def test_create_with_numa_affinity_policy_legacy(self): self._test_create_with_numa_affinity_policy('legacy') + def test_set_hints_invalid_json(self): + arglist = [ + '--network', + self._port.network_id, + '--hint', + 'invalid json', + 'test-port', + ] + self.assertRaises( + argparse.ArgumentTypeError, + self.check_parser, + self.cmd, + arglist, + None, + ) + + def test_set_hints_invalid_alias(self): + arglist = [ + '--hint', + 'invalid-alias=value', + 'test-port', + ] + verifylist = [ + ('hint', {'invalid-alias': 'value'}), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + self.assertRaises( + argparse.ArgumentTypeError, + self.cmd.take_action, + parsed_args, + ) + + def test_set_hints_invalid_value(self): + arglist = [ + '--hint', + 'ovs-tx-steering=invalid-value', + 'test-port', + ] + verifylist = [ + ('hint', {'ovs-tx-steering': 'invalid-value'}), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + self.assertRaises( + argparse.ArgumentTypeError, + self.cmd.take_action, + parsed_args, + ) + + def test_set_hints_valid_alias_value(self): + testport = network_fakes.create_one_port() + self.network.find_port = mock.Mock(return_value=testport) + self.network.find_extension = mock.Mock( + return_value=['port-hints', 'port-hint-ovs-tx-steering'] + ) + arglist = [ + '--hint', + 'ovs-tx-steering=hash', + testport.name, + ] + verifylist = [ + ('hint', {'ovs-tx-steering': 'hash'}), + ('port', testport.name), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + result = self.cmd.take_action(parsed_args) + + self.network.update_port.assert_called_once_with( + testport, + **{ + 'hints': { + 'openvswitch': {'other_config': {'tx-steering': 'hash'}} + } + }, + ) + self.assertIsNone(result) + + def test_set_hints_valid_json(self): + testport = network_fakes.create_one_port() + self.network.find_port = mock.Mock(return_value=testport) + self.network.find_extension = mock.Mock( + return_value=['port-hints', 'port-hint-ovs-tx-steering'] + ) + arglist = [ + '--hint', + '{"openvswitch": {"other_config": {"tx-steering": "hash"}}}', + testport.name, + ] + verifylist = [ + ( + 'hint', + {"openvswitch": {"other_config": {"tx-steering": "hash"}}}, + ), + ('port', testport.name), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + result = self.cmd.take_action(parsed_args) + + self.network.update_port.assert_called_once_with( + testport, + **{ + 'hints': { + 'openvswitch': {'other_config': {'tx-steering': 'hash'}} + } + }, + ) + self.assertIsNone(result) + class TestShowPort(TestPort): # The port to show. @@ -2474,3 +2712,23 @@ class TestUnsetPort(TestPort): self.network.update_port.assert_called_once_with(_fake_port, **attrs) self.assertIsNone(result) + + def test_unset_hints(self): + testport = network_fakes.create_one_port() + self.network.find_port = mock.Mock(return_value=testport) + arglist = [ + '--hints', + testport.name, + ] + verifylist = [ + ('hints', True), + ('port', testport.name), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + result = self.cmd.take_action(parsed_args) + + self.network.update_port.assert_called_once_with( + testport, + **{'hints': None}, + ) + self.assertIsNone(result) diff --git a/releasenotes/notes/add-port-hints-attribute-be1779e640a47d0d.yaml b/releasenotes/notes/add-port-hints-attribute-be1779e640a47d0d.yaml new file mode 100644 index 0000000000..5a52f3c13b --- /dev/null +++ b/releasenotes/notes/add-port-hints-attribute-be1779e640a47d0d.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + Enable management of Neutron port hints: ``port create --hint HINT``, + ``set port --hint HINT and ``unset port --hint``. Port hints allow + passing backend specific hints to Neutron mainly to tune backend + performance. The first hint controls Open vSwitch Tx steering.