Use waitUntil to check if bucket is deleted #177

krisis · 2017-10-03T00:07:43Z

[EDIT] testBucketPolicy creates a bucket immediately after deleting a bucket. Object storage services like Azure Blob Storage may not have deleted the bucket (container in this case) failing the create bucket operation. This change uses waitUntil method to avoid the explicit retry mechanism.
Found while testing #168

nitisht · 2017-10-03T06:28:38Z

@krisis Travis failed due to

Step 21/39 : RUN build/aws-sdk-php/install.sh
 ---> Running in c214f0b4622e
Could not open input file: /mint/run/core/aws-sdk-php/composer.phar
The command '/bin/sh -c build/aws-sdk-php/install.sh' returned a non-zero code: 1

ebozduman · 2017-10-05T19:12:32Z

run/core/aws-sdk-php/quick-tests.php

@@ -886,6 +886,8 @@ function testBucketPolicy($s3Client, $params) {
                throw new Exception('createBucket API failed for ' .
                                $bucket);
            }
+        } finally {


We try to find out if a bucket is deleted or not here. So, the determination process more like belongs to the delete logic, rather than retry logic for createBucket api. Hence, I suggest to start checking with bucketExists api in a timed-out loop instead. waitUntil api looks great to be used for this purpose.

@ebozduman Yes, this is a good point. I have made the necessary changes.

ebozduman · 2017-10-05T19:16:12Z

run/core/aws-sdk-php/quick-tests.php

@@ -991,15 +993,16 @@ function runTest($s3Client, $myfunc, $fnSignature, $args = []) {
        if ($errorCode != "NotImplemented") {
            $status = "FAIL";
            $error = $e->getMessage();
+        } else {
+            $status = "NA";
+            $message = "Not Implemented";


Since alert field is delivering the Not Implemented message, I don't think we need the $message = "Not Implemented" here. It can be eliminated.

Yes, makes sense. Done.

ebozduman · 2017-10-05T19:26:27Z

run/core/aws-sdk-php/quick-tests.php

+            // $fnSignature holds the specific API that is being
+            // tested. It is possible that functions used to create the
+            // test setup may not be implemented.
+            $alert = sprintf("%s or a related API is not implemented, see \"error\" for exact details.", $fnSignature);


It is a personal preference of course, but I'd change the alert message as:

"%s or a related API has NOT been IMPLEMENTED yet. Please see the \"error\" for exact details."

Good suggestion, I have updated the alert message accordingly.

nitisht · 2017-10-07T05:06:52Z

@ebozduman PR is updated, can you please check

ebozduman · 2017-10-10T00:04:56Z

run/core/aws-sdk-php/quick-tests.php

            }
        }
+    } while ($retries > 0);
+
+    // This means createBucket failed after $retries attempt separated


I suggest to change the comment like:
// This means createBucket failed after $retries attempt separated by $delay seconds.
==>
// $retries == 0 means createBucket failed and we timed out

ebozduman · 2017-10-10T00:12:07Z

run/core/aws-sdk-php/quick-tests.php

+    // This means createBucket failed after $retries attempt separated
+    // by $delay seconds.
+    if ($retries == 0) {
+        throw new Exception(sprintf("createBucket API failed after %d separated by %ds",


retries at this point is 0.
So, either we have to use a constant for the maximum number of retries and use it in this error message, while using a separate iterator to count the number of retries inside the while loop, or modify the error message without printing how many times we've tried, like:
Bucket delete failure, timed out!

ebozduman · 2017-10-10T00:28:21Z

run/core/aws-sdk-php/quick-tests.php

-        // test setup may not be implemented.
-        $alert = sprintf("%s or a related API is not implemented, see \"error\" for exact details.", $fnSignature);
-    } finally {
+    }finally {


missing space between } and finally {

harshavardhana · 2017-10-10T20:33:40Z

run/core/aws-sdk-php/quick-tests.php

-    $retries = 5;
-    while ($retries > 0) {
+    // least 30s to be truly deleted.
+    // Ref: https://docs.microsoft.com/en-us/rest/api/storageservices/create-container


what is the reason we are relying and ensuring that container is deleted? eventual consistency is fine. we don't need to verify backend consistency for deletes IMO.

@harshavardhana from our discussion I understand that this regression test could getBucketPolicy after a deleteBucket to confirm that bucket policies don't remain once the bucket is deleted. This allows us to simplify this test case by removing the retry mechanism.

nitisht · 2017-10-18T07:22:39Z

@krisis I tested this on local Minio server, tests fail

Running with
SERVER_ENDPOINT: 127.0.0.1:9000
ACCESS_KEY:      DAJ4WPGPJGYFJ0QUI5EH
SECRET_KEY:      ***REDACTED***
ENABLE_HTTPS:    0
SERVER_REGION:   us-east-1
MINT_DATA_DIR:   /mint/data
MINT_MODE:       core
To get intermittent logs, 'sudo docker cp 2c9c70dadfe6:/mint/log /tmp/mint-logs'
Running aws-sdk-php tests ... FAILED in 1 seconds
{
  "name": "aws-sdk-php",
  "function": "getBucketPolicy ( array $params = [] )",
  "args": {
    "Bucket": "aws-sdk-php-bucket-34012"
  },
  "duration": "32",
  "status": "FAIL",
  "error": "Error executing \"PutObject\" on \"http://127.0.0.1:9000/aws-sdk-php-bucket-34012/test-anon\"; AWS HTTP error: Client error: `PUT http://127.0.0.1:9000/aws-sdk-php-bucket-34012/test-anon` resulted in a `404 Not Found` response:\n<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<Error><Code>NoSuchBucket</Code><Message>The specified bucket does not exist</Mes (truncated...)\n NoSuchBucket (client): The specified bucket does not exist - <?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<Error><Code>NoSuchBucket</Code><Message>The specified bucket does not exist</Message><Key></Key><BucketName></BucketName><Resource>/aws-sdk-php-bucket-34012/test-anon</Resource><RequestId>3L137</RequestId><HostId>3L137</HostId></Error>"
}
Finished running all tests.
To get logs, run 'sudo docker cp 2c9c70dadfe6:/mint/log /tmp/mint-logs'

nitisht · 2017-10-18T07:23:33Z

blocked till failure is resolved/clarified

krisis · 2017-10-19T17:17:18Z

@nitisht I have fixed the failure you observe on server, but this doesn't fix the issue on Azure gateway.
@harshavardhana So far, only waiting 30s or more before creating a bucket just deleted, has been reliable. Thoughts?

harshavardhana · 2017-10-19T17:32:14Z

30s wait be fine we can revisit this..

deekoder · 2017-10-19T17:50:49Z

ok so is this ready to be merged.

krisis · 2017-10-19T18:28:53Z

@deekoder no, this PR isn't ready to be merged. As is it won't work with Minio gateway for Azure.

The test is applicable only to minio server and that can be tested without creating the bucket again. This allows us to remove the retry creation of bucket on failure for about 30s.

krisis · 2017-10-19T18:37:47Z

@nitisht @harshavardhana I have added an unconditional sleep to make mint aws-sdk-php functional tests run successfully on server and gateway (for Azure).

nitisht · 2017-10-20T09:34:37Z

@nitisht @harshavardhana I have added an unconditional sleep to make mint aws-sdk-php functional tests run successfully on server and gateway (for Azure).

Works fine @krisis will merge.

ebozduman · 2017-11-06T21:46:23Z

This issue is marked blocked per @krisis , until we have a discussion.

From: Harshavardhana notifications@github.com
This discussion starts from the question here #177, why do we need to add delays specifically for different gateways. This happens with Backblaze B2 for bucket policies in minio-java as well.

The reason we need to discuss this is to see if its a good idea to add these delays in our test code, perhaps even come up with modes for gateway where we have a separate setup and teardown such that we do not have to relax actual minio server tests which are strongly consistent and make them eventual consistent.

// cc @krisis @balamurugana @nitisht @kannappanr @ebozduman

From: Krishnan Parthasarathi notifications@github.com
@ebozduman @harshavardhana this issue should be marked blocked until we have a discussion.

deekoder requested review from harshavardhana and ebozduman October 3, 2017 01:49

ebozduman requested changes Oct 5, 2017

View reviewed changes

krisis force-pushed the fix/exception-handling branch from 4101297 to 2608213 Compare October 5, 2017 20:39

krisis changed the title ~~Reset retries for non-retryable exceptions~~ Use waitUntil to check if bucket is deleted Oct 5, 2017

krisis force-pushed the fix/exception-handling branch 2 times, most recently from aae2901 to 31a8c75 Compare October 6, 2017 03:26

krisis mentioned this pull request Oct 6, 2017

aws-sdk-php fails for listobjects and deleteobjects on gateways #148

Closed

ebozduman requested changes Oct 10, 2017

View reviewed changes

harshavardhana reviewed Oct 10, 2017

View reviewed changes

harshavardhana mentioned this pull request Oct 10, 2017

Question about consistency checks v/s eventual consistency checks added in tests #183

Closed

krisis force-pushed the fix/exception-handling branch from 31a8c75 to 86b9445 Compare October 10, 2017 21:39

ebozduman approved these changes Oct 17, 2017

View reviewed changes

harshavardhana approved these changes Oct 18, 2017

View reviewed changes

nitisht added the blocked label Oct 18, 2017

krisis force-pushed the fix/exception-handling branch from 86b9445 to 4454078 Compare October 19, 2017 17:14

Remove createBucket retry mechanism for Azure.

922a245

The test is applicable only to minio server and that can be tested without creating the bucket again. This allows us to remove the retry creation of bucket on failure for about 30s.

krisis force-pushed the fix/exception-handling branch from 4454078 to 922a245 Compare October 19, 2017 18:36

nitisht removed the blocked label Oct 20, 2017

nitisht merged commit f381846 into minio:master Oct 20, 2017

nitisht mentioned this pull request Oct 20, 2017

Mint PHP test, 'UploadPart' , fails with minio server running in s3 gateway mode #168

Closed

ebozduman added the blocked label Nov 6, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use waitUntil to check if bucket is deleted #177

Use waitUntil to check if bucket is deleted #177

krisis commented Oct 3, 2017 •

edited

Loading

nitisht commented Oct 3, 2017

ebozduman Oct 5, 2017

krisis Oct 5, 2017

ebozduman Oct 5, 2017

krisis Oct 5, 2017

ebozduman Oct 5, 2017

krisis Oct 5, 2017

nitisht commented Oct 7, 2017

ebozduman Oct 10, 2017 •

edited

Loading

ebozduman Oct 10, 2017

ebozduman Oct 10, 2017

harshavardhana Oct 10, 2017

krisis Oct 10, 2017

nitisht commented Oct 18, 2017

nitisht commented Oct 18, 2017

krisis commented Oct 19, 2017

harshavardhana commented Oct 19, 2017

deekoder commented Oct 19, 2017

krisis commented Oct 19, 2017

krisis commented Oct 19, 2017

nitisht commented Oct 20, 2017

ebozduman commented Nov 6, 2017 •

edited

Loading

Use waitUntil to check if bucket is deleted #177

Use waitUntil to check if bucket is deleted #177

Conversation

krisis commented Oct 3, 2017 • edited Loading

nitisht commented Oct 3, 2017

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

nitisht commented Oct 7, 2017

ebozduman Oct 10, 2017 • edited Loading

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

nitisht commented Oct 18, 2017

nitisht commented Oct 18, 2017

krisis commented Oct 19, 2017

harshavardhana commented Oct 19, 2017

deekoder commented Oct 19, 2017

krisis commented Oct 19, 2017

krisis commented Oct 19, 2017

nitisht commented Oct 20, 2017

ebozduman commented Nov 6, 2017 • edited Loading

krisis commented Oct 3, 2017 •

edited

Loading

ebozduman Oct 10, 2017 •

edited

Loading

ebozduman commented Nov 6, 2017 •

edited

Loading