tests/provider: Add concurrency to sweep (DB instance)

[YakDriver]

Community Note

• Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
• Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Relates #15334

Release note for CHANGELOG:

NONE

Sweeping one DB instance:

2020/09/24 14:48:56 [DEBUG] DB Instance status for instance terraform-20200924184121666000000001: deleting
2020/09/24 14:48:56 [TRACE] Waiting 10s before next try
2020/09/24 14:49:06 [DEBUG] DB Instance describe configuration: {
  DBInstanceIdentifier: "terraform-20200924184121666000000001"
}
2020/09/24 14:49:06 Sweeper Tests ran successfully:
	- aws_db_instance
ok  	github.com/terraform-providers/terraform-provider-aws/aws	243.507s

Sweeping two DB instances:

2020/09/24 15:03:24 [DEBUG] DB Instance status for instance terraform-20200924185147512600000002: deleting
2020/09/24 15:03:24 [TRACE] Waiting 10s before next try
2020/09/24 15:03:24 [DEBUG] DB Instance status for instance terraform-20200924185612374700000001: deleting
2020/09/24 15:03:24 [TRACE] Waiting 10s before next try
2020/09/24 15:03:34 [DEBUG] DB Instance describe configuration: {
  DBInstanceIdentifier: "terraform-20200924185612374700000001"
}
2020/09/24 15:03:34 [DEBUG] DB Instance status for instance terraform-20200924185147512600000002: deleting
2020/09/24 15:03:34 [TRACE] Waiting 10s before next try
2020/09/24 15:03:44 [DEBUG] DB Instance describe configuration: {
  DBInstanceIdentifier: "terraform-20200924185147512600000002"
}
2020/09/24 15:03:45 Sweeper Tests ran successfully:
	- aws_db_instance
ok  	github.com/terraform-providers/terraform-provider-aws/aws	233.885s

[gdavison]

One comment on the worker pool pattern

[bflad]

GitHub is automatically requesting reviews on force pushes which is a little annoying, but I'll move my out of band feedback here for posterity:

I think what I’d like to see here is the ResourceData and AWSClient being wholly handled outside the orchestrating goroutine so that any changes do not require broad changes in the codebase. So instead of passing a slice of resource IDs, which will not necessarily work for all resources since some require other d.Get(), the orchestrator could instead just be a slice of a new type that has the Resource, ResourceData, and AWSClient

e.g. roughly

type testSweepResource struct{
  d        *schema.ResourceData
  meta     interface{}
  resource *schema.Resource
}

func NewTestSweepResource(resource *schema.Resource, d *schema.ResourceData, meta interface{}) *testSweepResource {
  return &testSweepResource{
    d:        d,
    meta:     meta,
    resource: resource,
  }
}

func testSweepResourceOrchestrator([]*testSweepResource) {
  // ...
  r := resourceExampleThing()
  var sweepResources []*testSweepResource

  for _, thing := range page.Things {
    d := r.Data(nil)
    d.SetId(/*...*/)
    // other d.Set() or r.Read(d, client) as necessary
    sweepResources = append(sweepResources, NewTestSweepResource(r, d, client))
  }

  return testSweepResourceOrchestrator(sweepResources)
}

Then its generic for all resources and we can potentially promote it to the SDK. 👍

[gdavison]

I like @bflad's approach above. I had a different approach in #13516 which abstracts the API operations, but this leverages a lot of what the Terraform SDK provides.

[gdavison]

Added some feedback. It's looking good!

Changes made!

[YakDriver]

Since this potentially affects the 332 places where testAccCheckResourceDisappears() is called, I ran a subset of _disappears tests to make sure everything is okay. Also, the aws_db_instance sweeper is still happy.

--- PASS: TestAccAWSDeviceFarmProject_disappears (11.51s)
--- PASS: TestAccAWSXrayGroup_disappears (11.63s)
--- PASS: TestAccAWSGlueCatalogDatabase_disappears (11.68s)
--- PASS: TestAccAWSEIP_disappears (12.23s)
--- PASS: TestAccAWSAPIGatewayV2Api_disappears (13.02s)
--- PASS: TestAccAWSDefaultRouteTable_disappears_Vpc (16.41s)

2021/03/31 16:07:50 [DEBUG] DB Instance status for instance terraform-20210331194712463300000002: deleting
2021/03/31 16:07:50 [TRACE] Waiting 10s before next try
2021/03/31 16:07:50 [DEBUG] DB Instance status for instance terraform-20210331194712462400000001: deleting
2021/03/31 16:07:50 [TRACE] Waiting 10s before next try
2021/03/31 16:08:00 [DEBUG] DB Instance describe configuration: {
  DBInstanceIdentifier: "terraform-20210331194712463300000002"
}
2021/03/31 16:08:00 [DEBUG] DB Instance describe configuration: {
  DBInstanceIdentifier: "terraform-20210331194712462400000001"
}
2021/03/31 16:08:00 [DEBUG] DB Instance status for instance terraform-20210331194712462400000001: deleting
2021/03/31 16:08:00 [TRACE] Waiting 10s before next try
2021/03/31 16:08:10 [DEBUG] DB Instance describe configuration: {
  DBInstanceIdentifier: "terraform-20210331194712462400000001"
}
2021/03/31 16:08:11 Sweeper Tests ran successfully:
	- aws_db_instance
ok  	github.com/terraform-providers/terraform-provider-aws/aws	231.937s

[YakDriver]

If you want to test the sweeping, here's some HCL. It's especially fun to catch the nightly right when the RDS tests are running (hasn't happened yet but I imagine it would be).

data "aws_rds_engine_version" "default" {
  engine = "mysql"
}

data "aws_rds_orderable_db_instance" "test" {
  engine         = data.aws_rds_engine_version.default.engine
  engine_version = data.aws_rds_engine_version.default.version
  license_model  = "general-public-license"
  storage_type   = "standard"

  preferred_instance_classes = ["db.t3.micro", "db.t2.micro", "db.t2.medium"]
}

resource "aws_db_instance" "test1" {
  allocated_storage       = 10
  backup_retention_period = 0
  engine                  = data.aws_rds_engine_version.default.engine
  engine_version          = data.aws_rds_engine_version.default.version
  instance_class          = data.aws_rds_orderable_db_instance.test.instance_class
  name                    = "baz"
  parameter_group_name    = "default.${data.aws_rds_engine_version.default.parameter_group_family}"
  password                = "barbarbarbar"
  skip_final_snapshot     = true
  username                = "foo"

  # Maintenance Window is stored in lower case in the API, though not strictly
  # documented. Terraform will downcase this to match (as opposed to throw a
  # validation error).
  maintenance_window = "Fri:09:00-Fri:09:30"
}

resource "aws_db_instance" "test2" {
  allocated_storage       = 10
  backup_retention_period = 0
  engine                  = data.aws_rds_engine_version.default.engine
  engine_version          = data.aws_rds_engine_version.default.version
  instance_class          = data.aws_rds_orderable_db_instance.test.instance_class
  name                    = "baz"
  parameter_group_name    = "default.${data.aws_rds_engine_version.default.parameter_group_family}"
  password                = "barbarbarbar"
  skip_final_snapshot     = true
  username                = "foo"

  # Maintenance Window is stored in lower case in the API, though not strictly
  # documented. Terraform will downcase this to match (as opposed to throw a
  # validation error).
  maintenance_window = "Fri:09:00-Fri:09:30"
}

SWEEP=us-west-2 SWEEPARGS=-sweep-run=aws_db_instance make sweep

[gdavison]

LGTM 🚀

One suggestion about move the sweeper functions

Acceptance test results for Commercial partition

--- PASS: TestProvider_impl (0.24s)
--- PASS: TestReverseDns (0.00s)
--- PASS: TestReverseDns/empty (0.00s)
--- PASS: TestReverseDns/amazonaws.com (0.00s)
--- PASS: TestReverseDns/amazonaws.com.cn (0.00s)
--- PASS: TestReverseDns/sc2s.sgov.gov (0.00s)
--- PASS: TestReverseDns/c2s.ic.gov (0.00s)
--- PASS: TestProvider (0.87s)
--- PASS: TestAccAWSProvider_Region_AwsCommercial (39.88s)
--- PASS: TestAccAWSProvider_Region_AwsChina (41.37s)
--- PASS: TestAccAWSProvider_Region_AwsC2S (41.94s)
--- PASS: TestAccAWSProvider_Region_AwsGovCloudUs (42.68s)
--- PASS: TestAccAWSProvider_Region_AwsSC2S (43.33s)
--- PASS: TestAccAWSProvider_IgnoreTags_KeyPrefixes_One (53.55s)
--- PASS: TestAccAWSProvider_IgnoreTags_Keys_One (54.31s)
--- PASS: TestAccAWSProvider_DefaultAndIgnoreTags_EmptyConfigurationBlocks (54.18s)
--- PASS: TestAccAWSProvider_IgnoreTags_Keys_None (54.51s)
--- PASS: TestAccAWSProvider_DefaultTags_Tags_One (55.20s)
--- PASS: TestAccAWSProvider_IgnoreTags_EmptyConfigurationBlock (55.42s)
--- PASS: TestAccAWSProvider_IgnoreTags_KeyPrefixes_None (55.84s)
--- PASS: TestAccAWSProvider_IgnoreTags_Keys_Multiple (55.53s)
--- PASS: TestAccAWSProvider_IgnoreTags_KeyPrefixes_Multiple (55.76s)
--- PASS: TestAccProvider_DefaultTags_EmptyConfigurationBlock (55.47s)
--- PASS: TestAccAWSProvider_DefaultTags_Tags_Multiple (55.81s)
--- PASS: TestAccAWSProvider_DefaultTags_Tags_None (56.04s)
--- PASS: TestAccAWSProvider_Endpoints (56.08s)
--- FAIL: TestAccAWSDBInstance_DbSubnetGroupName_RamShared (1.55s)
--- PASS: TestAccAWSProvider_AssumeRole_Empty (60.99s)
--- PASS: TestAccAWSDBInstance_DbSubnetGroupName (362.53s)
--- PASS: TestAccAWSDBInstance_basic (441.60s)
--- PASS: TestAccAWSDBInstance_DeletionProtection (395.32s)
--- PASS: TestAccAWSDBInstance_kmsKey (427.69s)
--- PASS: TestAccAWSDBInstance_generatedName (436.10s)
--- FAIL: TestAccAWSDBInstance_ReplicateSourceDb_DbSubnetGroupName_RamShared (0.89s)
--- PASS: TestAccAWSDBInstance_Password (458.14s)
--- SKIP: TestAccAWSDBInstance_ReplicateSourceDb_DeletionProtection (0.00s)
--- PASS: TestAccAWSDBInstance_DbSubnetGroupName_VpcSecurityGroupIds (466.94s)
--- PASS: TestAccAWSDBInstance_AllowMajorVersionUpgrade (493.65s)
--- PASS: TestAccAWSDBInstance_IsAlreadyBeingDeleted (499.76s)
--- PASS: TestAccAWSDBInstance_iamAuth (513.52s)
--- PASS: TestAccAWSDBInstance_namePrefix (528.95s)
--- PASS: TestAccAWSDBInstance_optionGroup (558.00s)
--- PASS: TestAccAWSDBInstance_MaxAllocatedStorage (625.63s)
--- PASS: TestAccAWSDBInstance_FinalSnapshotIdentifier_SkipFinalSnapshot (656.97s)
--- PASS: TestAccAWSDBInstance_FinalSnapshotIdentifier (888.01s)
--- PASS: TestAccAWSDBInstance_subnetGroup (1059.59s)
--- PASS: TestAccAWSDBInstance_ReplicateSourceDb (1282.23s)
--- PASS: TestAccAWSDBInstance_ReplicateSourceDb_AutoMinorVersionUpgrade (1378.66s)
--- PASS: TestAccAWSDBInstance_ReplicateSourceDb_AllowMajorVersionUpgrade (1424.03s)
--- FAIL: TestAccAWSDBInstance_S3Import (825.81s)
--- PASS: TestAccAWSDBInstance_ReplicateSourceDb_Port (1167.28s)
--- PASS: TestAccAWSDBInstance_ReplicateSourceDb_BackupWindow (1319.34s)
--- PASS: TestAccAWSDBInstance_ReplicateSourceDb_AllocatedStorage (1717.80s)
--- PASS: TestAccAWSDBInstance_ReplicateSourceDb_AvailabilityZone (1380.92s)
--- PASS: TestAccAWSDBInstance_ReplicateSourceDb_MaintenanceWindow (1311.89s)
--- PASS: TestAccAWSDBInstance_ReplicateSourceDb_MaxAllocatedStorage (1310.73s)
--- FAIL: TestAccAWSDBInstance_ReplicateSourceDb_BackupRetentionPeriod (1466.28s)
--- FAIL: TestAccAWSDBInstance_SnapshotIdentifier_DbSubnetGroupName_RamShared (0.76s)
--- PASS: TestAccAWSDBInstance_ReplicateSourceDb_ParameterGroupName (1430.25s)
--- PASS: TestAccAWSDBInstance_ReplicateSourceDb_VpcSecurityGroupIds (1319.51s)
--- PASS: TestAccAWSDBInstance_ReplicateSourceDb_IamDatabaseAuthenticationEnabled (1522.34s)
--- PASS: TestAccAWSDBInstance_ReplicateSourceDb_CACertificateIdentifier (1349.66s)
--- PASS: TestAccAWSDBInstance_ReplicateSourceDb_Monitoring (1693.91s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier (1196.56s)
--- PASS: TestAccAWSDBInstance_ReplicateSourceDb_DbSubnetGroupName_VpcSecurityGroupIds (1929.86s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifierRemoved (1077.66s)
--- PASS: TestAccAWSDBInstance_ReplicateSourceDb_DbSubnetGroupName (1968.10s)
--- PASS: TestAccAWSDBInstance_ReplicateSourceDb_MultiAZ (2027.18s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_AllocatedStorage (1370.40s)
--- SKIP: TestAccAWSDBInstance_SnapshotIdentifier_Tags_Unset (0.00s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_AutoMinorVersionUpgrade (1127.34s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_Io1Storage (1470.68s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_MaintenanceWindow (936.35s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_AvailabilityZone (1237.52s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_IamDatabaseAuthenticationEnabled (1015.66s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_BackupWindow (1194.21s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_DbSubnetGroupName_VpcSecurityGroupIds (1187.53s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_DbSubnetGroupName (1238.22s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_MaxAllocatedStorage (1115.66s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_BackupRetentionPeriod (1409.45s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_DeletionProtection (1301.14s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_BackupRetentionPeriod_Unset (1663.06s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_ParameterGroupName (1181.04s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_Port (1160.00s)
--- SKIP: TestAccAWSDBInstance_ec2Classic (1.84s)
--- PASS: TestAccAWSDBInstance_MonitoringRoleArn_EnabledToRemoved (645.88s)
--- PASS: TestAccAWSDBInstance_portUpdate (589.59s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_Tags (1107.03s)
--- PASS: TestAccAWSDBInstance_separateIopsUpdate (713.11s)
--- PASS: TestAccAWSDBInstance_MonitoringRoleArn_RemovedToEnabled (749.29s)
--- FAIL: TestAccAWSDBInstance_MonitoringInterval (816.01s)
--- PASS: TestAccAWSDBInstance_MonitoringRoleArn_EnabledToDisabled (812.02s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_AllowMajorVersionUpgrade (2018.12s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_Monitoring (1571.10s)
--- PASS: TestAccAWSDBInstance_MinorVersion (428.83s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_VpcSecurityGroupIds_Tags (1117.66s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_VpcSecurityGroupIds (1207.65s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_MultiAZ (1843.79s)
--- PASS: TestAccAWSDBInstance_cloudwatchLogsExportConfiguration (574.37s)
--- PASS: TestAccAWSDBInstance_EnabledCloudwatchLogsExports_Postgresql (521.00s)
--- PASS: TestAccAWSDBInstance_NoDeleteAutomatedBackups (499.74s)
--- PASS: TestAccAWSDBInstance_EnabledCloudwatchLogsExports_MSSQL (632.95s)
--- PASS: TestAccAWSDBInstance_EnabledCloudwatchLogsExports_MySQL (840.91s)
--- PASS: TestAccAWSDBInstance_CACertificateIdentifier (437.76s)
--- PASS: TestAccAWSDBInstance_EnabledCloudwatchLogsExports_Oracle (772.88s)
--- PASS: TestAccAWSDBInstance_MSSQL_TZ (1454.41s)
--- PASS: TestAccAWSDBInstance_PerformanceInsightsEnabled_DisabledToEnabled (805.25s)
--- PASS: TestAccAWSDBInstance_PerformanceInsightsRetentionPeriod (916.86s)
--- PASS: TestAccAWSDBInstance_PerformanceInsightsEnabled_EnabledToDisabled (978.50s)
--- PASS: TestAccAWSDBInstance_PerformanceInsightsKmsKeyId (1011.14s)
--- PASS: TestAccAWSDBInstance_RestoreToPointInTime_SourceIdentifier (1302.11s)
--- PASS: TestAccAWSDBInstance_RestoreToPointInTime_SourceResourceID (1232.27s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_PerformanceInsightsEnabled (1491.22s)
--- PASS: TestAccAWSDBInstance_MySQL_SnapshotRestoreWithEngineVersion (2261.03s)
--- PASS: TestAccAWSDBInstance_ReplicateSourceDb_PerformanceInsightsEnabled (1962.46s)
--- PASS: TestAccAWSDBInstance_MSSQL_DomainSnapshotRestore (2887.94s)
--- PASS: TestAccAWSDBInstance_MSSQL_Domain (3252.90s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_MultiAZ_SQLServer (4410.35s)

Failures are unrelated to this change

[bflad]

Looks good to me 🚀 Thank you for sticking through this one.

[ghost]

This has been released in version 3.36.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!