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

Add quota_usage endpoint for v2 API. #16

Merged
merged 2 commits into from
May 18, 2021

Conversation

velp
Copy link

@velp velp commented May 17, 2021

This PR adds a new endpoint quota_usage to get all quota usage counters in one API request by project_id. Endpoint URL: /v2/lbaas/quota_usage/{project_id} accept GET request and returns something like:

"quota_usage": {
    "healthmonitor": 1,
    "listener": 1,
    "loadbalancer": 1,
    "pool": 1,
    "member": 2,
    "l7policy": 1,
    "l7rule": 1,
}

For now, this endpoint does several requests to the database to count number of resources, but in the future, we have to use data stored in quota table.

@majewsky
Copy link

majewsky commented May 18, 2021

The quota endpoint uses singular names:

{
  "quota": {
    "load_balancer": 1200,
    "listener": 1200,
    "member": 73,
    "pool": 1200,
    "health_monitor": 200
  }
}

For the sake of consistency, I would prefer the usage endpoint to use the same names. Also of note, later Octavia releases remove the underscores from the names, so it would be loadbalancer and healthmonitor instead, so I would furthermore prefer to use that naming scheme from the get-go here to have consistency with what Octavia will converge upon once we upgrade.

@velp
Copy link
Author

velp commented May 18, 2021

For the sake of consistency, I would prefer the usage endpoint to use the same names.

Endpoint v2/lbaas/qouta/{project_id} already taken https://github.com/sapcc/octavia/blob/stable/stein-m3/octavia/api/v2/controllers/quotas.py#L36 so I cannot use it.

Also of note, later Octavia releases remove the underscores from the names, so it would be loadbalancer and healthmonitor instead, so I would furthermore prefer to use that naming scheme from the get-go here to have consistency with what Octavia will converge upon once we upgrade.

Good point! Will fix

@velp
Copy link
Author

velp commented May 18, 2021

Never mind, I understood what you mean, will change to singular names

Copy link

@majewsky majewsky left a comment

Choose a reason for hiding this comment

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

I cannot comment on the implementation side, but the API side looks good.

Copy link
Collaborator

@notandy notandy left a comment

Choose a reason for hiding this comment

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

lgtm

@velp velp merged commit 9375856 into stable/stein-m3 May 18, 2021
@velp velp deleted the add_basic_quota_usage_endpoint branch May 18, 2021 21:57
@majewsky
Copy link

We found a bug with this: Deleted loadbalancers get counted into the usage value.

@BenjaminLudwigSAP
Copy link
Collaborator

Pushed a fix commit, currently deploying.

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.

4 participants