Merge "Add direction field to QoS bandwidth limit."

openstackclient/network/v2/network_qos_rule.py

@@ -14,7 +14,6 @@
 #    under the License.
 
 import itertools
-import six
 
 from osc_lib.command import command
 from osc_lib import exceptions
@@ -27,10 +26,14 @@ from openstackclient.network import sdk_utils
 RULE_TYPE_BANDWIDTH_LIMIT = 'bandwidth-limit'
 RULE_TYPE_DSCP_MARKING = 'dscp-marking'
 RULE_TYPE_MINIMUM_BANDWIDTH = 'minimum-bandwidth'
-REQUIRED_PARAMETERS = {
+MANDATORY_PARAMETERS = {
-    RULE_TYPE_MINIMUM_BANDWIDTH: ['min_kbps', 'direction'],
+    RULE_TYPE_MINIMUM_BANDWIDTH: {'min_kbps', 'direction'},
-    RULE_TYPE_DSCP_MARKING: ['dscp_mark'],
+    RULE_TYPE_DSCP_MARKING: {'dscp_mark'},
-    RULE_TYPE_BANDWIDTH_LIMIT: ['max_kbps', 'max_burst_kbps']}
+    RULE_TYPE_BANDWIDTH_LIMIT: {'max_kbps', 'max_burst_kbps'}}
+OPTIONAL_PARAMETERS = {
+    RULE_TYPE_MINIMUM_BANDWIDTH: set(),
+    RULE_TYPE_DSCP_MARKING: set(),
+    RULE_TYPE_BANDWIDTH_LIMIT: {'direction'}}
 DIRECTION_EGRESS = 'egress'
 DIRECTION_INGRESS = 'ingress'
 DSCP_VALID_MARKS = [0, 8, 10, 12, 14, 16, 18, 20, 22, 24, 26, 28, 30, 32,
@@ -51,17 +54,20 @@ def _get_columns(item):
 
 
 def _check_type_parameters(attrs, type, is_create):
-    req_params = REQUIRED_PARAMETERS[type]
+    req_params = MANDATORY_PARAMETERS[type]
-    notreq_params = list(itertools.chain(
+    opt_params = OPTIONAL_PARAMETERS[type]
-        *[v for k, v in six.iteritems(REQUIRED_PARAMETERS) if k != type]))
+    type_params = req_params | opt_params
+    notreq_params = set(itertools.chain(
+        *[v for k, v in MANDATORY_PARAMETERS.items() if k != type]))
+    notreq_params -= type_params
     if is_create and None in map(attrs.get, req_params):
         msg = (_('"Create" rule command for type "%(rule_type)s" requires '
-                 'arguments %(args)s') % {'rule_type': type,
+                 'arguments %(args)s') %
-                                          'args': ", ".join(req_params)})
+               {'rule_type': type, 'args': ", ".join(sorted(req_params))})
         raise exceptions.CommandError(msg)
-    if set(six.iterkeys(attrs)) & set(notreq_params):
+    if set(attrs.keys()) & notreq_params:
         msg = (_('Rule type "%(rule_type)s" only requires arguments %(args)s')
-               % {'rule_type': type, 'args': ", ".join(req_params)})
+               % {'rule_type': type, 'args': ", ".join(sorted(type_params))})
         raise exceptions.CommandError(msg)
 
 
@@ -183,7 +189,7 @@ class CreateNetworkQosRule(command.ShowOne):
                      RULE_TYPE_DSCP_MARKING,
                      RULE_TYPE_BANDWIDTH_LIMIT],
             help=(_('QoS rule type (%s)') %
-                  ", ".join(six.iterkeys(REQUIRED_PARAMETERS)))
+                  ", ".join(MANDATORY_PARAMETERS.keys()))
         )
         _add_rule_arguments(parser)
         return parser

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

@@ -169,6 +169,8 @@ class NetworkQosRuleTestsBandwidthLimit(common.NetworkTests):
     MAX_KBPS_MODIFIED = 15000
     MAX_BURST_KBITS = 1400
     MAX_BURST_KBITS_MODIFIED = 1800
+    RULE_DIRECTION = 'egress'
+    RULE_DIRECTION_MODIFIED = 'ingress'
     HEADERS = ['ID']
     FIELDS = ['id']
     TYPE = 'bandwidth-limit'
@@ -187,6 +189,7 @@ class NetworkQosRuleTestsBandwidthLimit(common.NetworkTests):
                 '--type ' + cls.TYPE + ' ' +
                 '--max-kbps ' + str(cls.MAX_KBPS) + ' ' +
                 '--max-burst-kbits ' + str(cls.MAX_BURST_KBITS) + ' ' +
+                '--' + cls.RULE_DIRECTION + ' ' +
                 cls.QOS_POLICY_NAME +
                 opts
             )
@@ -226,14 +229,13 @@ class NetworkQosRuleTestsBandwidthLimit(common.NetworkTests):
         self.openstack('network qos rule set --max-kbps ' +
                        str(self.MAX_KBPS_MODIFIED) + ' --max-burst-kbits ' +
                        str(self.MAX_BURST_KBITS_MODIFIED) + ' ' +
+                       '--' + self.RULE_DIRECTION_MODIFIED + ' ' +
                        self.QOS_POLICY_NAME + ' ' + self.RULE_ID)
-        opts = self.get_opts(['max_kbps'])
+        opts = self.get_opts(['direction', 'max_burst_kbps', 'max_kbps'])
         raw_output = self.openstack('network qos rule show ' +
                                     self.QOS_POLICY_NAME + ' ' + self.RULE_ID +
                                     opts)
-        self.assertEqual(str(self.MAX_KBPS_MODIFIED) + "\n", raw_output)
+        expected = (str(self.RULE_DIRECTION_MODIFIED) + "\n" +
-        opts = self.get_opts(['max_burst_kbps'])
+                    str(self.MAX_BURST_KBITS_MODIFIED) + "\n" +
-        raw_output = self.openstack('network qos rule show ' +
+                    str(self.MAX_KBPS_MODIFIED) + "\n")
-                                    self.QOS_POLICY_NAME + ' ' + self.RULE_ID +
+        self.assertEqual(expected, raw_output)
-                                    opts)
-        self.assertEqual(str(self.MAX_BURST_KBITS_MODIFIED) + "\n", raw_output)

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

@@ -921,6 +921,7 @@ class FakeNetworkQosRule(object):
         if type == RULE_TYPE_BANDWIDTH_LIMIT:
             qos_rule_attrs['max_kbps'] = randint(1, 10000)
             qos_rule_attrs['max_burst_kbits'] = randint(1, 10000)
+            qos_rule_attrs['direction'] = 'egress'
         elif type == RULE_TYPE_DSCP_MARKING:
             qos_rule_attrs['dscp_mark'] = choice(VALID_DSCP_MARKS)
         elif type == RULE_TYPE_MINIMUM_BANDWIDTH:

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

@@ -127,7 +127,7 @@ class TestCreateNetworkQosRuleMinimumBandwidth(TestNetworkQosRule):
             self.cmd.take_action(parsed_args)
         except exceptions.CommandError as e:
             msg = ('"Create" rule command for type "minimum-bandwidth" '
-                   'requires arguments min_kbps, direction')
+                   'requires arguments direction, min_kbps')
             self.assertEqual(msg, str(e))
 
 
@@ -229,6 +229,7 @@ class TestCreateNetworkQosRuleBandwidtLimit(TestNetworkQosRule):
         self.new_rule = network_fakes.FakeNetworkQosRule.create_one_qos_rule(
             attrs)
         self.columns = (
+            'direction',
             'id',
             'max_burst_kbits',
             'max_kbps',
@@ -238,6 +239,7 @@ class TestCreateNetworkQosRuleBandwidtLimit(TestNetworkQosRule):
         )
 
         self.data = (
+            self.new_rule.direction,
             self.new_rule.id,
             self.new_rule.max_burst_kbits,
             self.new_rule.max_kbps,
@@ -265,6 +267,7 @@ class TestCreateNetworkQosRuleBandwidtLimit(TestNetworkQosRule):
             '--type', RULE_TYPE_BANDWIDTH_LIMIT,
             '--max-kbps', str(self.new_rule.max_kbps),
             '--max-burst-kbits', str(self.new_rule.max_burst_kbits),
+            '--egress',
             self.new_rule.qos_policy_id,
         ]
 
@@ -272,6 +275,7 @@ class TestCreateNetworkQosRuleBandwidtLimit(TestNetworkQosRule):
             ('type', RULE_TYPE_BANDWIDTH_LIMIT),
             ('max_kbps', self.new_rule.max_kbps),
             ('max_burst_kbits', self.new_rule.max_burst_kbits),
+            ('egress', True),
             ('qos_policy', self.new_rule.qos_policy_id),
         ]
 
@@ -281,7 +285,8 @@ class TestCreateNetworkQosRuleBandwidtLimit(TestNetworkQosRule):
         self.network.create_qos_bandwidth_limit_rule.assert_called_once_with(
             self.qos_policy.id,
             **{'max_kbps': self.new_rule.max_kbps,
-               'max_burst_kbps': self.new_rule.max_burst_kbits}
+               'max_burst_kbps': self.new_rule.max_burst_kbits,
+               'direction': self.new_rule.direction}
         )
         self.assertEqual(self.columns, columns)
         self.assertEqual(self.data, data)
@@ -304,7 +309,7 @@ class TestCreateNetworkQosRuleBandwidtLimit(TestNetworkQosRule):
             self.cmd.take_action(parsed_args)
         except exceptions.CommandError as e:
             msg = ('"Create" rule command for type "bandwidth-limit" '
-                   'requires arguments max_kbps, max_burst_kbps')
+                   'requires arguments max_burst_kbps, max_kbps')
             self.assertEqual(msg, str(e))
 
 
@@ -574,8 +579,8 @@ class TestSetNetworkQosRuleMinimumBandwidth(TestNetworkQosRule):
             self.cmd.take_action(parsed_args)
         except exceptions.CommandError as e:
             msg = ('Failed to set Network QoS rule ID "%(rule)s": Rule type '
-                   '"minimum-bandwidth" only requires arguments min_kbps, '
+                   '"minimum-bandwidth" only requires arguments direction, '
-                   'direction' % {'rule': self.new_rule.id})
+                   'min_kbps' % {'rule': self.new_rule.id})
             self.assertEqual(msg, str(e))
 
 
@@ -716,9 +721,13 @@ class TestSetNetworkQosRuleBandwidthLimit(TestNetworkQosRule):
     def test_set_max_kbps_to_zero(self):
         self._set_max_kbps(max_kbps=0)
 
+    def _reset_max_kbps(self, max_kbps):
+        self.new_rule.max_kbps = max_kbps
+
     def _set_max_kbps(self, max_kbps=None):
         if max_kbps:
-            previous_max_kbps = self.new_rule.max_kbps
+            self.addCleanup(self._reset_max_kbps,
+                            self.new_rule.max_kbps)
             self.new_rule.max_kbps = max_kbps
 
         arglist = [
@@ -742,18 +751,19 @@ class TestSetNetworkQosRuleBandwidthLimit(TestNetworkQosRule):
             self.new_rule, self.qos_policy.id, **attrs)
         self.assertIsNone(result)
 
-        if max_kbps:
-            self.new_rule.max_kbps = previous_max_kbps
-
     def test_set_max_burst_kbits(self):
         self._set_max_burst_kbits()
 
     def test_set_max_burst_kbits_to_zero(self):
         self._set_max_burst_kbits(max_burst_kbits=0)
 
+    def _reset_max_burst_kbits(self, max_burst_kbits):
+        self.new_rule.max_burst_kbits = max_burst_kbits
+
     def _set_max_burst_kbits(self, max_burst_kbits=None):
         if max_burst_kbits:
-            previous_max_burst_kbits = self.new_rule.max_burst_kbits
+            self.addCleanup(self._reset_max_burst_kbits,
+                            self.new_rule.max_burst_kbits)
             self.new_rule.max_burst_kbits = max_burst_kbits
 
         arglist = [
@@ -777,8 +787,38 @@ class TestSetNetworkQosRuleBandwidthLimit(TestNetworkQosRule):
             self.new_rule, self.qos_policy.id, **attrs)
         self.assertIsNone(result)
 
-        if max_burst_kbits:
+    def test_set_direction_egress(self):
-            self.new_rule.max_burst_kbits = previous_max_burst_kbits
+        self._set_direction('egress')
+
+    def test_set_direction_ingress(self):
+        self._set_direction('ingress')
+
+    def _reset_direction(self, direction):
+        self.new_rule.direction = direction
+
+    def _set_direction(self, direction):
+        self.addCleanup(self._reset_direction, self.new_rule.direction)
+
+        arglist = [
+            '--%s' % direction,
+            self.new_rule.qos_policy_id,
+            self.new_rule.id,
+        ]
+        verifylist = [
+            (direction, True),
+            ('qos_policy', self.new_rule.qos_policy_id),
+            ('id', self.new_rule.id),
+        ]
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        result = self.cmd.take_action(parsed_args)
+
+        attrs = {
+            'direction': direction,
+        }
+        self.network.update_qos_bandwidth_limit_rule.assert_called_with(
+            self.new_rule, self.qos_policy.id, **attrs)
+        self.assertIsNone(result)
 
     def test_set_wrong_options(self):
         arglist = [
@@ -797,8 +837,8 @@ class TestSetNetworkQosRuleBandwidthLimit(TestNetworkQosRule):
             self.cmd.take_action(parsed_args)
         except exceptions.CommandError as e:
             msg = ('Failed to set Network QoS rule ID "%(rule)s": Rule type '
-                   '"bandwidth-limit" only requires arguments max_kbps, '
+                   '"bandwidth-limit" only requires arguments direction, '
-                   'max_burst_kbps' % {'rule': self.new_rule.id})
+                   'max_burst_kbps, max_kbps' % {'rule': self.new_rule.id})
             self.assertEqual(msg, str(e))
 
 
@@ -999,6 +1039,7 @@ class TestShowNetworkQosBandwidthLimit(TestNetworkQosRule):
             attrs)
         self.qos_policy.rules = [self.new_rule]
         self.columns = (
+            'direction',
             'id',
             'max_burst_kbits',
             'max_kbps',
@@ -1007,6 +1048,7 @@ class TestShowNetworkQosBandwidthLimit(TestNetworkQosRule):
             'type'
         )
         self.data = (
+            self.new_rule.direction,
             self.new_rule.id,
             self.new_rule.max_burst_kbits,
             self.new_rule.max_kbps,

releasenotes/notes/add-direction-to-network-qos-bw-limit-rule-a3c5b6892074d5ae.yaml

@@ -0,0 +1,8 @@
+---
+features:
+  - |
+    Added directionality to Network QoS rule type ``bandwidth-limit`` in
+    ``network qos rule create`` and ``network qos rule set`` commands.
+    This makes the options ``--egress`` and ``--ingress`` valid for the ``bandwidth-limit``
+    rule type.
+    [Bug `1614121 <https://bugs.launchpad.net/python-openstackclient/+bug/1614121>`_]