From 9b6d02d5f958e5b2ea6155879fd0514a15ce2231 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C5=82awek=20Kap=C5=82o=C5=84ski?= <slawek@kaplonski.pl>
Date: Fri, 11 May 2018 13:04:23 +0200
Subject: [PATCH] Make max_burst_kbps option as optional for bw limit QoS rule

Attribute max_burst_kbps of QoS bandwidth limit rule in Neutron's
is optional in API so it should be also optional on client's side.

Change-Id: Ie085b73fa885ff12f9ac080666cf3ca6a09b632a
Related-Bug:#1770622
Task: 19658
Story: 2002017
---
 .../network/v2/network_qos_rule.py            |  8 +--
 .../network/v2/test_network_qos_rule.py       |  1 -
 .../unit/network/v2/test_network_qos_rule.py  | 55 +++++++++++++++++--
 3 files changed, 53 insertions(+), 11 deletions(-)

diff --git a/openstackclient/network/v2/network_qos_rule.py b/openstackclient/network/v2/network_qos_rule.py
index f50e58b32d..9c4275a82b 100644
--- a/openstackclient/network/v2/network_qos_rule.py
+++ b/openstackclient/network/v2/network_qos_rule.py
@@ -29,11 +29,11 @@ RULE_TYPE_MINIMUM_BANDWIDTH = 'minimum-bandwidth'
 MANDATORY_PARAMETERS = {
     RULE_TYPE_MINIMUM_BANDWIDTH: {'min_kbps', 'direction'},
     RULE_TYPE_DSCP_MARKING: {'dscp_mark'},
-    RULE_TYPE_BANDWIDTH_LIMIT: {'max_kbps', 'max_burst_kbps'}}
+    RULE_TYPE_BANDWIDTH_LIMIT: {'max_kbps'}}
 OPTIONAL_PARAMETERS = {
     RULE_TYPE_MINIMUM_BANDWIDTH: set(),
     RULE_TYPE_DSCP_MARKING: set(),
-    RULE_TYPE_BANDWIDTH_LIMIT: {'direction'}}
+    RULE_TYPE_BANDWIDTH_LIMIT: {'direction', 'max_burst_kbps'}}
 DIRECTION_EGRESS = 'egress'
 DIRECTION_INGRESS = 'ingress'
 DSCP_VALID_MARKS = [0, 8, 10, 12, 14, 16, 18, 20, 22, 24, 26, 28, 30, 32,
@@ -62,11 +62,11 @@ def _check_type_parameters(attrs, type, is_create):
     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') %
+                 'arguments: %(args)s') %
                {'rule_type': type, 'args': ", ".join(sorted(req_params))})
         raise exceptions.CommandError(msg)
     if set(attrs.keys()) & notreq_params:
-        msg = (_('Rule type "%(rule_type)s" only requires arguments %(args)s')
+        msg = (_('Rule type "%(rule_type)s" only requires arguments: %(args)s')
                % {'rule_type': type, 'args': ", ".join(sorted(type_params))})
         raise exceptions.CommandError(msg)
 
diff --git a/openstackclient/tests/functional/network/v2/test_network_qos_rule.py b/openstackclient/tests/functional/network/v2/test_network_qos_rule.py
index 5d235405c5..98e588e82d 100644
--- a/openstackclient/tests/functional/network/v2/test_network_qos_rule.py
+++ b/openstackclient/tests/functional/network/v2/test_network_qos_rule.py
@@ -167,7 +167,6 @@ class NetworkQosRuleTestsBandwidthLimit(common.NetworkTests):
             'network qos rule create -f json '
             '--type bandwidth-limit '
             '--max-kbps 10000 '
-            '--max-burst-kbits 1400 '
             '--egress %s' %
             self.QOS_POLICY_NAME
         ))
diff --git a/openstackclient/tests/unit/network/v2/test_network_qos_rule.py b/openstackclient/tests/unit/network/v2/test_network_qos_rule.py
index 176bc86d2c..5b54d318ac 100644
--- a/openstackclient/tests/unit/network/v2/test_network_qos_rule.py
+++ b/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 direction, min_kbps')
+                   'requires arguments: direction, min_kbps')
             self.assertEqual(msg, str(e))
 
 
@@ -213,7 +213,7 @@ class TestCreateNetworkQosRuleDSCPMarking(TestNetworkQosRule):
             self.cmd.take_action(parsed_args)
         except exceptions.CommandError as e:
             msg = ('"Create" rule command for type "dscp-marking" '
-                   'requires arguments dscp_mark')
+                   'requires arguments: dscp_mark')
             self.assertEqual(msg, str(e))
 
 
@@ -263,6 +263,49 @@ class TestCreateNetworkQosRuleBandwidtLimit(TestNetworkQosRule):
                           self.cmd, arglist, verifylist)
 
     def test_create_default_options(self):
+        arglist = [
+            '--type', RULE_TYPE_BANDWIDTH_LIMIT,
+            '--max-kbps', str(self.new_rule.max_kbps),
+            '--egress',
+            self.new_rule.qos_policy_id,
+        ]
+
+        verifylist = [
+            ('type', RULE_TYPE_BANDWIDTH_LIMIT),
+            ('max_kbps', self.new_rule.max_kbps),
+            ('egress', True),
+            ('qos_policy', self.new_rule.qos_policy_id),
+        ]
+
+        rule = network_fakes.FakeNetworkQosRule.create_one_qos_rule(
+            {'qos_policy_id': self.qos_policy.id,
+             'type': RULE_TYPE_BANDWIDTH_LIMIT})
+        rule.max_burst_kbits = 0
+        expected_data = (
+            rule.direction,
+            rule.id,
+            rule.max_burst_kbits,
+            rule.max_kbps,
+            rule.project_id,
+            rule.qos_policy_id,
+            rule.type,
+        )
+
+        with mock.patch.object(
+                self.network, "create_qos_bandwidth_limit_rule",
+                return_value=rule) as create_qos_bandwidth_limit_rule:
+            parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+            columns, data = (self.cmd.take_action(parsed_args))
+
+        create_qos_bandwidth_limit_rule.assert_called_once_with(
+            self.qos_policy.id,
+            **{'max_kbps': self.new_rule.max_kbps,
+               'direction': self.new_rule.direction}
+        )
+        self.assertEqual(self.columns, columns)
+        self.assertEqual(expected_data, data)
+
+    def test_create_all_options(self):
         arglist = [
             '--type', RULE_TYPE_BANDWIDTH_LIMIT,
             '--max-kbps', str(self.new_rule.max_kbps),
@@ -309,7 +352,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_burst_kbps, max_kbps')
+                   'requires arguments: max_kbps')
             self.assertEqual(msg, str(e))
 
 
@@ -579,7 +622,7 @@ 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 direction, '
+                   '"minimum-bandwidth" only requires arguments: direction, '
                    'min_kbps' % {'rule': self.new_rule.id})
             self.assertEqual(msg, str(e))
 
@@ -673,7 +716,7 @@ class TestSetNetworkQosRuleDSCPMarking(TestNetworkQosRule):
             self.cmd.take_action(parsed_args)
         except exceptions.CommandError as e:
             msg = ('Failed to set Network QoS rule ID "%(rule)s": Rule type '
-                   '"dscp-marking" only requires arguments dscp_mark' %
+                   '"dscp-marking" only requires arguments: dscp_mark' %
                    {'rule': self.new_rule.id})
             self.assertEqual(msg, str(e))
 
@@ -837,7 +880,7 @@ 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 direction, '
+                   '"bandwidth-limit" only requires arguments: direction, '
                    'max_burst_kbps, max_kbps' % {'rule': self.new_rule.id})
             self.assertEqual(msg, str(e))