-
Notifications
You must be signed in to change notification settings - Fork 232
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: Introduce sweeper flag to continue other sweepers after failures #198
Conversation
…ter failures Due to various circumstances, such as a remote API unexpectedly breaking compatibility in a dependency sweeper, it can be desirable to allow the sweepers to continue after failures. This gives the operator the opportunity to try cleaning up other, unrelated infrastructure by using a new `-sweep-allow-failures` flag. It is still up to the resource implementations to allow continuing on failures within the same sweeper, rather than exiting on the first failure encountered. Without new flag (same previous behavior on successful sweepers): ```console $ go test ./aws -v -sweep=us-west-2 -sweep-run=aws_dx_gateway_association_proposal -timeout 10h 2019/10/08 11:39:30 [DEBUG] Running Sweepers for region (us-west-2): 2019/10/08 11:39:30 [DEBUG] Running Sweeper (aws_dx_gateway_association_proposal) in region (us-west-2) 2019/10/08 11:39:32 Sweeper Tests ran successfully: - aws_dx_gateway_association_proposal ok github.com/terraform-providers/terraform-provider-aws/aws 3.171s $ echo $? 0 ``` Without new flag (same previous behavior on failing sweepers): ```console $ go test ./aws -v -sweep=us-west-2 -sweep-run=aws_vpn_gateway -timeout 10h 2019/10/08 10:44:53 [DEBUG] Running Sweepers for region (us-west-2): 2019/10/08 10:44:53 [DEBUG] Sweeper (aws_vpn_gateway) has dependency (aws_dx_gateway_association), running.. 2019/10/08 10:44:53 [DEBUG] Sweeper (aws_dx_gateway_association) has dependency (aws_dx_gateway_association_proposal), running.. 2019/10/08 10:44:53 [DEBUG] Running Sweeper (aws_dx_gateway_association_proposal) in region (us-west-2) 2019/10/08 10:44:53 [INFO] Building AWS auth structure 2019/10/08 10:44:53 [INFO] Setting AWS metadata API timeout to 100ms 2019/10/08 10:44:54 [INFO] Ignoring AWS metadata API endpoint at default location as it doesn't return any instance-id 2019/10/08 10:44:54 [INFO] AWS Auth provider used: "SharedCredentialsProvider" 2019/10/08 10:44:54 [DEBUG] Trying to get account information via sts:GetCallerIdentity 2019/10/08 10:44:54 [DEBUG] Trying to get account information via sts:GetCallerIdentity 2019/10/08 10:44:55 [DEBUG] Running Sweeper (aws_dx_gateway_association) in region (us-west-2) 2019/10/08 10:44:55 [INFO] Building AWS auth structure 2019/10/08 10:44:55 [INFO] Setting AWS metadata API timeout to 100ms 2019/10/08 10:44:56 [INFO] Ignoring AWS metadata API endpoint at default location as it doesn't return any instance-id 2019/10/08 10:44:56 [INFO] AWS Auth provider used: "SharedCredentialsProvider" 2019/10/08 10:44:56 [DEBUG] Trying to get account information via sts:GetCallerIdentity 2019/10/08 10:44:56 [DEBUG] Trying to get account information via sts:GetCallerIdentity 2019/10/08 10:44:57 [INFO] Deleting Direct Connect Gateway (5a00fd97-9872-447d-822c-d2a6245e22d6) Association: vgw-0dc65a74a20f0ff68 2019/10/08 10:44:58 [ERROR] Error running Sweeper (aws_dx_gateway_association) in region (us-west-2): error deleting Direct Connect Gateway (5a00fd97-9872-447d-822c-d2a6245e22d6) Association (vgw-0dc65a74a20f0ff68): DirectConnectServerException: Unable to process request status code: 400, request id: 12c9dad0-add9-4726-ad39-5d02c529bd4c FAIL github.com/terraform-providers/terraform-provider-aws/aws 5.569s FAIL $ echo $? 1 ``` With new flag: ```console $ go test ./aws -v -sweep=us-west-2 -sweep-run=aws_vpn_gateway -sweep-allow-failures -timeout 10h 2019/10/08 10:47:13 [DEBUG] Running Sweepers for region (us-west-2): 2019/10/08 10:47:13 [DEBUG] Sweeper (aws_vpn_gateway) has dependency (aws_dx_gateway_association), running.. 2019/10/08 10:47:13 [DEBUG] Sweeper (aws_dx_gateway_association) has dependency (aws_dx_gateway_association_proposal), running.. 2019/10/08 10:47:13 [DEBUG] Running Sweeper (aws_dx_gateway_association_proposal) in region (us-west-2) 2019/10/08 10:47:15 [DEBUG] Running Sweeper (aws_dx_gateway_association) in region (us-west-2) 2019/10/08 10:47:16 [INFO] Deleting Direct Connect Gateway (5a00fd97-9872-447d-822c-d2a6245e22d6) Association: vgw-0dc65a74a20f0ff68 2019/10/08 10:47:17 [ERROR] Error running Sweeper (aws_dx_gateway_association) in region (us-west-2): error deleting Direct Connect Gateway (5a00fd97-9872-447d-822c-d2a6245e22d6) Association (vgw-0dc65a74a20f0ff68): DirectConnectServerException: Unable to process request status code: 400, request id: df997347-77eb-49d4-8a8f-2d3fd6bc7b59 2019/10/08 10:47:17 [ERROR] Error running Sweeper (aws_dx_gateway_association) in region (us-west-2): error deleting Direct Connect Gateway (5a00fd97-9872-447d-822c-d2a6245e22d6) Association (vgw-0dc65a74a20f0ff68): DirectConnectServerException: Unable to process request status code: 400, request id: df997347-77eb-49d4-8a8f-2d3fd6bc7b59 2019/10/08 10:47:17 [DEBUG] Running Sweeper (aws_vpn_gateway) in region (us-west-2) 2019/10/08 10:47:18 [DEBUG] Detaching VPN Gateway: { VpcId: "vpc-0d83012901b4c0903", VpnGatewayId: "vgw-068137a16116b907b" } 2019/10/08 10:47:18 [DEBUG] Waiting for VPN Gateway (vgw-068137a16116b907b) to detach from VPC (vpc-0d83012901b4c0903) 2019/10/08 10:47:18 [DEBUG] Waiting for state to become: [detached] 2019/10/08 10:52:15 [DEBUG] Deleting VPN Gateway: { VpnGatewayId: "vgw-068137a16116b907b" } 2019/10/08 10:52:16 [DEBUG] Detaching VPN Gateway: { VpcId: "vpc-0c62fd27265656c2e", VpnGatewayId: "vgw-01b70692831191788" } 2019/10/08 10:52:16 [DEBUG] Waiting for VPN Gateway (vgw-01b70692831191788) to detach from VPC (vpc-0c62fd27265656c2e) 2019/10/08 10:52:16 [DEBUG] Waiting for state to become: [detached] 2019/10/08 10:57:23 [DEBUG] Deleting VPN Gateway: { VpnGatewayId: "vgw-01b70692831191788" } 2019/10/08 10:57:24 [DEBUG] Detaching VPN Gateway: { VpcId: "vpc-05d5a615959157f27", VpnGatewayId: "vgw-0aa8c6fae0a604e10" } 2019/10/08 10:57:24 [DEBUG] Waiting for VPN Gateway (vgw-0aa8c6fae0a604e10) to detach from VPC (vpc-05d5a615959157f27) 2019/10/08 10:57:24 [DEBUG] Waiting for state to become: [detached] 2019/10/08 11:02:32 [DEBUG] Deleting VPN Gateway: { VpnGatewayId: "vgw-0aa8c6fae0a604e10" } 2019/10/08 11:02:32 [DEBUG] Detaching VPN Gateway: { VpcId: "vpc-0d3407641bd047759", VpnGatewayId: "vgw-07576dd2891f26542" } 2019/10/08 11:02:32 [DEBUG] Waiting for VPN Gateway (vgw-07576dd2891f26542) to detach from VPC (vpc-0d3407641bd047759) 2019/10/08 11:02:32 [DEBUG] Waiting for state to become: [detached] 2019/10/08 11:07:50 [DEBUG] Deleting VPN Gateway: { VpnGatewayId: "vgw-07576dd2891f26542" } 2019/10/08 11:07:51 [DEBUG] Detaching VPN Gateway: { VpcId: "vpc-06287cc0559c30024", VpnGatewayId: "vgw-01631d746dfe99ebf" } 2019/10/08 11:07:51 [DEBUG] Waiting for VPN Gateway (vgw-01631d746dfe99ebf) to detach from VPC (vpc-06287cc0559c30024) 2019/10/08 11:07:51 [DEBUG] Waiting for state to become: [detached] 2019/10/08 11:12:59 [DEBUG] Deleting VPN Gateway: { VpnGatewayId: "vgw-01631d746dfe99ebf" } 2019/10/08 11:12:59 [DEBUG] Detaching VPN Gateway: { VpcId: "vpc-08b202d4b25465bdc", VpnGatewayId: "vgw-0ed99c4a2b8d6a339" } 2019/10/08 11:13:00 [DEBUG] Waiting for VPN Gateway (vgw-0ed99c4a2b8d6a339) to detach from VPC (vpc-08b202d4b25465bdc) 2019/10/08 11:13:00 [DEBUG] Waiting for state to become: [detached] 2019/10/08 11:18:07 [DEBUG] Deleting VPN Gateway: { VpnGatewayId: "vgw-0ed99c4a2b8d6a339" } 2019/10/08 11:18:07 [DEBUG] Detaching VPN Gateway: { VpcId: "vpc-09424d73ee3dfb159", VpnGatewayId: "vgw-06169f1efed128de4" } 2019/10/08 11:18:08 [DEBUG] Waiting for VPN Gateway (vgw-06169f1efed128de4) to detach from VPC (vpc-09424d73ee3dfb159) 2019/10/08 11:18:08 [DEBUG] Waiting for state to become: [detached] 2019/10/08 11:23:15 [DEBUG] Deleting VPN Gateway: { VpnGatewayId: "vgw-06169f1efed128de4" } 2019/10/08 11:23:16 [DEBUG] Detaching VPN Gateway: { VpcId: "vpc-0401f3706243dad85", VpnGatewayId: "vgw-0dc65a74a20f0ff68" } 2019/10/08 11:23:16 [DEBUG] Waiting for VPN Gateway (vgw-0dc65a74a20f0ff68) to detach from VPC (vpc-0401f3706243dad85) 2019/10/08 11:23:16 [DEBUG] Waiting for state to become: [detached] 2019/10/08 11:28:23 [DEBUG] Deleting VPN Gateway: { VpnGatewayId: "vgw-0dc65a74a20f0ff68" } 2019/10/08 11:28:24 [DEBUG] Detaching VPN Gateway: { VpcId: "vpc-09e0bf3c6ea53ee5f", VpnGatewayId: "vgw-0f26b1614d922a13d" } 2019/10/08 11:28:24 [DEBUG] Waiting for VPN Gateway (vgw-0f26b1614d922a13d) to detach from VPC (vpc-09e0bf3c6ea53ee5f) 2019/10/08 11:28:24 [DEBUG] Waiting for state to become: [detached] 2019/10/08 11:33:42 [DEBUG] Deleting VPN Gateway: { VpnGatewayId: "vgw-0f26b1614d922a13d" } 2019/10/08 11:33:42 Sweeper Tests ran successfully: - aws_dx_gateway_association_proposal - aws_vpn_gateway 2019/10/08 11:33:42 Sweeper Tests ran unsuccessfully: - aws_dx_gateway_association: error deleting Direct Connect Gateway (5a00fd97-9872-447d-822c-d2a6245e22d6) Association (vgw-0dc65a74a20f0ff68): DirectConnectServerException: Unable to process request status code: 400, request id: df997347-77eb-49d4-8a8f-2d3fd6bc7b59 FAIL github.com/terraform-providers/terraform-provider-aws/aws 2790.472s FAIL $ echo $? 1 ```
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
There was a problem hiding this 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 PR @bflad
I'm happy with the implementation, but I'd love to see tests covering this new functionality.
It seems that there's some prior art around testing sweepers in general:
func TestTest_Main(t *testing.T) { |
so we could rename this to TestTest_Main_basic
and add a very similar test TestTest_Main_allowFailures
which can have 3 cases - no failures, 1 failure, 2 failures - all still verifying that all sweepers ran.
What do you think?
@radeksimko I'll see what I can come up with 😄 |
…global variables, add testing for allowing failures Reference: #198 (review)
@radeksimko it took a little rework to prevent the usage of global variables (which made the parallel unit testing very unhappy) but now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's great! Thank you for the refactoring and adding tests.
LGTM - let's
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. |
Due to various circumstances, such as a remote API unexpectedly breaking compatibility in a dependency sweeper, it can be desirable to allow the sweepers to continue after failures. This gives the operator the opportunity to try cleaning up other, unrelated infrastructure by using a new
-sweep-allow-failures
flag.It is still up to the resource implementations to allow continuing on failures within the same sweeper, rather than exiting on the first failure encountered.
Without new flag (same previous behavior on successful sweepers):
Without new flag (same previous behavior on failing sweepers):
With new flag: