Improvement/cldsrv 191 bump arsenal post breaking changes dev8x #4493

XinLi0207 · 2022-05-03T13:41:11Z

use an arsenal commit hashtag for testing, will replace it with 8.1.45 once it's released.

this PR is mainly doing two things:

update new arsenal error type:
- use err.is.Error instead of === error equal,
- use assert.strictEqual(err.is.Error, true) instead of deepStrictEqual(err, errors.Error)
after this change in arsenal, we can no longer use a random string to represent a non existing versionId for testing, it won't decode it successfully so will return InvalidArgument error instead of NoSuchVersion error that we expect. So even for non existing versionId, we have to give it a standard formated versionId, so I replaced several places where we use 000000000 for non existing versionId with a meaningful but also non existing versionId. b1d824c

green build in zenko https://eve.devsca.com/github/scality/zenko/#/builders/4/builds/21334

bert-e · 2022-05-03T13:41:13Z

Hello xinliscality,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Status report is not available.

bert-e · 2022-05-03T13:41:17Z

Incorrect fix version

The Fix Version/s in issue CLDSRV-191 contains:

8.3.13
8.4.9

Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:

8.4.9

Please check the Fix Version/s of CLDSRV-191, or the target
branch of this pull request.

bert-e · 2022-05-03T13:58:04Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
2 peers

miniscruff · 2022-05-03T18:05:56Z

Would it be possible to target 7.10 with this one?

XinLi0207 · 2022-05-04T13:19:46Z

Would it be possible to target 7.10 with this one?

Hi Ronnie, @miniscruff
there is already one pr for dev/7.10 #4466
and also considering dev/7.x and dev/8.x use different arsenal versions, so I didn't target 7.x and 8.x in one pr. how do you think?

tests/functional/aws-node-sdk/test/bucket/deleteBucketReplication.js

tests/functional/aws-node-sdk/test/bucket/getBucketReplication.js

tests/unit/api/objectPutTagging.js

williamlardier

LGTM, just two little improvements we can do.

tests/functional/aws-node-sdk/test/object/getObjectLegalHold.js

lib/api/apiUtils/bucket/bucketCreation.js

jbertran

Nice work!

You can kill ec68150f33babaad08395fa13cfd0d133d5c356d and squash d0bf45eccc09591634e3e2067e94564996ef0c36 into 3b2725f209e572231f1aadb0c3c588063fa4b269 🙂

We should get some green Zenko smoke tests before merging this, to be extra safe.

tests/functional/aws-node-sdk/test/bucket/deleteBucketReplication.js

tests/multipleBackend/objectPutCopyPart.js

francoisferrand · 2022-05-05T15:11:58Z

should this PR not target 8.2 ?
(the ticket is migrate latest arsenal to cloudserver dev/8.x)

XinLi0207 · 2022-05-06T07:08:26Z

should this PR not target 8.2 ? (the ticket is migrate latest arsenal to cloudserver dev/8.x)

Hi @francoisferrand , we are going to open dev/8.5 branch very soon, we want to maintain only the latest two branches, so I only target dev/8.4

francoisferrand · 2022-05-06T07:12:10Z

Maintaining 2 versions means 8.3 and 8.4 in addition to the Dev branch (8.5)

But my point is that the fix will land in 8.2, due to waterflow : so best to avoid extra/redundant work, and just target 8.2

Changes from your PR should be minimal I guess, and there is no extra work with federation/integration in this case.

XinLi0207 · 2022-05-06T08:00:38Z

Maintaining 2 versions means 8.3 and 8.4 in addition to the Dev branch (8.5)

But my point is that the fix will land in 8.2, due to waterflow : so best to avoid extra/redundant work, and just target 8.2

Changes from your PR should be minimal I guess, and there is no extra work with federation/integration in this case.

yeah, I understand. But I am not sure I can do this, rebasing to 8.2 will need some extra time, I am not sure I can still lose time for cold storage, I will check with PO quickly.

XinLi0207 · 2022-05-06T08:23:29Z

Maintaining 2 versions means 8.3 and 8.4 in addition to the Dev branch (8.5)

But my point is that the fix will land in 8.2, due to waterflow : so best to avoid extra/redundant work, and just target 8.2

Changes from your PR should be minimal I guess, and there is no extra work with federation/integration in this case.

hi @francoisferrand, sorry, I may not be able to do it this time, targeting 8.2 and 8.3 will bring a lot of extra work like rebasing and especially testing end2end in zenko, we don't want to add more risks to cold storage. But I will help to review the PR targeting 7.10 and also its integration PRs, hope can accelerate the migrating speed :)

jbertran · 2022-05-09T14:11:05Z

lib/api/multiObjectDelete.js

@@ -396,7 +396,8 @@ function multiObjectDelete(authInfo, request, log, callback) {
            return vault.checkPolicies(requestContextParams, authInfo.getArn(),
                log, (err, authorizationResults) => {
                    // there were no policies so received a blanket AccessDenied
-                    if (err.is.AccessDenied) {
+                    log.info('please see here: ', { err, authorizationResults });


XinLi0207 · 2022-05-09T15:28:07Z

/approve

bert-e · 2022-05-09T15:28:15Z

In the queue

The changeset has received all authorizations and has been added to the
relevant queue(s). The queue(s) will be merged in the target development
branch(es) as soon as builds have passed.

The changeset will be merged in:

✔️ development/8.4

The following branches will NOT be impacted:

development/6.4
development/7.10
development/7.4
development/8.2
development/8.3

There is no action required on your side. You will be notified here once
the changeset has been merged. In the unlikely event that the changeset
fails permanently on the queue, a member of the admin team will
contact you to help resolve the matter.

IMPORTANT

Please do not attempt to modify this pull request.

Any commit you add on the source branch will trigger a new cycle after the
current queue is merged.
Any commit you add on one of the integration branches will be lost.

If you need this pull request to be removed from the queue, please contact a
member of the admin team now.

The following options are set: approve

bert-e · 2022-05-09T15:28:23Z

I have successfully merged the changeset of this pull request
into targetted development branches:

✔️ development/8.4

The following branches have NOT changed:

development/6.4
development/7.10
development/7.4
development/8.2
development/8.3

Please check the status of the associated issue CLDSRV-191.

Goodbye xinliscality.

#4493

XinLi0207 force-pushed the improvement/CLDSRV-191-bump-arsenal-post-breaking-changes-dev8x branch from 7525709 to 3a20f69 Compare May 3, 2022 14:42

williamlardier reviewed May 5, 2022

View reviewed changes

tests/functional/aws-node-sdk/test/bucket/deleteBucketReplication.js Show resolved Hide resolved

tests/functional/aws-node-sdk/test/bucket/getBucketReplication.js Show resolved Hide resolved

tests/unit/api/objectPutTagging.js Show resolved Hide resolved

XinLi0207 force-pushed the improvement/CLDSRV-191-bump-arsenal-post-breaking-changes-dev8x branch 2 times, most recently from 5150705 to b1d824c Compare May 5, 2022 10:15

XinLi0207 requested review from williamlardier, jbertran, a user and KillianG May 5, 2022 11:55

williamlardier approved these changes May 5, 2022

View reviewed changes

tests/functional/aws-node-sdk/test/object/getObjectLegalHold.js Outdated Show resolved Hide resolved

lib/api/apiUtils/bucket/bucketCreation.js Outdated Show resolved Hide resolved

jbertran reviewed May 5, 2022

View reviewed changes

tests/functional/aws-node-sdk/test/bucket/deleteBucketReplication.js Show resolved Hide resolved

tests/multipleBackend/objectPutCopyPart.js Outdated Show resolved Hide resolved

XinLi0207 force-pushed the improvement/CLDSRV-191-bump-arsenal-post-breaking-changes-dev8x branch from 82dda9e to b37e4a9 Compare May 5, 2022 15:04

XinLi0207 force-pushed the improvement/CLDSRV-191-bump-arsenal-post-breaking-changes-dev8x branch from 35b4ba2 to 5b2baf2 Compare May 5, 2022 15:54

anurag4DSB requested a review from nicolas2bert May 5, 2022 16:50

KillianG approved these changes May 9, 2022

View reviewed changes

XinLi0207 force-pushed the improvement/CLDSRV-191-bump-arsenal-post-breaking-changes-dev8x branch from b32f6a9 to 582aa4e Compare May 9, 2022 13:51

jbertran approved these changes May 9, 2022

View reviewed changes

improvement: CLDSRV-191 migrate arsenal to cloudserver 8.x

03fa7d0

XinLi0207 force-pushed the improvement/CLDSRV-191-bump-arsenal-post-breaking-changes-dev8x branch from 5edee36 to 03fa7d0 Compare May 9, 2022 14:45

bert-e merged commit 03fa7d0 into development/8.4 May 9, 2022

bert-e deleted the improvement/CLDSRV-191-bump-arsenal-post-breaking-changes-dev8x branch May 9, 2022 15:28

XinLi0207 pushed a commit that referenced this pull request May 12, 2022

improvement: CLDSRV-183 remove stray log from the last PR

69043e0

#4493

XinLi0207 pushed a commit that referenced this pull request May 13, 2022

improvement: CLDSRV-183 remove stray log from the last PR

8c85dc6

#4493

XinLi0207 pushed a commit that referenced this pull request May 13, 2022

improvement: CLDSRV-183 remove stray log from the last PR

d42c292

#4493

XinLi0207 pushed a commit that referenced this pull request May 13, 2022

improvement: CLDSRV-183 remove stray log from the last PR

92630a3

#4493

XinLi0207 pushed a commit that referenced this pull request May 13, 2022

improvement: CLDSRV-183 remove stray log from the last PR

c9221ea

#4493

XinLi0207 pushed a commit that referenced this pull request May 16, 2022

improvement: CLDSRV-183 remove stray log from the last PR

cd915ee

#4493

XinLi0207 pushed a commit that referenced this pull request May 23, 2022

improvement: CLDSRV-183 remove stray log from the last PR

6909a4b

#4493

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improvement/cldsrv 191 bump arsenal post breaking changes dev8x #4493

Improvement/cldsrv 191 bump arsenal post breaking changes dev8x #4493

XinLi0207 commented May 3, 2022 •

edited

Loading

bert-e commented May 3, 2022

bert-e commented May 3, 2022

bert-e commented May 3, 2022

miniscruff commented May 3, 2022

XinLi0207 commented May 4, 2022 •

edited

Loading

williamlardier left a comment

jbertran left a comment

francoisferrand commented May 5, 2022 •

edited

Loading

XinLi0207 commented May 6, 2022

francoisferrand commented May 6, 2022

XinLi0207 commented May 6, 2022

XinLi0207 commented May 6, 2022

jbertran May 9, 2022

XinLi0207 commented May 9, 2022

bert-e commented May 9, 2022

bert-e commented May 9, 2022

Improvement/cldsrv 191 bump arsenal post breaking changes dev8x #4493

Improvement/cldsrv 191 bump arsenal post breaking changes dev8x #4493

Conversation

XinLi0207 commented May 3, 2022 • edited Loading

bert-e commented May 3, 2022

Hello xinliscality,

bert-e commented May 3, 2022

Incorrect fix version

bert-e commented May 3, 2022

Waiting for approval

miniscruff commented May 3, 2022

XinLi0207 commented May 4, 2022 • edited Loading

williamlardier left a comment

Choose a reason for hiding this comment

jbertran left a comment

Choose a reason for hiding this comment

francoisferrand commented May 5, 2022 • edited Loading

XinLi0207 commented May 6, 2022

francoisferrand commented May 6, 2022

XinLi0207 commented May 6, 2022

XinLi0207 commented May 6, 2022

jbertran May 9, 2022

Choose a reason for hiding this comment

XinLi0207 commented May 9, 2022

bert-e commented May 9, 2022

In the queue

bert-e commented May 9, 2022

XinLi0207 commented May 3, 2022 •

edited

Loading

XinLi0207 commented May 4, 2022 •

edited

Loading

francoisferrand commented May 5, 2022 •

edited

Loading