From c35587491c4cb83564c7bc7cf0fc8f9bf2c7f473 Mon Sep 17 00:00:00 2001 From: Ronald Ding Date: Mon, 3 Nov 2025 17:04:48 -0800 Subject: [PATCH 1/2] fix: add insufficent capacity handling for shadeform --- v1/providers/shadeform/instance.go | 32 +++- v1/providers/shadeform/validation_test.go | 174 ++++++++++++++-------- 2 files changed, 143 insertions(+), 63 deletions(-) diff --git a/v1/providers/shadeform/instance.go b/v1/providers/shadeform/instance.go index 8175cd6c..3a85d4d2 100644 --- a/v1/providers/shadeform/instance.go +++ b/v1/providers/shadeform/instance.go @@ -2,6 +2,7 @@ package v1 import ( "context" + "encoding/json" "fmt" "io" "strings" @@ -21,6 +22,15 @@ const ( instanceNameSeparator = "_" ) +const ( + OutOfStockErrorCode = "OUT_OF_STOCK" +) + +type DefaultErrorResponse struct { + ErrorCode string `json:"error_code"` + Error string `json:"error"` +} + func (c *ShadeformClient) CreateInstance(ctx context.Context, attrs v1.CreateInstanceAttrs) (*v1.Instance, error) { //nolint:gocyclo,funlen // ok authCtx := c.makeAuthContext(ctx) @@ -110,8 +120,26 @@ func (c *ShadeformClient) CreateInstance(ctx context.Context, attrs v1.CreateIns defer func() { _ = httpResp.Body.Close() }() } if err != nil { - httpMessage, _ := io.ReadAll(httpResp.Body) - return nil, errors.WrapAndTrace(fmt.Errorf("failed to create instance: %w, %s", err, string(httpMessage))) + if httpResp.StatusCode == 409 { + // Shadeform provides more details on 409 errors in the response body + httpMessage, _ := io.ReadAll(httpResp.Body) + jsonStr := string(httpMessage) + + var errorResponse DefaultErrorResponse + err = json.Unmarshal([]byte(jsonStr), &errorResponse) + if err != nil { + return nil, errors.WrapAndTrace(fmt.Errorf("failed to create instance: %w, %s", err, string(httpMessage))) + } + + if errorResponse.ErrorCode == OutOfStockErrorCode { + return nil, v1.ErrInsufficientResources + } else { + return nil, errors.WrapAndTrace(fmt.Errorf("failed to create instance: %w, %s", err, string(httpMessage))) + } + } else { + httpMessage, _ := io.ReadAll(httpResp.Body) + return nil, errors.WrapAndTrace(fmt.Errorf("failed to create instance: %w, %s", err, string(httpMessage))) + } } if resp == nil { diff --git a/v1/providers/shadeform/validation_test.go b/v1/providers/shadeform/validation_test.go index d62756bc..e2b70787 100644 --- a/v1/providers/shadeform/validation_test.go +++ b/v1/providers/shadeform/validation_test.go @@ -7,39 +7,117 @@ import ( "time" "github.com/brevdev/cloud/internal/ssh" - "github.com/brevdev/cloud/internal/validation" v1 "github.com/brevdev/cloud/v1" - openapi "github.com/brevdev/cloud/v1/providers/shadeform/gen/shadeform" "github.com/google/uuid" "github.com/stretchr/testify/require" ) -func TestValidationFunctions(t *testing.T) { - t.Parallel() - checkSkip(t) - apiKey := getAPIKey() - - config := validation.ProviderConfig{ - Credential: NewShadeformCredential("validation-test", apiKey), - StableIDs: []v1.InstanceTypeID{"datacrunch_B200_helsinki-finland-5", "massedcompute_L40_desmoines-usa-1"}, - } - - validation.RunValidationSuite(t, config) -} - -func TestInstanceLifecycleValidation(t *testing.T) { - t.Parallel() - checkSkip(t) - apiKey := getAPIKey() - - config := validation.ProviderConfig{ - Credential: NewShadeformCredential("validation-test", apiKey), - } - - validation.RunInstanceLifecycleValidation(t, config) -} - -func TestInstanceTypeFilter(t *testing.T) { +// +//func TestValidationFunctions(t *testing.T) { +// t.Parallel() +// checkSkip(t) +// apiKey := getAPIKey() +// +// config := validation.ProviderConfig{ +// Credential: NewShadeformCredential("validation-test", apiKey), +// StableIDs: []v1.InstanceTypeID{"datacrunch_B200_helsinki-finland-5", "massedcompute_L40_desmoines-usa-1"}, +// } +// +// validation.RunValidationSuite(t, config) +//} +// +//func TestInstanceLifecycleValidation(t *testing.T) { +// t.Parallel() +// checkSkip(t) +// apiKey := getAPIKey() +// +// config := validation.ProviderConfig{ +// Credential: NewShadeformCredential("validation-test", apiKey), +// } +// +// validation.RunInstanceLifecycleValidation(t, config) +//} +// +//func TestInstanceTypeFilter(t *testing.T) { +// checkSkip(t) +// apiKey := getAPIKey() +// +// ctx, cancel := context.WithTimeout(context.Background(), 15*time.Minute) +// t.Cleanup(cancel) +// +// client := NewShadeformClient("validation-test", apiKey) +// client.WithConfiguration(Configuration{ +// AllowedInstanceTypes: map[openapi.Cloud]map[string]bool{ +// openapi.HYPERSTACK: { +// "A4000": true, +// }, +// }, +// }) +// +// types, err := client.GetInstanceTypes(ctx, v1.GetInstanceTypeArgs{}) +// require.NoError(t, err) +// require.NotEmpty(t, types, "Should have instance types") +// require.True(t, len(types) == 1, "Instance types should return only one entry") +// require.True(t, types[0].Type == "hyperstack_A4000", "returned instance type does not match expectations") +// +// if !types[0].IsAvailable { +// return +// } +// +// instance, err := client.CreateInstance(ctx, v1.CreateInstanceAttrs{ +// RefID: uuid.New().String(), +// InstanceType: types[0].Type, +// Location: types[0].Location, +// PublicKey: ssh.GetTestPublicKey(), +// Name: "test_name", +// FirewallRules: v1.FirewallRules{ +// EgressRules: []v1.FirewallRule{ +// { +// ID: "test-rule1", +// FromPort: 80, +// ToPort: 8080, +// IPRanges: []string{"127.0.0.1", "10.0.0.0/24"}, +// }, +// { +// ID: "test-rule2", +// FromPort: 5432, +// ToPort: 5432, +// IPRanges: []string{"127.0.0.1", "10.0.0.0/24"}, +// }, +// }, +// IngressRules: []v1.FirewallRule{ +// { +// ID: "test-rule3", +// FromPort: 80, +// ToPort: 8080, +// IPRanges: []string{"127.0.0.1", "10.0.0.0/24"}, +// }, +// { +// ID: "test-rule4", +// FromPort: 5432, +// ToPort: 5432, +// IPRanges: []string{"127.0.0.1", "10.0.0.0/24"}, +// }, +// }, +// }, +// }) +// if err != nil { +// t.Fatalf("ValidateCreateInstance failed: %v", err) +// } +// require.NotNil(t, instance) +// +// t.Run("ValidateSSHAccessible", func(t *testing.T) { +// err := v1.ValidateInstanceSSHAccessible(ctx, client, instance, ssh.GetTestPrivateKey()) +// require.NoError(t, err, "ValidateSSHAccessible should pass") +// }) +// +// t.Run("ValidateTerminateInstance", func(t *testing.T) { +// err := v1.ValidateTerminateInstance(ctx, client, instance) +// require.NoError(t, err, "ValidateTerminateInstance should pass") +// }) +//} + +func TestOutOfStockError(t *testing.T) { checkSkip(t) apiKey := getAPIKey() @@ -47,28 +125,12 @@ func TestInstanceTypeFilter(t *testing.T) { t.Cleanup(cancel) client := NewShadeformClient("validation-test", apiKey) - client.WithConfiguration(Configuration{ - AllowedInstanceTypes: map[openapi.Cloud]map[string]bool{ - openapi.HYPERSTACK: { - "A4000": true, - }, - }, - }) - - types, err := client.GetInstanceTypes(ctx, v1.GetInstanceTypeArgs{}) - require.NoError(t, err) - require.NotEmpty(t, types, "Should have instance types") - require.True(t, len(types) == 1, "Instance types should return only one entry") - require.True(t, types[0].Type == "hyperstack_A4000", "returned instance type does not match expectations") + client.WithConfiguration(Configuration{}) - if !types[0].IsAvailable { - return - } - - instance, err := client.CreateInstance(ctx, v1.CreateInstanceAttrs{ + _, err := client.CreateInstance(ctx, v1.CreateInstanceAttrs{ RefID: uuid.New().String(), - InstanceType: types[0].Type, - Location: types[0].Location, + InstanceType: "datacrunch_H200x8", + Location: "abc123", // Put a region that is not valid PublicKey: ssh.GetTestPublicKey(), Name: "test_name", FirewallRules: v1.FirewallRules{ @@ -102,20 +164,10 @@ func TestInstanceTypeFilter(t *testing.T) { }, }, }) - if err != nil { - t.Fatalf("ValidateCreateInstance failed: %v", err) + if err == nil { + t.Fatalf("ValidateCreateInstance failed: Should have resulted in an insufficientResourcesError") } - require.NotNil(t, instance) - - t.Run("ValidateSSHAccessible", func(t *testing.T) { - err := v1.ValidateInstanceSSHAccessible(ctx, client, instance, ssh.GetTestPrivateKey()) - require.NoError(t, err, "ValidateSSHAccessible should pass") - }) - - t.Run("ValidateTerminateInstance", func(t *testing.T) { - err := v1.ValidateTerminateInstance(ctx, client, instance) - require.NoError(t, err, "ValidateTerminateInstance should pass") - }) + require.True(t, err.Error() == v1.ErrInsufficientResources.Error(), "Error must be ErrInsufficientResources") } func checkSkip(t *testing.T) { From 4859f475d49de6685242fd27f305779cdabc5bbf Mon Sep 17 00:00:00 2001 From: Ronald Ding Date: Mon, 3 Nov 2025 17:09:57 -0800 Subject: [PATCH 2/2] fix: uncomment shadeform tests --- v1/providers/shadeform/validation_test.go | 209 +++++++++++----------- 1 file changed, 105 insertions(+), 104 deletions(-) diff --git a/v1/providers/shadeform/validation_test.go b/v1/providers/shadeform/validation_test.go index e2b70787..65a5f60c 100644 --- a/v1/providers/shadeform/validation_test.go +++ b/v1/providers/shadeform/validation_test.go @@ -2,6 +2,8 @@ package v1 import ( "context" + "github.com/brevdev/cloud/internal/validation" + openapi "github.com/brevdev/cloud/v1/providers/shadeform/gen/shadeform" "os" "testing" "time" @@ -12,110 +14,109 @@ import ( "github.com/stretchr/testify/require" ) -// -//func TestValidationFunctions(t *testing.T) { -// t.Parallel() -// checkSkip(t) -// apiKey := getAPIKey() -// -// config := validation.ProviderConfig{ -// Credential: NewShadeformCredential("validation-test", apiKey), -// StableIDs: []v1.InstanceTypeID{"datacrunch_B200_helsinki-finland-5", "massedcompute_L40_desmoines-usa-1"}, -// } -// -// validation.RunValidationSuite(t, config) -//} -// -//func TestInstanceLifecycleValidation(t *testing.T) { -// t.Parallel() -// checkSkip(t) -// apiKey := getAPIKey() -// -// config := validation.ProviderConfig{ -// Credential: NewShadeformCredential("validation-test", apiKey), -// } -// -// validation.RunInstanceLifecycleValidation(t, config) -//} -// -//func TestInstanceTypeFilter(t *testing.T) { -// checkSkip(t) -// apiKey := getAPIKey() -// -// ctx, cancel := context.WithTimeout(context.Background(), 15*time.Minute) -// t.Cleanup(cancel) -// -// client := NewShadeformClient("validation-test", apiKey) -// client.WithConfiguration(Configuration{ -// AllowedInstanceTypes: map[openapi.Cloud]map[string]bool{ -// openapi.HYPERSTACK: { -// "A4000": true, -// }, -// }, -// }) -// -// types, err := client.GetInstanceTypes(ctx, v1.GetInstanceTypeArgs{}) -// require.NoError(t, err) -// require.NotEmpty(t, types, "Should have instance types") -// require.True(t, len(types) == 1, "Instance types should return only one entry") -// require.True(t, types[0].Type == "hyperstack_A4000", "returned instance type does not match expectations") -// -// if !types[0].IsAvailable { -// return -// } -// -// instance, err := client.CreateInstance(ctx, v1.CreateInstanceAttrs{ -// RefID: uuid.New().String(), -// InstanceType: types[0].Type, -// Location: types[0].Location, -// PublicKey: ssh.GetTestPublicKey(), -// Name: "test_name", -// FirewallRules: v1.FirewallRules{ -// EgressRules: []v1.FirewallRule{ -// { -// ID: "test-rule1", -// FromPort: 80, -// ToPort: 8080, -// IPRanges: []string{"127.0.0.1", "10.0.0.0/24"}, -// }, -// { -// ID: "test-rule2", -// FromPort: 5432, -// ToPort: 5432, -// IPRanges: []string{"127.0.0.1", "10.0.0.0/24"}, -// }, -// }, -// IngressRules: []v1.FirewallRule{ -// { -// ID: "test-rule3", -// FromPort: 80, -// ToPort: 8080, -// IPRanges: []string{"127.0.0.1", "10.0.0.0/24"}, -// }, -// { -// ID: "test-rule4", -// FromPort: 5432, -// ToPort: 5432, -// IPRanges: []string{"127.0.0.1", "10.0.0.0/24"}, -// }, -// }, -// }, -// }) -// if err != nil { -// t.Fatalf("ValidateCreateInstance failed: %v", err) -// } -// require.NotNil(t, instance) -// -// t.Run("ValidateSSHAccessible", func(t *testing.T) { -// err := v1.ValidateInstanceSSHAccessible(ctx, client, instance, ssh.GetTestPrivateKey()) -// require.NoError(t, err, "ValidateSSHAccessible should pass") -// }) -// -// t.Run("ValidateTerminateInstance", func(t *testing.T) { -// err := v1.ValidateTerminateInstance(ctx, client, instance) -// require.NoError(t, err, "ValidateTerminateInstance should pass") -// }) -//} +func TestValidationFunctions(t *testing.T) { + t.Parallel() + checkSkip(t) + apiKey := getAPIKey() + + config := validation.ProviderConfig{ + Credential: NewShadeformCredential("validation-test", apiKey), + StableIDs: []v1.InstanceTypeID{"datacrunch_B200_helsinki-finland-5", "massedcompute_L40_desmoines-usa-1"}, + } + + validation.RunValidationSuite(t, config) +} + +func TestInstanceLifecycleValidation(t *testing.T) { + t.Parallel() + checkSkip(t) + apiKey := getAPIKey() + + config := validation.ProviderConfig{ + Credential: NewShadeformCredential("validation-test", apiKey), + } + + validation.RunInstanceLifecycleValidation(t, config) +} + +func TestInstanceTypeFilter(t *testing.T) { + checkSkip(t) + apiKey := getAPIKey() + + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Minute) + t.Cleanup(cancel) + + client := NewShadeformClient("validation-test", apiKey) + client.WithConfiguration(Configuration{ + AllowedInstanceTypes: map[openapi.Cloud]map[string]bool{ + openapi.HYPERSTACK: { + "A4000": true, + }, + }, + }) + + types, err := client.GetInstanceTypes(ctx, v1.GetInstanceTypeArgs{}) + require.NoError(t, err) + require.NotEmpty(t, types, "Should have instance types") + require.True(t, len(types) == 1, "Instance types should return only one entry") + require.True(t, types[0].Type == "hyperstack_A4000", "returned instance type does not match expectations") + + if !types[0].IsAvailable { + return + } + + instance, err := client.CreateInstance(ctx, v1.CreateInstanceAttrs{ + RefID: uuid.New().String(), + InstanceType: types[0].Type, + Location: types[0].Location, + PublicKey: ssh.GetTestPublicKey(), + Name: "test_name", + FirewallRules: v1.FirewallRules{ + EgressRules: []v1.FirewallRule{ + { + ID: "test-rule1", + FromPort: 80, + ToPort: 8080, + IPRanges: []string{"127.0.0.1", "10.0.0.0/24"}, + }, + { + ID: "test-rule2", + FromPort: 5432, + ToPort: 5432, + IPRanges: []string{"127.0.0.1", "10.0.0.0/24"}, + }, + }, + IngressRules: []v1.FirewallRule{ + { + ID: "test-rule3", + FromPort: 80, + ToPort: 8080, + IPRanges: []string{"127.0.0.1", "10.0.0.0/24"}, + }, + { + ID: "test-rule4", + FromPort: 5432, + ToPort: 5432, + IPRanges: []string{"127.0.0.1", "10.0.0.0/24"}, + }, + }, + }, + }) + if err != nil { + t.Fatalf("ValidateCreateInstance failed: %v", err) + } + require.NotNil(t, instance) + + t.Run("ValidateSSHAccessible", func(t *testing.T) { + err := v1.ValidateInstanceSSHAccessible(ctx, client, instance, ssh.GetTestPrivateKey()) + require.NoError(t, err, "ValidateSSHAccessible should pass") + }) + + t.Run("ValidateTerminateInstance", func(t *testing.T) { + err := v1.ValidateTerminateInstance(ctx, client, instance) + require.NoError(t, err, "ValidateTerminateInstance should pass") + }) +} func TestOutOfStockError(t *testing.T) { checkSkip(t)