Improvement/CLDSRV-825 drop ceph support#6097
Improvement/CLDSRV-825 drop ceph support#6097benzekrimaha wants to merge 6 commits intodevelopment/9.3from
Conversation
Hello benzekrimaha,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
4365d2c to
178412f
Compare
tests/functional/aws-node-sdk/test/multipleBackend/objectCopy/objectCopy.js
Outdated
Show resolved
Hide resolved
tests/functional/aws-node-sdk/test/multipleBackend/objectCopy/objectCopyAwsVersioning.js
Outdated
Show resolved
Hide resolved
Request integration branchesWaiting for integration branch creation to be requested by the user. To request integration branches, please comment on this pull request with the following command: Alternatively, the |
b15a533 to
a4528f1
Compare
This commit drops Ceph CI artifacts and location config, removes Ceph-specific runtime defaults, and updates reported capability flags and related unit coverage to match supported backends. Issue: CLDSRV-825
a4528f1 to
ef671c1
Compare
|
LGTM |
This commit remove Ceph-specific skip logic and behavior branches from multiple-backend/functional suites, and cleans stale Ceph comments in test files. Issue: CLDSRV-825
0390b29 to
a79a3c6
Compare
|
LGTM |
…ties fixing tests after re enabling them
DarkIsDude
left a comment
There was a problem hiding this comment.
I found some CEPH reference in the product doc that we should also clean. The CI is still unstable
lib/Config.js
Outdated
| if (config.capabilities) { | ||
| if (config.capabilities.locationTypes) { | ||
| this.supportedLocationTypes = new Set(config.capabilities.locationTypes); | ||
| this.supportedLocationTypes = new Set( |
There was a problem hiding this comment.
why?
this should not be needed, unless there is an issue in tests.... so the tests should be updated instead: we should not adapt production code to tests, but tests to match how it happens in production
| @@ -531,7 +523,7 @@ describe('Multipart Upload API with AWS Backend', function mpuTestSuite() { | |||
| })).then(() => { | |||
| assert.fail('Expected an error listing parts of aborted MPU'); | |||
| }).catch(err => { | |||
| const wantedError = isCEPH ? 'NoSuchKey' : 'NoSuchUpload'; | |||
| const wantedError = 'NoSuchUpload'; | |||
There was a problem hiding this comment.
variable probably not needed here
tests/unit/utils/reportHandler.js
Outdated
| awsIngestLocation: false, | ||
| }); | ||
| assert.strictEqual( | ||
| Object.prototype.hasOwnProperty.call(caps, 'locationTypeCephRadosGW'), |
There was a problem hiding this comment.
useless: redundant with assert.deepStrictEqual above, can never fail...
tests/unit/utils/reportHandler.js
Outdated
| customCapability: 'test-value', | ||
| }); | ||
| assert.strictEqual( | ||
| Object.prototype.hasOwnProperty.call(caps, 'locationTypeCephRadosGW'), |
There was a problem hiding this comment.
useless: redundant with assert.deepStrictEqual above, can never fail...
francoisferrand
left a comment
There was a problem hiding this comment.
a few small things, but more importantly it seems to me this cleanup is not complete: this is modifying only the tests, but we keep the code handling specific corner cases for cepth (which may be either/both in cloudserver and arsenal).
this code MUST be cleaned as well, otherwise we just continue maintaining the same code, just with less coverage
Branches have divergedThis pull request's source branch To avoid any integration risks, please re-synchronize them using one of the
Note: If you choose to rebase, you may have to ask me to rebuild |
|
@francoisferrand Thanks for the careful review — I agree with your point that removing tests without removing related code would leave us with untested legacy behavior. |
lib/utilities/reportHandler.js
Outdated
| delete caps.locationTypeCephRadosGW; | ||
| delete caps.cephIngestLocation; |
There was a problem hiding this comment.
this was an earlier comment, we need to keep these!
cloudserver will not support ceph anymore, so we don't want to advertise that we still support it
this is not "ceph-specific" logic, this is logic which was introduced earlier on purpose to correctly report that we DO NOT support ceph anymore. So it must be kept.
yes, as discussed on Monday we need to check in Arsenal as well : where the actual specifics of each (data) backend are implemented |
Issue: CLDSRV-825