Skip to content

test(e2e): wait for the S3 gateway to accept a key before using it - #4694

Merged
IvanHunters merged 1 commit into
mainfrom
fix/e2e-bucket-write-readiness
Oct 5, 2026
Merged

IvanHunters merged 1 commit into
mainfrom
fix/e2e-bucket-write-readiness

Conversation

@lexfrei

@lexfrei Aleksei Sviridkin (lexfrei) commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

What this PR does

The bucket e2e suite fails now and then because the S3 gateway rejects a key that COSI already reports as granted. The error is The access key ID you provided does not exist in our records. from mc alias set. In job 109130131391 the BucketAccess assert finished at 21:23:43.57 and the first mc alias set failed at 21:23:44.82. In job 109396174094 cap1 and cap2 were accepted, and cap3 was rejected two seconds into the step.

Granted access is not the same as a usable key. In seaweedfs-cosi-driver v0.3.1, DriverGrantBucketAccess writes the new key into /etc/iam/identity.json on the filer and returns. The S3 gateway in SeaweedFS 4.45 reloads that file later, from a filer metadata subscription (weed/s3api/auth_credentials_subscribe.go). In our chart the driver writes through seaweedfs-filer and the two gateways follow seaweedfs-filer-client, so the change may have to pass through the other filer replica first.

The suite now waits until the gateway accepts the key before it writes. Every alias in both write steps goes through await_key, which uses the same port-forward as the writes and so asks the same gateway pod. It waits only while the answer is the InvalidAccessKeyId text, for at most 60s. Any other error fails at once, and so does an attempt cut off at its 15s cap. When the window runs out, the step fails with a message that names the BucketAccess.

One probe costs at most 77s, so both steps go from a 5m to an 8m op timeout. The inner budgets now sum to 409s and 441s, and the budget comments say so.

I checked the probe locally against a fake mc:

case=ok rc=0 attempts=1 elapsed=0s
  Added `a` successfully.
case=lag rc=0 attempts=3 elapsed=5s
  Added `a` successfully.
case=other rc=1 attempts=1 elapsed=0s
  FAIL: mc alias set for BucketAccess bucket-test-admin failed (exit 1) with an error other than the key not being loaded yet
case=never rc=1 attempts=31 elapsed=60s
  FAIL: S3 gateway still rejects the key of BucketAccess bucket-test-admin after 60s of probing (the gateway never loaded it, or the driver never wrote it)

A hanging mc fails on its first attempt after 15s with exit 124. chainsaw lint test from v0.2.15 accepts the file.

The read-only upload check gets fixed along the way. Before, an unloaded ro-user key also made the upload fail, and that failure passed as the expected denial.

One thing I left as it is: await_key is copied into both steps, because chainsaw steps share no shell, and moving it to _lib/ would run every suite on each edit.

Closes #4580

Screenshots

Not applicable, no UI change.

Downstream repositories

Release note

test(e2e): the bucket suite waits for the S3 gateway to accept a new key before writing with it, instead of trusting BucketAccess readiness.

@github-actions github-actions Bot added area/testing Issues or PRs related to testing (e2e, bats, unit tests) size/M This PR changes 30-99 lines, ignoring generated files labels Oct 2, 2026
@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: cozystack/cozystack/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 588ec4a2-2b78-4536-bda9-844ed19306fa

📥 Commits

Reviewing files that changed from the base of the PR and between bc400df and b306abd.

📒 Files selected for processing (1)
  • hack/e2e-chainsaw/bucket/chainsaw-test.yaml

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 4 remain after this review.


📝 Walkthrough

Walkthrough

Both bucket Chainsaw scripts retry S3 alias setup when the gateway reports that an access key does not exist. The scripts increase their step timeouts to eight minutes and update their command-budget calculations.

Changes

Bucket S3 key readiness

Layer / File(s) Summary
S3 access verification retry
hack/e2e-chainsaw/bucket/chainsaw-test.yaml
The access verification script retries admin and viewer alias setup for up to 60 seconds when the gateway reports an unknown key. Other errors and 15-second command timeouts fail immediately. The step timeout increases to eight minutes, and the command budget is updated.
Capacity probe retry
hack/e2e-chainsaw/bucket/chainsaw-test.yaml
The capacity script applies the bounded key probe to each bucket. Its step timeout increases to eight minutes, and its per-bucket and total command budgets are updated.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to b306a

The bucket checks can wait for delayed gateway key acceptance without an identified merge-blocking issue. Normal checks remain appropriate.

Architecture Summary

Architecture risk: 🔵 Low · up to b306a

The change affects 1 system.

Changed systems: hack

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — hack (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in hack/e2e-chainsaw/bucket/chainsaw-test.yaml: The S3 access verification script timeout increases from 5 to 8 minutes.
  • observed — Modified behavior in hack/e2e-chainsaw/bucket/chainsaw-test.yaml: The documented command-budget calculation changes from a maximum of 285 seconds under the 5-minute step timeout to 409 seconds under the 8-minute timeout. It accounts for two 30-second reads, two 45-second PUTs, two probes of up to 77 seconds, up to five 15-second other calls, and the 30-second port-forward wait.
  • observed — Modified behavior in hack/e2e-chainsaw/bucket/chainsaw-test.yaml: The two direct alias setup calls are replaced with await_key calls for the admin and viewer credentials. The helper retries only the specific not-yet-loaded-key error, for up to 60 seconds with 2-second sleeps; each attempt is capped at 15 seconds. It succeeds on alias setup, but fails immediately on timeout or other errors, and fails with the relevant BucketAccess name if the key remains unknown at the deadline.
  • observed — Modified behavior in hack/e2e-chainsaw/bucket/chainsaw-test.yaml: The capacity verification script timeout increases from 5 to 8 minutes.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Issue #4580 requires the bucket Chainsaw suite to wait for gateway acceptance before writes in both affected steps. The PR adds await_key before the read/write verification operations and before eac…
Out of Scope Changes check ✅ Passed The diff is limited to hack/e2e-chainsaw/bucket/chainsaw-test.yaml. The timeout updates, probe function, failure diagnostics, and budget comments support the linked issue's bounded gateway-readiness…
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the primary change: the e2e test now waits for the S3 gateway to accept a key before using it.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

BucketAccess turns accessGranted as soon as the COSI driver has written
the new key into the filer's IAM file. The S3 gateway loads that file
later, from a filer metadata subscription, and with two filer replicas
the change can reach the filer a gateway follows only after the other
replica passes it on. Both bucket suites call mc alias set right after
the assert, so a key the gateway has not loaded yet is rejected with
InvalidAccessKeyId and the suite goes red.

Probe the key through the same port-forward the writes use. Only the
InvalidAccessKeyId text is waited on, for at most 60s; any other error
or a timed-out attempt fails at once, and the window expiring fails
with a message naming the BucketAccess. The two steps get an 8m op
ceiling so the inner budgets still sit under it.

Signed-off-by: Aleksei Sviridkin <[email protected]>
Assisted-by: LLM

@IvanHunters IvanHunters left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

Fixes the #4580 race at the root. It waits for the S3 gateway to accept the key, bounded, rather than sleeping a fixed amount, and without needing the driver-side change the test can't make.

What I checked:

  • The wait gates on the key actually being accepted. await_key returns success only when mc alias set exits 0, so there's no other path to green and no false-green branch. mc alias set really does contact the gateway (the two CI logs in the issue show it returning the does not exist in our records error), so a success means this gateway took the key.
  • The ceiling is bounded: a 60s deadline off date +%s, every attempt under timeout 15, sleep 2 between tries, ~77s worst case per call. Both step budgets recompute correctly and sit under the 8m ceiling, 409s for verify-s3-access and 441s for verify-every-bucket-takes-a-write.
  • It reddens on a real failure. await_key runs as a plain command under set -eu, so the deadline return 1 fails the script, and chainsaw's script op fails on a non-zero exit by default (step 2 writes it out as ($error == null): true, step 1 leans on the default). Pull the wait and the test drops back to the intermittent red this PR is fixing.
  • No inert negated assertions in here: it's a chainsaw inline /bin/sh, not a .bats under the cozytest runner. The single ! grep -q sits in an if that leads to return 1, and an empty alias.log degrades to a fast fail, which is the safe direction.

Non-blocking:

  • The retryable-error match keys on SeaweedFS's text does not exist in our records. If that wording ever changes, await_key degrades to a fast fail and the flake comes back, but never to a false green. mc gives the shell no structured error code, so the text is the only signal there is; a deliberate, narrow choice.
  • await_key is copied verbatim into both script steps. The two copies are byte-identical today, and a single chainsaw file has no way to share a shell function across two isolated script ops, so there's no avoiding it here. The only risk is future drift.
  • Cosmetic: the budget comment around the "= at most 409s. mc and kubectl both exit on SIGTERM" line has an awkward mid-sentence wrap left over from an edit.

@IvanHunters
IvanHunters enabled auto-merge October 5, 2026 11:06
@IvanHunters
IvanHunters merged commit 1ca603a into main Oct 5, 2026
20 checks passed
@IvanHunters
IvanHunters deleted the fix/e2e-bucket-write-readiness branch October 5, 2026 11:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/testing Issues or PRs related to testing (e2e, bats, unit tests) size/M This PR changes 30-99 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(e2e): bucket suite writes before SeaweedFS accepts a key that BucketAccess already reports Ready

2 participants