Skip to content

LoadBalancer V2: add quotas package#2010

Merged
jtopjian merged 3 commits intogophercloud:masterfrom
mercedes-benz:loadbalancersv2-quotas-get
Sep 10, 2020
Merged

LoadBalancer V2: add quotas package#2010
jtopjian merged 3 commits intogophercloud:masterfrom
mercedes-benz:loadbalancersv2-quotas-get

Conversation

@chrischdi
Copy link
Copy Markdown
Contributor

Add openstack/loadbalancer/v2/quotas.Get.

Add unit and acceptance tests.

Signed-off-by: Schlotter, Christian <christian.schlotter@daimler.com>
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.01%) to 79.526% when pulling 081a2ed on Daimler:loadbalancersv2-quotas-get into 77b238f on gophercloud:master.

@coveralls
Copy link
Copy Markdown

coveralls commented Aug 27, 2020

Coverage Status

Coverage increased (+0.04%) to 79.547% when pulling 5d38f6e on Daimler:loadbalancersv2-quotas-get into 77b238f on gophercloud:master.

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci bot commented Aug 27, 2020

Build succeeded.

Copy link
Copy Markdown
Contributor

@jtopjian jtopjian left a comment

Choose a reason for hiding this comment

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

@chrischdi I'm sorry for the late review on this.

I've added a few comments - please let me know if you have any questions.

@@ -0,0 +1,23 @@
package v2
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 55e3ace

// Quota contains load balancer quotas for a project.
type Quota struct {
// Loadbalancer represents the number of load balancers. A "-1" value means no limit.
Loadbalancer int `json:"load_balancer"`
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

According to https://github.com/openstack/octavia/blob/bd0206a6ea4efa342fe73e0d099238ca78ca1208/releasenotes/notes/Correct-naming-for-quota-resources-8e4309a839208cd1.yaml, load_balancer and health_monitor were deprecated 2 years ago.

This is also noted throughout the quotas type code (https://github.com/openstack/octavia/blob/master/octavia/api/v2/types/quotas.py#L25).

Can you confirm if this is just for the objects named throughout the Python code and not actually the JSON response? In other words, are you seeing both load_balancer and loadbalancer being returned?

If so, we'll want to change this to loadbalancer so we use the future-proof version.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You were totally right. Our OpenStack does currently provide both, also another one having Queens release works fine using loadbalancer :-)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 55e3ace

Pool int `json:"pool"`

// HealthMonitor represents the number of healthmonitors. A "-1" value means no limit.
Healthmonitor int `json:"health_monitor"`
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Similar here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Same as above: Fixed in 55e3ace

* use the not-deprecated loadbalancer and healthmonitor instead of
  load_balancer and health_monitor
* fix header in acceptance/openstack/loadbalancer/v2/quotas_test.go
@chrischdi
Copy link
Copy Markdown
Contributor Author

@chrischdi I'm sorry for the late review on this.

I've added a few comments - please let me know if you have any questions.

Hi @jtopjian thanks for you review. I think I addressed your points.

I'll now wait for the openlab test and see if I made another mistake there :-)

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci bot commented Sep 7, 2020

Build succeeded.

@jtopjian
Copy link
Copy Markdown
Contributor

jtopjian commented Sep 7, 2020

hm... this is really odd. OpenLab, which is testing with the Octavia master branch, shows that the underscore formats are returned:

2020-09-07 15:14:35.641292 | ubuntu-bionic | === RUN   TestQuotasGet
2020-09-07 15:14:35.682822 | ubuntu-bionic | 2020/09/07 15:14:35 [DEBUG] OpenStack Request URL: GET http://192.168.0.7/load-balancer/v2.0/quotas/admin
2020-09-07 15:14:35.682962 | ubuntu-bionic | 2020/09/07 15:14:35 [DEBUG] OpenStack request Headers:
2020-09-07 15:14:35.683004 | ubuntu-bionic | Accept: application/json
2020-09-07 15:14:35.683045 | ubuntu-bionic | User-Agent: gophercloud/2.0.0
2020-09-07 15:14:35.683073 | ubuntu-bionic | X-Auth-Token: ***
2020-09-07 15:14:35.776302 | ubuntu-bionic | 2020/09/07 15:14:35 [DEBUG] OpenStack Response Code: 200
2020-09-07 15:14:35.776489 | ubuntu-bionic | 2020/09/07 15:14:35 [DEBUG] OpenStack Response Headers:
2020-09-07 15:14:35.776540 | ubuntu-bionic | Content-Length: 126
2020-09-07 15:14:35.776599 | ubuntu-bionic | Content-Type: application/json
2020-09-07 15:14:35.776664 | ubuntu-bionic | Date: Mon, 07 Sep 2020 15:14:35 GMT
2020-09-07 15:14:35.776722 | ubuntu-bionic | Server: Apache/2.4.29 (Ubuntu)
2020-09-07 15:14:35.776850 | ubuntu-bionic | X-Openstack-Request-Id: req-bb6e92b2-ad2f-442a-8848-c04c77ef3cdf
2020-09-07 15:14:35.776943 | ubuntu-bionic | 2020/09/07 15:14:35 [DEBUG] OpenStack Response Body: {
2020-09-07 15:14:35.776976 | ubuntu-bionic |   "quota": {
2020-09-07 15:14:35.777027 | ubuntu-bionic |     "health_monitor": -1,
2020-09-07 15:14:35.777070 | ubuntu-bionic |     "l7policy": -1,
2020-09-07 15:14:35.777109 | ubuntu-bionic |     "l7rule": -1,
2020-09-07 15:14:35.777152 | ubuntu-bionic |     "listener": -1,
2020-09-07 15:14:35.777201 | ubuntu-bionic |     "load_balancer": -1,
2020-09-07 15:14:35.777255 | ubuntu-bionic |     "member": -1,
2020-09-07 15:14:35.777293 | ubuntu-bionic |     "pool": -1
2020-09-07 15:14:35.777313 | ubuntu-bionic |   }
2020-09-07 15:14:35.777331 | ubuntu-bionic | }
2020-09-07 15:14:35.777390 | ubuntu-bionic | --- PASS: TestQuotasGet (0.14s)
2020-09-07 15:14:35.777431 | ubuntu-bionic |     tools.go:80: {
2020-09-07 15:14:35.777486 | ubuntu-bionic |           "loadbalancer": 0,
2020-09-07 15:14:35.777629 | ubuntu-bionic |           "listener": -1,
2020-09-07 15:14:35.777698 | ubuntu-bionic |           "member": -1,
2020-09-07 15:14:35.777743 | ubuntu-bionic |           "pool": -1,
2020-09-07 15:14:35.777799 | ubuntu-bionic |           "healthmonitor": 0,
2020-09-07 15:14:35.777850 | ubuntu-bionic |           "l7policy": -1,
2020-09-07 15:14:35.777908 | ubuntu-bionic |           "l7rule": -1
2020-09-07 15:14:35.777938 | ubuntu-bionic |         }
2020-09-07 15:14:35.777973 | ubuntu-bionic | PASS

I should have checked this previously. I thought the test would not have run because of the missing build tags.

I'm at a loss as to why the underscores are being returned in OpenLab. Digging into this more, it appears that while changes were made to the Octavia API service, the Python client/library has still not been updated to account for this (https://storyboard.openstack.org/#!/story/2002997). While Gophercloud does not leverage the Python client at all, the Python client being out of sync with the API code points to inconsistency within OpenStack itself. This might get even more messier once we start implementing POST and PUT.

Our OpenStack does currently provide both

What method are you using to see that both are being returned?

The easiest method is to add the --debug flag to the openstack command-line tool. This will print the JSON requests and responses.

also another one having Queens release works fine using loadbalancer

One thing to note is that if a struct field with a JSON tag does not exist in the JSON response, the value will be set to the "zero-value" of the type. You can see this above with loadbalancer and healthmonitor being set to 0 instead of -1.

If you're really seeing both load_balancer and loadbalancer in the JSON responses, then we should account for both of them in Gophercloud. I put together the following which should work:

diff --git a/openstack/loadbalancer/v2/quotas/results.go b/openstack/loadbalancer/v2/quotas/results.go
index 0ccd3a2e..f201745a 100644
--- a/openstack/loadbalancer/v2/quotas/results.go
+++ b/openstack/loadbalancer/v2/quotas/results.go
@@ -1,6 +1,9 @@
 package quotas

-import "github.com/gophercloud/gophercloud"
+import (
+       "encoding/json"
+       "github.com/gophercloud/gophercloud"
+)

 type commonResult struct {
        gophercloud.Result
@@ -24,7 +27,7 @@ type GetResult struct {
 // Quota contains load balancer quotas for a project.
 type Quota struct {
        // Loadbalancer represents the number of load balancers. A "-1" value means no limit.
-       Loadbalancer int `json:"loadbalancer"`
+       Loadbalancer int `json:"-"`

        // Listener represents the number of listeners. A "-1" value means no limit.
        Listener int `json:"listener"`
@@ -36,7 +39,7 @@ type Quota struct {
        Pool int `json:"pool"`

        // HealthMonitor represents the number of healthmonitors. A "-1" value means no limit.
-       Healthmonitor int `json:"healthmonitor"`
+       Healthmonitor int `json:"-"`

        // L7Policy represents the number of l7policies. A "-1" value means no limit.
        L7Policy int `json:"l7policy"`
@@ -44,3 +47,42 @@ type Quota struct {
        // L7Rule represents the number of l7rules. A "-1" value means no limit.
        L7Rule int `json:"l7rule"`
 }
+
+func (r *Quota) UnmarshalJSON(b []byte) error {
+       type tmp Quota
+
+       // Support both underscore and non-underscore naming.
+       var s struct {
+               tmp
+               LoadBalancer *int `json:"load_balancer"`
+               Loadbalancer *int `json:"loadbalancer"`
+
+               HealthMonitor *int `json:"health_monitor"`
+               Healthmonitor *int `json:"healthmonitor"`
+       }
+
+       err := json.Unmarshal(b, &s)
+       if err != nil {
+               return err
+       }
+
+       *r = Quota(s.tmp)
+
+       if s.LoadBalancer != nil {
+               r.Loadbalancer = *s.LoadBalancer
+       }
+
+       if s.Loadbalancer != nil {
+               r.Loadbalancer = *s.Loadbalancer
+       }
+
+       if s.HealthMonitor != nil {
+               r.Healthmonitor = *s.HealthMonitor
+       }
+
+       if s.Healthmonitor != nil {
+               r.Healthmonitor = *s.Healthmonitor
+       }
+
+       return nil
+}
diff --git a/openstack/loadbalancer/v2/quotas/testing/fixtures.go b/openstack/loadbalancer/v2/quotas/testing/fixtures.go
index 8bffb358..e180557a 100644
--- a/openstack/loadbalancer/v2/quotas/testing/fixtures.go
+++ b/openstack/loadbalancer/v2/quotas/testing/fixtures.go
@@ -2,7 +2,7 @@ package testing

 import "github.com/gophercloud/gophercloud/openstack/loadbalancer/v2/quotas"

-const GetResponseRaw = `
+const GetResponseRaw_1 = `
 {
     "quota": {
         "loadbalancer": 15,
@@ -16,6 +16,20 @@ const GetResponseRaw = `
 }
 `

+const GetResponseRaw_2 = `
+{
+    "quota": {
+        "load_balancer": 15,
+        "listener": 30,
+        "member": -1,
+        "pool": 15,
+        "health_monitor": 30,
+        "l7policy": 100,
+        "l7rule": -1
+    }
+}
+`
+
 var GetResponse = quotas.Quota{
        Loadbalancer:  15,
        Listener:      30,
diff --git a/openstack/loadbalancer/v2/quotas/testing/requests_test.go b/openstack/loadbalancer/v2/quotas/testing/requests_test.go
index 39a4c962..4bf9872e 100644
--- a/openstack/loadbalancer/v2/quotas/testing/requests_test.go
+++ b/openstack/loadbalancer/v2/quotas/testing/requests_test.go
@@ -10,7 +10,7 @@ import (
        th "github.com/gophercloud/gophercloud/testhelper"
 )

-func TestGet(t *testing.T) {
+func TestGet_1(t *testing.T) {
        th.SetupHTTP()
        defer th.TeardownHTTP()

@@ -21,7 +21,26 @@ func TestGet(t *testing.T) {
                w.Header().Add("Content-Type", "application/json")
                w.WriteHeader(http.StatusOK)

-               fmt.Fprintf(w, GetResponseRaw)
+               fmt.Fprintf(w, GetResponseRaw_1)
+       })
+
+       q, err := quotas.Get(fake.ServiceClient(), "0a73845280574ad389c292f6a74afa76").Extract()
+       th.AssertNoErr(t, err)
+       th.AssertDeepEquals(t, q, &GetResponse)
+}
+
+func TestGet_2(t *testing.T) {
+       th.SetupHTTP()
+       defer th.TeardownHTTP()
+
+       th.Mux.HandleFunc("/v2.0/quotas/0a73845280574ad389c292f6a74afa76", func(w http.ResponseWriter, r *http.Request) {
+               th.TestMethod(t, r, "GET")
+               th.TestHeader(t, r, "X-Auth-Token", fake.TokenID)
+
+               w.Header().Add("Content-Type", "application/json")
+               w.WriteHeader(http.StatusOK)
+
+               fmt.Fprintf(w, GetResponseRaw_2)
        })

        q, err := quotas.Get(fake.ServiceClient(), "0a73845280574ad389c292f6a74afa76").Extract()

If you agree with the proposed changes, feel free to integrate this diff into your branch.

Thoughts?

@chrischdi
Copy link
Copy Markdown
Contributor Author

Sound good to me, I think I have some distinction:

  • Octavia still returns health_monitor and load_balancer (Canonical Openstack at Ocata Release)
  • Neutron returns healthmonitor and loadbalancer in my case (running VMware Integrated OpenStack at Queens Release)

Example app:

package main

import (
	"fmt"
	"net/http"
	"os"

	"github.com/gophercloud/gophercloud/openstack"
	"github.com/gophercloud/utils/client"
	osClient "github.com/gophercloud/utils/client"

	"github.com/gophercloud/gophercloud"
	"github.com/gophercloud/gophercloud/openstack/loadbalancer/v2/quotas"
)

func main() {
	authOptions := gophercloud.AuthOptions{
		IdentityEndpoint: os.Getenv("OS_AUTH_URL"),
		Username:         os.Getenv("OS_USERNAME"),
		Password:         os.Getenv("OS_PASSWORD"),
		DomainName:       os.Getenv("OS_DOMAIN_NAME"),
		TenantID:         os.Getenv("OS_PROJECT_ID"),
		AllowReauth:      true,
	}
	endpointOpts := gophercloud.EndpointOpts{Region: os.Getenv("OS_REGION_NAME")}

	fmt.Printf("%++v\n", authOptions)

	providerClient, err := openstack.AuthenticatedClient(authOptions)
	if err != nil {
		fmt.Printf("err: %v\n", err)
		os.Exit(1)
	}
	// add debugging
	providerClient.HTTPClient = http.Client{
		Transport: &client.RoundTripper{
			Rt:     &http.Transport{Proxy: http.ProxyFromEnvironment},
			Logger: &osClient.DefaultLogger{},
		},
	}

	var client *gophercloud.ServiceClient
	if os.Getenv("USE_OCTAVIA") == "true" {
		client, err = openstack.NewLoadBalancerV2(providerClient, endpointOpts)
	} else {
		client, err = openstack.NewNetworkV2(providerClient, endpointOpts)
	}
	if err != nil {
		fmt.Printf("err: %v\n", err)
		os.Exit(1)
	}

	quotasInfo, err := quotas.Get(client, authOptions.TenantID).Extract()
	if err != nil {
		fmt.Printf("err: %v\n", err)
		os.Exit(1)
	}
	fmt.Printf("%++v\n", quotasInfo)
}

Output when using Octavia@Ocata as client (USE_OCTAVIA=true):

$ go run ./acceptance/openstack/loadbalancer/v2/test/main.go
2020/09/08 13:57:08 [DEBUG] OpenStack Request URL: GET https://REPLACED_URL:9876/v2.0/quotas/aaac70f4956549798ba3494a85933873
2020/09/08 13:57:08 [DEBUG] OpenStack Request Headers:
Accept: application/json
User-Agent: gophercloud/2.0.0
X-Auth-Token: ***
2020/09/08 13:57:08 [DEBUG] OpenStack Response Code: 200
2020/09/08 13:57:08 [DEBUG] OpenStack Response Headers:
Content-Length: 96
Content-Type: application/json
Date: Tue, 08 Sep 2020 11:57:08 GMT
Server: Apache/2.4.29 (Ubuntu)
X-Openstack-Request-Id: req-7fc6e26c-6318-47b9-a2a6-6c105e1a20e0
2020/09/08 13:57:08 [DEBUG] OpenStack Response Body: {
  "quota": {
    "health_monitor": 50,
    "listener": 50,
    "load_balancer": 20,
    "member": -1,
    "pool": 50
  }
}
&{Loadbalancer:0 Listener:50 Member:-1 Pool:50 Healthmonitor:0 L7Policy:0 L7Rule:0}

Output for lbaasv2@Queens:

$ go run ./acceptance/openstack/loadbalancer/v2/test/main.go
2020/09/08 13:55:41 [DEBUG] OpenStack Request URL: GET https://REPLACED_URL:9696/v2.0/quotas/1b08db53fdec4c498a15ad7d027eac4f
2020/09/08 13:55:41 [DEBUG] OpenStack Request Headers:
Accept: application/json
User-Agent: gophercloud/2.0.0
X-Auth-Token: ***
2020/09/08 13:55:42 [DEBUG] OpenStack Response Code: 200
2020/09/08 13:55:42 [DEBUG] OpenStack Response Headers:
Content-Length: 377
Content-Type: application/json
Date: Tue, 08 Sep 2020 11:55:42 GMT
Server: Apache/2.4.18 (Ubuntu)
X-Openstack-Request-Id: req-0693e5b8-deaf-4531-8953-cc3c6b8f9be3
2020/09/08 13:55:42 [DEBUG] OpenStack Response Body: {
  "quota": {
    "firewall": 10,
    "firewall_policy": 10,
    "firewall_rule": 500,
    "floatingip": 14,
    "healthmonitor": 50,
    "housekeeper": -1,
    "l2-gateway-connection": -1,
    "l7policy": -1,
    "listener": 50,
    "loadbalancer": 20,
    "member": -1,
    "network": 5,
    "pool": 50,
    "port": 50,
    "rbac_policy": 10,
    "router": 10,
    "security_group": 40,
    "security_group_rule": 100,
    "subnet": 5,
    "subnetpool": -1
  }
}
&{Loadbalancer:20 Listener:50 Member:-1 Pool:50 Healthmonitor:50 L7Policy:-1 L7Rule:0}

So I'll integrate your suggested diff so we will be compatible to both :-)

Thank you very much!

Support the current and deprecated syntaxes:
* `loadbalancer` and `load_balancer`
* `healthmonitor` and `health_monitor`

Signed-off-by: Schlotter, Christian <christian.schlotter@daimler.com>
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci bot commented Sep 9, 2020

Build succeeded.

@chrischdi
Copy link
Copy Markdown
Contributor Author

@jtopjian I added the patch and tests also look good so far :-)

Copy link
Copy Markdown
Contributor

@jtopjian jtopjian left a comment

Choose a reason for hiding this comment

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

LGTM - thank you for your help and patience with this!

@jtopjian jtopjian merged commit 0491243 into gophercloud:master Sep 10, 2020
@chrischdi chrischdi deleted the loadbalancersv2-quotas-get branch September 10, 2020 12:09
@chrischdi chrischdi restored the loadbalancersv2-quotas-get branch September 11, 2020 08:50
@kayrus kayrus mentioned this pull request Jun 14, 2025
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.

3 participants