ceph / s3-tests

Compatibility tests for S3 clones
MIT License
299 stars 288 forks source link

test_versioning_concurrent_multi_object_delete creates the bucket twice and fails on BucketAlreadyExists #588

Open romayalon opened 2 months ago

romayalon commented 2 months ago

Hey! While running test_versioning_concurrent_multi_object_delete on NooBaa, I saw that the test is trying to create the bucket twice -

  1. on get_new_bucket() - client.create_bucket(Bucket=name)
  2. on _create_objects() -> get_new_bucket_resource() -> bucket.create()

See code references after the suggested fix.

According to my understanding, these are the needed changes for fixing the issue -

 def test_versioning_concurrent_multi_object_delete():
     num_objects = 5
     num_threads = 5
-    bucket_name = get_new_bucket()
+    bucket = get_new_bucket_resource()
+    bucket_name = bucket.name

     check_configure_versioning_retry(bucket_name, "Enabled", "Enabled")

     key_names = ["key_{:d}".format(x) for x in range(num_objects)]
-    bucket = _create_objects(bucket_name=bucket_name, keys=key_names)
+    _create_objects(bucket=bucket, bucket_name=bucket_name, keys=key_names)

     client = get_client()
     versions = client.list_object_versions(Bucket=bucket_name)['Versions']

It would be great if you would fix that, thanks a lot!

Code References -

  1. get_new_bucket() -

    def get_new_bucket(client=None, name=None):
    """
    Get a bucket that exists and is empty.
    
    Always recreates a bucket from scratch. This is useful to also
    reset ACLs and such.
    """
    if client is None:
        client = get_client()
    if name is None:
        name = get_new_bucket_name()
    
    client.create_bucket(Bucket=name) // create bucket first time
    return name
  2. get_new_bucket_resource() -

    def get_new_bucket_resource(name=None):
    """
    Get a bucket that exists and is empty.
    
    Always recreates a bucket from scratch. This is useful to also
    reset ACLs and such.
    """
    s3 = boto3.resource('s3',
                        aws_access_key_id=config.main_access_key,
                        aws_secret_access_key=config.main_secret_key,
                        endpoint_url=config.default_endpoint,
                        use_ssl=config.default_is_secure,
                        verify=config.default_ssl_verify)
    if name is None:
        name = get_new_bucket_name()
    bucket = s3.Bucket(name)
    bucket_location = bucket.create() // create bucket second time
    return bucket
  3. Test code -

    def test_versioning_concurrent_multi_object_delete():
    num_objects = 5
    num_threads = 5
    bucket_name = get_new_bucket() // 1
    
    check_configure_versioning_retry(bucket_name, "Enabled", "Enabled")
    
    key_names = ["key_{:d}".format(x) for x in range(num_objects)]
    bucket = _create_objects(bucket_name=bucket_name, keys=key_names) // 2
    
    client = get_client()
    versions = client.list_object_versions(Bucket=bucket_name)['Versions']
    assert len(versions) == num_objects
    objs_dict = {'Objects': [dict((k, v[k]) for k in ["Key", "VersionId"]) for v in versions]}
    results = [None] * num_threads
    
    def do_request(n):
        results[n] = client.delete_objects(Bucket=bucket_name, Delete=objs_dict)
    
    t = []
    for i in range(num_threads):
        thr = threading.Thread(target = do_request, args=[i])
        thr.start()
        t.append(thr)
    _do_wait_completion(t)
    
    for response in results:
        assert len(response['Deleted']) == num_objects
        assert 'Errors' not in response
    
    response = client.list_objects(Bucket=bucket_name)
    assert 'Contents' not in response