[WIP] Early stages of some ideas to standardize/generate sweepers #13516

gdavison · 2020-05-27T17:12:28Z

Description

This draft PR starts sketching out some ways to standardize utility functions so that e.g. sweepers can be generated using those standard operations.

aws/resource_aws_cloudformation_stack_set.go

gdavison · 2020-05-27T17:38:46Z

aws/resource_aws_cloudformation_stack_set.go

+}
+
+// generate
+func listAllCloudFormationStackSetsPages(conn *cloudformation.CloudFormation, fn func(*cloudformation.ListStackSetsOutput, bool) bool) error {


Here we're creating our own List*Pages() from the List*() function the AWS API provides. This can be easily generated, since it follows a standard pattern, and could be swapped out if/when the API adds a paged function.

gdavison · 2020-05-27T17:39:19Z

aws/resource_aws_cloudformation_stack_set_instance.go

@@ -13,6 +13,12 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/helper/validation"
 )

+const (


These should be moved to the service package

aws/resource_aws_cloudformation_stack_set_instance.go

gdavison · 2020-05-27T17:41:46Z

aws/resource_aws_cloudformation_stack_set_instance_test.go

@@ -22,76 +21,64 @@ func init() {
 }

 func testSweepCloudformationStackSetInstances(region string) error {


This is an example of a two-level iteration for nested objects

gdavison · 2020-05-27T17:45:07Z

aws/resource_aws_cloudformation_stack_set_test.go

@@ -24,46 +24,54 @@ func init() {
 	})
 }

-func testSweepCloudformationStackSets(region string) error {
+// generate
+func sharedCloudFormationClientForRegion(region string) (*cloudformation.CloudFormation, error) {


This wrapping might be overkill, but it gives a standard naming pattern

gdavison · 2020-05-27T17:45:23Z

aws/resource_aws_cloudformation_stack_set_test.go

-		return nil
-	}
+// generate
+func serviceConnectionCloudFormation(client interface{}) *cloudformation.CloudFormation {


This wrapping might be overkill, but it gives a standard naming pattern

ewbankkit · 2020-05-28T17:05:45Z

aws/resource_aws_cloudformation_stack_set.go

 	if err != nil {
 		return fmt.Errorf("error deleting CloudFormation StackSet (%s): %s", d.Id(), err)
 	}

 	return nil
 }

-func listCloudFormationStackSets(conn *cloudformation.CloudFormation) ([]*cloudformation.StackSetSummary, error) {
+func deleteCloudFormationStackSet(conn *cloudformation.CloudFormation, name string) error {


Any thoughts around having the resource manipulation functions in a service-specific package (e.g internal/service/cloudformation/actions)?

The implementation becomes

func DeleteStackSet(conn *cloudformation.CloudFormation, name string) error {...}

and the caller

err := actions.DeleteStackSet(conn, d.Id())

It may make code generation easier.

Yes, they would definitely belong there

It should definitely be moved to something like that. We'll have to figure out where to move isAWSErr() first, because otherwise we'll have a dependency loop

#13036 should help with that.

Thanks, I was about to create an issue for that

ewbankkit · 2020-06-02T13:32:35Z

aws/internal/generators/listpages/main.go

+			break
+		}
+
+		input.NextToken = output.NextToken


This field isn't always named NextToken, for example RDS Cluster Parameters use `Marker'.

Ah, good to know. Thanks!

We'll probably need some kind of customization spec. Or punt and do it manually :)

… Set Instances

…ions to `lister` package

Needed to add a paginator field name to the generator function, since not all services use `NextToken`. Removed the waiter for `aws_rds_cluster_parameter_group` deletion, since it always exited on the first call.

…ata and API resource

gdavison · 2020-06-16T16:46:10Z

aws/internal/service/cloudformation/generators/sweepers/main.go

+
+	var sweeperErrs *multierror.Error
+
+	err = lister.ListAllStackSetsPages(conn, func(page *cloudformation.ListStackSetsOutput, lastPage bool) bool {


Committed too early 🤦

zhelding · 2021-10-19T16:43:58Z

Pull request #21306 has significantly refactored the AWS Provider codebase. As a result, most PRs opened prior to the refactor now have merge conflicts that must be resolved before proceeding.

Specifically, PR #21306 relocated the code for all AWS resources and data sources from a single aws directory to a large number of separate directories in internal/service, each corresponding to a particular AWS service. This separation of code has also allowed for us to simplify the names of underlying functions -- while still avoiding namespace collisions.

We recognize that many pull requests have been open for some time without yet being addressed by our maintainers. Therefore, we want to make it clear that resolving these conflicts in no way affects the prioritization of a particular pull request. Once a pull request has been prioritized for review, the necessary changes will be made by a maintainer -- either directly or in collaboration with the pull request author.

For a more complete description of this refactor, including examples of how old filepaths and function names correspond to their new counterparts: please refer to issue #20000.

For a quick guide on how to amend your pull request to resolve the merge conflicts resulting from this refactor and bring it in line with our new code patterns: please refer to our Service Package Refactor Pull Request Guide.

github-actions · 2023-11-16T02:06:50Z

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

gdavison commented May 27, 2020

View reviewed changes

aws/resource_aws_cloudformation_stack_set.go Outdated Show resolved Hide resolved

gdavison commented May 27, 2020

View reviewed changes

aws/resource_aws_cloudformation_stack_set_instance.go Outdated Show resolved Hide resolved

gdavison commented May 27, 2020

View reviewed changes

ewbankkit reviewed May 28, 2020

View reviewed changes

ghost added the provider Pertains to the provider itself, rather than any interaction with AWS. label Jun 1, 2020

gdavison force-pushed the f-common-resource-operations branch from a682c08 to 5101128 Compare June 1, 2020 20:52

ewbankkit reviewed Jun 2, 2020

View reviewed changes

ghost added service/rds Issues and PRs that pertain to the rds service. size/XXL Managed by automation to categorize the size of a PR. and removed size/XL Managed by automation to categorize the size of a PR. labels Jun 8, 2020

gdavison added 9 commits June 10, 2020 16:55

Factors out sweeper bits

8208981

Moves MSK status and waiter to service package

04837ef

Factors out common operations for CloudFormation Stack Sets and Stack…

37d97d3

… Set Instances

Moves MSK delete waiter into the delete function

68e4079

Linting cleanup

4e1bfea

Removes commented code

8a4d717

Adds generator for List*Pages() functions and moves List*() funct…

f08e0bb

…ions to `lister` package

Adds listPages() function for aws_rds_cluster_parameter_group

42a4ae3

Needed to add a paginator field name to the generator function, since not all services use `NextToken`. Removed the waiter for `aws_rds_cluster_parameter_group` deletion, since it always exited on the first call.

Adds functions to generate the DeleteInput structures from resource d…

e5a798e

…ata and API resource

gdavison force-pushed the f-common-resource-operations branch from f63f342 to e5a798e Compare June 10, 2020 23:55

gdavison added 2 commits June 15, 2020 15:58

Minor fixes

fe4bc00

Generates CloudFormation Stack Set sweeper

d96ddd2

gdavison commented Jun 16, 2020

View reviewed changes

gdavison added 6 commits June 18, 2020 16:51

Removes last StackSet reference from template

ebe8f9e

Factors out Cloudformation-specific parts of sweeper generator

dfdf9b8

Ties CloudFormation Stack Set sweeper generator into Makefile

41314c6

Moves internal MSK package to Kafka to match API package

8a6d893

Adds generator for MSK Cluster sweeper

af14446

Moves sweeper loop to nested template

7dbe02c

anGie44 force-pushed the master branch from 54db70d to 48ba2f7 Compare July 21, 2020 18:26

Base automatically changed from master to main January 23, 2021 00:58

ewbankkit mentioned this pull request Feb 5, 2021

[PLACEHOLDER] Consistent Resource Not Found Error Handling #15945

Closed

gdavison mentioned this pull request Feb 16, 2021

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

Merged

ewbankkit mentioned this pull request Jul 5, 2023

sweepers: Adds new registration and removes boilerplate #32291

Merged

2 tasks

gdavison closed this in #32291 Oct 17, 2023

github-actions bot locked as resolved and limited conversation to collaborators Nov 16, 2023

ewbankkit deleted the f-common-resource-operations branch February 9, 2024 14:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Early stages of some ideas to standardize/generate sweepers #13516

[WIP] Early stages of some ideas to standardize/generate sweepers #13516

gdavison commented May 27, 2020

gdavison May 27, 2020

gdavison May 27, 2020

gdavison May 27, 2020

gdavison May 27, 2020

gdavison May 27, 2020

ewbankkit May 28, 2020

gdavison May 28, 2020

gdavison May 29, 2020

ewbankkit May 30, 2020

gdavison Jun 2, 2020

ewbankkit Jun 2, 2020

gdavison Jun 2, 2020

gdavison Jun 16, 2020

zhelding commented Oct 19, 2021

github-actions bot commented Nov 16, 2023

		@@ -22,76 +21,64 @@ func init() {
		}

		func testSweepCloudformationStackSetInstances(region string) error {


		var sweeperErrs *multierror.Error

		err = lister.ListAllStackSetsPages(conn, func(page *cloudformation.ListStackSetsOutput, lastPage bool) bool {

[WIP] Early stages of some ideas to standardize/generate sweepers #13516

[WIP] Early stages of some ideas to standardize/generate sweepers #13516

Conversation

gdavison commented May 27, 2020

Description

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhelding commented Oct 19, 2021

github-actions bot commented Nov 16, 2023