Skip to content

Commit 13327cb

Browse files
drewmalincholical
andauthored
fix(BREV-2099): Handle Shadeform 409 Errors (brevdev#57)
* Shadeform insufficient capacity (brevdev#53) * fix: add insufficent capacity handling for shadeform * fix: uncomment shadeform tests * fix(BREV-2099): Handle shadeform 409 errors * wrap and trace, use errors is in test --------- Co-authored-by: Ronald Ding <ronaldhding@gmail.com>
1 parent 25d97ee commit 13327cb

2 files changed

Lines changed: 130 additions & 5 deletions

File tree

v1/providers/shadeform/instance.go

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@ package v1
22

33
import (
44
"context"
5+
"encoding/json"
56
"fmt"
67
"io"
8+
"net/http"
79
"strings"
810

911
"github.com/alecthomas/units"
@@ -19,8 +21,14 @@ const (
1921
cloudCredRefIDTagName = "cloudCredRefID" //nolint:gosec // not a secret
2022
instanceNameFormat = "%v_%v"
2123
instanceNameSeparator = "_"
24+
outOfStockErrorCode = "OUT_OF_STOCK"
2225
)
2326

27+
type DefaultErrorResponse struct {
28+
ErrorCode string `json:"error_code"`
29+
Error string `json:"error"`
30+
}
31+
2432
func (c *ShadeformClient) CreateInstance(ctx context.Context, attrs v1.CreateInstanceAttrs) (*v1.Instance, error) { //nolint:gocyclo,funlen // ok
2533
authCtx := c.makeAuthContext(ctx)
2634

@@ -110,10 +118,8 @@ func (c *ShadeformClient) CreateInstance(ctx context.Context, attrs v1.CreateIns
110118
defer func() { _ = httpResp.Body.Close() }()
111119
}
112120
if err != nil {
113-
httpMessage, _ := io.ReadAll(httpResp.Body)
114-
return nil, errors.WrapAndTrace(fmt.Errorf("failed to create instance: %w, %s", err, string(httpMessage)))
121+
return nil, errors.WrapAndTrace(fmt.Errorf("failed to create instance: %w", handleShadeformAPIErrorResponse(httpResp)))
115122
}
116-
117123
if resp == nil {
118124
return nil, errors.WrapAndTrace(fmt.Errorf("no instance returned from create request"))
119125
}
@@ -128,6 +134,36 @@ func (c *ShadeformClient) CreateInstance(ctx context.Context, attrs v1.CreateIns
128134
return createdInstance, nil
129135
}
130136

137+
func handleShadeformAPIErrorResponse(httpResp *http.Response) error {
138+
// Read the details of the API response
139+
httpStatusCode := httpResp.StatusCode
140+
httpMessageBytes, err := io.ReadAll(httpResp.Body)
141+
if err != nil {
142+
return errors.WrapAndTrace(fmt.Errorf("failed to read shadeform API response body: %w", err))
143+
}
144+
145+
// Most well-structured errors are 409, so handle these as a special case
146+
if httpStatusCode == http.StatusConflict {
147+
// Unmarshal the response body into a Shadeform DefaultErrorResponse
148+
var shadeformErrorResponse DefaultErrorResponse
149+
err = json.Unmarshal(httpMessageBytes, &shadeformErrorResponse)
150+
if err != nil {
151+
return errors.WrapAndTrace(fmt.Errorf("failed to unmarshal shadeform API response body: %w", err))
152+
}
153+
154+
// Handle Shadeform specific errors and attempt to translate them into Brev errors
155+
if shadeformErrorResponse.ErrorCode == outOfStockErrorCode {
156+
return v1.ErrInsufficientResources
157+
} else {
158+
// For all other error codes, return the error object from the API response
159+
return errors.WrapAndTrace(fmt.Errorf("shadeform API error: error code: %v, error: %v", shadeformErrorResponse.ErrorCode, shadeformErrorResponse.Error))
160+
}
161+
}
162+
163+
// For all other HTTP status codes, return the status code and body
164+
return errors.WrapAndTrace(fmt.Errorf("shadeform HTTP error: [%d], %s", httpStatusCode, string(httpMessageBytes)))
165+
}
166+
131167
func (c *ShadeformClient) getInstanceNameForShadeform(refID string, providedName string) string {
132168
return fmt.Sprintf(instanceNameFormat, refID, providedName)
133169
}

v1/providers/shadeform/validation_test.go

Lines changed: 91 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,19 @@ package v1
22

33
import (
44
"context"
5+
"errors"
6+
"io"
7+
"net/http"
58
"os"
9+
"strings"
610
"testing"
711
"time"
812

9-
"github.com/brevdev/cloud/internal/ssh"
1013
"github.com/brevdev/cloud/internal/validation"
11-
v1 "github.com/brevdev/cloud/v1"
1214
openapi "github.com/brevdev/cloud/v1/providers/shadeform/gen/shadeform"
15+
16+
"github.com/brevdev/cloud/internal/ssh"
17+
v1 "github.com/brevdev/cloud/v1"
1318
"github.com/google/uuid"
1419
"github.com/stretchr/testify/require"
1520
)
@@ -118,6 +123,90 @@ func TestInstanceTypeFilter(t *testing.T) {
118123
})
119124
}
120125

126+
func TestHandleShadeformAPIErrorResponse(t *testing.T) {
127+
tests := []struct {
128+
name string
129+
httpResp *http.Response
130+
expectedError error
131+
}{
132+
{
133+
name: "OutOfStockError",
134+
httpResp: &http.Response{StatusCode: http.StatusConflict, Body: io.NopCloser(strings.NewReader(`{"error_code": "OUT_OF_STOCK", "error": "Out of stock"}`))},
135+
expectedError: v1.ErrInsufficientResources,
136+
},
137+
{
138+
name: "OtherShadeformError",
139+
httpResp: &http.Response{StatusCode: http.StatusConflict, Body: io.NopCloser(strings.NewReader(`{"error_code": "INTERNAL_SERVER_ERROR", "error": "Internal server error"}`))},
140+
expectedError: errors.New(`shadeform API error: error code: INTERNAL_SERVER_ERROR, error: Internal server error`),
141+
},
142+
{
143+
name: "OtherHTTPError",
144+
httpResp: &http.Response{StatusCode: http.StatusInternalServerError, Body: io.NopCloser(strings.NewReader("error"))},
145+
expectedError: errors.New("shadeform HTTP error: [500], error"),
146+
},
147+
}
148+
149+
for _, tt := range tests {
150+
t.Run(tt.name, func(t *testing.T) {
151+
err := handleShadeformAPIErrorResponse(tt.httpResp)
152+
require.Contains(t, err.Error(), tt.expectedError.Error())
153+
})
154+
}
155+
}
156+
157+
func TestOutOfStockError(t *testing.T) {
158+
checkSkip(t)
159+
apiKey := getAPIKey()
160+
161+
ctx, cancel := context.WithTimeout(context.Background(), 15*time.Minute)
162+
t.Cleanup(cancel)
163+
164+
client := NewShadeformClient("validation-test", apiKey)
165+
client.WithConfiguration(Configuration{})
166+
167+
_, err := client.CreateInstance(ctx, v1.CreateInstanceAttrs{
168+
RefID: uuid.New().String(),
169+
InstanceType: "datacrunch_H200x8",
170+
Location: "abc123", // Put a region that is not valid
171+
PublicKey: ssh.GetTestPublicKey(),
172+
Name: "test_name",
173+
FirewallRules: v1.FirewallRules{
174+
EgressRules: []v1.FirewallRule{
175+
{
176+
ID: "test-rule1",
177+
FromPort: 80,
178+
ToPort: 8080,
179+
IPRanges: []string{"127.0.0.1", "10.0.0.0/24"},
180+
},
181+
{
182+
ID: "test-rule2",
183+
FromPort: 5432,
184+
ToPort: 5432,
185+
IPRanges: []string{"127.0.0.1", "10.0.0.0/24"},
186+
},
187+
},
188+
IngressRules: []v1.FirewallRule{
189+
{
190+
ID: "test-rule3",
191+
FromPort: 80,
192+
ToPort: 8080,
193+
IPRanges: []string{"127.0.0.1", "10.0.0.0/24"},
194+
},
195+
{
196+
ID: "test-rule4",
197+
FromPort: 5432,
198+
ToPort: 5432,
199+
IPRanges: []string{"127.0.0.1", "10.0.0.0/24"},
200+
},
201+
},
202+
},
203+
})
204+
if err == nil {
205+
t.Fatalf("ValidateCreateInstance failed: Should have resulted in an insufficientResourcesError")
206+
}
207+
require.True(t, errors.Is(err, v1.ErrInsufficientResources), "Error must be ErrInsufficientResources")
208+
}
209+
121210
func checkSkip(t *testing.T) {
122211
apiKey := getAPIKey()
123212
isValidationTest := os.Getenv("VALIDATION_TEST")

0 commit comments

Comments
 (0)