Skip to content

fix(smoke): delete S3 Files mount targets first; tolerate per-item failures#337

Closed
chrisns wants to merge 1 commit into
mainfrom
fix/smoke-s3files-mount-targets
Closed

fix(smoke): delete S3 Files mount targets first; tolerate per-item failures#337
chrisns wants to merge 1 commit into
mainfrom
fix/smoke-s3files-mount-targets

Conversation

@chrisns
Copy link
Copy Markdown
Member

@chrisns chrisns commented May 20, 2026

Summary

PR 321 retry surfaced two more pre-deploy bugs:

  1. `aws s3files delete-file-system` fails with `ConflictException: The file system 'fs-...' cannot be deleted because it has mount targets`. Need to enumerate + delete mount targets first.
  2. `delete_bucket_completely` returns 1 when all attempts fail (it opens a stranded-stack issue and gives up). The bare call inside the sweep loop is NOT a conditional context, so `set -e` fires and the whole pre-deploy script exits — even when only one bucket couldn't be deleted.

Fix

  • Before `delete-file-system`, list mount targets and `delete-mount-target` each; sleep 30s for the async deletes to complete.
  • Add `|| true` to `delete_bucket_completely` and `cleanup_orphan` calls inside their respective sweep loops so per-item failures don't kill the whole sweep. Stranded-stack issues remain the audit trail.
  • Capture file-system-delete stderr to a variable + echo, matching the pattern used elsewhere — avoids the `cmd | sed` pipefail trap.

Test plan

  • `bash -n` clean
  • PR 321 (or any post-merge smoke run from a torn-down state) reaches success

…ilures

PR 321 retry uncovered two more issues:

1. delete-file-system fails with ConflictException ("has mount targets")
   on file systems that still have mount targets attached. Add a
   list-mount-targets + delete-mount-target loop before delete-file-system,
   plus a 30s grace for the async deletes to complete.

2. delete_bucket_completely returns 1 when all attempts fail (it opens a
   stranded-stack issue and gives up). The bare function call inside the
   bucket sweep loop is NOT a conditional context, so set -e fires and
   kills the whole pre-deploy script. Adding `|| true` to both the
   bucket-sweep and orphan-stack-sweep loops keeps them tolerant of
   per-item failures; the stranded-stack issues remain the audit trail.

Also captured stderr explicitly (var + echo) for the file-system delete,
matching the pattern used elsewhere — avoids the `cmd | sed` pipefail
trap that killed earlier iterations.
@chrisns chrisns had a problem deploying to smoke-test-deploy May 20, 2026 10:02 — with GitHub Actions Failure
@chrisns
Copy link
Copy Markdown
Member Author

chrisns commented May 20, 2026

Superseded — fix commit cherry-picked into PR #321 (renovate/node-26.x) directly so it lands together with the node bump.

@chrisns chrisns closed this May 20, 2026
@chrisns chrisns deleted the fix/smoke-s3files-mount-targets branch May 20, 2026 10:10
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.

1 participant