Skip to content

Commit

Permalink
Merge branch 'bugfix/BB-617' into q/8.7
Browse files Browse the repository at this point in the history
  • Loading branch information
@bert-e
bert-e committed Nov 13, 2024
2 parents 6372ec0 + 0ab64e0 commit 0aca138
Show file tree
Hide file tree
Showing 3 changed files with 107 additions and 49 deletions.
98 changes: 61 additions & 37 deletions extensions/lifecycle/tasks/LifecycleDeleteObjectTask.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,25 +103,62 @@ class LifecycleDeleteObjectTask extends BackbeatTask {
return done();
}

_getS3Action(actionType, accountId) {
let reqMethod;
if (actionType === 'deleteObject') {
const backbeatClient = this.getBackbeatClient(accountId);
if (!backbeatClient) {
return null;
}
// Zenko supports the "deleteObjectFromExpiration" API, which
// sets the proper originOp in the metadata to trigger a
// nortification when an object gets expired.
if (typeof backbeatClient.deleteObjectFromExpiration === 'function') {
return backbeatClient.deleteObjectFromExpiration.bind(backbeatClient);
}
reqMethod = 'deleteObject';
} else {
reqMethod = 'abortMultipartUpload';
}
_abortMPU(reqParams, accountId, transitionTime, startTime, location, log, done) {
const client = this.getS3Client(accountId);
return client[reqMethod].bind(client);
if (!client) {
log.error('failed to get s3 client', {
accountId,
actionType: 'deleteMPU',
method: 'LifecycleDeleteObjectTask._abortMPU',
});
done(errors.InternalError
.customizeDescription('Unable to obtain s3 client'));
return;
}
LifecycleMetrics.onLifecycleStarted(log,
'expiration:mpu',
location, startTime - transitionTime);
const req = client.abortMultipartUpload(reqParams);
attachReqUids(req, log);
req.send(done);
}

_deleteObject(reqParams, accountId, transitionTime, startTime, location, log, done) {
const logDetails = {
accountId,
actionType: 'deleteObject',
method: 'LifecycleDeleteObjectTask._deleteObject',
};
const client = this.getBackbeatClient(accountId);
if (!client) {
log.error('failed to get s3 backbeat client', logDetails);
done(errors.InternalError
.customizeDescription('Unable to obtain s3 client'));
return;
}
LifecycleMetrics.onLifecycleStarted(log,
'expiration',
location, startTime - transitionTime);
const req = client.deleteObjectFromExpiration(reqParams);
attachReqUids(req, log);
req.send(err => {
if (err?.statusCode === errors.MethodNotAllowed.code) {
log.warn('deleteObjectFromExpiration API not supported, falling back to deleteObject',
logDetails);
// fallback to s3 deleteObject when using a cloudserver that
// doesn't support deleteObjectFromExpiration
const s3Client = this.getS3Client(accountId);
if (!s3Client) {
log.error('failed to get s3 client', logDetails);
return done(errors.InternalError
.customizeDescription('Unable to obtain client'));
}
const s3Req = s3Client.deleteObject(reqParams);
attachReqUids(s3Req, log);
return s3Req.send(done);
}
return done(err);
});
}

_executeDelete(entry, startTime, log, done) {
Expand All @@ -135,37 +172,24 @@ class LifecycleDeleteObjectTask extends BackbeatTask {
if (version !== undefined) {
reqParams.VersionId = version;
}
let reqMethod;

const actionType = entry.getActionType();
const transitionTime = entry.getAttribute('transitionTime');
const location = this.objectMD?.dataStoreName || entry.getAttribute('details.dataStoreName');
let req = null;

const s3Action = this._getS3Action(actionType, accountId);
if (!s3Action) {
log.error('failed to get s3 action', {
accountId,
actionType,
method: 'LifecycleDeleteObjectTask._executeDelete',
});
return done(errors.InternalError
.customizeDescription('Unable to obtain s3 action'));
}
LifecycleMetrics.onLifecycleStarted(log,
actionType === 'deleteMPU' ? 'expiration:mpu' : 'expiration',
location, startTime - transitionTime);
let reqMethod = 'deleteObject';
let actionMethod = this._deleteObject.bind(this);
if (actionType === 'deleteMPU') {
reqMethod = 'abortMultipartUpload';
reqParams.UploadId = entry.getAttribute('details.UploadId');
actionMethod = this._abortMPU.bind(this);
}
req = s3Action(reqParams);
attachReqUids(req, log);
return req.send(err => {

actionMethod(reqParams, accountId, transitionTime, startTime, location, log, err => {
LifecycleMetrics.onS3Request(log, reqMethod, 'expiration', err);
LifecycleMetrics.onLifecycleCompleted(log,
actionType === 'deleteMPU' ? 'expiration:mpu' : 'expiration',
location, Date.now() - entry.getAttribute('transitionTime'));

if (err) {
log.error(
`an error occurred on ${reqMethod} to S3`, Object.assign({
Expand Down
49 changes: 43 additions & 6 deletions tests/unit/lifecycle/LifecycleDeleteObjectTask.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ describe('LifecycleDeleteObjectTask', () => {
});
});

it('should expire object using the deleteObjectFromExpiration method if supported', done => {
it('should expire object using the deleteObjectFromExpiration method', done => {
const entry = ActionQueueEntry.create('deleteObject')
.setAttribute('target.owner', 'testowner')
.setAttribute('target.bucket', 'testbucket')
Expand All @@ -278,9 +278,7 @@ describe('LifecycleDeleteObjectTask', () => {
});
});

it('should expire object using the deleteObject method when not in Zenko', done => {
backbeatClient = new BackbeatClientMock({ isS3c: true });
sinon.stub(task, 'getBackbeatClient').returns(backbeatClient);
it('should fallback to deleteObject method if deleteObjectFromExpiration is not supported', done => {
const entry = ActionQueueEntry.create('deleteObject')
.setAttribute('target.owner', 'testowner')
.setAttribute('target.bucket', 'testbucket')
Expand All @@ -289,11 +287,32 @@ describe('LifecycleDeleteObjectTask', () => {
.setAttribute('target.version', 'testversion')
.setAttribute('details.lastModified', '2022-05-13T17:51:31.261Z');
s3Client.setResponse(null, {});
backbeatClient.setResponse(null, {});
const methodNotAllowedErr = new Error('MethodNotAllowed');
methodNotAllowedErr.statusCode = 405;
backbeatClient.setResponse(methodNotAllowedErr, {});
task.processActionEntry(entry, err => {
assert.ifError(err);
assert.strictEqual(backbeatClient.times.deleteObjectFromExpiration, 1);
assert.strictEqual(s3Client.calls.deleteObject, 1);
assert.strictEqual(backbeatClient.times.deleteObjectFromExpiration, 0);
done();
});
});

it('should fail to fallback if it fails to get the s3 client', done => {
sinon.stub(task, 'getS3Client').returns(null);
const entry = ActionQueueEntry.create('deleteObject')
.setAttribute('target.owner', 'testowner')
.setAttribute('target.bucket', 'testbucket')
.setAttribute('target.accountId', 'testid')
.setAttribute('target.key', 'testkey')
.setAttribute('target.version', 'testversion')
.setAttribute('details.lastModified', '2022-05-13T17:51:31.261Z');
s3Client.setResponse(null, {});
const methodNotAllowedErr = new Error('MethodNotAllowed');
methodNotAllowedErr.statusCode = 405;
backbeatClient.setResponse(methodNotAllowedErr, {});
task.processActionEntry(entry, err => {
assert(err);
done();
});
});
Expand Down Expand Up @@ -334,4 +353,22 @@ describe('LifecycleDeleteObjectTask', () => {
done();
});
});

it('should return an error when it can\'t get the S3 client', done => {
sinon.stub(task, 'getS3Client').returns(null);
const entry = ActionQueueEntry.create('deleteMPU')
.setAttribute('target.owner', 'testowner')
.setAttribute('target.bucket', 'testbucket')
.setAttribute('target.accountId', 'testid')
.setAttribute('target.key', 'testkey')
.setAttribute('target.version', 'testversion')
.setAttribute('details.UploadId', 'someUploadId')
.setAttribute('details.lastModified', '2022-05-13T17:51:31.261Z');
s3Client.setResponse(null, {});
backbeatClient.setResponse(null, {});
task.processActionEntry(entry, err => {
assert(err);
done();
});
});
});
9 changes: 3 additions & 6 deletions tests/unit/mocks.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,21 +61,18 @@ class MockRequestAPI extends EventEmitter {
}

class BackbeatClientMock {
constructor(params) {
constructor() {
this.response = null;
this.batchDeleteResponse = {};
this.times = {
batchDeleteResponse: 0,
deleteObjectFromExpiration: 0,
};
if (params?.isS3c) {
this.deleteObjectFromExpiration = undefined;
}
}

deleteObjectFromExpiration() {
deleteObjectFromExpiration(params) {
this.times.deleteObjectFromExpiration += 1;
return new MockRequestAPI(this.response.error, this.response.data);
return new MockRequestAPI(params, this.response);
}

setResponse(error, data) {
Expand Down

0 comments on commit 0aca138

Please sign in to comment.