Handle BucketAlreadyExists races and bump arsenal for MPU afterEach fix

package.json

@@ -30,7 +30,7 @@
         "@azure/storage-blob": "^12.28.0",
         "@hapi/joi": "^17.1.1",
         "@smithy/node-http-handler": "^3.0.0",
-        "arsenal": "git+https://github.com/scality/Arsenal#8.4.16",
+        "arsenal": "git+https://github.com/scality/Arsenal#88e7b332",
         "async": "2.6.4",
         "bucketclient": "scality/bucketclient#8.2.7",
         "bufferutil": "^4.0.8",

tests/unit/bucket/bucketCreation.js

@@ -1,8 +1,12 @@
 const assert = require('assert');
+const sinon = require('sinon');
 
+const { errors } = require('arsenal');
+const BucketInfo = require('arsenal').models.BucketInfo;
+const constants = require('../../../constants');
+const metadata = require('../../../lib/metadata/wrapper');
 const { cleanup, DummyRequestLogger } = require('../helpers');
-const { createBucket } =
-    require('../../../lib/api/apiUtils/bucket/bucketCreation');
+const { createBucket } = require('../../../lib/api/apiUtils/bucket/bucketCreation');
 const { makeAuthInfo } = require('../helpers');
 
 const bucketName = 'creationbucket';
@@ -15,34 +19,30 @@ const specialBehaviorLocationConstraint = 'us-east-1';
 
 describe('bucket creation', () => {
     it('should create a bucket', done => {
-        createBucket(authInfo, bucketName, headers,
-            normalBehaviorLocationConstraint, log, err => {
-                assert.ifError(err);
-                done();
-            });
+        createBucket(authInfo, bucketName, headers, normalBehaviorLocationConstraint, log, err => {
+            assert.ifError(err);
+            done();
+        });
     });
 
     describe('when you already created the bucket in us-east-1', () => {
         beforeEach(done => {
             cleanup();
-            createBucket(authInfo, bucketName, headers,
-                specialBehaviorLocationConstraint, log, err => {
-                    assert.ifError(err);
-                    done();
-                });
+            createBucket(authInfo, bucketName, headers, specialBehaviorLocationConstraint, log, err => {
+                assert.ifError(err);
+                done();
+            });
         });
 
         it('should return 200 if try to recreate in us-east-1', done => {
-            createBucket(authInfo, bucketName, headers,
-            specialBehaviorLocationConstraint, log, err => {
+            createBucket(authInfo, bucketName, headers, specialBehaviorLocationConstraint, log, err => {
                 assert.ifError(err);
                 done();
             });
         });
 
         it('should return 409 if try to recreate in non-us-east-1', done => {
-            createBucket(authInfo, bucketName, headers,
-            normalBehaviorLocationConstraint, log, err => {
+            createBucket(authInfo, bucketName, headers, normalBehaviorLocationConstraint, log, err => {
                 assert.strictEqual(err.is.BucketAlreadyOwnedByYou, true);
                 done();
             });
@@ -52,32 +52,72 @@ describe('bucket creation', () => {
     describe('when you already created the bucket in non-us-east-1', () => {
         beforeEach(done => {
             cleanup();
-            createBucket(authInfo, bucketName, headers,
-                normalBehaviorLocationConstraint, log, err => {
-                    assert.ifError(err);
-                    done();
-                });
+            createBucket(authInfo, bucketName, headers, normalBehaviorLocationConstraint, log, err => {
+                assert.ifError(err);
+                done();
+            });
         });
 
         it('should return 409 if try to recreate in us-east-1', done => {
-            createBucket(authInfo, bucketName, headers,
-            specialBehaviorLocationConstraint, log, err => {
+            createBucket(authInfo, bucketName, headers, specialBehaviorLocationConstraint, log, err => {
                 assert.strictEqual(err.is.BucketAlreadyOwnedByYou, true);
                 done();
             });
         });
     });
 });
+
+describe('bucket creation when createBucket races', () => {
+    const raceBucketName = 'race-creation-bucket';
+    let sandbox;
+
+    beforeEach(() => {
+        cleanup();
+        sandbox = sinon.createSandbox();
+    });
+
+    afterEach(() => {
+        sandbox.restore();
+    });
+
+    it('should complete creation when metadata returns BucketAlreadyExists', done => {
+        const usersBucketMD = new BucketInfo(constants.usersBucket, 'admin', 'admin', new Date().toJSON());
+
+        sandbox.stub(metadata, 'getBucket').callsFake((name, log, cb) => {
+            if (name === raceBucketName) {
+                return cb(errors.NoSuchBucket);
+            }
+            if (name === constants.usersBucket) {
+                return cb(null, usersBucketMD);
+            }
+            return cb(errors.NoSuchBucket);
+        });
+        sandbox.stub(metadata, 'createBucket').callsFake((name, bucket, log, cb) => {
+            if (name === raceBucketName) {
+                return cb(errors.BucketAlreadyExists);
+            }
+            return cb(null);
+        });
+        sandbox.stub(metadata, 'putObjectMD').yields(null);
+        sandbox.stub(metadata, 'updateBucket').yields(null);
+
+        createBucket(authInfo, raceBucketName, headers, normalBehaviorLocationConstraint, log, err => {
+            assert.ifError(err);
+            sandbox.assert.calledOnce(metadata.createBucket);
+            done();
+        });
+    });
+});
+
 describe('bucket creation with object lock', () => {
     it('should return 200 when creating a bucket with object lock', done => {
         const bucketName = 'test-bucket-with-objectlock';
         const headers = {
             'x-amz-bucket-object-lock-enabled': 'true',
         };
-        createBucket(authInfo, bucketName, headers,
-            normalBehaviorLocationConstraint, log, err => {
-                assert.ifError(err);
-                done();
-            });
+        createBucket(authInfo, bucketName, headers, normalBehaviorLocationConstraint, log, err => {
+            assert.ifError(err);
+            done();
+        });
     });
 });

tests/unit/lib/services.spec.js

@@ -91,8 +91,12 @@ describe('services', () => {
                 IsTruncated: false,
             });
 
-            services.findObjectVersionByUploadId(bucketName, objectKey, 'non-existent-upload-id',
-                log, (err, foundVersion) => {
+            services.findObjectVersionByUploadId(
+                bucketName,
+                objectKey,
+                'non-existent-upload-id',
+                log,
+                (err, foundVersion) => {
                     assert.ifError(err);
                     sinon.assert.calledTwice(getObjectListingStub);
 
@@ -101,7 +105,8 @@ describe('services', () => {
                     assert.strictEqual(secondCallParams.versionIdMarker, 'version-marker');
                     assert.strictEqual(foundVersion, null);
                     done();
-                });
+                },
+            );
         });
 
         it('should find a version on the first page of many and stop listing', done => {
@@ -154,4 +159,61 @@ describe('services', () => {
             });
         });
     });
+
+    describe('getMPUBucket', () => {
+        const { errors } = require('arsenal');
+        const BucketInfo = require('arsenal').models.BucketInfo;
+        const metadata = require('../../../lib/metadata/wrapper');
+        const constants = require('../../../constants');
+        const destinationBucket = new BucketInfo('dest-bucket', 'owner', 'ownerDisplay', new Date().toJSON());
+        let getBucketStub;
+        let createBucketStub;
+
+        beforeEach(() => {
+            getBucketStub = sinon.stub(metadata, 'getBucket');
+            createBucketStub = sinon.stub(metadata, 'createBucket');
+        });
+
+        it('should return existing MPU bucket when createBucket races', done => {
+            const mpuBucketName = `${constants.mpuBucketPrefix}${bucketName}`;
+            const existingMpuBucket = new BucketInfo(mpuBucketName, 'owner', 'ownerDisplay', new Date().toJSON());
+            getBucketStub.onFirstCall().yields(errors.NoSuchBucket);
+            createBucketStub.yields(errors.BucketAlreadyExists);
+            getBucketStub.onSecondCall().yields(null, existingMpuBucket);
+
+            services.getMPUBucket(destinationBucket, bucketName, log, (err, bucket) => {
+                assert.ifError(err);
+                assert.strictEqual(bucket, existingMpuBucket);
+                sinon.assert.calledTwice(getBucketStub);
+                sinon.assert.calledOnce(createBucketStub);
+                done();
+            });
+        });
+
+        it('should create MPU bucket when it does not exist', done => {
+            getBucketStub.yields(errors.NoSuchBucket);
+            createBucketStub.yields(null);
+
+            services.getMPUBucket(destinationBucket, bucketName, log, (err, bucket) => {
+                assert.ifError(err);
+                assert.strictEqual(bucket.getName(), `${constants.mpuBucketPrefix}${bucketName}`);
+                sinon.assert.calledOnce(createBucketStub);
+                done();
+            });
+        });
+
+        it('should return error when refetch fails after BucketAlreadyExists', done => {
+            const testError = errors.InternalError;
+            getBucketStub.onFirstCall().yields(errors.NoSuchBucket);
+            createBucketStub.yields(errors.BucketAlreadyExists);
+            getBucketStub.onSecondCall().yields(testError);
+
+            services.getMPUBucket(destinationBucket, bucketName, log, err => {
+                assert.strictEqual(err, testError);
+                sinon.assert.calledTwice(getBucketStub);
+                sinon.assert.calledOnce(createBucketStub);
+                done();
+            });
+        });
+    });
 });

yarn.lock

@@ -6591,9 +6591,9 @@ arraybuffer.prototype.slice@^1.0.4:
   optionalDependencies:
     ioctl "^2.0.2"
 
-"arsenal@git+https://github.com/scality/Arsenal#8.4.16":
-  version "8.4.16"
-  resolved "git+https://github.com/scality/Arsenal#65223fdfc37f4471332c44775def9e985b320855"
+"arsenal@git+https://github.com/scality/Arsenal#88e7b332":
+  version "8.4.20"
+  resolved "git+https://github.com/scality/Arsenal#88e7b33205f6ce3fda3be0b313a68609eaac7daa"
   dependencies:
     "@aws-sdk/client-kms" "^3.975.0"
     "@aws-sdk/client-s3" "^3.975.0"