chore(tests): add sweeper to remove workspaces

[mitchnielsen]

Summary

Adds sweepers to clean up workspaces in cases where they were not automatically cleaned up.

https://developer.hashicorp.com/terraform/plugin/testing/acceptance-tests/sweepers

Related to https://linear.app/prefect/issue/PLA-642/ensure-ephemeral-workspaces-are-cleaned-up-consistently

Testing

Initially I want to disable the actual deletion so we can maintain some dangling workspaces in the account for testing purposes. So I made a small change:

diff --git a/internal/provider/resources/sweeper_test.go b/internal/provider/resources/sweeper_test.go
index ce3f4a9..bc58065 100644
--- a/internal/provider/resources/sweeper_test.go
+++ b/internal/provider/resources/sweeper_test.go
@@ -55,10 +55,11 @@ func addWorkspaceSweeper() {
 				if strings.HasPrefix(workspace.Name, testutils.TestAccPrefix) {
 					log.Printf("found acceptance testing workspace %s, deleting...\n", workspace.Name)
 
-					err := workspacesClient.Delete(context.Background(), workspace.ID)
-					if err != nil {
-						log.Printf("unable to delete workspaces %s during sweep: %s\n", workspace.Name, err)
-					}
+					log.Printf("pretending to delete workspaces %s\n", workspace.Name)
 				} else {
 					log.Printf("workspace %s does not match acceptance testing prefix, skipping...\n", workspace.Name)
 				}

And ran the following:

$ SWEEP="yes" make testacc-dev
./scripts/testacc-dev "" "INFO" yes
sweeping...
2025/01/13 16:46:13 [DEBUG] Running Sweepers for region (all):
2025/01/13 16:46:13 [DEBUG] Running Sweeper (workspaces) in region (all)
2025/01/13 16:46:13 [DEBUG] POST https://api.stg.prefect.dev/api/accounts/bb19c492-73c2-4ecd-9cd7-d82c4aac08e6/workspaces/filter
2025/01/13 16:46:13 workspace github-ci-tests does not match acceptance testing prefix, skipping...
2025/01/13 16:46:13 found acceptance testing workspace terraformacc07to6nvdgi, deleting...
2025/01/13 16:46:13 pretending to delete workspaces terraformacc07to6nvdgi

... and so on ...

2025/01/13 16:46:13 found acceptance testing workspace terraformaccy3jwhpmhkx, deleting...
2025/01/13 16:46:13 pretending to delete workspaces terraformaccy3jwhpmhkx
2025/01/13 16:46:13 found acceptance testing workspace terraformaccz6zhmdb8ep, deleting...
2025/01/13 16:46:13 pretending to delete workspaces terraformaccz6zhmdb8ep
2025/01/13 16:46:13 workspace test does not match acceptance testing prefix, skipping...
2025/01/13 16:46:13 [DEBUG] Completed Sweeper (workspaces) in region (all) in 188.201667ms
2025/01/13 16:46:13 Completed Sweepers for region (all) in 188.543042ms
2025/01/13 16:46:13 Sweeper Tests for region (all) ran successfully:
2025/01/13 16:46:13     - workspaces
ok      github.com/prefecthq/terraform-provider-prefect/internal/provider/resources     0.731s

[mitchnielsen]

Hitting some snags here. The sweeper docs are pretty short and vague, which led someone to create hashicorp/terraform-plugin-testing#378 with very fair questions. It's a few months old now with no response, so will keep an eye on that. Haven't found many other sweeper-related issues logged yet.

I also noticed that even if I put an os.Exit(1) in the sweeper function, I never see that exit failure in the output. It just always looks like:

2025/01/10 16:51:12 [DEBUG] Running Sweepers for region (foo):
2025/01/10 16:51:12 Completed Sweepers for region (foo) in 145.458µs
2025/01/10 16:51:12 Sweeper Tests for region (foo) ran successfully:

Note that foo is what you pass in with -sweep=foo. I've tried different values here and aligned them with the sweeper function, but it always just exits without any of my logging statements present (even with tflog.Debug).

In general, I'm a bit hesitant to pursue this much further because:

• Documentation is so scarce
• It's not very clear when in the testing workflow this runs
• I'm not able to get the sweeper to run so far

I'll dig into this a bit more, but if I don't make much progress (assuming no developments in the sweeper docs) we can use the logic in this function and just call it directly after the tests run, rather than using Terraform's sweeper facilities.

FYI @parkedwards (no action required)

[mitchnielsen]

Some links for reference later:

• Document how to write and use sweepers in contributor docs hashicorp/terraform-provider-google#17891
• https://github.com/hashicorp/terraform-provider-google/wiki/Developer-Best-Practices#sweepers
• 🧹 Enhance Terraform Sweeper documentation hashicorp/terraform-plugin-testing#378
• https://developer.hashicorp.com/terraform/plugin/testing/acceptance-tests/sweepers

[parkedwards]

im guessing -sweep=all didn't work? or is all supposed to be an arbitrary selector?

[mitchnielsen]

Looks like sweep specifies the provider to sweep, and sweep-run lets you specify specific sweepers. Also learned that providing the sweep flag only runs the sweepers, and that it's often recommended to:

1. Run the sweepers (for a clean slate)
2. Run the acceptance tests
3. Run the sweepers again

[mitchnielsen]

More context in a4d16f7

The ephemeral workspaces we make during tests have random values.
Because sweepers are run in a separate command from the acceptance
tests, there's currently no way to persist those unique values between
commands so the sweeper can delete those specific workspaces.

Instead, as a first iteration, this change lists all workspaces in an
account and deletes any that match the acceptance testing prefix we use.
This is designed to run either by hand or by CI at a given interval,
such as overnight, when other acceptance tests are not running.

Welcoming feedback here. If we want to do the more traditional implementation, we'll need to find a way to persist the unique names of the ephemeral workspaces between running the acceptance tests and then running the sweepers.

Also, the nature of us using an entirely separate (ephemeral) workspace for each test is not reflected in the basic scenario shown in the documentation. It would be more fitting if we had a static prefix for the resource in the test, and could then turn around and delete all of those specific resources matching that prefix.

I think there's a few ways we can approach this, so now that I understand sweepers a bit better, I'm very open to making adjustments here.

[mitchnielsen]

Put another way: we're not fully following the true spirit of sweepers here, in the sense that we're just making a sweeper separated from any specific resource test. That's because of our approach with making ephemeral workspaces as part of each test, so it's most effective to just go clean up those ephemeral workspaces.

But it gets the ball rolling because we can start moving in that direction if it makes sense, and we implemented the cleanup following (loosely) a Terraform framework rather than writing external scripts/etc to do the work.

It even makes me wonder if ephemeral workspaces are as vital as long as we use ephemeral name prefixes for each resource, and then use sweepers to make sure they're cleaned up. Both seem to have their own pros and cons.

[parkedwards]

im OK with this. maybe I had misunderstood the original intention of implementing sweepers, because I had always imagined them running independently / being idempotent based off of that resource name prefix we use

[mitchnielsen]

TODO: pending the approach we land on, we'll want to add a scheduled CI action to run the sweeper at a given interval (such as overnight when things are quiet).

[parkedwards]

should SWEEP be a bool?

[parkedwards]

oh i see, the shell script checks for any value