Repository navigation
Fix flaky S3 races in test_cli put_object helper - #10765
GarrettBeatty wants to merge 2 commits into
Conversation
Replace the fixed 5s sleep with a bucket_exists waiter, then poll get_public_access_block until it consistently reports NoSuchPublicAccessBlockConfiguration before uploading. Also retry PutObject on AccessDenied as a backstop, since the public ACL can still be rejected briefly after the block is reported deleted. Fixes flaky test_no_sign_request (CLI-8332).
TestBasicCommandFunctionality.put_object registered a bare client.delete_bucket cleanup, so a transient OperationAborted from S3 failed test_streaming_output_operation (and the other tests using the helper). Retry with exponential backoff on that error code. CLI-8387
| # Deleting the public access block is eventually consistent, so | ||
| # wait until it is consistently reported as gone. | ||
| ConsistencyWaiter(min_successes=3, delay=2, max_attempts=30).wait( | ||
| self._public_access_block_deleted, client, bucket | ||
| ) |
There was a problem hiding this comment.
The consistency that you're claiming is not true as ConsistencyWaiter.wait succeeded even the success are not consistent (e.g. 0 0 0 0 1 0 1 0 0 1 still succeeded). This is because a failure doesn't reset the success count. Are there cases we get false positive so that we need consistent success? if not I suggest changing min_success to 1 and changing the comment.
| client.get_waiter('bucket_exists').wait(Bucket=bucket) | ||
| self.addCleanup(self._delete_bucket_with_retry, client, bucket) |
There was a problem hiding this comment.
Now that get_waiter is called before cleanup, if get_waiter raises, we have a bucket leak, meaning bucket won't be deleted.
|
|
||
| def _put_object_with_retry(self, client, call_args, attempts=5, delay=5): |
There was a problem hiding this comment.
Nit: _put_object_with_retry and _delete_bucket_with_retry are essentially the same, consider creating a helper function to avoid duplicate codes
Description of changes:
Fixes two intermittent failures in the
TestBasicCommandFunctionality.put_objecthelper intests/integration/test_cli.py, which is used bytest_streaming_output_operation,test_no_sign_request, andtest_no_paginate_arg.test_no_sign_request): replace the fixed 5s sleep with abucket_existswaiter, pollGetPublicAccessBlockviaConsistencyWaiteruntil it consistently reportsNoSuchPublicAccessBlockConfiguration, and retryPutObjectonAccessDeniedas a backstop for the public ACL being briefly rejected.test_streaming_output_operation): S3 can intermittently rejectDeleteBucketwithOperationAborted("A conflicting conditional operation is currently in progress"). Cleanup now retries on that error code with exponential backoff (5/10/20/40s).Test-only change; no customer-facing behavior changes.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.