Support deleting Magnum bays (#38)

* Fix variable name to match standards

* Support deleting a Magnum bay

* Extract the error message from requests that fail with 400

The problem may not actually be due to a bad request and actually
is caused by the bay state, so display a more appropriate error message.

* Move delete error handling into errors.go

* Enable special handling of 409 Conflict errors

* Attempt to unwrap custom error messages from the server

Fallback to the default error message for the status code if
a custom error message isn't present

* Unwrap errors from create and get too

* Preserve the original UnexpectedResponseCode

This lets consumers get a nice message but still inspect the failed
response code, body, etc.

* Remove redundant else statements

* Use standard DeleteResult instead of DeleteHeader

Author: carolynvs

acceptance/openstack/containerorchestration/v1/bay_test.go

@@ -19,8 +19,7 @@ func TestBayCRUDOperations(t *testing.T) {
 	bayModelID := "5b793604-fc76-4886-a834-ed522812cdcb"
 	b, err := bays.Create(Client, bays.CreateOpts{BayModelID: bayModelID}).Extract()
 	th.AssertNoErr(t, err)
-	// TODO: cleanup after ourselves once DELETE is implemented
-	// defer bays.Delete(Client, b.ID)
+	defer bays.Delete(Client, b.ID)
 	th.AssertEquals(t, b.Status, "CREATE_IN_PROGRESS")
 	th.AssertEquals(t, b.BayModelID, bayModelID)
 	th.AssertEquals(t, b.Masters, 1)

errors.go

@@ -87,6 +87,11 @@ type ErrDefault408 struct {
 	ErrUnexpectedResponseCode
 }
 
+// ErrDefault409 is the default error type returned on a 409 HTTP response code.
+type ErrDefault409 struct {
+	ErrUnexpectedResponseCode
+}
+
 // ErrDefault429 is the default error type returned on a 429 HTTP response code.
 type ErrDefault429 struct {
 	ErrUnexpectedResponseCode
@@ -117,6 +122,9 @@ func (e ErrDefault405) Error() string {
 func (e ErrDefault408) Error() string {
 	return "The server timed out waiting for the request"
 }
+func (e ErrDefault409) Error() string {
+	return "Request failed due to a conflict, such as duplicate data or an edit conflict between multiple simultaneous updates."
+}
 func (e ErrDefault429) Error() string {
 	return "Too many requests have been sent in a given amount of time. Pause" +
 		" requests, wait up to one minute, and try again."
@@ -159,6 +167,12 @@ type Err408er interface {
 	Error408(ErrUnexpectedResponseCode) error
 }
 
+// Err409er is the interface resource error types implement to override the error message
+// from a 409 error.
+type Err409er interface {
+	Error409(ErrUnexpectedResponseCode) error
+}
+
 // Err429er is the interface resource error types implement to override the error message
 // from a 429 error.
 type Err429er interface {

openstack/containerorchestration/v1/bays/requests.go

@@ -2,6 +2,7 @@ package bays
 
 import (
 	"github.com/gophercloud/gophercloud"
+	"github.com/gophercloud/gophercloud/openstack/containerorchestration/v1/common"
 	"github.com/gophercloud/gophercloud/pagination"
 )
 
@@ -47,7 +48,8 @@ func List(c *gophercloud.ServiceClient, opts ListOptsBuilder) pagination.Pager {
 
 // Get retrieves a specific bay based on its unique ID.
 func Get(c *gophercloud.ServiceClient, id string) (r GetResult) {
-	_, r.Err = c.Get(getURL(c, id), &r.Body, nil)
+	ro := &gophercloud.RequestOpts{ErrorContext: &common.ErrorResponse{}}
+	_, r.Err = c.Get(getURL(c, id), &r.Body, ro)
 	return
 }
 
@@ -81,6 +83,15 @@ func Create(c *gophercloud.ServiceClient, opts CreateOptsBuilder) (r CreateResul
 		r.Err = err
 		return
 	}
-	_, r.Err = c.Post(createURL(c), b, &r.Body, nil)
+
+	ro := &gophercloud.RequestOpts{ErrorContext: &common.ErrorResponse{}}
+	_, r.Err = c.Post(createURL(c), b, &r.Body, ro)
+	return
+}
+
+// Delete accepts a unique ID and deletes the bay associated with it.
+func Delete(c *gophercloud.ServiceClient, bayID string) (r DeleteResult) {
+	ro := &gophercloud.RequestOpts{ErrorContext: &common.ErrorResponse{}}
+	_, r.Err = c.Delete(deleteURL(c, bayID), ro)
 	return
 }

openstack/containerorchestration/v1/bays/results.go

@@ -26,6 +26,11 @@ type GetResult struct {
 	commonResult
 }
 
+// DeleteResult represents the result of a delete operation.
+type DeleteResult struct {
+	gophercloud.ErrResult
+}
+
 // Represents a Container Orchestration Engine Bay, i.e. a cluster
 type Bay struct {
 	// UUID for the bay

openstack/containerorchestration/v1/bays/testing/requests_test.go

@@ -141,6 +141,39 @@ func TestGet(t *testing.T) {
 	th.AssertEquals(t, b.ID, "a56a6cd8-0779-461b-b1eb-26cec904284a")
 }
 
+func TestGetFailed(t *testing.T) {
+	th.SetupHTTP()
+	defer th.TeardownHTTP()
+
+	th.Mux.HandleFunc("/v1/bays/duplicatename", func(w http.ResponseWriter, r *http.Request) {
+		th.TestMethod(t, r, "GET")
+		th.TestHeader(t, r, "X-Auth-Token", fake.TokenID)
+		w.WriteHeader(http.StatusConflict)
+		fmt.Fprintf(w, `
+{
+  "errors": [
+    {
+      "status": 409,
+      "code": "client",
+      "links": [],
+      "title": "Multiple bays exist with same name",
+      "detail": "Multiple bays exist with same name. Please use the bay uuid instead.",
+      "request_id": ""
+    }
+  ]
+}
+		`)
+	})
+
+	res := bays.Get(fake.ServiceClient(), "duplicatename")
+
+	th.AssertEquals(t, "Multiple bays exist with same name. Please use the bay uuid instead.", res.Err.Error())
+
+	er, ok := res.Err.(*fake.ErrorResponse)
+	th.AssertEquals(t, true, ok)
+	th.AssertEquals(t, http.StatusConflict, er.Actual)
+}
+
 func TestCreate(t *testing.T) {
 	th.SetupHTTP()
 	defer th.TeardownHTTP()
@@ -203,3 +236,85 @@ func TestCreate(t *testing.T) {
 	th.AssertEquals(t, b.Masters, 1)
 	th.AssertEquals(t, b.Nodes, 2)
 }
+
+func TestCreateFailed(t *testing.T) {
+	th.SetupHTTP()
+	defer th.TeardownHTTP()
+
+	th.Mux.HandleFunc("/v1/bays", func(w http.ResponseWriter, r *http.Request) {
+		th.TestMethod(t, r, "POST")
+		th.TestHeader(t, r, "X-Auth-Token", fake.TokenID)
+		w.WriteHeader(http.StatusInternalServerError)
+		fmt.Fprintf(w, `
+{
+  "errors": [
+    {
+      "status": 500,
+      "code": "client",
+      "links": [],
+      "title": "Nova is down",
+      "detail": "Nova is down. Try again later.",
+      "request_id": ""
+    }
+  ]
+}
+		`)
+	})
+
+	options := bays.CreateOpts{Name: "mycluster", Nodes: 2, BayModelID: "5b793604-fc76-4886-a834-ed522812cdcb"}
+
+	res := bays.Create(fake.ServiceClient(), options)
+
+	th.AssertEquals(t, "Nova is down. Try again later.", res.Err.Error())
+
+	er, ok := res.Err.(*fake.ErrorResponse)
+	th.AssertEquals(t, true, ok)
+	th.AssertEquals(t, http.StatusInternalServerError, er.Actual)
+}
+
+func TestDelete(t *testing.T) {
+	th.SetupHTTP()
+	defer th.TeardownHTTP()
+
+	th.Mux.HandleFunc("/v1/bays/a56a6cd8-0779-461b-b1eb-26cec904284a", func(w http.ResponseWriter, r *http.Request) {
+		th.TestMethod(t, r, "DELETE")
+		th.TestHeader(t, r, "X-Auth-Token", fake.TokenID)
+		w.WriteHeader(http.StatusNoContent)
+	})
+
+	res := bays.Delete(fake.ServiceClient(), "a56a6cd8-0779-461b-b1eb-26cec904284a")
+	th.AssertNoErr(t, res.Err)
+}
+
+func TestDeleteFailed(t *testing.T) {
+	th.SetupHTTP()
+	defer th.TeardownHTTP()
+
+	th.Mux.HandleFunc("/v1/bays/a56a6cd8-0779-461b-b1eb-26cec904284a", func(w http.ResponseWriter, r *http.Request) {
+		th.TestMethod(t, r, "DELETE")
+		th.TestHeader(t, r, "X-Auth-Token", fake.TokenID)
+		w.WriteHeader(http.StatusBadRequest)
+		fmt.Fprintf(w, `
+{
+  "errors": [
+    {
+      "status": 400,
+      "code": "client",
+      "links": [],
+      "title": "Bay k8sbay already has an operation in progress",
+      "detail": "Bay k8sbay already has an operation in progress.",
+      "request_id": ""
+    }
+  ]
+}
+		`)
+	})
+
+	res := bays.Delete(fake.ServiceClient(), "a56a6cd8-0779-461b-b1eb-26cec904284a")
+
+	th.AssertEquals(t, "Bay k8sbay already has an operation in progress.", res.Err.Error())
+
+	er, ok := res.Err.(*fake.ErrorResponse)
+	th.AssertEquals(t, true, ok)
+	th.AssertEquals(t, http.StatusBadRequest, er.Actual)
+}

openstack/containerorchestration/v1/bays/urls.go

@@ -2,14 +2,18 @@ package bays
 
 import "github.com/gophercloud/gophercloud"
 
-func createURL(client *gophercloud.ServiceClient) string {
-	return client.ServiceURL("bays")
+func createURL(c *gophercloud.ServiceClient) string {
+	return c.ServiceURL("bays")
 }
 
-func listURL(client *gophercloud.ServiceClient) string {
-	return client.ServiceURL("bays")
+func listURL(c *gophercloud.ServiceClient) string {
+	return c.ServiceURL("bays")
 }
 
-func getURL(client *gophercloud.ServiceClient, id string) string {
-	return client.ServiceURL("bays", id)
+func getURL(c *gophercloud.ServiceClient, id string) string {
+	return c.ServiceURL("bays", id)
+}
+
+func deleteURL(c *gophercloud.ServiceClient, id string) string {
+	return c.ServiceURL("bays", id)
 }

openstack/containerorchestration/v1/common/errors.go

@@ -0,0 +1,116 @@
+package common
+
+import (
+	"encoding/json"
+	"github.com/gophercloud/gophercloud"
+	"strings"
+)
+
+// ErrorResponse represents failed response
+type ErrorResponse struct {
+	gophercloud.ErrUnexpectedResponseCode
+	Errors []Error `json:"errors" required:"true"`
+}
+
+// Error represents a Magnum API error message
+type Error struct {
+	Status    int    `json:"status"`
+	Code      string `json:"code"`
+	Title     string `json:"title"`
+	Detail    string `json:"detail"`
+	RequestID string `json:"request_id,omitempty"`
+}
+
+// Error returns the error message details from the server
+func (e *ErrorResponse) Error() string {
+	errors := []string{}
+	for _, e := range e.Errors {
+		errors = append(errors, e.Detail)
+	}
+	return strings.Join(errors, "\n")
+}
+
+func (e *ErrorResponse) unwrapError(r gophercloud.ErrUnexpectedResponseCode) bool {
+	e.ErrUnexpectedResponseCode = r
+	err := json.Unmarshal(r.Body, &e)
+	return err == nil
+}
+
+// Error400 extracts the actual error message from the body of the response
+func (e *ErrorResponse) Error400(r gophercloud.ErrUnexpectedResponseCode) error {
+	if e.unwrapError(r) {
+		return e
+	}
+
+	return gophercloud.ErrDefault400{ErrUnexpectedResponseCode: r}
+}
+
+// Error401 extracts the actual error message from the body of the response
+func (e *ErrorResponse) Error401(r gophercloud.ErrUnexpectedResponseCode) error {
+	if e.unwrapError(r) {
+		return e
+	}
+	return gophercloud.ErrDefault401{ErrUnexpectedResponseCode: r}
+}
+
+// Error404 extracts the actual error message from the body of the response
+func (e *ErrorResponse) Error404(r gophercloud.ErrUnexpectedResponseCode) error {
+	if e.unwrapError(r) {
+		return e
+	}
+	return gophercloud.ErrDefault404{ErrUnexpectedResponseCode: r}
+}
+
+// Error405 extracts the actual error message from the body of the response
+func (e *ErrorResponse) Error405(r gophercloud.ErrUnexpectedResponseCode) error {
+	if e.unwrapError(r) {
+		return e
+	}
+
+	return gophercloud.ErrDefault405{ErrUnexpectedResponseCode: r}
+}
+
+// Error408 extracts the actual error message from the body of the response
+func (e *ErrorResponse) Error408(r gophercloud.ErrUnexpectedResponseCode) error {
+	if e.unwrapError(r) {
+		return e
+	}
+
+	return gophercloud.ErrDefault408{ErrUnexpectedResponseCode: r}
+}
+
+// Error409 extracts the actual error message from the body of the response
+func (e *ErrorResponse) Error409(r gophercloud.ErrUnexpectedResponseCode) error {
+	if e.unwrapError(r) {
+		return e
+	}
+
+	return gophercloud.ErrDefault409{ErrUnexpectedResponseCode: r}
+}
+
+// Error429 extracts the actual error message from the body of the response
+func (e *ErrorResponse) Error429(r gophercloud.ErrUnexpectedResponseCode) error {
+	if e.unwrapError(r) {
+		return e
+	}
+
+	return gophercloud.ErrDefault429{ErrUnexpectedResponseCode: r}
+}
+
+// Error500 extracts the actual error message from the body of the response
+func (e *ErrorResponse) Error500(r gophercloud.ErrUnexpectedResponseCode) error {
+	if e.unwrapError(r) {
+		return e
+	}
+
+	return gophercloud.ErrDefault500{ErrUnexpectedResponseCode: r}
+}
+
+// Error503 extracts the actual error message from the body of the response
+func (e *ErrorResponse) Error503(r gophercloud.ErrUnexpectedResponseCode) error {
+	if e.unwrapError(r) {
+		return e
+	}
+
+	return gophercloud.ErrDefault503{ErrUnexpectedResponseCode: r}
+}

provider_client.go

@@ -254,6 +254,11 @@ func (client *ProviderClient) Request(method, url string, options *RequestOpts)
 			if error408er, ok := errType.(Err408er); ok {
 				err = error408er.Error408(respErr)
 			}
+		case http.StatusConflict:
+			err = ErrDefault409{respErr}
+			if error409er, ok := errType.(Err409er); ok {
+				err = error409er.Error409(respErr)
+			}
 		case 429:
 			err = ErrDefault429{respErr}
 			if error429er, ok := errType.(Err429er); ok {