LoadBalancer V2: add quotas package#2010
Conversation
Add openstack/loadbalancer/v2/quotas.Get. Add unit and acceptance tests. Signed-off-by: Schlotter, Christian <christian.schlotter@daimler.com>
|
Build succeeded.
|
jtopjian
left a comment
There was a problem hiding this comment.
@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 | |||
There was a problem hiding this comment.
This file needs a build line followed by a blank line. For example: https://github.com/gophercloud/gophercloud/blob/master/acceptance/openstack/loadbalancer/v2/l7policies_test.go#L1
| // 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"` |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
You were totally right. Our OpenStack does currently provide both, also another one having Queens release works fine using loadbalancer :-)
| Pool int `json:"pool"` | ||
|
|
||
| // HealthMonitor represents the number of healthmonitors. A "-1" value means no limit. | ||
| Healthmonitor int `json:"health_monitor"` |
* use the not-deprecated loadbalancer and healthmonitor instead of load_balancer and health_monitor * fix header in acceptance/openstack/loadbalancer/v2/quotas_test.go
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 :-) |
|
Build succeeded.
|
|
hm... this is really odd. OpenLab, which is testing with the Octavia master branch, shows that the underscore formats are returned: 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.
What method are you using to see that both are being returned? The easiest method is to add the
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 If you're really seeing both 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? |
|
Sound good to me, I think I have some distinction:
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 ( Output for lbaasv2@Queens: 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>
|
Build succeeded.
|
|
@jtopjian I added the patch and tests also look good so far :-) |
jtopjian
left a comment
There was a problem hiding this comment.
LGTM - thank you for your help and patience with this!
Add openstack/loadbalancer/v2/quotas.Get.
Add unit and acceptance tests.
For #1826
Links to the line numbers/files in the OpenStack source code that support the
code in this PR:
- https://github.com/openstack/octavia/blob/bf3d5372b9fc670ecd08339fa989c9b738ad8d69/octavia/api/v2/types/quotas.py
- https://github.com/openstack/octavia/blob/master/octavia/api/v2/controllers/quotas.py#L43
- https://github.com/openstack/octavia/blob/0b1d8dd5e76edc4b3058a36b47f6c059f970e516/octavia/api/v2/controllers/base.py#L192
Christian Schlotter , Daimler TSS GmbH, Imprint