Merge pull request #437 from smashwilson/clb-creation-err

Rackspace CLB ExtractNodes call does not propagate errors correctly

Author: smashwilson

pagination/http.go

@@ -36,13 +36,19 @@ func PageResultFrom(resp *http.Response) (PageResult, error) {
 		parsedBody = rawBody
 	}
 
+	return PageResultFromParsed(resp, parsedBody), err
+}
+
+// PageResultFromParsed constructs a PageResult from an HTTP response that has already had its
+// body parsed as JSON (and closed).
+func PageResultFromParsed(resp *http.Response, body interface{}) PageResult {
 	return PageResult{
 		Result: gophercloud.Result{
-			Body:   parsedBody,
+			Body:   body,
 			Header: resp.Header,
 		},
 		URL: *resp.Request.URL,
-	}, err
+	}
 }
 
 // Request performs an HTTP request and extracts the http.Response from the result.

rackspace/lb/v1/nodes/fixtures.go

@@ -107,6 +107,42 @@ func mockCreateResponse(t *testing.T, lbID int) {
 	})
 }
 
+func mockCreateErrResponse(t *testing.T, lbID int) {
+	th.Mux.HandleFunc(_rootURL(lbID), func(w http.ResponseWriter, r *http.Request) {
+		th.TestMethod(t, r, "POST")
+		th.TestHeader(t, r, "X-Auth-Token", fake.TokenID)
+
+		th.TestJSONRequest(t, r, `
+{
+  "nodes": [
+    {
+      "address": "10.2.2.3",
+      "port": 80,
+      "condition": "ENABLED",
+      "type": "PRIMARY"
+    },
+    {
+      "address": "10.2.2.4",
+      "port": 81,
+      "condition": "ENABLED",
+      "type": "SECONDARY"
+    }
+  ]
+}
+    `)
+
+		w.Header().Add("Content-Type", "application/json")
+		w.WriteHeader(422) // Unprocessable Entity
+
+		fmt.Fprintf(w, `
+{
+  "code": 422,
+  "message": "Load Balancer '%d' has a status of 'PENDING_UPDATE' and is considered immutable."
+}
+  `, lbID)
+	})
+}
+
 func mockBatchDeleteResponse(t *testing.T, lbID int, ids []int) {
 	th.Mux.HandleFunc(_rootURL(lbID), func(w http.ResponseWriter, r *http.Request) {
 		th.TestMethod(t, r, "DELETE")

rackspace/lb/v1/nodes/requests.go

@@ -119,12 +119,7 @@ func Create(client *gophercloud.ServiceClient, loadBalancerID int, opts CreateOp
 		return res
 	}
 
-	pr, err := pagination.PageResultFrom(resp)
-	if err != nil {
-		res.Err = err
-		return res
-	}
-
+	pr := pagination.PageResultFromParsed(resp, res.Body)
 	return CreateResult{pagination.SinglePageBase(pr)}
 }
 

rackspace/lb/v1/nodes/requests_test.go

@@ -108,6 +108,38 @@ func TestCreate(t *testing.T) {
 	th.CheckDeepEquals(t, expected, actual)
 }
 
+func TestCreateErr(t *testing.T) {
+	th.SetupHTTP()
+	defer th.TeardownHTTP()
+
+	mockCreateErrResponse(t, lbID)
+
+	opts := CreateOpts{
+		CreateOpt{
+			Address:   "10.2.2.3",
+			Port:      80,
+			Condition: ENABLED,
+			Type:      PRIMARY,
+		},
+		CreateOpt{
+			Address:   "10.2.2.4",
+			Port:      81,
+			Condition: ENABLED,
+			Type:      SECONDARY,
+		},
+	}
+
+	page := Create(client.ServiceClient(), lbID, opts)
+
+	actual, err := page.ExtractNodes()
+	if err == nil {
+		t.Fatal("Did not receive expected error from ExtractNodes")
+	}
+	if actual != nil {
+		t.Fatalf("Received non-nil result from failed ExtractNodes: %#v", actual)
+	}
+}
+
 func TestBulkDelete(t *testing.T) {
 	th.SetupHTTP()
 	defer th.TeardownHTTP()

rackspace/lb/v1/nodes/results.go

@@ -126,6 +126,9 @@ type CreateResult struct {
 
 // ExtractNodes extracts a slice of Node structs from a CreateResult.
 func (res CreateResult) ExtractNodes() ([]Node, error) {
+	if res.Err != nil {
+		return nil, res.Err
+	}
 	return commonExtractNodes(res.Body)
 }