Skip to content

Commit 1fc7a42

Browse files
authored
Brev 9311/fix invalid instance type error (#386)
* fix: clear errors for invalid/unavailable instance types * fix: drop redundant type prefix in skip log + add unit tests
1 parent 6ea0f72 commit 1fc7a42

4 files changed

Lines changed: 252 additions & 0 deletions

File tree

pkg/cmd/gpucreate/gpucreate.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -784,10 +784,38 @@ type typeCreateResult struct {
784784
fatalError error
785785
}
786786

787+
// validateInstanceTypeAvailability errors when the type is invalid or has no capacity. Returns nil when the listing is missing.
788+
func (c *createContext) validateInstanceTypeAvailability(instanceType string) error {
789+
if c.allInstanceTypes == nil {
790+
return nil
791+
}
792+
if c.allInstanceTypes.GetWorkspaceGroupID(instanceType) != "" {
793+
return nil
794+
}
795+
if !c.allInstanceTypes.HasInstanceType(instanceType) {
796+
return breverrors.NewValidationError(fmt.Sprintf(
797+
"instance type %q is not a recognized type; run 'brev search' to see available types",
798+
instanceType,
799+
))
800+
}
801+
return breverrors.NewValidationError(fmt.Sprintf(
802+
"instance type %q is currently unavailable (no capacity); try again later or run 'brev search' to find another type",
803+
instanceType,
804+
))
805+
}
806+
787807
// createInstancesWithType attempts to create instances using a specific type
788808
func (c *createContext) createInstancesWithType(spec InstanceSpec, startIdx, count int) typeCreateResult {
789809
result := typeCreateResult{}
790810

811+
if c.opts.LaunchableID == "" {
812+
if err := c.validateInstanceTypeAvailability(spec.Type); err != nil {
813+
c.logf("Skipping: %s\n", err.Error())
814+
result.hadFailure = true
815+
return result
816+
}
817+
}
818+
791819
var mu sync.Mutex
792820
var wg sync.WaitGroup
793821

@@ -1027,6 +1055,19 @@ func (c *createContext) createWorkspace(name string, spec InstanceSpec) (*entity
10271055
}
10281056
}
10291057

1058+
if cwOptions.WorkspaceGroupID == "" {
1059+
if c.allInstanceTypes == nil {
1060+
return nil, breverrors.NewValidationError(fmt.Sprintf(
1061+
"could not resolve workspace group for %q (instance-type listing was unavailable); please retry",
1062+
spec.Type,
1063+
))
1064+
}
1065+
return nil, breverrors.NewValidationError(fmt.Sprintf(
1066+
"instance type %q is invalid or unavailable; run 'brev search' to see available types",
1067+
spec.Type,
1068+
))
1069+
}
1070+
10301071
workspace, err := c.store.CreateWorkspace(c.org.ID, cwOptions)
10311072
if err != nil {
10321073
return nil, breverrors.WrapAndTrace(err)

pkg/cmd/gpucreate/gpucreate_test.go

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77

88
"github.com/brevdev/brev-cli/pkg/cmd/gpusearch"
99
"github.com/brevdev/brev-cli/pkg/entity"
10+
breverrors "github.com/brevdev/brev-cli/pkg/errors"
1011
"github.com/brevdev/brev-cli/pkg/store"
1112
"github.com/brevdev/brev-cli/pkg/terminal"
1213
"github.com/stretchr/testify/assert"
@@ -747,3 +748,158 @@ func TestInlineLaunchableLifeCycleScript(t *testing.T) {
747748
assert.Equal(t, "", info.BuildRequest.VMBuild.LifeCycleScriptAttr.Script)
748749
})
749750
}
751+
752+
func TestValidateInstanceTypeAvailability(t *testing.T) {
753+
t.Run("returns nil when listing is unavailable", func(t *testing.T) {
754+
ctx := &createContext{}
755+
assert.NoError(t, ctx.validateInstanceTypeAvailability("hyperstack_H100x8_one"))
756+
})
757+
758+
t.Run("returns nil when type has a workspace group", func(t *testing.T) {
759+
ctx := &createContext{
760+
allInstanceTypes: &gpusearch.AllInstanceTypesResponse{
761+
AllInstanceTypes: []gpusearch.InstanceType{
762+
{
763+
Type: "hyperstack_H100_sxm5x8",
764+
WorkspaceGroups: []gpusearch.WorkspaceGroup{
765+
{ID: "wg-1", Name: "Shadeform", PlatformType: "shadeform"},
766+
},
767+
},
768+
},
769+
},
770+
}
771+
assert.NoError(t, ctx.validateInstanceTypeAvailability("hyperstack_H100_sxm5x8"))
772+
})
773+
774+
t.Run("returns invalid-type error for unknown type", func(t *testing.T) {
775+
ctx := &createContext{
776+
allInstanceTypes: &gpusearch.AllInstanceTypesResponse{
777+
AllInstanceTypes: []gpusearch.InstanceType{
778+
{Type: "hyperstack_H100_sxm5x8", WorkspaceGroups: []gpusearch.WorkspaceGroup{{ID: "wg-1"}}},
779+
},
780+
},
781+
}
782+
err := ctx.validateInstanceTypeAvailability("hyperstack_H100x8_one")
783+
assert.Error(t, err)
784+
assert.Contains(t, err.Error(), `"hyperstack_H100x8_one"`)
785+
assert.Contains(t, err.Error(), "not a recognized type")
786+
assert.Contains(t, err.Error(), "brev search")
787+
})
788+
789+
t.Run("returns unavailable error for known type without workspace groups", func(t *testing.T) {
790+
ctx := &createContext{
791+
allInstanceTypes: &gpusearch.AllInstanceTypesResponse{
792+
AllInstanceTypes: []gpusearch.InstanceType{
793+
{Type: "hyperstack_H100x8_NVLINK", WorkspaceGroups: nil},
794+
},
795+
},
796+
}
797+
err := ctx.validateInstanceTypeAvailability("hyperstack_H100x8_NVLINK")
798+
assert.Error(t, err)
799+
assert.Contains(t, err.Error(), `"hyperstack_H100x8_NVLINK"`)
800+
assert.Contains(t, err.Error(), "currently unavailable")
801+
assert.Contains(t, err.Error(), "brev search")
802+
})
803+
804+
t.Run("error type is ValidationError so no stack trace is appended", func(t *testing.T) {
805+
ctx := &createContext{
806+
allInstanceTypes: &gpusearch.AllInstanceTypesResponse{
807+
AllInstanceTypes: []gpusearch.InstanceType{},
808+
},
809+
}
810+
err := ctx.validateInstanceTypeAvailability("missing")
811+
assert.Error(t, err)
812+
var ve breverrors.ValidationError
813+
assert.ErrorAs(t, err, &ve)
814+
assert.NotContains(t, err.Error(), "gpucreate.go", "validation error should not include source-file traces")
815+
})
816+
}
817+
818+
func TestCreateInstancesWithTypeSkipsInvalidType(t *testing.T) {
819+
mock := NewMockGPUCreateStore()
820+
ctx := &createContext{
821+
t: terminal.New(),
822+
store: mock,
823+
opts: GPUCreateOptions{Count: 1, Parallel: 1, Name: "jt-4"},
824+
org: mock.Org,
825+
user: mock.User,
826+
piped: true,
827+
allInstanceTypes: &gpusearch.AllInstanceTypesResponse{
828+
AllInstanceTypes: []gpusearch.InstanceType{
829+
{
830+
Type: "hyperstack_H100_sxm5x8",
831+
WorkspaceGroups: []gpusearch.WorkspaceGroup{
832+
{ID: "wg-shadeform", Name: "Shadeform", PlatformType: "shadeform"},
833+
},
834+
},
835+
},
836+
},
837+
}
838+
ctx.logf = func(_ string, _ ...interface{}) {}
839+
840+
result := ctx.createInstancesWithType(InstanceSpec{Type: "hyperstack_H100x8_one"}, 0, 1)
841+
842+
assert.True(t, result.hadFailure, "expected hadFailure for an invalid instance type")
843+
assert.Empty(t, result.successes, "expected no successes for invalid type")
844+
assert.NoError(t, result.fatalError, "invalid type should not be fatal — caller may try the next type")
845+
assert.Empty(t, mock.CreatedWorkspaces, "CreateWorkspace must not be called when the type is unrecognized")
846+
}
847+
848+
func TestCreateInstancesWithTypeSkipsUnavailableType(t *testing.T) {
849+
mock := NewMockGPUCreateStore()
850+
ctx := &createContext{
851+
t: terminal.New(),
852+
store: mock,
853+
opts: GPUCreateOptions{Count: 1, Parallel: 1, Name: "jt-4"},
854+
org: mock.Org,
855+
user: mock.User,
856+
piped: true,
857+
allInstanceTypes: &gpusearch.AllInstanceTypesResponse{
858+
AllInstanceTypes: []gpusearch.InstanceType{
859+
{Type: "hyperstack_H100x8_NVLINK", WorkspaceGroups: nil},
860+
},
861+
},
862+
}
863+
ctx.logf = func(_ string, _ ...interface{}) {}
864+
865+
result := ctx.createInstancesWithType(InstanceSpec{Type: "hyperstack_H100x8_NVLINK"}, 0, 1)
866+
867+
assert.True(t, result.hadFailure, "expected hadFailure for an unavailable instance type")
868+
assert.Empty(t, result.successes)
869+
assert.Empty(t, mock.CreatedWorkspaces, "CreateWorkspace must not be called when no workspace group is available")
870+
}
871+
872+
func TestCreateInstancesWithTypeBypassesValidationForLaunchable(t *testing.T) {
873+
mock := NewMockGPUCreateStore()
874+
ctx := &createContext{
875+
t: terminal.New(),
876+
store: mock,
877+
opts: GPUCreateOptions{
878+
Count: 1,
879+
Parallel: 1,
880+
Name: "jt-4",
881+
LaunchableID: "env-abc",
882+
LaunchableInfo: &store.LaunchableResponse{
883+
ID: "env-abc",
884+
Name: "test-launchable",
885+
CreateWorkspaceRequest: store.LaunchableWorkspaceRequest{
886+
WorkspaceGroupID: "wg-from-launchable",
887+
InstanceType: "n2-standard-4",
888+
},
889+
},
890+
},
891+
org: mock.Org,
892+
user: mock.User,
893+
piped: true,
894+
allInstanceTypes: &gpusearch.AllInstanceTypesResponse{
895+
AllInstanceTypes: []gpusearch.InstanceType{}, // launchable's type is not in the org listing
896+
},
897+
}
898+
ctx.logf = func(_ string, _ ...interface{}) {}
899+
900+
result := ctx.createInstancesWithType(InstanceSpec{Type: "n2-standard-4"}, 0, 1)
901+
902+
assert.False(t, result.hadFailure, "launchable should not be blocked by pre-flight validation")
903+
assert.Len(t, result.successes, 1, "expected the launchable instance to be created")
904+
assert.Len(t, mock.CreatedWorkspaces, 1)
905+
}

pkg/cmd/gpusearch/gpusearch.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,16 @@ func (r *AllInstanceTypesResponse) GetWorkspaceGroupID(instanceType string) stri
9898
return ""
9999
}
100100

101+
// HasInstanceType reports whether the type exists in the API listing, independent of capacity.
102+
func (r *AllInstanceTypesResponse) HasInstanceType(instanceType string) bool {
103+
for _, it := range r.AllInstanceTypes {
104+
if it.Type == instanceType {
105+
return true
106+
}
107+
}
108+
return false
109+
}
110+
101111
// GPUSearchStore defines the interface for fetching instance types
102112
type GPUSearchStore interface {
103113
GetInstanceTypes(includeCPU bool) (*InstanceTypesResponse, error)

pkg/cmd/gpusearch/gpusearch_test.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -585,3 +585,48 @@ func TestProcessInstancesCloudExtraction(t *testing.T) {
585585
assert.Equal(t, "nebius", instances[1].Cloud)
586586
assert.Equal(t, "nebius", instances[1].Provider)
587587
}
588+
589+
func TestAllInstanceTypesResponseLookup(t *testing.T) {
590+
resp := &AllInstanceTypesResponse{
591+
AllInstanceTypes: []InstanceType{
592+
{
593+
Type: "hyperstack_H100_sxm5x8",
594+
WorkspaceGroups: []WorkspaceGroup{
595+
{ID: "wg-shadeform", Name: "Shadeform", PlatformType: "shadeform"},
596+
},
597+
},
598+
{
599+
Type: "hyperstack_H100x8_NVLINK",
600+
WorkspaceGroups: nil,
601+
},
602+
{
603+
Type: "verda-b300-8x",
604+
WorkspaceGroups: []WorkspaceGroup{},
605+
},
606+
},
607+
}
608+
609+
t.Run("GetWorkspaceGroupID returns id when type has groups", func(t *testing.T) {
610+
assert.Equal(t, "wg-shadeform", resp.GetWorkspaceGroupID("hyperstack_H100_sxm5x8"))
611+
})
612+
613+
t.Run("GetWorkspaceGroupID returns empty for type without groups", func(t *testing.T) {
614+
assert.Equal(t, "", resp.GetWorkspaceGroupID("hyperstack_H100x8_NVLINK"))
615+
assert.Equal(t, "", resp.GetWorkspaceGroupID("verda-b300-8x"))
616+
})
617+
618+
t.Run("GetWorkspaceGroupID returns empty for unknown type", func(t *testing.T) {
619+
assert.Equal(t, "", resp.GetWorkspaceGroupID("hyperstack_H100x8_one"))
620+
})
621+
622+
t.Run("HasInstanceType is true even when groups are empty", func(t *testing.T) {
623+
assert.True(t, resp.HasInstanceType("hyperstack_H100_sxm5x8"))
624+
assert.True(t, resp.HasInstanceType("hyperstack_H100x8_NVLINK"))
625+
assert.True(t, resp.HasInstanceType("verda-b300-8x"))
626+
})
627+
628+
t.Run("HasInstanceType is false for unknown type", func(t *testing.T) {
629+
assert.False(t, resp.HasInstanceType("hyperstack_H100x8_one"))
630+
assert.False(t, resp.HasInstanceType(""))
631+
})
632+
}

0 commit comments

Comments
 (0)