Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(kuma-cp) provide total field when listing resources in the HTTP API #723

Merged
merged 17 commits into from
May 12, 2020

Conversation

nickolaev
Copy link
Contributor

Summary

When we fetch entities via the HTTP API (and therefore via the GUI and kumactl) we don't know what is the total number of sites for a particular resource stored in the Kuma control plane. Return a total count of items for each resource that works on both Universal and Kubernetes.

Adds a new total top-level property in the HTTP API responses, so they look like

$ curl localhost:5681/meshes
{
 "total": 3,
 "items": [
  {
   "type": "Mesh",
   "name": "default",
   ...
  },
  {
   "type": "Mesh",
   "name": "default1",
   ...
  },
  {
   "type": "Mesh",
   "name": "default2",
   ...
  }
 ],
 "next": null
}

The implementation in all supported resource store mechanisms (memory, PostgreSQL and Kubernetes) is based on naive resource counting each time a List requests gets in. That can be optimized (cached?) for better scalability once Kuma has to handle huge amount of resources with multiple concurrent API requests happening. This optimisation is left for a follow-up PR.

Full changelog

  • feat(kuma-cp) add naive total implementation for k8s store
  • fix(kuma-cp) 32 bits for Total should be enough
  • chore(*) reorder to make Total on the top of the structure
  • feat(kuma-cp) add support for total count in PostgreSQL store
  • (http/feat/count_entities) fix(*) fix testing when totals introduced
  • feat(kuma-cp) expand ResourceList interface

Issues resolved

None

@nickolaev nickolaev requested a review from a team May 7, 2020 13:23
pkg/core/resources/model/resource.go Outdated Show resolved Hide resolved
pkg/plugins/resources/k8s/store.go Outdated Show resolved Hide resolved
pkg/plugins/resources/postgres/store.go Outdated Show resolved Hide resolved
@bloqhead
Copy link

bloqhead commented May 7, 2020

I've got this working in the GUI. It can be tested in this PR.

@nickolaev nickolaev force-pushed the feat/count_entities branch from 93fa7a2 to 5eab14e Compare May 8, 2020 16:45
@CLAassistant
Copy link

CLAassistant commented May 8, 2020

CLA assistant check
All committers have signed the CLA.

@nickolaev nickolaev force-pushed the feat/count_entities branch from 5eab14e to 3dc20ab Compare May 8, 2020 16:57
return nil
}

func (s *KubernetesStore) countK8sResources(ctx context.Context, rs core_model.ResourceList) (int, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you check if total elements counting works with different meshes?
Create 2 meshes: "default" and "demo"
Create 2 traffic permissions in "default" mesh and in 2 in demo
List traffic permissions by "demo" and see if the total is 2 or 4.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, so I need additional filtering when counting the resources here.

pkg/test/resources/apis/sample/sample_types.go Outdated Show resolved Hide resolved
pkg/plugins/resources/remote/unmarshalling.go Outdated Show resolved Hide resolved
pkg/core/resources/model/resource.go Show resolved Hide resolved
@nickolaev nickolaev force-pushed the feat/count_entities branch 2 times, most recently from da4bf11 to 9bff2b9 Compare May 11, 2020 13:17
Nikolay Nikolaev added 12 commits May 11, 2020 16:35
Add GetItemsCount and SetItemsCount to facilitate the overall
items of that kind. This is expanidn on the pagination ideas
so that one can have the total items available for pagination.

Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
This makes setting values inside actually work.

Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
Now we use GetPagination and use the accessors to the Total and NextOffset
fields.

Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
@nickolaev nickolaev force-pushed the feat/count_entities branch from 9bff2b9 to 2bcaf2e Compare May 11, 2020 14:29
Copy link
Contributor

@jakubdyszkiewicz jakubdyszkiewicz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a test in store_test_template.go it's a test that checks the contract for all the stores, so you will write the test once for all stores.

pkg/plugins/resources/k8s/store.go Show resolved Hide resolved
Nikolay Nikolaev added 2 commits May 12, 2020 10:57
Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
Add a specific test in the K8s List() verification
to account for multi mesh resource filtering.

Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
pkg/plugins/resources/memory/store.go Outdated Show resolved Hide resolved
pkg/plugins/resources/k8s/store_test.go Show resolved Hide resolved
Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
@nickolaev nickolaev merged commit 9c402cf into master May 12, 2020
@nickolaev nickolaev deleted the feat/count_entities branch May 14, 2020 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants