fix(sfcompute): keep starting nodes listed as pending on SSH 404

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 {

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{},
+	}
+}