Skip to content

Cloudserver adaptation post arsenal refacto #6044

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
benzekrimaha wants to merge 3 commits into
base: development/9.3
Choose a base branch
from
Open

Cloudserver adaptation post arsenal refacto #6044

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a3a207a
bump latest up to date arsenal
benzekrimaha Jan 14, 2026
bd529bb
tests updates post gcpService unused functions removal
benzekrimaha Jan 14, 2026
b5f9d21
update arsenal tag
benzekrimaha Jan 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,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.3.0-preview.1",
"arsenal": "git+https://github.com/scality/Arsenal#aed443f02efebeb3f5c2e74786baadb5c19eaded",
"async": "2.6.4",
"bucketclient": "scality/bucketclient#8.2.7",
"bufferutil": "^4.0.8",
Expand Down
30 changes: 30 additions & 0 deletions tests/functional/raw-node/test/GCP/bucket/get.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const assert = require('assert');
const async = require('async');
const arsenal = require('arsenal');
const { ListObjectsCommand } = require('@aws-sdk/client-s3');
const { GCP } = arsenal.storage.data.external.GCP;
const { makeGcpRequest } = require('../../../utils/makeRequest');
const { gcpRequestRetry, genUniqID } = require('../../../utils/gcpUtils');
Expand All @@ -15,6 +16,35 @@
const config = getRealAwsConfig(credentialOne);
const gcpClient = new GCP(config);

gcpClient.listObjects = (params, callback) => {
const command = new ListObjectsCommand(params);
return gcpClient.send(command)
.then(data => callback(null, data))
.catch(err => {
if ( err.statusCode === undefined) {

Check warning on line 24 in tests/functional/raw-node/test/GCP/bucket/get.js

View workflow job for this annotation

GitHub Actions / lint 

There should be no space after this paren
// eslint-disable-next-line no-param-reassign
err.statusCode = err.$metadata.httpStatusCode;
}
return callback(err);
});
};

gcpClient.getBucket = (params, callback) =>
gcpClient.headBucket(params, (err, res) => {
if (err) {
if (err.statusCode === undefined) {
// eslint-disable-next-line no-param-reassign
err.statusCode = err.$metadata.httpStatusCode;
}
if (err.$metadata && err.$metadata.httpStatusCode === 404) {
// eslint-disable-next-line no-param-reassign
err.name = 'NoSuchBucket';
}
return callback(err);
}
return callback(null, res);
});

Comment on lines +19 to +47
Copy link
Contributor

@francoisferrand francoisferrand Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why keep these "legacy" functions, and not migrate the actual tests to the new SDK api?
since this PR is focused, it does not seem like a large change...

function populateBucket(createdObjects, callback) {
process.stdout.write(
`Putting ${createdObjects.length} objects into bucket\n`);
Expand Down
41 changes: 27 additions & 14 deletions tests/functional/raw-node/test/GCP/bucket/getVersioning.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const assert = require('assert');
const async = require('async');
const arsenal = require('arsenal');
const { GetBucketVersioningCommand } = require('@aws-sdk/client-s3');
const { GCP } = arsenal.storage.data.external.GCP;
const { makeGcpRequest } = require('../../../utils/makeRequest');
const { gcpRequestRetry, genUniqID } = require('../../../utils/gcpUtils');
Expand Down Expand Up @@ -67,14 +68,19 @@ describe('GCP: GET Bucket Versioning', () => {
return next(err);
}),
next => {
gcpClient.getBucketVersioning({
const command = new GetBucketVersioningCommand({
Bucket: this.test.bucketName,
}, (err, res) => {
assert.equal(err, null,
`Expected success, but got err ${err}`);
assert.deepStrictEqual(res.Status, verEnabledObj);
return next();
});
return gcpClient.send(command)
.then(res => {
assert.deepStrictEqual(res.Status, verEnabledObj);
return next();
})
.catch(err => {
assert.equal(err, null,
`Expected success, but got err ${err}`);
return next(err);
});
},
], err => done(err));
});
Expand All @@ -93,14 +99,21 @@ describe('GCP: GET Bucket Versioning', () => {
}
return next(err);
}),
next => gcpClient.getBucketVersioning({
Bucket: this.test.bucketName,
}, (err, res) => {
assert.equal(err, null,
`Expected success, but got err ${err}`);
assert.deepStrictEqual(res.Status, verDisabledObj);
return next();
}),
next => {
const command = new GetBucketVersioningCommand({
Bucket: this.test.bucketName,
});
return gcpClient.send(command)
.then(res => {
assert.deepStrictEqual(res.Status, verDisabledObj);
return next();
})
.catch(err => {
assert.equal(err, null,
`Expected success, but got err ${err}`);
return next(err);
});
},
], err => done(err));
});
});
51 changes: 31 additions & 20 deletions tests/functional/raw-node/test/GCP/bucket/putVersioning.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const assert = require('assert');
const async = require('async');
const arsenal = require('arsenal');
const { PutBucketVersioningCommand } = require('@aws-sdk/client-s3');
const xml2js = require('xml2js');
const { GCP } = arsenal.storage.data.external.GCP;
const { makeGcpRequest } = require('../../../utils/makeRequest');
Expand Down Expand Up @@ -56,16 +57,21 @@ describe('GCP: PUT Bucket Versioning', () => {

it('should enable bucket versioning', function testFn(done) {
return async.waterfall([
next => gcpClient.putBucketVersioning({
Bucket: this.test.bucketName,
VersioningConfiguration: {
Status: 'Enabled',
},
}, err => {
assert.equal(err, null,
`Expected success, but got err ${err}`);
return next();
}),
next => {
const command = new PutBucketVersioningCommand({
Bucket: this.test.bucketName,
VersioningConfiguration: {
Status: 'Enabled',
},
});
return gcpClient.send(command)
.then(() => next())
.catch(err => {
assert.equal(err, null,
`Expected success, but got err ${err}`);
return next(err);
});
},
next => makeGcpRequest({
method: 'GET',
bucket: this.test.bucketName,
Expand All @@ -83,16 +89,21 @@ describe('GCP: PUT Bucket Versioning', () => {

it('should disable bucket versioning', function testFn(done) {
return async.waterfall([
next => gcpClient.putBucketVersioning({
Bucket: this.test.bucketName,
VersioningConfiguration: {
Status: 'Suspended',
},
}, err => {
assert.equal(err, null,
`Expected success, but got err ${err}`);
return next();
}),
next => {
const command = new PutBucketVersioningCommand({
Bucket: this.test.bucketName,
VersioningConfiguration: {
Status: 'Suspended',
},
});
return gcpClient.send(command)
.then(() => next())
.catch(err => {
assert.equal(err, null,
`Expected success, but got err ${err}`);
return next(err);
});
},
next => makeGcpRequest({
method: 'GET',
bucket: this.test.bucketName,
Expand Down
14 changes: 14 additions & 0 deletions tests/functional/raw-node/test/GCP/object/completeMpu.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const assert = require('assert');
const async = require('async');
const arsenal = require('arsenal');
const { ListObjectsCommand } = require('@aws-sdk/client-s3');
const { GCP, GcpUtils } = arsenal.storage.data.external.GCP;
const {
gcpRequestRetry,
Expand Down Expand Up @@ -100,6 +101,19 @@ describe('GCP: Complete MPU', function testSuite() {
before(done => {
config = getRealAwsConfig(credentialOne);
gcpClient = new GCP(config);
gcpClient.listObjects = (params, callback) => {
Copy link
Contributor

@SylvainSenechal SylvainSenechal Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to understand : this was available in Arsenal GcpService but you just removed it and now you are redefining it in all these files, why not keeping it in Arsenal ? I think the GcpService already has several functions that are used for tests only 🤔
Or is it because you want to reassign the statusCode ?

Copy link
Contributor

@SylvainSenechal SylvainSenechal Jan 15, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Upon looking : consider updating the gcpClientRetry function : check err.$metadata.statuscode == 429 on top of the existing check err.statusCode == 429.
This way, can keep listObjects in Arsenal. (unless there are other reason I didn't see)

Copy link
Contributor Author

@benzekrimaha benzekrimaha Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

even if we added the $metadata.httpStatusCode check inside gcpClientRetry, we still need bespoke error munging in these raw-node tests. Rather than keeping a half-working helper in Arsenal plus a bunch of local overrides, I’d rather keep the shared GcpService minimal and let the tests own whatever extra plumbing they require.

const command = new ListObjectsCommand(params);
Copy link
Contributor

@francoisferrand francoisferrand Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above, should migrate each test? (and avoid duplicating this piece of code)

return gcpClient.send(command)
.then(data => callback(null, data))
.catch(err => {
if (err && err.$metadata && err.$metadata.httpStatusCode &&
err.statusCode === undefined) {
// eslint-disable-next-line no-param-reassign
err.statusCode = err.$metadata.httpStatusCode;
}
return callback(err);
});
};
async.eachSeries(bucketNames,
(bucket, next) => gcpRequestRetry({
method: 'PUT',
Expand Down
14 changes: 14 additions & 0 deletions tests/functional/raw-node/test/GCP/object/deleteMpu.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const assert = require('assert');
const async = require('async');
const arsenal = require('arsenal');
const { ListObjectsCommand } = require('@aws-sdk/client-s3');
const { GCP } = arsenal.storage.data.external.GCP;
const { gcpRequestRetry, setBucketClass, gcpMpuSetup, genUniqID } =
require('../../../utils/gcpUtils');
Expand Down Expand Up @@ -40,6 +41,19 @@ describe('GCP: Abort MPU', function testSuite() {
before(done => {
config = getRealAwsConfig(credentialOne);
gcpClient = new GCP(config);
gcpClient.listObjects = (params, callback) => {
const command = new ListObjectsCommand(params);
return gcpClient.send(command)
.then(data => callback(null, data))
.catch(err => {
if (err && err.$metadata && err.$metadata.httpStatusCode &&
err.statusCode === undefined) {
// eslint-disable-next-line no-param-reassign
err.statusCode = err.$metadata.httpStatusCode;
}
return callback(err);
});
};
async.eachSeries(bucketNames,
(bucket, next) => gcpRequestRetry({
method: 'PUT',
Expand Down
14 changes: 14 additions & 0 deletions tests/functional/raw-node/test/GCP/object/upload.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const assert = require('assert');
const async = require('async');
const arsenal = require('arsenal');
const { ListObjectsCommand } = require('@aws-sdk/client-s3');
const { GCP } = arsenal.storage.data.external.GCP;
const { gcpRequestRetry, setBucketClass, genUniqID } =
require('../../../utils/gcpUtils');
Expand Down Expand Up @@ -32,6 +33,19 @@ describe('GCP: Upload Object', function testSuite() {
before(done => {
config = getRealAwsConfig(credentialOne);
gcpClient = new GCP(config);
gcpClient.listObjects = (params, callback) => {
const command = new ListObjectsCommand(params);
return gcpClient.send(command)
.then(data => callback(null, data))
.catch(err => {
if (err && err.$metadata && err.$metadata.httpStatusCode &&
err.statusCode === undefined) {
// eslint-disable-next-line no-param-reassign
err.statusCode = err.$metadata.httpStatusCode;
}
return callback(err);
});
};
async.eachSeries(bucketNames,
(bucket, next) => gcpRequestRetry({
method: 'PUT',
Expand Down
4 changes: 2 additions & 2 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4645,9 +4645,9 @@ arraybuffer.prototype.slice@^1.0.4:
optionalDependencies:
ioctl "^2.0.2"

"arsenal@git+https://github.com/scality/Arsenal#8.3.0-preview.1":
"arsenal@git+https://github.com/scality/Arsenal#aed443f02efebeb3f5c2e74786baadb5c19eaded":
version "8.3.0-preview.1"
resolved "git+https://github.com/scality/Arsenal#00033b55fc22d8e58ab38211842fc0ccf41b9766"
resolved "git+https://github.com/scality/Arsenal#aed443f02efebeb3f5c2e74786baadb5c19eaded"
dependencies:
"@aws-sdk/client-kms" "^3.901.0"
"@aws-sdk/client-s3" "^3.901.0"
Expand Down
Loading