Add update tests for policies and rules

This patch adds tests for 'update' for both policies and rules. This
completes the CRUD tests and leaves the association/disassociation for
a later patch. Note that deleting a rule isn't tested explicitly
because of a bug with the rule delete logic on the server side. Once
that code is fixed, the test should be added.

to_dict() for policies should also convert any rules inside rule lists
to dicts too, otherwise API layer receives rule object __repr__ strings
instead of actual dicts. This patch introduces a fix to the existing
to_dict() code to properly support policies.

This patch also modifies the base infra to create policies and rules for
admins and not for tenant.

Partially-Implements: blueprint quantum-qos-api
Change-Id: I13870680d7756be9dd020135bc8e91d1c12f728d
Co-Authored-By: Ihar Hrachyshka <ihrachys@redhat.com>

diff --git a/neutron/objects/base.py b/neutron/objects/base.py
index 5e1f59263710907ad3c2863594ffa8503b733fc5..cf51cf3d20ef44ae43543bf9666bd578a1a3cbe2 100644 (file)
--- a/neutron/objects/base.py
+++ b/neutron/objects/base.py
@@ -18,15 +18,11 @@ import six
 from neutron.db import api as db_api
 
 
-# TODO(QoS): revisit dict compatibility and how we can isolate dict behavior
-
-
 @six.add_metaclass(abc.ABCMeta)
 class NeutronObject(obj_base.VersionedObject,
                     obj_base.VersionedObjectDictCompat,
                     obj_base.ComparableVersionedObject):
 
-    # TODO(QoS): this should be revisited on how we plan to work with dicts
     def to_dict(self):
         return dict(self.items())
 

diff --git a/neutron/objects/qos/policy.py b/neutron/objects/qos/policy.py
index 53c34a9934b55182e91c0a48790848fac369c96f..51602a3eafbdab984850bdd6dcd9646f35f39a91 100644 (file)
--- a/neutron/objects/qos/policy.py
+++ b/neutron/objects/qos/policy.py
@@ -65,6 +65,13 @@ class QosPolicy(base.NeutronDbObject):
 
     fields_no_update = ['id', 'tenant_id']
 
+    def to_dict(self):
+        dict_ = super(QosPolicy, self).to_dict()
+        for field in self.rule_fields:
+            if field in dict_:
+                dict_[field] = [rule.to_dict() for rule in dict_[field]]
+        return dict_
+
     def obj_load_attr(self, attrname):
         if attrname not in self.rule_fields:
             raise exceptions.ObjectActionError(

diff --git a/neutron/tests/api/base.py b/neutron/tests/api/base.py
index f23e52826f4cb2a24764a9bbdec8dca15b386a2e..0e8b6fffda873e31404576f736b6cb954752d75a 100644 (file)
--- a/neutron/tests/api/base.py
+++ b/neutron/tests/api/base.py
@@ -111,11 +111,11 @@ class BaseNetworkTest(neutron.tests.tempest.test.BaseTestCase):
                                          fw_rule['id'])
             # Clean up QoS policies
             for qos_policy in cls.qos_policies:
-                cls._try_delete_resource(cls.client.delete_qos_policy,
+                cls._try_delete_resource(cls.admin_client.delete_qos_policy,
                                          qos_policy['id'])
             # Clean up QoS rules
             for qos_rule in cls.qos_rules:
-                cls._try_delete_resource(cls.client.delete_qos_rule,
+                cls._try_delete_resource(cls.admin_client.delete_qos_rule,
                                          qos_rule['id'])
             # Clean up ike policies
             for ikepolicy in cls.ikepolicies:
@@ -444,7 +444,7 @@ class BaseNetworkTest(neutron.tests.tempest.test.BaseTestCase):
     @classmethod
     def create_qos_policy(cls, name, description, shared):
         """Wrapper utility that returns a test QoS policy."""
-        body = cls.client.create_qos_policy(name, description, shared)
+        body = cls.admin_client.create_qos_policy(name, description, shared)
         qos_policy = body['policy']
         cls.qos_policies.append(qos_policy)
         return qos_policy
@@ -453,7 +453,7 @@ class BaseNetworkTest(neutron.tests.tempest.test.BaseTestCase):
     def create_qos_bandwidth_limit_rule(cls, policy_id,
                                        max_kbps, max_burst_kbps):
         """Wrapper utility that returns a test QoS bandwidth limit rule."""
-        body = cls.client.create_bandwidth_limit_rule(
+        body = cls.admin_client.create_bandwidth_limit_rule(
             policy_id, max_kbps, max_burst_kbps)
         qos_rule = body['bandwidth_limit_rule']
         cls.qos_rules.append(qos_rule)

diff --git a/neutron/tests/api/test_qos.py b/neutron/tests/api/test_qos.py
index a1247039795616307c28070a99f292558f6facbe..3683b4628882b7459acc6723b78ef9420f280be5 100644 (file)
--- a/neutron/tests/api/test_qos.py
+++ b/neutron/tests/api/test_qos.py
@@ -12,6 +12,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from tempest_lib import exceptions
+
 from neutron.services.qos import qos_consts
 from neutron.tests.api import base
 from neutron.tests.tempest import config
@@ -47,9 +49,39 @@ class QosTestJSON(base.BaseAdminNetworkTest):
         policies_ids = [p['id'] for p in policies]
         self.assertIn(policy['id'], policies_ids)
 
+    @test.attr(type='smoke')
+    @test.idempotent_id('8e88a54b-f0b2-4b7d-b061-a15d93c2c7d6')
+    def test_policy_update(self):
+        policy = self.create_qos_policy(name='test-policy',
+                                        description='',
+                                        shared=False)
+        self.admin_client.update_qos_policy(policy['id'],
+                                            description='test policy desc',
+                                            shared=True)
+
+        retrieved_policy = self.admin_client.show_qos_policy(policy['id'])
+        retrieved_policy = retrieved_policy['policy']
+        self.assertEqual('test policy desc', retrieved_policy['description'])
+        self.assertEqual(True, retrieved_policy['shared'])
+        self.assertEqual([], retrieved_policy['bandwidth_limit_rules'])
+
+    @test.attr(type='smoke')
+    @test.idempotent_id('1cb42653-54bd-4a9a-b888-c55e18199201')
+    def test_delete_policy(self):
+        policy = self.admin_client.create_qos_policy(
+            'test-policy', 'desc', True)['policy']
+
+        retrieved_policy = self.admin_client.show_qos_policy(policy['id'])
+        retrieved_policy = retrieved_policy['policy']
+        self.assertEqual('test-policy', retrieved_policy['name'])
+
+        self.admin_client.delete_qos_policy(policy['id'])
+        self.assertRaises(exceptions.ServerFault,
+                          self.admin_client.show_qos_policy, policy['id'])
+
     @test.attr(type='smoke')
     @test.idempotent_id('8a59b00b-3e9c-4787-92f8-93a5cdf5e378')
-    def test_create_rule(self):
+    def test_bandwidth_limit_rule_create(self):
         policy = self.create_qos_policy(name='test-policy',
                                         description='test policy',
                                         shared=False)
@@ -58,12 +90,12 @@ class QosTestJSON(base.BaseAdminNetworkTest):
                                                     max_burst_kbps=1337)
 
         # Test 'show rule'
-        retrieved_policy = self.admin_client.show_bandwidth_limit_rule(
+        retrieved_rule = self.admin_client.show_bandwidth_limit_rule(
             policy['id'], rule['id'])
-        retrieved_policy = retrieved_policy['bandwidth_limit_rule']
-        self.assertEqual(rule['id'], retrieved_policy['id'])
-        self.assertEqual(200, retrieved_policy['max_kbps'])
-        self.assertEqual(1337, retrieved_policy['max_burst_kbps'])
+        retrieved_rule = retrieved_rule['bandwidth_limit_rule']
+        self.assertEqual(rule['id'], retrieved_rule['id'])
+        self.assertEqual(200, retrieved_rule['max_kbps'])
+        self.assertEqual(1337, retrieved_rule['max_burst_kbps'])
 
         # Test 'list rules'
         rules = self.admin_client.list_bandwidth_limit_rules(policy['id'])
@@ -71,6 +103,52 @@ class QosTestJSON(base.BaseAdminNetworkTest):
         rules_ids = [r['id'] for r in rules]
         self.assertIn(rule['id'], rules_ids)
 
+        # Test 'show policy'
+        retrieved_policy = self.admin_client.show_qos_policy(policy['id'])
+        policy_rules = retrieved_policy['policy']['bandwidth_limit_rules']
+        self.assertEqual(1, len(policy_rules))
+        self.assertEqual(rule['id'], policy_rules[0]['id'])
+
+    @test.idempotent_id('149a6988-2568-47d2-931e-2dbc858943b3')
+    def test_bandwidth_limit_rule_update(self):
+        policy = self.create_qos_policy(name='test-policy',
+                                        description='test policy',
+                                        shared=False)
+        rule = self.create_qos_bandwidth_limit_rule(policy_id=policy['id'],
+                                                    max_kbps=1,
+                                                    max_burst_kbps=1)
+
+        self.admin_client.update_bandwidth_limit_rule(policy['id'],
+                                                      rule['id'],
+                                                      max_kbps=200,
+                                                      max_burst_kbps=1337)
+
+        retrieved_policy = self.admin_client.show_bandwidth_limit_rule(
+            policy['id'], rule['id'])
+        retrieved_policy = retrieved_policy['bandwidth_limit_rule']
+        self.assertEqual(200, retrieved_policy['max_kbps'])
+        self.assertEqual(1337, retrieved_policy['max_burst_kbps'])
+
+    #TODO(QoS): Uncomment once the rule-delete logic is fixed.
+#    @test.attr(type='smoke')
+#    @test.idempotent_id('67ee6efd-7b33-4a68-927d-275b4f8ba958')
+#    def test_bandwidth_limit_rule_delete(self):
+#        policy = self.create_qos_policy(name='test-policy',
+#                                        description='test policy',
+#                                        shared=False)
+#        rule = self.admin_client.create_bandwidth_limit_rule(
+#            policy['id'], 200, 1337)['bandwidth_limit_rule']
+#
+#        retrieved_policy = self.admin_client.show_bandwidth_limit_rule(
+#            policy['id'], rule['id'])
+#        retrieved_policy = retrieved_policy['bandwidth_limit_rule']
+#        self.assertEqual(rule['id'], retrieved_policy['id'])
+#
+#        self.admin_client.delete_bandwidth_limit_rule(policy['id'], rule['id']
+#        self.assertRaises(exceptions.ServerFault,
+#                          self.admin_client.show_bandwidth_limit_rule,
+#                          policy['id'], rule['id'])
+
     @test.attr(type='smoke')
     @test.idempotent_id('cf776f77-8d3d-49f2-8572-12d6a1557224')
     def test_list_rule_types(self):
@@ -90,9 +168,7 @@ class QosTestJSON(base.BaseAdminNetworkTest):
         for rule in expected_rule_types:
             self.assertIn(rule, actual_rule_types)
 
-    #TODO(QoS): policy update (name)
     #TODO(QoS): create several bandwidth-limit rules (not sure it makes sense,
     #           but to test more than one rule)
-    #TODO(QoS): update bandwidth-limit rule
     #TODO(QoS): associate/disassociate policy with network
     #TODO(QoS): associate/disassociate policy with port

diff --git a/neutron/tests/tempest/services/network/json/network_client.py b/neutron/tests/tempest/services/network/json/network_client.py
index b17fa4864453724f7789dc0288e9d0e9652530c0..bc8eaa2c04bfb78885a7c7443dcbcde8660fcca8 100644 (file)
--- a/neutron/tests/tempest/services/network/json/network_client.py
+++ b/neutron/tests/tempest/services/network/json/network_client.py
@@ -645,6 +645,14 @@ class NetworkClientJSON(service_client.ServiceClient):
         self.expected_success(201, resp.status)
         return service_client.ResponseBody(resp, body)
 
+    def update_qos_policy(self, policy_id, **kwargs):
+        uri = '%s/qos/policies/%s' % (self.uri_prefix, policy_id)
+        post_data = self.serialize({'policy': kwargs})
+        resp, body = self.put(uri, post_data)
+        body = self.deserialize_single(body)
+        self.expected_success(200, resp.status)
+        return service_client.ResponseBody(resp, body)
+
     def get_qos_policy(self, policy_id):
         uri = '%s/qos/policies/%s' % (self.uri_prefix, policy_id)
         resp, body = self.get(uri)
@@ -681,19 +689,22 @@ class NetworkClientJSON(service_client.ServiceClient):
         self.expected_success(200, resp.status)
         return service_client.ResponseBody(resp, body)
 
-    def update_bandwidth_limit_rule(self, policy_id, rule_id,
-                                    max_kbps, max_burst_kbps):
+    def update_bandwidth_limit_rule(self, policy_id, rule_id, **kwargs):
         uri = '%s/qos/policies/%s/bandwidth_limit_rules/%s' % (
             self.uri_prefix, policy_id, rule_id)
-        post_data = {
-            'bandwidth_limit_rule': {
-                'max_kbps': max_kbps,
-                'max_burst_kbps': max_burst_kbps}
-            }
+        post_data = {'bandwidth_limit_rule': kwargs}
         resp, body = self.put(uri, json.dumps(post_data))
+        body = self.deserialize_single(body)
         self.expected_success(200, resp.status)
         return service_client.ResponseBody(resp, body)
 
+    def delete_bandwidth_limit_rule(self, policy_id, rule_id):
+        uri = '%s/qos/policies/%s/bandwidth_limit_rules/%s' % (
+            self.uri_prefix, policy_id, rule_id)
+        resp, body = self.delete(uri)
+        self.expected_success(204, resp.status)
+        return service_client.ResponseBody(resp, body)
+
     def list_qos_rule_types(self):
         uri = '%s/qos/rule-types' % self.uri_prefix
         resp, body = self.get(uri)

diff --git a/neutron/tests/unit/objects/qos/test_policy.py b/neutron/tests/unit/objects/qos/test_policy.py
index ed8a1bf55b831250a7b3cfc3cd6afcd6827d07be..9369f03a8c63c749893c7a1d96980a36e6985f44 100644 (file)
--- a/neutron/tests/unit/objects/qos/test_policy.py
+++ b/neutron/tests/unit/objects/qos/test_policy.py
@@ -230,3 +230,18 @@ class QosPolicyDbObjectTestCase(test_base.BaseDbObjectTestCase,
         primitive = policy_obj.obj_to_primitive()
         self.assertNotEqual([], (primitive['versioned_object.data']
                                           ['bandwidth_limit_rules']))
+
+    def test_to_dict_returns_rules_as_dicts(self):
+        policy_obj, rule_obj = self._create_test_policy_with_rule()
+        policy_obj = policy.QosPolicy.get_by_id(self.context, policy_obj.id)
+
+        obj_dict = policy_obj.to_dict()
+        rule_dict = rule_obj.to_dict()
+
+        # first make sure that to_dict() is still sane and does not return
+        # objects
+        for obj in (rule_dict, obj_dict):
+            self.assertIsInstance(obj, dict)
+
+        self.assertEqual(rule_dict,
+                         obj_dict['bandwidth_limit_rules'][0])