Skip to content

ARSN-652: Backport DataWrapper error improvement & more - #2720

Merged
bert-e merged 9 commits into
development/7.10from
improvement/ARSN-652-leak-kms-7.10
Oct 7, 2026
Merged

ARSN-652: Backport DataWrapper error improvement & more#2720
bert-e merged 9 commits into
development/7.10from
improvement/ARSN-652-leak-kms-7.10

Conversation

@BourgoisMickael

@BourgoisMickael BourgoisMickael commented Oct 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Cherry-pick and backport for DataWrapper and copy tests from above branches.

This is for RD-2378 for a customer stuck on an old RING 8

In Integration branches backport #2668 to fix kmip ci

@bert-e

bert-e commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Hello bourgoismickael,

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.

Available options
name description privileged authored
/after_pull_request Wait for the given pull request id to be merged before continuing with the current one.
/bypass_author_approval Bypass the pull request author's approval ⭐
/bypass_build_status Bypass the build and test status ⭐
/bypass_commit_size Bypass the check on the size of the changeset TBA ⭐
/bypass_incompatible_branch Bypass the check on the source branch prefix ⭐
/bypass_jira_check Bypass the Jira issue check ⭐
/bypass_peer_approval Bypass the pull request peers' approval ⭐
/bypass_leader_approval Bypass the pull request leaders' approval ⭐
/bypass_source_branch_lineage Bypass the cross-branch contamination check ⭐
/approve Instruct Bert-E that the author has approved the pull request. ✍️
/create_pull_requests Allow the creation of integration pull requests.
/create_integration_branches Allow the creation of integration branches.
/no_octopus Prevent Wall-E from doing any octopus merge and use multiple consecutive merge instead
/unanimity Change review acceptance criteria from one reviewer at least to all reviewers
/wait Instruct Bert-E not to run until further notice.
Available commands
name description privileged
/help Print Bert-E's manual in the pull request.
/status Print Bert-E's current status in the pull request.
/clear Remove all comments from Bert-E from the history TBA
/retry Re-start a fresh build TBA
/build Re-start a fresh build TBA
/force_reset Delete integration branches & pull requests, and restart merge process from the beginning.
/reset Try to remove integration branches unless there are commits on them which do not appear on the source branch.

Status report is not available.

@bert-e

bert-e commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Request integration branches

Waiting for integration branch creation to be requested by the user.

To request integration branches, please comment on this pull request with the following command:

/create_integration_branches

Alternatively, the /approve and /create_pull_requests commands will automatically
create the integration branches.

@BourgoisMickael

BourgoisMickael commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

/create_integration_branches

@bert-e

This comment was marked as resolved.

Sign in to view
BourgoisMickael and others added 9 commits October 2, 2026 15:23
@BourgoisMickael
ARSN-481: DataWrapper use preexisting KMS decipher
e0343c3 
KMS error generated in the data wrapper
will not be returned to client as connection
will be aborted

Cloudserver getObject needs to prepare KMS
calls to handle errors

(cherry picked from commit 471580b)

Edit from the original commit to drop the warning that got
removed later.
@BourgoisMickael
ARSN-652: Fix log method name
43accea 
From refacto 0d526df for ARSN-128

(cherry picked from commit 81aec71)
@BourgoisMickael
ARSN-652: Destroy the data stream on backend error
6a7ef5a 
- destroy the get stream on get with KMS error
- destroy the get stream on copy (for object and part)
  on both KMS or put error

(cherry picked from commit 0666d2e)
@BourgoisMickael
ARSN-652: Replace pipe with pipeline for cleanup
355e1a5 
`.pipe()` only forwards data, backpressure and end.
`pipeline` forwards errors and destroy streams for cleanup and prevent socket leak.

Otherwise some socket might stay open for a while until garbage collection
if any issue arise anywhere in the pipeline.

Note: for _put value stream and error downstream is not propagated
this is by design to let the V4Transform drain the incoming HTTP upload stream

(cherry picked from commit 668d8a4)
@nicolas2bert @BourgoisMickael
bugfix/ARSN-247 data.delete 404 errors not handled properly
5562b74 
(cherry picked from commit 28779db)
@BourgoisMickael
ARSN-652: Backport DataWrapper unit tests
729d40b 
Bring tests/unit/storage/data/DataWrapper.spec.js as present on
development/8.2+ (03dcf4f), adapted to 7.10:
- uploadPartCopy reads SSE from destBucketMD (no ARSN-485)
- copyObject only skips SSE on strict null (no ARSN-497)
- 7.10 eslint rules: no object spread, comma-dangle
@BourgoisMickael
ARSN-652: Test destroy on KMS failure
6844032 
(cherry picked from commit 3058ccd)
@BourgoisMickael
ARSN-652: Test pipeline error propagation
f949d32 
(cherry picked from commit 48a94e9)
@BourgoisMickael
ARSN-652: Bump version
931e3f1
@BourgoisMickael
BourgoisMickael force-pushed the improvement/ARSN-652-leak-kms-7.10 branch from 1b2d2c3 to 931e3f1 Compare October 2, 2026 13:24
@bert-e

This comment was marked as resolved.

Sign in to view
@bert-e

This comment was marked as resolved.

Sign in to view
@bert-e

This comment was marked as resolved.

Sign in to view
@bert-e

bert-e commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Integration data created

I have created the integration data for the additional destination branches.

The following branches will NOT be impacted:

  • development/6.4
  • development/7.4

You can set option create_pull_requests if you need me to create
integration pull requests in addition to integration branches, with:

@bert-e create_pull_requests

The following options are set: create_integration_branches

@BourgoisMickael

BourgoisMickael commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

@bert-e create_pull_requests

This was referenced Oct 2, 2026
Merged
Merged
Merged
Merged
Merged
Merged
Merged
@bert-e

bert-e commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Integration data created

I have created the integration data for the additional destination branches.

The following branches will NOT be impacted:

  • development/6.4
  • development/7.4

Follow integration pull requests if you would like to be notified of
build statuses by email.

The following options are set: create_pull_requests, create_integration_branches

@bert-e

bert-e commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Waiting for approval

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

  • the author

  • 2 peers

The following options are set: create_pull_requests, create_integration_branches

@scality scality deleted a comment from bert-e Oct 5, 2026
@BourgoisMickael
BourgoisMickael requested a review from a team October 5, 2026 14:48
@BourgoisMickael
BourgoisMickael requested review from a team, SylvainSenechal, anurag4DSB, leif-scality, maeldonn and nicolas2bert and removed request for a team October 5, 2026 14:48
BourgoisMickael added a commit to scality/cloudserver that referenced this pull request Oct 5, 2026
@BourgoisMickael
CLDSRV-629: Handle KMS before data to return error
b004c1b 
(cherry picked from commit 5184ec2)

Dropped monitoring, as it appeared only in 7.70 and not 7.10

This goes with scality/Arsenal#2720
For RD-2378
@BourgoisMickael BourgoisMickael mentioned this pull request Oct 5, 2026
Merged
jonathan-gramain
jonathan-gramain approved these changes Oct 5, 2026
View reviewed changes

@jonathan-gramain jonathan-gramain left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just curious, is there a reason why ARSN-652 is a task ticket, not a bugfix ticket (seems to fix real bugs, hence may be better as a bugfix ticket for tracking what's in a given version)?

@BourgoisMickael

BourgoisMickael commented Oct 6, 2026 •
edited by atlassian Bot
Loading

Copy link
Copy Markdown
Contributor Author

Just curious, is there a reason why ARSN-652 is a task ticket, not a bugfix ticket (seems to fix real bugs, hence may be better as a bugfix ticket for tracking what's in a given version)?

Right, it should have been a bug ticket. I'll change its type after merge (otherwise I'd need to reopen a new PR with a new branch name I think) for bert-e

@BourgoisMickael

BourgoisMickael commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

/approve

@bert-e

bert-e commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Build failed

The build for commit did not succeed in branch w/8.2/improvement/ARSN-652-leak-kms-7.10

The following options are set: approve, create_pull_requests, create_integration_branches

@bert-e

bert-e commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Build failed

The build for commit did not succeed in branch w/8.4/improvement/ARSN-652-leak-kms-7.10

The following options are set: approve, create_pull_requests, create_integration_branches

@bert-e

This comment was marked as resolved.

Sign in to view
@bert-e

This comment was marked as resolved.

Sign in to view
@scality scality deleted a comment from bert-e Oct 7, 2026
@bert-e

bert-e commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Build failed

The build for commit did not succeed in branch w/8.6/improvement/ARSN-652-leak-kms-7.10

The following options are set: approve, create_pull_requests, create_integration_branches

@bert-e

bert-e commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

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

  • ✔️ development/7.10

  • ✔️ development/7.70

  • ✔️ development/8.1

  • ✔️ development/8.2

  • ✔️ development/8.3

  • ✔️ development/8.4

  • ✔️ development/8.5

  • ✔️ development/8.6

The following branches have NOT changed:

  • development/6.4
  • development/7.4

This pull request did not target the following hotfix branch(es) so they
were left untouched:

  • hotfix/7.4.2
  • hotfix/7.4.3
  • hotfix/7.4.10
  • hotfix/7.10.46
  • hotfix/7.70.14
  • hotfix/7.7.0
  • hotfix/7.4.0
  • hotfix/6.4.7
  • hotfix/7.4.8
  • hotfix/7.8.0
  • hotfix/7.10.4
  • hotfix/7.10.31
  • hotfix/7.70.20
  • hotfix/7.4.7
  • hotfix/7.4.1
  • hotfix/7.9.0
  • hotfix/7.4.4
  • hotfix/7.70.25
  • hotfix/7.5.0
  • hotfix/7.10.57
  • hotfix/8.2.48
  • hotfix/7.70.4
  • hotfix/7.10.3
  • hotfix/7.4.9
  • hotfix/7.10.2
  • hotfix/7.10.47
  • hotfix/7.10.43
  • hotfix/7.10.0
  • hotfix/7.4.5
  • hotfix/7.6.0
  • hotfix/7.10.1
  • hotfix/7.4.6
  • hotfix/7.10.36
  • hotfix/7.2.0
  • hotfix/7.10.29

Please check the status of the associated issue ARSN-652.

Goodbye bourgoismickael.

The following options are set: approve, create_pull_requests, create_integration_branches

@bert-e
bert-e merged commit 931e3f1 into development/7.10 Oct 7, 2026
7 checks passed
@bert-e
bert-e deleted the improvement/ARSN-652-leak-kms-7.10 branch October 7, 2026 23:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@anurag4DSB anurag4DSB anurag4DSB approved these changes
@jonathan-gramain jonathan-gramain jonathan-gramain approved these changes
@nicolas2bert nicolas2bert Awaiting requested review from nicolas2bert nicolas2bert was automatically assigned from scality/object
@leif-scality leif-scality Awaiting requested review from leif-scality leif-scality was automatically assigned from scality/object
@SylvainSenechal SylvainSenechal Awaiting requested review from SylvainSenechal SylvainSenechal was automatically assigned from scality/raving-robots
@maeldonn maeldonn Awaiting requested review from maeldonn maeldonn was automatically assigned from scality/raving-robots

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@BourgoisMickael @bert-e @anurag4DSB @jonathan-gramain @nicolas2bert