Durable async cleanup to speed up tests (#109)

* Simplify cleanup using centralized tf/destroy, return cleanup closures on error, and apply review fixes for robust cleanup

* Hardcode cleanup dir

* Address David's comments

* Fix nit in main.tf

Author: aabmass

.gitignore

@@ -19,3 +19,6 @@ terraform*
 
 go.work
 go.work.sum
+
+# Ignore nested jj workspace
+concurrent_task/

cloudbuild-cleanup.yaml

@@ -0,0 +1,13 @@
+steps:
+  - name: $_TEST_RUNNER_IMAGE
+    id: cleanup
+    dir: /
+    timeout: 600s
+    args:
+      - cleanup
+      - --environment=$_E2E_ENVIRONMENT
+      - --project-id=$PROJECT_ID
+      - --test-run-id=$_TEST_RUN_ID
+
+substitutions:
+  _TEST_RUN_ID: $BUILD_ID

cloudbuild-e2e-cloud-functions-gen2.yaml

@@ -61,14 +61,19 @@ steps:
     id: run-tests-cloudfunctions
     dir: /
     timeout: 1800s
-    env: ["PROJECT_ID=$PROJECT_ID"]
+    env: ["PROJECT_ID=$PROJECT_ID", "TEST_RUN_ID=$BUILD_ID"]
     args:
       - cloud-functions-gen2
       - --runtime=go125
       - --functionsource=/workspace/opentelemetry-operations-go/e2e-test-server/cloud_functions/function-source.zip
       - --entrypoint=HandleCloudFunction
+      - --skip-cleanup
 
 logsBucket: gs://opentelemetry-ops-e2e-cloud-build-logs
 substitutions:
   _TEST_RUNNER_IMAGE: us-central1-docker.pkg.dev/${PROJECT_ID}/e2e-testing/opentelemetry-operations-e2e-testing:${SHORT_SHA}
   _TEST_SERVER_IMAGE: us-central1-docker.pkg.dev/${PROJECT_ID}/e2e-testing/opentelemetry-operations-go-e2e-test-server:${SHORT_SHA}
+
+options:
+  # Notify for Cloud Build async cleanup trigger
+  pubsubTopic: projects/opentelemetry-ops-e2e/topics/e2e-cleanup

cloudbuild-e2e-cloud-run.yaml

@@ -39,12 +39,17 @@ steps:
     id: run-tests-cloudrun
     dir: /
     timeout: 10m
-    env: ["PROJECT_ID=$PROJECT_ID"]
+    env: ["PROJECT_ID=$PROJECT_ID", "TEST_RUN_ID=$BUILD_ID"]
     args:
       - cloud-run
       - --image=$_TEST_SERVER_IMAGE
+      - --skip-cleanup
 
 logsBucket: gs://opentelemetry-ops-e2e-cloud-build-logs
 substitutions:
   _TEST_RUNNER_IMAGE: us-central1-docker.pkg.dev/${PROJECT_ID}/e2e-testing/opentelemetry-operations-e2e-testing:${SHORT_SHA}
   _TEST_SERVER_IMAGE: us-central1-docker.pkg.dev/${PROJECT_ID}/e2e-testing/opentelemetry-operations-go-e2e-test-server:${SHORT_SHA}
+
+options:
+  # Notify for Cloud Build async cleanup trigger
+  pubsubTopic: projects/opentelemetry-ops-e2e/topics/e2e-cleanup

cloudbuild-e2e-gae-standard.yaml

@@ -61,13 +61,18 @@ steps:
     id: run-tests-gae-standard
     dir: /
     timeout: 10m
-    env: ["PROJECT_ID=$PROJECT_ID"]
+    env: ["PROJECT_ID=$PROJECT_ID", "TEST_RUN_ID=$BUILD_ID"]
     args:
       - gae-standard
       - --runtime=go125
       - --appsource=/workspace/opentelemetry-operations-go/e2e-test-server/appsource.zip
+      - --skip-cleanup
 
 logsBucket: gs://opentelemetry-ops-e2e-cloud-build-logs
 substitutions:
   _TEST_RUNNER_IMAGE: us-central1-docker.pkg.dev/${PROJECT_ID}/e2e-testing/opentelemetry-operations-e2e-testing:${SHORT_SHA}
   _TEST_SERVER_IMAGE: us-central1-docker.pkg.dev/${PROJECT_ID}/e2e-testing/opentelemetry-operations-go-e2e-test-server:${SHORT_SHA}
+
+options:
+  # Notify for Cloud Build async cleanup trigger
+  pubsubTopic: projects/opentelemetry-ops-e2e/topics/e2e-cleanup

cloudbuild-e2e-gae.yaml

@@ -39,13 +39,18 @@ steps:
     id: run-tests-gae
     dir: /
     timeout: 10m
-    env: ["PROJECT_ID=$PROJECT_ID"]
+    env: ["PROJECT_ID=$PROJECT_ID", "TEST_RUN_ID=$BUILD_ID"]
     args:
       - gae
       - --image=$_TEST_SERVER_IMAGE
       - --runtime=go125
+      - --skip-cleanup
 
 logsBucket: gs://opentelemetry-ops-e2e-cloud-build-logs
 substitutions:
   _TEST_RUNNER_IMAGE: us-central1-docker.pkg.dev/${PROJECT_ID}/e2e-testing/opentelemetry-operations-e2e-testing:${SHORT_SHA}
   _TEST_SERVER_IMAGE: us-central1-docker.pkg.dev/${PROJECT_ID}/e2e-testing/opentelemetry-operations-go-e2e-test-server:${SHORT_SHA}
+
+options:
+  # Notify for Cloud Build async cleanup trigger
+  pubsubTopic: projects/opentelemetry-ops-e2e/topics/e2e-cleanup

cloudbuild-e2e-gce.yaml

@@ -39,12 +39,17 @@ steps:
     id: run-tests-gce
     dir: /
     timeout: 10m
-    env: ["PROJECT_ID=$PROJECT_ID"]
+    env: ["PROJECT_ID=$PROJECT_ID", "TEST_RUN_ID=$BUILD_ID"]
     args:
       - gce
       - --image=$_TEST_SERVER_IMAGE
+      - --skip-cleanup
 
 logsBucket: gs://opentelemetry-ops-e2e-cloud-build-logs
 substitutions:
   _TEST_RUNNER_IMAGE: us-central1-docker.pkg.dev/${PROJECT_ID}/e2e-testing/opentelemetry-operations-e2e-testing:${SHORT_SHA}
   _TEST_SERVER_IMAGE: us-central1-docker.pkg.dev/${PROJECT_ID}/e2e-testing/opentelemetry-operations-go-e2e-test-server:${SHORT_SHA}
+
+options:
+  # Notify for Cloud Build async cleanup trigger
+  pubsubTopic: projects/opentelemetry-ops-e2e/topics/e2e-cleanup

cloudbuild-e2e-gke.yaml

@@ -39,12 +39,17 @@ steps:
     id: run-tests-gke
     dir: /
     timeout: 10m
-    env: ["PROJECT_ID=$PROJECT_ID"]
+    env: ["PROJECT_ID=$PROJECT_ID", "TEST_RUN_ID=$BUILD_ID"]
     args:
       - gke
       - --image=$_TEST_SERVER_IMAGE
+      - --skip-cleanup
 
 logsBucket: gs://opentelemetry-ops-e2e-cloud-build-logs
 substitutions:
   _TEST_RUNNER_IMAGE: us-central1-docker.pkg.dev/${PROJECT_ID}/e2e-testing/opentelemetry-operations-e2e-testing:${SHORT_SHA}
   _TEST_SERVER_IMAGE: us-central1-docker.pkg.dev/${PROJECT_ID}/e2e-testing/opentelemetry-operations-go-e2e-test-server:${SHORT_SHA}
+
+options:
+  # Notify for Cloud Build async cleanup trigger
+  pubsubTopic: projects/opentelemetry-ops-e2e/topics/e2e-cleanup

e2etesting/e2e_testing.go

@@ -23,6 +23,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/GoogleCloudPlatform/opentelemetry-operations-e2e-testing/e2etesting/setuptf"
 	"github.com/GoogleCloudPlatform/opentelemetry-operations-e2e-testing/e2etestrunner/testclient"
 	"github.com/alexflint/go-arg"
 )
@@ -106,12 +107,16 @@ type CloudFunctionsGen2Cmd struct {
 	FunctionSource string `arg:"required" help:"The full path of the zip file that contains the code source that needs to run within the CloudFunction"`
 }
 
+type CleanupCmd struct{}
+
 type Args struct {
 	// This subcommand is a special case, it doesn't run any tests. It just
 	// applies the persistent resources which are used across tests. See
 	// tf/persistent/README.md for details on what is in there.
 	ApplyPersistent *ApplyPersistent `arg:"subcommand:apply-persistent" help:"Terraform apply the resources in tf/persistent and exit (does not run tests)."`
 
+	Cleanup              *CleanupCmd              `arg:"subcommand:cleanup" help:"Clean up resources for a test run"`
+
 	Local                *LocalCmd                `arg:"subcommand:local" help:"Deploy the test server locally with docker and execute tests"`
 	Gke                  *GkeCmd                  `arg:"subcommand:gke" help:"Deploy the test server on GKE and execute tests"`
 	Gce                  *GceCmd                  `arg:"subcommand:gce" help:"Deploy the test server on GCE and execute tests"`
@@ -135,6 +140,7 @@ type Args struct {
 	// resources created for debugging. If not provided, we generate a hex
 	// string.
 	TestRunID string `arg:"--test-run-id,env:TEST_RUN_ID" help:"Optional test run id to use to partition terraform resources"`
+	SkipCleanup bool   `arg:"--skip-cleanup" help:"Skip automatic cleanup after tests (default: false). If enabled, you must run the 'cleanup' subcommand manually or via trigger."`
 }
 
 type Cleanup func()
@@ -164,6 +170,15 @@ func InitTestMain(args *Args, applyPersistent ApplyPersistentFunc) (*log.Logger,
 	logger := log.New(os.Stdout, "TestMain: ", log.LstdFlags|log.Lshortfile)
 	ctx := context.Background()
 
+	// Handle cleanup subcommand
+	if args.Cleanup != nil {
+		err := setuptf.CleanupTf(ctx, args.ProjectID, args.TestRunID, logger)
+		if err != nil {
+			logger.Panic(err)
+		}
+		return nil, nil, true
+	}
+
 	// Handle special case of just creating persistent resources
 	if args.ApplyPersistent != nil {
 		err := applyPersistent(ctx, args.ProjectID, args.ApplyPersistent.AutoApprove, logger)

e2etesting/setuptf/setuptf.go

@@ -69,9 +69,8 @@ func initCommand(ctx context.Context, projectID string) *exec.Cmd {
 	)
 }
 
-// Runs the sequence of terraform commands most environemnts need, returns the
-// output bytes of `terraform output -json` and a cleanup function to teardown
-// the created resources.
+// Runs the sequence of terraform commands most environments need, returns the
+// parsed PubsubInfo from `terraform output -json`.
 //
 // 1. Run terraform init
 // 2. Create a new terraform workspace for the test run ID
@@ -86,35 +85,17 @@ func SetupTf(
 	tfDir string, // the Dir to set when running terraform commands in e.g. tf/gke
 	tfVars map[string]string, // key-values for terraform input vars to send to terraform
 	logger *log.Logger,
-) (*PubsubInfo, func(), error) {
+) (*PubsubInfo, error) {
 	tfVarArgs := tfVarMapToArgs(projectID, tfVars)
 	cmd := initCommand(ctx, projectID)
 	cmd.Args = append(cmd.Args, tfVarArgs...)
 	cmd.Dir = tfDir
 	if err := runWithOutput(cmd, logger); err != nil {
-		return nil, func() {}, err
+		return nil, err
 	}
 
 	logger.Printf("Running %s with image: %s\n", tfDir, tfVars["image"])
 
-	cleanup := func() {
-		defer deleteWorkspace(ctx, testRunID, tfDir, logger)
-
-		// Run terraform destroy
-		cmd = exec.CommandContext(
-			ctx,
-			"terraform",
-			"destroy",
-			"-input=false",
-			"-auto-approve",
-		)
-		cmd.Args = append(cmd.Args, tfVarArgs...)
-		cmd.Dir = tfDir
-		if err := runWithOutput(cmd, logger); err != nil {
-			logger.Panic(err)
-		}
-	}
-
 	// Create new terraform workspace
 	cmd = exec.CommandContext(ctx, "terraform", "workspace", "new", testRunID)
 	cmd.Dir = tfDir
@@ -124,7 +105,7 @@ func SetupTf(
 		cmd.Dir = tfDir
 
 		if err := runWithOutput(cmd, logger); err != nil {
-			return nil, cleanup, err
+			return nil, err
 		}
 	}
 
@@ -139,7 +120,7 @@ func SetupTf(
 	cmd.Args = append(cmd.Args, tfVarArgs...)
 	cmd.Dir = tfDir
 	if err := runWithOutput(cmd, logger); err != nil {
-		return nil, cleanup, err
+		return nil, err
 	}
 
 	// Run terraform output
@@ -148,14 +129,14 @@ func SetupTf(
 	out, err := cmd.Output()
 	if err != nil {
 		logger.Println(err)
-		return nil, cleanup, err
+		return nil, err
 	}
 
 	tfOutput := &tfOutput{}
 	if err := json.Unmarshal(out, tfOutput); err != nil {
-		return nil, cleanup, err
+		return nil, err
 	}
-	return &tfOutput.PubsubInfoWrapper.Value, cleanup, nil
+	return &tfOutput.PubsubInfoWrapper.Value, nil
 }
 
 func ApplyPersistent(
@@ -255,3 +236,47 @@ func tfVarMapToArgs(
 	}
 	return tfVarArgs
 }
+
+// CleanupTf runs terraform destroy and deletes the workspace.
+func CleanupTf(
+	ctx context.Context,
+	projectID string,
+	testRunID string,
+	logger *log.Logger,
+) error {
+	const tfDir = "tf/destroy"
+	tfVarArgs := []string{fmt.Sprintf("-var=project_id=%s", projectID)}
+	cmd := initCommand(ctx, projectID)
+	cmd.Dir = tfDir
+	if err := runWithOutput(cmd, logger); err != nil {
+		logger.Printf("error cleaning up terraform (init) in %s: %v", tfDir, err)
+		return err
+	}
+
+	// Switch to target workspace
+	cmd = exec.CommandContext(ctx, "terraform", "workspace", "select", testRunID)
+	cmd.Dir = tfDir
+	if err := runWithOutput(cmd, logger); err != nil {
+		logger.Printf("error cleaning up terraform (workspace select %s) in %s: %v", testRunID, tfDir, err)
+		return err
+	}
+
+	// Run terraform destroy
+	cmd = exec.CommandContext(
+		ctx,
+		"terraform",
+		"destroy",
+		"-input=false",
+		"-auto-approve",
+	)
+	cmd.Args = append(cmd.Args, tfVarArgs...)
+	cmd.Dir = tfDir
+	if err := runWithOutput(cmd, logger); err != nil {
+		logger.Printf("error cleaning up terraform (destroy) in %s: %v", tfDir, err)
+		return err
+	}
+
+	deleteWorkspace(ctx, testRunID, tfDir, logger)
+
+	return nil
+}