Skip to content

Support Days=0 lifecycle expiration: bump arsenal and add functional tests#6192

Open
delthas wants to merge 3 commits into
development/9.3from
improvement/CLDSRV-928/days-0-lifecycle-expiration
Open

Support Days=0 lifecycle expiration: bump arsenal and add functional tests#6192
delthas wants to merge 3 commits into
development/9.3from
improvement/CLDSRV-928/days-0-lifecycle-expiration

Conversation

@delthas

@delthas delthas commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

What

CloudServer side of the Days = 0 lifecycle support. Two commits:

  1. Functional tests (tests/functional/aws-node-sdk/test/bucket/putBucketLifecycle.js):
    • Expiration Days: 0 accepted and round-trips via GetBucketLifecycle
    • NoncurrentVersionExpiration NoncurrentDays: 0 and AbortIncompleteMultipartUpload DaysAfterInitiation: 0 accepted
    • negative Expiration Days rejected (must be nonnegative) and > MAX_DAYS rejected (MalformedXML)
  2. arsenal bump to pick up the validation/round-trip/consumer changes.

Why

Days = 0 is an explicit "empty this bucket" lifecycle signal. Because it was rejected everywhere until now, it is unambiguous (unlike Days = 1, used by legitimate short-retention buckets) and lets backbeat (BB-779) skip building costly lifecycle indexes for buckets being drained. CloudServer delegates all lifecycle validation to arsenal, so the only changes here are the dependency bump and the tests.

⚠️ Temporary arsenal pin — do not merge as-is

The arsenal dependency is pinned to the ARSN-597 commit f5d43abeb8139ec8835ef0354888ff13ddb14d8b (arsenal PR scality/Arsenal#2648), not a released tag, because the version is not cut yet. Before merge: amend the bump to the released arsenal version (>= 8.4.8) and re-run yarn install. Draft until then.

Related

Issue: CLDSRV-928

@bert-e

bert-e commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Hello delthas,

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
/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 TBA
/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 Jun 18, 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.

Comment thread package.json Outdated
"@hapi/joi": "^17.1.1",
"@smithy/node-http-handler": "^3.0.0",
"arsenal": "git+https://github.com/scality/Arsenal#8.4.4",
"arsenal": "git+https://github.com/scality/Arsenal#f5d43abeb8139ec8835ef0354888ff13ddb14d8b",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Arsenal is pinned to a commit hash, not a released tag. The PR description acknowledges this, but flagging per policy: git-based deps must pin to a tag before merge.

— Claude Code

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks Sherlock

Comment thread tests/functional/aws-node-sdk/test/bucket/putBucketLifecycle.js
@codecov

codecov Bot commented Jun 18, 2026

Copy link
Copy Markdown

❌ 6 Tests Failed:

Tests completed Failed Passed Skipped
3216 6 3210 0
View the top 1 failed test(s) by shortest run time
"before each" hook for "should get a range from the second part of an object and include the end if the range requested goes beyond the actual object end"::aws-node-sdk range tests aws-node-sdk range test for object put by MPU With v4 signature "before each" hook for "should get a range from the second part of an object and include the end if the range requested goes beyond the actual object end"
Stack Traces | 5.14s run time
Error uploading part 1: Connection timed out after 5000 ms
View the full list of 5 ❄️ flaky test(s)
"after each" hook for "should fail if trying to overwrite a delete marker"::MPU with x-scal-s3-version-id header With default signature "after each" hook for "should fail if trying to overwrite a delete marker"

Flake rate in main: 100.00% (Passed 0 times, Failed 66 times)

Stack Traces | 0.015s run time
We encountered an internal error. Please try again.
"after each" hook for "should fail if trying to overwrite a delete marker"::MPU with x-scal-s3-version-id header With v4 signature "after each" hook for "should fail if trying to overwrite a delete marker"

Flake rate in main: 100.00% (Passed 0 times, Failed 53 times)

Stack Traces | 0.015s run time
We encountered an internal error. Please try again.
should not accept configuration when 'Destination' properties of two or more rules specify different buckets::aws-node-sdk test putBucketReplication configuration rules should not accept configuration when 'Destination' properties of two or more rules specify different buckets

Flake rate in main: 13.81% (Passed 668 times, Failed 107 times)

Stack Traces | 0.006s run time
incorrect error response code: should be 'InvalidRequest' but got 'Error'
+ actual - expected

+ 'Error'
- 'InvalidRequest'
should not accept configuration when rules contain overlapping 'Prefix' values: new prefix starts with used prefix::aws-node-sdk test putBucketReplication configuration rules should not accept configuration when rules contain overlapping 'Prefix' values: new prefix starts with used prefix

Flake rate in main: 13.81% (Passed 668 times, Failed 107 times)

Stack Traces | 0.005s run time
incorrect error response code: should be 'InvalidRequest' but got 'Error'
+ actual - expected

+ 'Error'
- 'InvalidRequest'
should not accept configuration when rules contain overlapping 'Prefix' values: used prefix starts with new prefix::aws-node-sdk test putBucketReplication configuration rules should not accept configuration when rules contain overlapping 'Prefix' values: used prefix starts with new prefix

Flake rate in main: 13.81% (Passed 668 times, Failed 107 times)

Stack Traces | 0.005s run time
incorrect error response code: should be 'InvalidRequest' but got 'Error'
+ actual - expected

+ 'Error'
- 'InvalidRequest'

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@delthas delthas force-pushed the improvement/CLDSRV-928/days-0-lifecycle-expiration branch from f6b1245 to a934286 Compare June 18, 2026 11:44
Comment thread package.json
@scality scality deleted a comment from claude Bot Jun 18, 2026
@delthas delthas marked this pull request as ready for review June 18, 2026 11:49
delthas added 3 commits June 18, 2026 13:50
Pure formatting pass (no logic change) on the functional test file touched by
the Days=0 work, so the prettier CI check (which runs on whole files modified
by the PR) passes. Isolating the reformat keeps the test commit reviewable.

Issue: CLDSRV-928
Cover the new Days=0 ("empty this bucket") capability in PutBucketLifecycle:
- Expiration Days=0 accepted and round-trips via GetBucketLifecycle
- NoncurrentVersionExpiration NoncurrentDays=0 accepted
- AbortIncompleteMultipartUpload DaysAfterInitiation=0 accepted
- negative Expiration Days rejected (must be nonnegative)
- Expiration Days exceeding MAX_DAYS rejected (MalformedXML)

These pass once the arsenal dependency is bumped to the version including
ARSN-597.

Issue: CLDSRV-928
Point arsenal at the ARSN-597 commit
(72e5d59ea4ca10e060e55dccb17063f98d1a86a1) rather than a released tag, since
the version is not cut yet. This delivers Days=0 support (the explicit
"empty this bucket" lifecycle signal) and makes the functional tests added in
the previous commit pass.

To amend once ARSN-597 is released: replace the commit pin with the released
arsenal version (>= 8.4.8) and re-run yarn install.

Issue: CLDSRV-928
@delthas delthas force-pushed the improvement/CLDSRV-928/days-0-lifecycle-expiration branch from a934286 to c7099f8 Compare June 18, 2026 11:51
Comment thread package.json
@claude

claude Bot commented Jun 18, 2026

Copy link
Copy Markdown
  • Arsenal pinned to commit hash instead of a release tag (acknowledged in PR description as a merge blocker). The hash in the code (72e5d59ea4ca) doesn't match the one referenced in the PR description (f5d43abeb813) — update the description when switching to the released tag.

    Tests look good: proper coverage for Days=0 acceptance and round-trip, negative Days rejection, and MAX_DAYS boundary. No .only() tests, imports at top of file, assertions match tested behavior.

    Review by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants