Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

helper/resource: Ensure -sweep-run filtering includes sweeper dependencies #213

Merged
merged 1 commit into from
Oct 31, 2019

Conversation

bflad
Copy link
Contributor

@bflad bflad commented Oct 27, 2019

Previously, the -sweep-run filtering would only add sweepers matching the filter, but miss the dependencies. In real world usage:

	resource.AddTestSweepers("aws_vpc", &resource.Sweeper{
		Name: "aws_vpc",
		Dependencies: []string{
			"aws_egress_only_internet_gateway",
			"aws_internet_gateway",
			"aws_nat_gateway",
			"aws_network_acl",
			"aws_route_table",
			"aws_security_group",
			"aws_subnet",
			"aws_vpc_peering_connection",
			"aws_vpn_gateway",
		},
		F: testSweepVPCs,
	})

Where resources like aws_subnet would contain further dependencies that would cause failures if not removed, e.g.

$ go test ./aws -v -timeout=10h -sweep=us-west-2 -sweep-run=aws_vpc -sweep-allow-failures
...
2019/10/27 16:10:29 [DEBUG] Sweeper (aws_vpc) has dependency (aws_egress_only_internet_gateway), but that sweeper was not found
2019/10/27 16:10:29 [DEBUG] Sweeper (aws_vpc) has dependency (aws_internet_gateway), but that sweeper was not found
2019/10/27 16:10:29 [DEBUG] Sweeper (aws_vpc) has dependency (aws_nat_gateway), but that sweeper was not found
2019/10/27 16:10:29 [DEBUG] Sweeper (aws_vpc) has dependency (aws_network_acl), but that sweeper was not found
2019/10/27 16:10:29 [DEBUG] Sweeper (aws_vpc) has dependency (aws_route_table), but that sweeper was not found
2019/10/27 16:10:29 [DEBUG] Sweeper (aws_vpc) has dependency (aws_security_group), but that sweeper was not found
2019/10/27 16:10:29 [DEBUG] Sweeper (aws_vpc) has dependency (aws_subnet), but that sweeper was not found
2019/10/27 16:10:29 [DEBUG] Sweeper (aws_vpc) has dependency (aws_vpc_peering_connection), running..
...
2019/10/27 16:11:32 Sweeper Tests ran successfully:
	- aws_vpc_dhcp_options
	- aws_vpc_peering_connection
	- aws_vpc_endpoint
	- aws_vpc_endpoint_service
2019/10/27 16:11:32 Sweeper Tests ran unsuccessfully:
	- aws_vpc: Error deleting VPC (vpc-0f3da92fc03d683b7): DependencyViolation: The vpc 'vpc-0f3da92fc03d683b7' has dependencies and cannot be deleted.

After changes:

$ go test ./aws -v -timeout=10h -sweep=us-west-2 -sweep-run=aws_vpc -sweep-allow-failures
...
2019/10/27 18:44:58 Sweeper Tests ran successfully:
	- aws_autoscaling_group
	- aws_db_subnet_group
	- aws_vpc_endpoint_service
	- aws_route_table
	- aws_batch_job_queue
	- aws_beanstalk_environment
	- aws_cloudhsm_v2_cluster
	- aws_fsx_windows_file_system
	- aws_ec2_client_vpn_endpoint
	- aws_elasticache_replication_group
	- aws_sagemaker_notebook_instance
	- aws_vpc_dhcp_options
	- aws_vpc_endpoint
	- aws_mq_broker
	- aws_msk_cluster
	- aws_network_interface
	- aws_batch_compute_environment
	- aws_elasticache_cluster
	- aws_lambda_function
	- aws_instance
	- aws_vpn_gateway
	- aws_nat_gateway
	- aws_vpc_peering_connection
	- aws_directory_service_directory
	- aws_ec2_transit_gateway_vpc_attachment
	- aws_elb
	- aws_emr_cluster
	- aws_fsx_lustre_file_system
	- aws_security_group
	- aws_egress_only_internet_gateway
	- aws_db_instance
	- aws_lb
	- aws_redshift_cluster
	- aws_internet_gateway
	- aws_dx_gateway_association_proposal
	- aws_eks_cluster
	- aws_network_acl
	- aws_elasticsearch_domain
	- aws_api_gateway_vpc_link
	- aws_route53_resolver_endpoint
	- aws_spot_fleet_request
	- aws_dx_gateway_association
2019/10/27 18:44:58 Sweeper Tests ran unsuccessfully:
	- aws_vpc: Error deleting VPC (vpc-0f3da92fc03d683b7): DependencyViolation: The vpc 'vpc-0f3da92fc03d683b7' has dependencies and cannot be deleted.
	status code: 400, request id: 00c178dd-1a3c-45fc-9802-e8215322a54b
	- aws_subnet: Error deleting Subnet (subnet-075212958430d977f): DependencyViolation: The subnet 'subnet-075212958430d977f' has dependencies and cannot be deleted.
	status code: 400, request id: ded1a6e7-c13f-45df-82e3-a53c27f9c8e6

It's worth noting that the sweeper framework would likely benefit from creating a proper directed acyclic graph, but that is certainly outside the scope of this fix.

…ncies

Previously, the filtering would only add sweepers matching the filter, but miss their dependencies. In real world usage:

```go
	resource.AddTestSweepers("aws_vpc", &resource.Sweeper{
		Name: "aws_vpc",
		Dependencies: []string{
			"aws_egress_only_internet_gateway",
			"aws_internet_gateway",
			"aws_nat_gateway",
			"aws_network_acl",
			"aws_route_table",
			"aws_security_group",
			"aws_subnet",
			"aws_vpc_peering_connection",
			"aws_vpn_gateway",
		},
		F: testSweepVPCs,
	})
```

```console
$ go test ./aws -v -timeout=10h -sweep=us-west-2 -sweep-run=aws_vpc -sweep-allow-failures
...
2019/10/27 16:10:29 [DEBUG] Sweeper (aws_vpc) has dependency (aws_egress_only_internet_gateway), but that sweeper was not found
2019/10/27 16:10:29 [DEBUG] Sweeper (aws_vpc) has dependency (aws_internet_gateway), but that sweeper was not found
2019/10/27 16:10:29 [DEBUG] Sweeper (aws_vpc) has dependency (aws_nat_gateway), but that sweeper was not found
2019/10/27 16:10:29 [DEBUG] Sweeper (aws_vpc) has dependency (aws_network_acl), but that sweeper was not found
2019/10/27 16:10:29 [DEBUG] Sweeper (aws_vpc) has dependency (aws_route_table), but that sweeper was not found
2019/10/27 16:10:29 [DEBUG] Sweeper (aws_vpc) has dependency (aws_security_group), but that sweeper was not found
2019/10/27 16:10:29 [DEBUG] Sweeper (aws_vpc) has dependency (aws_subnet), but that sweeper was not found
2019/10/27 16:10:29 [DEBUG] Sweeper (aws_vpc) has dependency (aws_vpc_peering_connection), running..
...
2019/10/27 16:11:32 Sweeper Tests ran successfully:
	- aws_vpc_dhcp_options
	- aws_vpc_peering_connection
	- aws_vpc_endpoint
	- aws_vpc_endpoint_service
2019/10/27 16:11:32 Sweeper Tests ran unsuccessfully:
	- aws_vpc: Error deleting VPC (vpc-0f3da92fc03d683b7): DependencyViolation: The vpc 'vpc-0f3da92fc03d683b7' has dependencies and cannot be deleted.
```

After changes:

```console
$ go test ./aws -v -timeout=10h -sweep=us-west-2 -sweep-run=aws_vpc -sweep-allow-failures
...
2019/10/27 18:44:58 Sweeper Tests ran successfully:
	- aws_autoscaling_group
	- aws_db_subnet_group
	- aws_vpc_endpoint_service
	- aws_route_table
	- aws_batch_job_queue
	- aws_beanstalk_environment
	- aws_cloudhsm_v2_cluster
	- aws_fsx_windows_file_system
	- aws_ec2_client_vpn_endpoint
	- aws_elasticache_replication_group
	- aws_sagemaker_notebook_instance
	- aws_vpc_dhcp_options
	- aws_vpc_endpoint
	- aws_mq_broker
	- aws_msk_cluster
	- aws_network_interface
	- aws_batch_compute_environment
	- aws_elasticache_cluster
	- aws_lambda_function
	- aws_instance
	- aws_vpn_gateway
	- aws_nat_gateway
	- aws_vpc_peering_connection
	- aws_directory_service_directory
	- aws_ec2_transit_gateway_vpc_attachment
	- aws_elb
	- aws_emr_cluster
	- aws_fsx_lustre_file_system
	- aws_security_group
	- aws_egress_only_internet_gateway
	- aws_db_instance
	- aws_lb
	- aws_redshift_cluster
	- aws_internet_gateway
	- aws_dx_gateway_association_proposal
	- aws_eks_cluster
	- aws_network_acl
	- aws_elasticsearch_domain
	- aws_api_gateway_vpc_link
	- aws_route53_resolver_endpoint
	- aws_spot_fleet_request
	- aws_dx_gateway_association
2019/10/27 18:44:58 Sweeper Tests ran unsuccessfully:
	- aws_vpc: Error deleting VPC (vpc-0f3da92fc03d683b7): DependencyViolation: The vpc 'vpc-0f3da92fc03d683b7' has dependencies and cannot be deleted.
	status code: 400, request id: 00c178dd-1a3c-45fc-9802-e8215322a54b
	- aws_subnet: Error deleting Subnet (subnet-075212958430d977f): DependencyViolation: The subnet 'subnet-075212958430d977f' has dependencies and cannot be deleted.
	status code: 400, request id: ded1a6e7-c13f-45df-82e3-a53c27f9c8e6
```
@bflad bflad added the bug Something isn't working label Oct 27, 2019
Copy link
Member

@radeksimko radeksimko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the bug fix and very thorough testing! 👍

@appilon
Copy link
Contributor

appilon commented Oct 31, 2019

I'm going to merge this since it was approved okay @radeksimko and @bflad

@appilon appilon merged commit f587e3b into master Oct 31, 2019
@appilon appilon deleted the b-helper-resource-sweep-run-missing-dependencies branch October 31, 2019 18:01
@ghost
Copy link

ghost commented Apr 1, 2020

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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants