From 21ea9d380937177713d0835720e6bb6d2faac9aa Mon Sep 17 00:00:00 2001 From: hakhandelwal11 Date: Tue, 15 Sep 2026 16:14:39 +0530 Subject: [PATCH 1/3] fix(nebius): sweep orphaned validation CI resources and make TerminateInstance idempotent --- .github/workflows/validation-nebius.yml | 16 ++ internal/validation/suite.go | 63 +++++-- internal/validation/sweep.go | 89 ++++++++++ internal/validation/sweep_test.go | 104 +++++++++++ v1/providers/nebius/errors.go | 4 + v1/providers/nebius/instance.go | 42 ++++- v1/providers/nebius/integration_test.go | 20 ++- v1/providers/nebius/sweep_pass_test.go | 185 ++++++++++++++++++++ v1/providers/nebius/sweep_test.go | 221 ++++++++++++++++++++++++ v1/providers/nebius/validation_test.go | 9 + v1/utils.go | 4 + 11 files changed, 732 insertions(+), 25 deletions(-) create mode 100644 internal/validation/sweep.go create mode 100644 internal/validation/sweep_test.go create mode 100644 v1/providers/nebius/sweep_pass_test.go create mode 100644 v1/providers/nebius/sweep_test.go diff --git a/.github/workflows/validation-nebius.yml b/.github/workflows/validation-nebius.yml index 9f9c3e3c..050ff283 100644 --- a/.github/workflows/validation-nebius.yml +++ b/.github/workflows/validation-nebius.yml @@ -42,10 +42,26 @@ jobs: TEST_PRIVATE_KEY_BASE64: ${{ secrets.TEST_PRIVATE_KEY_BASE64 }} TEST_PUBLIC_KEY_BASE64: ${{ secrets.TEST_PUBLIC_KEY_BASE64 }} VALIDATION_TEST: true + # Stamps this run's resources for the sweep below. run_id (not run_attempt) so a + # re-run also reaps resources a prior attempt leaked before its cleanup ran. + CI_RUN_ID: ${{ github.run_id }} run: | cd v1/providers/nebius go test -v -short=false -timeout=30m ./... + # Deletes any VM/network/subnet/disk this run left behind. Matches only this run's + # ci-run-id label, so it never touches another run's or a production resource. + - name: Sweep leaked validation/integration resources + if: always() + env: + NEBIUS_SERVICE_ACCOUNT_JSON: ${{ secrets.NEBIUS_SERVICE_ACCOUNT_JSON }} + NEBIUS_TENANT_ID: ${{ secrets.NEBIUS_TENANT_ID }} + SWEEP_ORPHANS: "true" + CI_RUN_ID: ${{ github.run_id }} + run: | + cd v1/providers/nebius + go test -v -run '^TestSweepOrphans$' -timeout=20m . + - name: Upload test results uses: actions/upload-artifact@v4 if: always() diff --git a/internal/validation/suite.go b/internal/validation/suite.go index 048e8994..cd7754ae 100644 --- a/internal/validation/suite.go +++ b/internal/validation/suite.go @@ -2,11 +2,13 @@ package validation import ( "context" + "errors" "testing" "time" "github.com/brevdev/cloud/internal/ssh" v1 "github.com/brevdev/cloud/v1" + "github.com/cenkalti/backoff/v4" "github.com/stretchr/testify/require" ) @@ -14,6 +16,40 @@ type ProviderConfig struct { Location string StableIDs []v1.InstanceTypeID Credential v1.CloudCredential + // Tags are extra labels (e.g. the CI run ID) stamped on every instance and its + // network/subnet/disk so a post-run sweep can delete this run's resources. + Tags map[string]string +} + +// registerInstanceCleanup schedules termination via t.Cleanup so it runs even after a t.Fatalf, +// on a fresh context with retry. Not-found counts as success; a terminal failure fails the test. +// Returns markTerminated, which the caller invokes once it has terminated the instance itself to +// skip the redundant delete. Call this right after a create, before any assertion. +func registerInstanceCleanup(t *testing.T, client v1.CloudCreateTerminateInstance, cloudID v1.CloudProviderInstanceID) (markTerminated func()) { + t.Helper() + terminated := false + markTerminated = func() { terminated = true } + if cloudID == "" { + return markTerminated + } + t.Cleanup(func() { + if terminated { + return + } + op := func() error { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) + defer cancel() + err := client.TerminateInstance(ctx, cloudID) + if err == nil || errors.Is(err, v1.ErrInstanceNotFound) || errors.Is(err, v1.ErrResourceNotFound) { + return nil + } + return err + } + if err := backoff.Retry(op, backoff.WithMaxRetries(backoff.NewExponentialBackOff(), 4)); err != nil { + t.Errorf("LEAKED INSTANCE %s: cleanup failed after retries: %v", cloudID, err) + } + }) + return markTerminated } func RunValidationSuite(t *testing.T, config ProviderConfig) { @@ -123,18 +159,19 @@ func RunInstanceLifecycleValidation(t *testing.T, config ProviderConfig) { break } } + attrs.Tags = config.Tags instance, err := v1.ValidateCreateInstance(ctx, client, attrs, selectedType) + // Register cleanup before the fatal below: create can return a non-nil instance with an + // error, and a t.Fatalf would skip a later defer and leak the VM. + markTerminated := func() {} + if instance != nil { + markTerminated = registerInstanceCleanup(t, client, instance.CloudID) + } if err != nil { t.Fatalf("ValidateCreateInstance failed: %v", err) } require.NotNil(t, instance) - defer func() { - if instance != nil { - _ = client.TerminateInstance(ctx, instance.CloudID) - } - }() - t.Run("ValidateListCreatedInstance", func(t *testing.T) { err := v1.ValidateListCreatedInstance(ctx, client, instance) require.NoError(t, err, "ValidateListCreatedInstance should pass") @@ -165,6 +202,7 @@ func RunInstanceLifecycleValidation(t *testing.T, config ProviderConfig) { t.Run("ValidateTerminateInstance", func(t *testing.T) { err := v1.ValidateTerminateInstance(ctx, client, instance) require.NoError(t, err, "ValidateTerminateInstance should pass") + markTerminated() // already terminated; skip redundant delete }) }) } @@ -312,16 +350,16 @@ func RunFirewallValidation(t *testing.T, config ProviderConfig, opts FirewallVal require.NotEmpty(t, attrs.InstanceType, "Should find available instance type") // Create instance for firewall testing + attrs.Tags = config.Tags instance, err := v1.ValidateCreateInstance(ctx, client, attrs, selectedType) + // Register cleanup before the assertions below so a failed require cannot leak the VM. + markTerminated := func() {} + if instance != nil { + markTerminated = registerInstanceCleanup(t, client, instance.CloudID) + } require.NoError(t, err, "ValidateCreateInstance should pass") require.NotNil(t, instance) - defer func() { - if instance != nil { - _ = client.TerminateInstance(ctx, instance.CloudID) - } - }() - // Wait for instance to be running and SSH accessible t.Run("ValidateSSHAccessible", func(t *testing.T) { err := v1.ValidateInstanceSSHAccessible(ctx, client, instance, ssh.GetTestPrivateKey()) @@ -349,6 +387,7 @@ func RunFirewallValidation(t *testing.T, config ProviderConfig, opts FirewallVal t.Run("ValidateTerminateInstance", func(t *testing.T) { err := v1.ValidateTerminateInstance(ctx, client, instance) require.NoError(t, err, "ValidateTerminateInstance should pass") + markTerminated() // already terminated; skip redundant delete }) } diff --git a/internal/validation/sweep.go b/internal/validation/sweep.go new file mode 100644 index 00000000..bc8d21b2 --- /dev/null +++ b/internal/validation/sweep.go @@ -0,0 +1,89 @@ +package validation + +import ( + "context" + "errors" + "fmt" + "time" + + v1 "github.com/brevdev/cloud/v1" + "github.com/cenkalti/backoff/v4" +) + +// CIRunIDLabel re-exports v1.CIRunIDLabel so the sweeper and create paths share one value. +const CIRunIDLabel = v1.CIRunIDLabel + +// SweepOpts configures SweepOrphanedInstances. +type SweepOpts struct { + // RunID is the CIRunIDLabel label value to match. It is REQUIRED and must be unique per CI run. + RunID string + // Logf, if set, receives human-readable progress lines. Defaults to fmt.Printf. + Logf func(format string, args ...any) +} + +// SweepResult reports what a sweep scanned and did. +type SweepResult struct { + Scanned int + Matched []v1.Instance + Deleted []v1.CloudProviderInstanceID + Failed map[v1.CloudProviderInstanceID]error +} + +// SweepOrphanedInstances lists every instance visible to client and terminates exactly the ones +// whose CIRunIDLabel label equals opts.RunID — i.e. the VMs created by this CI run. +func SweepOrphanedInstances(ctx context.Context, client v1.CloudClient, opts SweepOpts) (SweepResult, error) { + res := SweepResult{Failed: map[v1.CloudProviderInstanceID]error{}} + logf := opts.Logf + if logf == nil { + logf = func(format string, args ...any) { fmt.Printf(format+"\n", args...) } + } + + // A run ID is mandatory: without it there is nothing safe to match on. + if opts.RunID == "" { + return res, errors.New("SweepOrphanedInstances: RunID is required") + } + + instances, err := client.ListInstances(ctx, v1.ListInstancesArgs{}) + if err != nil { + return res, fmt.Errorf("failed to list instances: %w", err) + } + res.Scanned = len(instances) + + for i := range instances { + inst := instances[i] + if inst.Tags[CIRunIDLabel] != opts.RunID { + continue + } + res.Matched = append(res.Matched, inst) + } + + logf("[sweep] scanned=%d matched=%d runID=%q", res.Scanned, len(res.Matched), opts.RunID) + + for i := range res.Matched { + inst := res.Matched[i] + logf("[sweep] terminating cloudID=%s name=%q owner=%s location=%s", + inst.CloudID, inst.Name, inst.CloudCredRefID, inst.Location) + if err := terminateWithRetry(ctx, client, inst.CloudID); err != nil { + res.Failed[inst.CloudID] = err + logf("[sweep] FAILED to terminate cloudID=%s: %v", inst.CloudID, err) + continue + } + res.Deleted = append(res.Deleted, inst.CloudID) + } + return res, nil +} + +// terminateWithRetry terminates an instance with a bounded exponential backoff, treating a +// not-found result as success (the instance is already gone). +func terminateWithRetry(ctx context.Context, client v1.CloudCreateTerminateInstance, id v1.CloudProviderInstanceID) error { + op := func() error { + termCtx, cancel := context.WithTimeout(ctx, 10*time.Minute) + defer cancel() + err := client.TerminateInstance(termCtx, id) + if err == nil || errors.Is(err, v1.ErrInstanceNotFound) || errors.Is(err, v1.ErrResourceNotFound) { + return nil + } + return err + } + return backoff.Retry(op, backoff.WithMaxRetries(backoff.NewExponentialBackOff(), 4)) +} diff --git a/internal/validation/sweep_test.go b/internal/validation/sweep_test.go new file mode 100644 index 00000000..aad9667b --- /dev/null +++ b/internal/validation/sweep_test.go @@ -0,0 +1,104 @@ +package validation + +import ( + "context" + "testing" + "time" + + v1 "github.com/brevdev/cloud/v1" +) + +// fakeSweepClient is a CloudClient whose ListInstances returns a canned set and whose +// TerminateInstance records the IDs it was asked to delete. All other methods come from +// NotImplCloudClient and return ErrNotImplemented (the sweeper never calls them). +type fakeSweepClient struct { + v1.NotImplCloudClient + instances []v1.Instance + terminated []v1.CloudProviderInstanceID +} + +var _ v1.CloudClient = (*fakeSweepClient)(nil) + +func (c *fakeSweepClient) ListInstances(context.Context, v1.ListInstancesArgs) ([]v1.Instance, error) { + return c.instances, nil +} + +func (c *fakeSweepClient) TerminateInstance(_ context.Context, id v1.CloudProviderInstanceID) error { + c.terminated = append(c.terminated, id) + return nil +} + +func matchedIDs(res SweepResult) []v1.CloudProviderInstanceID { + ids := make([]v1.CloudProviderInstanceID, 0, len(res.Matched)) + for _, m := range res.Matched { + ids = append(ids, m.CloudID) + } + return ids +} + +func assertSameSet(t *testing.T, got, want []v1.CloudProviderInstanceID) { + t.Helper() + gm := map[v1.CloudProviderInstanceID]bool{} + for _, g := range got { + gm[g] = true + } + wm := map[v1.CloudProviderInstanceID]bool{} + for _, w := range want { + wm[w] = true + } + for w := range wm { + if !gm[w] { + t.Fatalf("missing %q: got %v want %v", w, got, want) + } + } + for g := range gm { + if !wm[g] { + t.Fatalf("unexpected %q: got %v want %v", g, got, want) + } + } +} + +// TestSweepOrphanedInstances_OnlyMatchesRunID is the safety-critical test: the sweep terminates +// exactly the VMs carrying this run's ci-run-id label and nothing else. +func TestSweepOrphanedInstances_OnlyMatchesRunID(t *testing.T) { + old := time.Now().Add(-10 * 24 * time.Hour) + fresh := time.Now().Add(-1 * time.Minute) + + instances := []v1.Instance{ + // This run's VMs — must be deleted, regardless of age. + {CloudID: "A-thisrun", CloudCredRefID: "validation-test", CreatedAt: fresh, Tags: v1.Tags{CIRunIDLabel: "run-1"}}, + {CloudID: "B-thisrun-int", CloudCredRefID: "integration-test-ref", CreatedAt: fresh, Tags: v1.Tags{CIRunIDLabel: "run-1"}}, + // Another run's VM — must NOT be deleted (different run id). + {CloudID: "C-otherrun", CloudCredRefID: "validation-test", CreatedAt: old, Tags: v1.Tags{CIRunIDLabel: "run-2"}}, + // Legacy CI VM with no run label (predates this feature) — must NOT be deleted (manual cleanup). + {CloudID: "D-legacy-nolabel", CloudCredRefID: "validation-test", CreatedAt: old}, + // Production VM that reused a CI credential ref but has no run label — must NOT be deleted. + {CloudID: "E-prod-reused-cred", CloudCredRefID: "validation-test", CreatedAt: old}, + // Ordinary production VM — must NOT be deleted. + {CloudID: "F-prod", CloudCredRefID: "cloud-cred-abc123", CreatedAt: old}, + } + + fc := &fakeSweepClient{instances: instances} + res, err := SweepOrphanedInstances(context.Background(), fc, SweepOpts{RunID: "run-1", Logf: t.Logf}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + want := []v1.CloudProviderInstanceID{"A-thisrun", "B-thisrun-int"} + assertSameSet(t, matchedIDs(res), want) + assertSameSet(t, res.Deleted, want) + assertSameSet(t, fc.terminated, want) +} + +// TestSweepOrphanedInstances_RequiresRunID pins the safety invariant: without a run ID the sweep +// refuses to run and terminates nothing. +func TestSweepOrphanedInstances_RequiresRunID(t *testing.T) { + fc := &fakeSweepClient{instances: []v1.Instance{ + {CloudID: "x", CloudCredRefID: "validation-test", Tags: v1.Tags{CIRunIDLabel: "run-1"}}, + }} + if _, err := SweepOrphanedInstances(context.Background(), fc, SweepOpts{RunID: ""}); err == nil { + t.Error("expected an error when RunID is empty") + } + if len(fc.terminated) != 0 { + t.Errorf("empty RunID must terminate nothing, terminated: %v", fc.terminated) + } +} diff --git a/v1/providers/nebius/errors.go b/v1/providers/nebius/errors.go index 5811b356..695342c8 100644 --- a/v1/providers/nebius/errors.go +++ b/v1/providers/nebius/errors.go @@ -27,6 +27,10 @@ func (e *NebiusError) Error() string { // isNotFoundError checks if an error is a "not found" error func isNotFoundError(err error) bool { + // Also match the v1 sentinels, for errors already mapped by callers like TerminateInstance. + if errors.Is(err, v1.ErrInstanceNotFound) || errors.Is(err, v1.ErrResourceNotFound) { + return true + } // Check for gRPC NotFound status code if status, ok := status.FromError(err); ok { return status.Code() == codes.NotFound diff --git a/v1/providers/nebius/instance.go b/v1/providers/nebius/instance.go index 8635e9c7..8c6f4c5d 100644 --- a/v1/providers/nebius/instance.go +++ b/v1/providers/nebius/instance.go @@ -70,7 +70,7 @@ func (c *NebiusClient) CreateInstance(ctx context.Context, attrs v1.CreateInstan // Create isolated networking infrastructure for this instance // Use RefID (environmentId) for resource correlation var err error - networkID, subnetID, err = c.createIsolatedNetwork(ctx, attrs.RefID) + networkID, subnetID, err = c.createIsolatedNetwork(ctx, attrs.RefID, ciRunLabels(attrs.Tags)) if err != nil { return nil, fmt.Errorf("failed to create isolated network: %w", err) } @@ -607,6 +607,11 @@ func (c *NebiusClient) TerminateInstance(ctx context.Context, instanceID v1.Clou Id: string(instanceID), }) if err != nil { + // Already gone: wrap both the v1 sentinel (for errors.Is) and the gRPC error (keeps its + // status code and message). Two %w verbs preserve both. + if isNotFoundError(err) { + return fmt.Errorf("instance %s already terminated: %w: %w", instanceID, v1.ErrInstanceNotFound, err) + } return fmt.Errorf("failed to get instance details: %w", err) } @@ -1009,10 +1014,31 @@ func (c *NebiusClient) MergeInstanceForUpdate(currInst v1.Instance, newInst v1.I return merged } +// labelsWithTags merges the given tags into a base label set; base (reserved) keys always win. +func labelsWithTags(tags, base map[string]string) map[string]string { + out := make(map[string]string, len(tags)+len(base)) + for k, v := range tags { + out[k] = v + } + for k, v := range base { + out[k] = v + } + return out +} + +// ciRunLabels returns only the CI run-ID label from tags. That is the sole label propagated to the +// network/subnet/disk (the sweep matches on it); the full tag set stays on the instance alone. +func ciRunLabels(tags map[string]string) map[string]string { + if id, ok := tags[v1.CIRunIDLabel]; ok && id != "" { + return map[string]string{v1.CIRunIDLabel: id} + } + return nil +} + // createIsolatedNetwork creates a dedicated VPC and subnet for a single instance // This ensures complete network isolation between instances // Uses refID (environmentId) for resource correlation -func (c *NebiusClient) createIsolatedNetwork(ctx context.Context, refID string) (networkID, subnetID string, err error) { +func (c *NebiusClient) createIsolatedNetwork(ctx context.Context, refID string, tags map[string]string) (networkID, subnetID string, err error) { // Create VPC network (unique per instance, named with refID for correlation) networkName := fmt.Sprintf("%s-vpc", refID) @@ -1020,11 +1046,11 @@ func (c *NebiusClient) createIsolatedNetwork(ctx context.Context, refID string) Metadata: &common.ResourceMetadata{ ParentId: c.projectID, Name: networkName, - Labels: map[string]string{ + Labels: labelsWithTags(tags, map[string]string{ "created-by": "brev-cloud-sdk", "brev-user": c.refID, "environment-id": refID, - }, + }), }, Spec: &vpc.NetworkSpec{ // Use default network pools @@ -1058,12 +1084,12 @@ func (c *NebiusClient) createIsolatedNetwork(ctx context.Context, refID string) Metadata: &common.ResourceMetadata{ ParentId: c.projectID, Name: subnetName, - Labels: map[string]string{ + Labels: labelsWithTags(tags, map[string]string{ "created-by": "brev-cloud-sdk", "brev-user": c.refID, "environment-id": refID, "network-id": networkID, - }, + }), }, Spec: &vpc.SubnetSpec{ NetworkId: networkID, @@ -1217,12 +1243,12 @@ func (c *NebiusClient) buildDiskCreateRequest(_ context.Context, diskName string Metadata: &common.ResourceMetadata{ ParentId: c.projectID, Name: diskName, - Labels: map[string]string{ + Labels: labelsWithTags(ciRunLabels(attrs.Tags), map[string]string{ "created-by": "brev-cloud-sdk", "brev-user": c.refID, "environment-id": attrs.RefID, "image-family": imageFamily, - }, + }), }, Spec: &compute.DiskSpec{ Size: &compute.DiskSpec_SizeGibibytes{ diff --git a/v1/providers/nebius/integration_test.go b/v1/providers/nebius/integration_test.go index ae2dec53..9c6e80cd 100644 --- a/v1/providers/nebius/integration_test.go +++ b/v1/providers/nebius/integration_test.go @@ -51,6 +51,20 @@ func setupIntegrationTest(t *testing.T) *NebiusClient { return client } +// integrationTestTags returns the integration test's tags, adding the CI run ID when CI_RUN_ID is +// set so the post-run sweep can find this run's resources. +func integrationTestTags() map[string]string { + tags := map[string]string{ + "test": "integration", + "created-by": "nebius-integration-test", + "auto-delete": "true", + } + if id := os.Getenv("CI_RUN_ID"); id != "" { + tags[CIRunIDLabel] = id + } + return tags +} + // generateTestSSHKeyPair generates an RSA SSH key pair for testing // Returns private key (PEM format) and public key (OpenSSH format) func generateTestSSHKeyPair(t *testing.T) (privateKey, publicKey string) { @@ -275,11 +289,7 @@ func TestIntegration_InstanceLifecycle(t *testing.T) { DiskSize: 50 * 1024 * 1024 * 1024, // 50 GiB in bytes Location: selectedInstanceType.Location, // Use the instance type's location PublicKey: publicKey, // SSH public key for access (like Shadeform) - Tags: map[string]string{ - "test": "integration", - "created-by": "nebius-integration-test", - "auto-delete": "true", - }, + Tags: integrationTestTags(), } t.Logf("Creating instance with RefID: %s", instanceRefID) diff --git a/v1/providers/nebius/sweep_pass_test.go b/v1/providers/nebius/sweep_pass_test.go new file mode 100644 index 00000000..2d890ffd --- /dev/null +++ b/v1/providers/nebius/sweep_pass_test.go @@ -0,0 +1,185 @@ +package v1 + +import ( + "context" + "errors" + "reflect" + "testing" + + v1 "github.com/brevdev/cloud/v1" + common "github.com/nebius/gosdk/proto/nebius/common/v1" + compute "github.com/nebius/gosdk/proto/nebius/compute/v1" +) + +const ( + sweepTestRunID = "run-1" + testDiskID = "d1" +) + +var ( + errListFailed = errors.New("list failed") + errDeleteFailed = errors.New("delete failed") +) + +// fakeItem is a minimal element for exercising resourceSweepPass without the Nebius SDK. +type fakeItem struct { + id string + match bool +} + +// fakePage is one page returned by a fake list callback; err makes that page return an error. +type fakePage struct { + items []fakeItem + err bool +} + +// listFromPages returns a list callback that serves pages in order, one per call. +func listFromPages(pages []fakePage) func(context.Context, string, string) ([]fakeItem, string, error) { + call := 0 + return func(_ context.Context, _, _ string) ([]fakeItem, string, error) { + if call >= len(pages) { + return nil, "", nil + } + p := pages[call] + call++ + if p.err { + return nil, "", errListFailed + } + next := "" + if call < len(pages) { + next = "more" + } + return p.items, next, nil + } +} + +// TestResourceSweepPass covers keep filtering, pagination, and failure counting: a list error or a +// real delete error counts as a failure, while a not-found delete is success. +func TestResourceSweepPass(t *testing.T) { + match := fakeItem{id: "a", match: true} + other := fakeItem{id: "c", match: true} + skip := fakeItem{id: "b", match: false} + + tests := []struct { + name string + pages []fakePage + delErr error + wantDeleted int + wantFailures int + wantIDs []string + }{ + { + name: "deletes only matching items across two pages", + pages: []fakePage{{items: []fakeItem{match, skip}}, {items: []fakeItem{other}}}, + wantDeleted: 2, + wantIDs: []string{"a", "c"}, + }, + { + name: "list error counts as a failure and deletes nothing", + pages: []fakePage{{err: true}}, + wantFailures: 1, + }, + { + name: "not-found on delete is treated as success", + pages: []fakePage{{items: []fakeItem{match}}}, + delErr: v1.ErrResourceNotFound, + wantDeleted: 1, + }, + { + name: "real delete error counts as a failure", + pages: []fakePage{{items: []fakeItem{match}}}, + delErr: errDeleteFailed, + wantFailures: 1, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var deleted []string + del := func(_ context.Context, id string) error { + if tt.delErr != nil { + return tt.delErr + } + deleted = append(deleted, id) + return nil + } + keep := func(it fakeItem) (string, string, bool) { return it.id, it.id, it.match } + + gotDeleted, gotFailures := resourceSweepPass( + context.Background(), "fake", []string{"proj-1"}, t.Logf, + listFromPages(tt.pages), keep, del) + + if gotDeleted != tt.wantDeleted { + t.Errorf("deleted = %d, want %d", gotDeleted, tt.wantDeleted) + } + if gotFailures != tt.wantFailures { + t.Errorf("failures = %d, want %d", gotFailures, tt.wantFailures) + } + if tt.wantIDs != nil && !reflect.DeepEqual(deleted, tt.wantIDs) { + t.Errorf("deleted ids = %v, want %v", deleted, tt.wantIDs) + } + }) + } +} + +// TestResourceSweepPass_PerProjectFailureIsolation verifies a list error in one project counts as a +// failure but still lets another project be swept. +func TestResourceSweepPass_PerProjectFailureIsolation(t *testing.T) { + list := func(_ context.Context, projectID, _ string) ([]fakeItem, string, error) { + if projectID == "bad" { + return nil, "", errListFailed + } + return []fakeItem{{id: "g", match: true}}, "", nil + } + var deleted []string + del := func(_ context.Context, id string) error { + deleted = append(deleted, id) + return nil + } + keep := func(it fakeItem) (string, string, bool) { return it.id, it.id, it.match } + + gotDeleted, gotFailures := resourceSweepPass( + context.Background(), "fake", []string{"bad", "good"}, t.Logf, list, keep, del) + + if gotFailures != 1 { + t.Errorf("failures = %d, want 1 (the failing project)", gotFailures) + } + if gotDeleted != 1 || !reflect.DeepEqual(deleted, []string{"g"}) { + t.Errorf("good project not swept: deleted=%d ids=%v", gotDeleted, deleted) + } +} + +// diskMeta builds resource metadata with the given labels for the disk keep-predicate tests. +func diskMeta(labels map[string]string) *common.ResourceMetadata { + return &common.ResourceMetadata{Id: testDiskID, Name: "disk-1", Labels: labels} +} + +// TestDiskKeepPredicates verifies keepByLabel matches only this run's label (nil metadata safe) and +// keepUnattachedDisk also skips an attached disk. +func TestDiskKeepPredicates(t *testing.T) { + matching := map[string]string{CIRunIDLabel: sweepTestRunID} + byLabel := keepByLabel[*compute.Disk](sweepTestRunID) + unattached := keepUnattachedDisk(sweepTestRunID) + + tests := []struct { + name string + keep func(*compute.Disk) (string, string, bool) + disk *compute.Disk + wantOK bool + }{ + {"label match", byLabel, &compute.Disk{Metadata: diskMeta(matching)}, true}, + {"label mismatch", byLabel, &compute.Disk{Metadata: diskMeta(map[string]string{CIRunIDLabel: "run-2"})}, false}, + {"nil metadata", byLabel, &compute.Disk{}, false}, + {"unattached kept", unattached, &compute.Disk{Metadata: diskMeta(matching)}, true}, + {"read-write attached skipped", unattached, &compute.Disk{Metadata: diskMeta(matching), Status: &compute.DiskStatus{ReadWriteAttachment: "vm-1"}}, false}, + {"read-only attached skipped", unattached, &compute.Disk{Metadata: diskMeta(matching), Status: &compute.DiskStatus{ReadOnlyAttachments: []string{"vm-1"}}}, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, _, ok := tt.keep(tt.disk) + if ok != tt.wantOK { + t.Errorf("ok = %v, want %v", ok, tt.wantOK) + } + }) + } +} diff --git a/v1/providers/nebius/sweep_test.go b/v1/providers/nebius/sweep_test.go new file mode 100644 index 00000000..57efc704 --- /dev/null +++ b/v1/providers/nebius/sweep_test.go @@ -0,0 +1,221 @@ +package v1 + +import ( + "context" + "fmt" + "os" + "testing" + "time" + + "github.com/brevdev/cloud/internal/validation" + common "github.com/nebius/gosdk/proto/nebius/common/v1" + compute "github.com/nebius/gosdk/proto/nebius/compute/v1" + vpc "github.com/nebius/gosdk/proto/nebius/vpc/v1" +) + +// CIRunIDLabel aliases the validation key so the VM sweep and this resource sweep agree. +const CIRunIDLabel = validation.CIRunIDLabel + +// sweepPageSize is the max page size ([1...1000] per the Nebius API). Filtering is client-side, so +// every page must be fetched. +const sweepPageSize = 1000 + +type resourceSweepResult struct { + disks int + subnets int + networks int + failures int +} + +// sweepStandaloneResources deletes this run's leftover network/subnet/disk (VM gone), in dependency +// order: disks, subnets, then VPCs. Matches only ci-run-id == runID, so it never touches prod. +func (c *NebiusClient) sweepStandaloneResources(ctx context.Context, runID string, logf func(string, ...any)) (resourceSweepResult, error) { + var res resourceSweepResult + if runID == "" { + return res, fmt.Errorf("sweepStandaloneResources: runID is required") + } + projects, discovered := c.sweepProjects(ctx, logf) + if !discovered { + res.failures++ // narrowed scan may miss this run's resources in other projects; fail loudly + } + + var failed int + res.disks, failed = resourceSweepPass(ctx, "disk", projects, logf, c.listDisks, keepUnattachedDisk(runID), c.deleteBootDiskIfExists) + res.failures += failed + res.subnets, failed = resourceSweepPass(ctx, "subnet", projects, logf, c.listSubnets, keepByLabel[*vpc.Subnet](runID), c.deleteSubnetIfExists) + res.failures += failed + res.networks, failed = resourceSweepPass(ctx, "network", projects, logf, c.listNetworks, keepByLabel[*vpc.Network](runID), c.deleteNetworkIfExists) + res.failures += failed + return res, nil +} + +// sweepProjects lists every project to scan. discovered is false when tenant-wide discovery fails +// and it falls back to the primary project alone — a narrowed scan that may miss resources elsewhere, +// which the caller treats as a failure. +func (c *NebiusClient) sweepProjects(ctx context.Context, logf func(string, ...any)) (projects []string, discovered bool) { + projectToRegion, err := c.discoverAllProjectsWithRegions(ctx) + if err != nil || len(projectToRegion) == 0 { + logf("[sweep] WARNING: project discovery failed (err=%v); scanning ONLY primary project %s — resources in other projects will be missed", err, c.projectID) + return []string{c.projectID}, false + } + projects = make([]string, 0, len(projectToRegion)) + for projectID := range projectToRegion { + projects = append(projects, projectID) + } + return projects, true +} + +// resourceSweepPass paginates one resource type across every project: keep selects this run's +// deletable items, del removes them. A list error counts as a failure (so a partial scan is never +// silent) and stops that project's pages; a not-found on delete is success. +func resourceSweepPass[T any]( + ctx context.Context, + kind string, + projects []string, + logf func(string, ...any), + list func(ctx context.Context, projectID, pageToken string) (items []T, nextPageToken string, err error), + keep func(item T) (id, name string, ok bool), + del func(ctx context.Context, id string) error, +) (deleted, failures int) { + for _, projectID := range projects { + pageToken := "" + for { + items, next, err := list(ctx, projectID, pageToken) + if err != nil { + logf("[sweep] list %ss in %s failed: %v", kind, projectID, err) + failures++ + break + } + for _, item := range items { + id, name, ok := keep(item) + if !ok { + continue + } + logf("[sweep] %s %s name=%q", kind, id, name) + if derr := del(ctx, id); derr != nil && !isNotFoundError(derr) { + logf("[sweep] FAILED %s %s: %v", kind, id, derr) + failures++ + continue + } + deleted++ + } + if pageToken = next; pageToken == "" { + break + } + } + } + return deleted, failures +} + +// keepByLabel keeps resources whose ci-run-id label == runID, returning their id/name. +func keepByLabel[T interface { + GetMetadata() *common.ResourceMetadata +}](runID string) func(item T) (id, name string, ok bool) { + return func(item T) (string, string, bool) { + md := item.GetMetadata() + if md == nil || md.GetLabels()[CIRunIDLabel] != runID { + return "", "", false + } + return md.GetId(), md.GetName(), true + } +} + +// keepUnattachedDisk keeps this run's disks that aren't attached to a VM (attached ones belong to a +// live VM the VM pass handles). +func keepUnattachedDisk(runID string) func(disk *compute.Disk) (id, name string, ok bool) { + base := keepByLabel[*compute.Disk](runID) + return func(disk *compute.Disk) (string, string, bool) { + id, name, ok := base(disk) + if !ok { + return "", "", false + } + if st := disk.GetStatus(); st.GetReadWriteAttachment() != "" || len(st.GetReadOnlyAttachments()) > 0 { + return "", "", false + } + return id, name, true + } +} + +// listDisks, listSubnets, and listNetworks each fetch one page of a resource type within a project. +func (c *NebiusClient) listDisks(ctx context.Context, projectID, pageToken string) ([]*compute.Disk, string, error) { + resp, err := c.sdk.Services().Compute().V1().Disk().List(ctx, &compute.ListDisksRequest{ + ParentId: projectID, PageSize: sweepPageSize, PageToken: pageToken, + }) + if err != nil { + return nil, "", err + } + return resp.GetItems(), resp.GetNextPageToken(), nil +} + +func (c *NebiusClient) listSubnets(ctx context.Context, projectID, pageToken string) ([]*vpc.Subnet, string, error) { + resp, err := c.sdk.Services().VPC().V1().Subnet().List(ctx, &vpc.ListSubnetsRequest{ + ParentId: projectID, PageSize: sweepPageSize, PageToken: pageToken, + }) + if err != nil { + return nil, "", err + } + return resp.GetItems(), resp.GetNextPageToken(), nil +} + +func (c *NebiusClient) listNetworks(ctx context.Context, projectID, pageToken string) ([]*vpc.Network, string, error) { + resp, err := c.sdk.Services().VPC().V1().Network().List(ctx, &vpc.ListNetworksRequest{ + ParentId: projectID, PageSize: sweepPageSize, PageToken: pageToken, + }) + if err != nil { + return nil, "", err + } + return resp.GetItems(), resp.GetNextPageToken(), nil +} + +// TestSweepOrphans deletes this run's leftover VMs then its standalone network/subnet/disk. Gated on +// SWEEP_ORPHANS=true and CI_RUN_ID; deletes only resources whose ci-run-id == CI_RUN_ID. +func TestSweepOrphans(t *testing.T) { + if os.Getenv("SWEEP_ORPHANS") != "true" { + t.Skip("SWEEP_ORPHANS is not set to true, skipping orphan sweep") + } + runID := os.Getenv("CI_RUN_ID") + if runID == "" { + t.Skip("CI_RUN_ID is not set; the run-scoped sweep has nothing to match") + } + + serviceAccountJSON := os.Getenv("NEBIUS_SERVICE_ACCOUNT_JSON") + tenantID := os.Getenv("NEBIUS_TENANT_ID") + if serviceAccountJSON == "" || tenantID == "" { + t.Skip("Skipping sweep: NEBIUS_SERVICE_ACCOUNT_JSON and NEBIUS_TENANT_ID must be set") + } + if _, statErr := os.Stat(serviceAccountJSON); statErr == nil { + //nolint:gosec // maintenance entrypoint reading the service account from a controlled CI env + data, err := os.ReadFile(serviceAccountJSON) + if err != nil { + t.Fatalf("failed to read service account file: %v", err) + } + serviceAccountJSON = string(data) + } + + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Minute) + defer cancel() + + // Ref ID is arbitrary; ownership comes from the ci-run-id label, not this client. + client, err := NewNebiusClient(ctx, "orphan-sweeper", serviceAccountJSON, tenantID, "", defaultNebiusLocation) + if err != nil { + t.Fatalf("failed to create Nebius client: %v", err) + } + + // VMs first — terminate cascades their attached network/disk. + vmRes, err := validation.SweepOrphanedInstances(ctx, client, validation.SweepOpts{RunID: runID, Logf: t.Logf}) + if err != nil { + t.Fatalf("VM sweep failed: %v", err) + } + + // Then standalone network/subnet/disk left behind. + resRes, err := client.sweepStandaloneResources(ctx, runID, t.Logf) + if err != nil { + t.Fatalf("resource sweep failed: %v", err) + } + + t.Logf("sweep summary: runID=%s vms_deleted=%d vms_failed=%d disks=%d subnets=%d networks=%d res_failures=%d", + runID, len(vmRes.Deleted), len(vmRes.Failed), resRes.disks, resRes.subnets, resRes.networks, resRes.failures) + if len(vmRes.Failed) > 0 || resRes.failures > 0 { + t.Errorf("sweep had failures: vms=%d resources=%d (see logs above)", len(vmRes.Failed), resRes.failures) + } +} diff --git a/v1/providers/nebius/validation_test.go b/v1/providers/nebius/validation_test.go index 460ae98a..45dd50de 100644 --- a/v1/providers/nebius/validation_test.go +++ b/v1/providers/nebius/validation_test.go @@ -38,11 +38,20 @@ func TestInstanceLifecycleValidation(t *testing.T) { config := validation.ProviderConfig{ Credential: newNebiusCredential(t), Location: "eu-north1", + Tags: ciRunTags(), } validation.RunInstanceLifecycleValidation(t, config) } +// ciRunTags returns the CI run label when CI_RUN_ID is set (nil on local runs). +func ciRunTags() map[string]string { + if id := os.Getenv("CI_RUN_ID"); id != "" { + return map[string]string{CIRunIDLabel: id} + } + return nil +} + func newNebiusCredential(t *testing.T) *NebiusCredential { serviceAccountJSON := os.Getenv(nebiusServiceAccountJSONEnvVar) tenantID := os.Getenv(nebiusTenantIDEnvVar) diff --git a/v1/utils.go b/v1/utils.go index a5b3563f..60ba90c7 100644 --- a/v1/utils.go +++ b/v1/utils.go @@ -1,3 +1,7 @@ package v1 type Tags map[string]string + +// CIRunIDLabel labels a CI run's instances and their sub-resources with a per-run value so a +// post-run sweep can delete them. Production never sets it. +const CIRunIDLabel = "ci-run-id" From c13d55444a6acf57eba7af204af09f9365bf54b4 Mon Sep 17 00:00:00 2001 From: hakhandelwal11 Date: Thu, 17 Sep 2026 16:09:12 +0530 Subject: [PATCH 2/3] fix: bound cleanup/sweep terminate to a single 8-min attempt --- internal/validation/suite.go | 21 ++++++++------------- internal/validation/sweep.go | 22 +++++++++------------- 2 files changed, 17 insertions(+), 26 deletions(-) diff --git a/internal/validation/suite.go b/internal/validation/suite.go index cd7754ae..1d5f6be8 100644 --- a/internal/validation/suite.go +++ b/internal/validation/suite.go @@ -8,7 +8,6 @@ import ( "github.com/brevdev/cloud/internal/ssh" v1 "github.com/brevdev/cloud/v1" - "github.com/cenkalti/backoff/v4" "github.com/stretchr/testify/require" ) @@ -22,7 +21,7 @@ type ProviderConfig struct { } // registerInstanceCleanup schedules termination via t.Cleanup so it runs even after a t.Fatalf, -// on a fresh context with retry. Not-found counts as success; a terminal failure fails the test. +// on a fresh 8-min context. Not-found counts as success; a terminal failure fails the test. // Returns markTerminated, which the caller invokes once it has terminated the instance itself to // skip the redundant delete. Call this right after a create, before any assertion. func registerInstanceCleanup(t *testing.T, client v1.CloudCreateTerminateInstance, cloudID v1.CloudProviderInstanceID) (markTerminated func()) { @@ -36,17 +35,13 @@ func registerInstanceCleanup(t *testing.T, client v1.CloudCreateTerminateInstanc if terminated { return } - op := func() error { - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) - defer cancel() - err := client.TerminateInstance(ctx, cloudID) - if err == nil || errors.Is(err, v1.ErrInstanceNotFound) || errors.Is(err, v1.ErrResourceNotFound) { - return nil - } - return err - } - if err := backoff.Retry(op, backoff.WithMaxRetries(backoff.NewExponentialBackOff(), 4)); err != nil { - t.Errorf("LEAKED INSTANCE %s: cleanup failed after retries: %v", cloudID, err) + // Single 8-min attempt on a fresh context (real terminates ~1 min; internal wait caps at + // 5 minutes). + ctx, cancel := context.WithTimeout(context.Background(), 8*time.Minute) + defer cancel() + err := client.TerminateInstance(ctx, cloudID) + if err != nil && !errors.Is(err, v1.ErrInstanceNotFound) && !errors.Is(err, v1.ErrResourceNotFound) { + t.Errorf("LEAKED INSTANCE %s: cleanup terminate failed: %v", cloudID, err) } }) return markTerminated diff --git a/internal/validation/sweep.go b/internal/validation/sweep.go index bc8d21b2..287a008a 100644 --- a/internal/validation/sweep.go +++ b/internal/validation/sweep.go @@ -7,7 +7,6 @@ import ( "time" v1 "github.com/brevdev/cloud/v1" - "github.com/cenkalti/backoff/v4" ) // CIRunIDLabel re-exports v1.CIRunIDLabel so the sweeper and create paths share one value. @@ -63,7 +62,7 @@ func SweepOrphanedInstances(ctx context.Context, client v1.CloudClient, opts Swe inst := res.Matched[i] logf("[sweep] terminating cloudID=%s name=%q owner=%s location=%s", inst.CloudID, inst.Name, inst.CloudCredRefID, inst.Location) - if err := terminateWithRetry(ctx, client, inst.CloudID); err != nil { + if err := terminateWithTimeout(ctx, client, inst.CloudID); err != nil { res.Failed[inst.CloudID] = err logf("[sweep] FAILED to terminate cloudID=%s: %v", inst.CloudID, err) continue @@ -73,17 +72,14 @@ func SweepOrphanedInstances(ctx context.Context, client v1.CloudClient, opts Swe return res, nil } -// terminateWithRetry terminates an instance with a bounded exponential backoff, treating a +// terminateWithTimeout terminates an instance in a single attempt on an 8-min context, treating a // not-found result as success (the instance is already gone). -func terminateWithRetry(ctx context.Context, client v1.CloudCreateTerminateInstance, id v1.CloudProviderInstanceID) error { - op := func() error { - termCtx, cancel := context.WithTimeout(ctx, 10*time.Minute) - defer cancel() - err := client.TerminateInstance(termCtx, id) - if err == nil || errors.Is(err, v1.ErrInstanceNotFound) || errors.Is(err, v1.ErrResourceNotFound) { - return nil - } - return err +func terminateWithTimeout(ctx context.Context, client v1.CloudCreateTerminateInstance, id v1.CloudProviderInstanceID) error { + termCtx, cancel := context.WithTimeout(ctx, 8*time.Minute) + defer cancel() + err := client.TerminateInstance(termCtx, id) + if err == nil || errors.Is(err, v1.ErrInstanceNotFound) || errors.Is(err, v1.ErrResourceNotFound) { + return nil } - return backoff.Retry(op, backoff.WithMaxRetries(backoff.NewExponentialBackOff(), 4)) + return err } From 1d9de3878b9bbe83b762391f1ae530ec059a976e Mon Sep 17 00:00:00 2001 From: hakhandelwal11 Date: Sat, 19 Sep 2026 14:08:14 +0530 Subject: [PATCH 3/3] ci: disable test cache on the sweep --- .github/workflows/validation-nebius.yml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/workflows/validation-nebius.yml b/.github/workflows/validation-nebius.yml index 050ff283..9b93647e 100644 --- a/.github/workflows/validation-nebius.yml +++ b/.github/workflows/validation-nebius.yml @@ -42,8 +42,7 @@ jobs: TEST_PRIVATE_KEY_BASE64: ${{ secrets.TEST_PRIVATE_KEY_BASE64 }} TEST_PUBLIC_KEY_BASE64: ${{ secrets.TEST_PUBLIC_KEY_BASE64 }} VALIDATION_TEST: true - # Stamps this run's resources for the sweep below. run_id (not run_attempt) so a - # re-run also reaps resources a prior attempt leaked before its cleanup ran. + # Stamps this run's resources for the sweep below. CI_RUN_ID: ${{ github.run_id }} run: | cd v1/providers/nebius @@ -60,7 +59,7 @@ jobs: CI_RUN_ID: ${{ github.run_id }} run: | cd v1/providers/nebius - go test -v -run '^TestSweepOrphans$' -timeout=20m . + go test -v -run '^TestSweepOrphans$' -count=1 -timeout=20m . - name: Upload test results uses: actions/upload-artifact@v4