Merge "nodegroup list with --limit gives wrong next URL"

magnum/api/controllers/v1/nodegroup.py

@@ -180,12 +180,13 @@ class NodeGroupCollection(collection.Collection):
         self._type = 'nodegroups'
 
     @staticmethod
-    def convert(nodegroups, limit, expand=True, **kwargs):
+    def convert(nodegroups, cluster_id, limit, expand=True, **kwargs):
         collection = NodeGroupCollection()
         collection.nodegroups = [NodeGroup.convert(ng, expand)
                                  for ng in nodegroups]
+        url = "clusters/%s/nodegroups" % cluster_id
         collection.next = collection.get_next(limit,
-                                              marker_attribute='id',
+                                              url=url,
                                               **kwargs)
         return collection
 
@@ -217,13 +218,14 @@ class NodeGroupController(base.Controller):
                                             filters=filters)
 
         return NodeGroupCollection.convert(nodegroups,
+                                           cluster_id,
                                            limit,
                                            expand=expand,
                                            sort_key=sort_key,
                                            sort_dir=sort_dir)
 
     @base.Controller.api_version("1.9")
-    @expose.expose(NodeGroupCollection, types.uuid_or_name, int, int,
+    @expose.expose(NodeGroupCollection, types.uuid_or_name, types.uuid, int,
                    wtypes.text, wtypes.text, wtypes.text)
     def get_all(self, cluster_id, marker=None, limit=None, sort_key='id',
                 sort_dir='asc', role=None):

magnum/tests/unit/api/controllers/v1/test_nodegroup.py

@@ -114,11 +114,29 @@ class TestListNodegroups(NodeGroupControllerTest):
         self._test_list_nodegroups(self.cluster.name, expected=expected)
 
     def test_get_all_with_pagination_marker(self):
-        ng_uuid = self.cluster.default_ng_master.uuid
-        url = '/clusters/%s/nodegroups?limit=1&marker=1' % (self.cluster_uuid)
+        worker_ng_uuid = self.cluster.default_ng_worker.uuid
+        master_ng_uuid = self.cluster.default_ng_master.uuid
+        # First make sure that the api returns 1 ng and since they
+        # are sorted by id, the ng should be the default-worker
+        url = '/clusters/%s/nodegroups?limit=1' % (self.cluster_uuid)
         response = self.get_json(url)
         self.assertEqual(1, len(response['nodegroups']))
-        self.assertEqual(ng_uuid, response['nodegroups'][0]['uuid'])
+        self.assertEqual(worker_ng_uuid, response['nodegroups'][0]['uuid'])
+        marker = "marker=%s" % worker_ng_uuid
+        self.assertIn(marker, response['next'])
+        # Now using the next url make sure that we get the default-master
+        next_url = response['next'].split('v1')[1]
+        response = self.get_json(next_url)
+        self.assertEqual(1, len(response['nodegroups']))
+        self.assertEqual(master_ng_uuid, response['nodegroups'][0]['uuid'])
+        marker = "marker=%s" % master_ng_uuid
+        self.assertIn(marker, response['next'])
+        # Now we should not get any other entry since the cluster only has two
+        # nodegroups and the marker is set at the default-master.
+        next_url = response['next'].split('v1')[1]
+        response = self.get_json(next_url)
+        self.assertEqual(0, len(response['nodegroups']))
+        self.assertNotIn('next', response)
 
     def test_get_all_by_role(self):
         filters = {'role': 'master'}

releasenotes/notes/nodegroup-limit-89930d45ee06c621.yaml

@@ -0,0 +1,4 @@
+---
+fixes:
+  - |
+    Fixes the next url in the list nodegroups API response.