From 39d0ee28567820a89f216bb83631bec571124dcf Mon Sep 17 00:00:00 2001 From: mmoulikk Date: Wed, 4 Nov 2026 14:26:36 +0530 Subject: [PATCH] fix(sfcompute): keep starting nodes listed as pending on SSH 404 --- v1/providers/sfcomputev2/instance.go | 30 +++++-- v1/providers/sfcomputev2/instance_test.go | 98 +++++++++++++++++++++++ 2 files changed, 123 insertions(+), 5 deletions(-) diff --git a/v1/providers/sfcomputev2/instance.go b/v1/providers/sfcomputev2/instance.go index a9953ea..722107b 100644 --- a/v1/providers/sfcomputev2/instance.go +++ b/v1/providers/sfcomputev2/instance.go @@ -25,6 +25,8 @@ var ( sfcNameLeading = regexp.MustCompile(`^[^a-zA-Z0-9]+`) ) +const sshDetailsPendingWindow = 45 * time.Minute + // sanitizeSFCName coerces a requested instance name into SFC's required format: // disallowed characters are replaced with '-', leading non-alphanumeric characters are // dropped (SFC requires an alphanumeric first character), and the result is truncated to @@ -118,7 +120,7 @@ func (c *SFCClientV2) GetInstance(ctx context.Context, id v1.CloudProviderInstan return nil, errors.WrapAndTrace(fmt.Errorf("instance %s not found", id)) } - sshInfo, err := c.getSSHInfo(ctx, string(id), resp.Status) + sshInfo, err := c.getSSHInfo(ctx, resp) if err != nil { return nil, errors.WrapAndTrace(err) } @@ -157,7 +159,7 @@ func (c *SFCClientV2) ListInstances(ctx context.Context, args v1.ListInstancesAr continue } - sshInfo, err := c.getSSHInfo(ctx, inst.ID, inst.Status) + sshInfo, err := c.getSSHInfo(ctx, &inst) if err != nil { c.logger.Error(ctx, err, v1.LogField("msg", "sfcv2: ListInstances skipping instance due to SSH error"), @@ -210,13 +212,19 @@ func normalizeTerminateInstanceError(err error) error { return errors.WrapAndTrace(err) } -func (c *SFCClientV2) getSSHInfo(ctx context.Context, id string, status instanceStatus) (*instanceSSHInfo, error) { - if status != instanceStatusRunning { +func (c *SFCClientV2) getSSHInfo(ctx context.Context, inst *instanceResponse) (*instanceSSHInfo, error) { + if inst.Status != instanceStatusRunning { return nil, nil } - resp, err := c.client.getSSHInfo(ctx, id) + resp, err := c.client.getSSHInfo(ctx, inst.ID) if err != nil { + if isSSHDetailsPending(err, inst.CreatedAt, time.Now()) { + c.logger.Info(ctx, "sfcv2: SSH details not available yet, reporting instance as pending", + v1.LogField("instanceID", inst.ID), + ) + return nil, nil + } return nil, errors.WrapAndTrace(err) } if resp == nil { @@ -226,6 +234,14 @@ func (c *SFCClientV2) getSSHInfo(ctx context.Context, id string, status instance return resp, nil } +func isSSHDetailsPending(err error, createdAt int64, now time.Time) bool { + var responseErr *apiError + if !errors.As(err, &responseErr) || responseErr.statusCode != http.StatusNotFound { + return false + } + return now.Sub(time.Unix(createdAt, 0)) < sshDetailsPendingWindow +} + func (c *SFCClientV2) sfcInstanceToBrevInstance(inst *instanceResponse, sshInfo *instanceSSHInfo) (*v1.Instance, error) { tags := inst.Tags @@ -244,6 +260,10 @@ func (c *SFCClientV2) sfcInstanceToBrevInstance(inst *instanceResponse, sshInfo } status := sfcStatusToLifecycleStatus(inst.Status) + if status == v1.LifecycleStatusRunning && sshInfo.GetHostname() == "" { + // Brev can't reach the instance until SFC returns its SSH details. + status = v1.LifecycleStatusPending + } diskInt64, err := h100InstanceTypeMetadata.diskBytes.ByteCountInUnitInt64(v1.Gibibyte) if err != nil { diff --git a/v1/providers/sfcomputev2/instance_test.go b/v1/providers/sfcomputev2/instance_test.go index 85dcd59..6d14908 100644 --- a/v1/providers/sfcomputev2/instance_test.go +++ b/v1/providers/sfcomputev2/instance_test.go @@ -1,15 +1,20 @@ package v2 import ( + "context" "errors" "fmt" "net/http" + "net/http/httptest" "testing" + "time" v1 "github.com/brevdev/cloud/v1" "github.com/stretchr/testify/require" ) +const sshDetailsNotAvailableBody = `{"error":{"type":"not_found","message":"SSH details not currently available"}}` + func TestNormalizeTerminateInstanceErrorTreatsNotFoundAsAlreadyGone(t *testing.T) { t.Parallel() @@ -57,3 +62,96 @@ func TestMakeSFCNameIsUniquePerInstance(t *testing.T) { require.NotEqual(t, first, second) } + +func TestIsSSHDetailsPending(t *testing.T) { + t.Parallel() + + now := time.Unix(1_800_000_000, 0) + notFound := fmt.Errorf("get ssh info: %w", &apiError{statusCode: http.StatusNotFound, body: sshDetailsNotAvailableBody}) + serverError := &apiError{statusCode: http.StatusInternalServerError, body: "internal error"} + + tests := []struct { + name string + err error + createdAt time.Time + want bool + }{ + {name: "404 for a new instance", err: notFound, createdAt: now.Add(-time.Minute), want: true}, + {name: "404 just before the window ends", err: notFound, createdAt: now.Add(-sshDetailsPendingWindow + time.Second), want: true}, + {name: "404 once the window has ended", err: notFound, createdAt: now.Add(-sshDetailsPendingWindow), want: false}, + {name: "server error for a new instance", err: serverError, createdAt: now.Add(-time.Minute), want: false}, + {name: "transport error for a new instance", err: errors.New("connection reset"), createdAt: now.Add(-time.Minute), want: false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + require.Equal(t, tt.want, isSSHDetailsPending(tt.err, tt.createdAt.Unix(), now)) + }) + } +} + +func TestListInstancesKeepsNewInstanceWithoutSSHDetailsAsPending(t *testing.T) { + t.Parallel() + + recently := time.Now().Add(-time.Minute).Unix() + listed := []instanceResponse{ + {ID: "inst_starting", Status: instanceStatusRunning, CreatedAt: recently}, + {ID: "inst_ready", Status: instanceStatusRunning, CreatedAt: recently}, + {ID: "inst_old", Status: instanceStatusRunning, CreatedAt: time.Now().Add(-2 * sshDetailsPendingWindow).Unix()}, + } + server := httptest.NewServer(http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { + switch request.URL.Path { + case "/integrations/brev/v1/instances": + writeJSON(t, writer, listInstancesResponse{Data: listed}) + case "/integrations/brev/v1/instances/inst_ready/ssh": + writeJSON(t, writer, instanceSSHInfo{Hostname: "192.0.2.1", Port: 2222}) + default: + http.Error(writer, sshDetailsNotAvailableBody, http.StatusNotFound) + } + })) + t.Cleanup(server.Close) + + instances, err := newTestSFCClient(server.URL).ListInstances(context.Background(), v1.ListInstancesArgs{}) + require.NoError(t, err) + + byID := map[v1.CloudProviderInstanceID]v1.Instance{} + for _, instance := range instances { + byID[instance.CloudID] = instance + } + require.Equal(t, v1.LifecycleStatusPending, byID["inst_starting"].Status.LifecycleStatus) + require.Empty(t, byID["inst_starting"].PublicIP) + require.Equal(t, v1.LifecycleStatusRunning, byID["inst_ready"].Status.LifecycleStatus) + require.Equal(t, "192.0.2.1", byID["inst_ready"].PublicIP) + require.Equal(t, 2222, byID["inst_ready"].SSHPort) + require.NotContains(t, byID, v1.CloudProviderInstanceID("inst_old")) +} + +func TestGetInstanceReportsNewInstanceWithoutSSHDetailsAsPending(t *testing.T) { + t.Parallel() + + server := httptest.NewServer(http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { + if request.URL.Path == "/integrations/brev/v1/instances/inst_starting" { + writeJSON(t, writer, instanceResponse{ID: "inst_starting", Status: instanceStatusRunning, CreatedAt: time.Now().Add(-time.Minute).Unix()}) + return + } + http.Error(writer, sshDetailsNotAvailableBody, http.StatusNotFound) + })) + t.Cleanup(server.Close) + + instance, err := newTestSFCClient(server.URL).GetInstance(context.Background(), "inst_starting") + require.NoError(t, err) + require.Equal(t, v1.LifecycleStatusPending, instance.Status.LifecycleStatus) + require.Empty(t, instance.PublicIP) +} + +func newTestSFCClient(baseURL string) *SFCClientV2 { + client := newAPIClient("api-key") + client.baseURL = baseURL + return &SFCClientV2{ + refID: "cloud-cred", + organization: "account", + workspace: "workspace", + client: client, + logger: &v1.NoopLogger{}, + } +}