Fixes #6818: Harden storage Compose scripts - #7151
BobSong-dev wants to merge 5 commits into
Conversation
| ## run e2e-test | ||
| sleep 60s | ||
|
|
||
| ./mvnw -B -f ./shenyu-e2e/pom.xml -pl shenyu-e2e-case/shenyu-e2e-case-storage -am test |
There was a problem hiding this comment.
Blocking: with set -euo pipefail, this line aborts the script on failure, so the log dumps below (lines 36-41) become unreachable.
Those two blocks are the only reason they exist - this PR's own CI run e2e-storage (shenyu-e2e-case-storage, e2e-postgres-compose) aborted here and no admin/bootstrap logs were printed to the job output. Previously the script carried on and still emitted them.
Suggestion: keep fail-fast behaviour but always dump the logs first, e.g.
if ! ./mvnw -B -f ./shenyu-e2e/pom.xml -pl shenyu-e2e-case/shenyu-e2e-case-storage -am test; then
docker compose -f "$COMPOSE_FILE" logs shenyu-admin || true
docker compose -f "$COMPOSE_FILE" logs shenyu-bootstrap || true
exit 1
fi
Aias00
left a comment
There was a problem hiding this comment.
The direction is right. I checked the four compose files (shenyu-e2e-case/compose/storage/shenyu-storage-{h2,mysql,opengauss,postgres}.yml): all four declare healthcheck for shenyu-admin (9095) and shenyu-bootstrap (9195) with start_period: 30s, plus a healthcheck on the database service, so docker compose up --wait is a legitimate replacement for the fixed sleep 30s + sleep 60s + healthcheck.sh dance. Adding set -euo pipefail, an EXIT trap and network create ... || true are all net improvements. Two things need fixing before merge.
- [blocking]
set -euo pipefailmakes the log dumps unreachable
All five scripts now run the test with -e active, and the two docker compose logs blocks afterwards are the entire diagnostics value of these scripts. When the test fails the shell exits immediately and nothing is printed - exactly what happened in this PR's own CI: e2e-storage (shenyu-e2e-case-storage, e2e-postgres-compose) aborted and the job output contains no admin/bootstrap logs. Before this change the script continued and always emitted them.
Please keep fail-fast but guarantee the logs, e.g.
if ! ./mvnw -B -f ./shenyu-e2e/pom.xml -pl shenyu-e2e-case/shenyu-e2e-case-storage -am test; then
docker compose -f "$COMPOSE_FILE" logs shenyu-admin || true
docker compose -f "$COMPOSE_FILE" logs shenyu-bootstrap || true
exit 1
fi
(in e2e-storage-compose.sh that goes inside the loop; the trailing docker compose down there already resets COMPOSE_FILE.)
- [blocking] bound the wait and re-run CI
docker compose up --wait has no upper timeout by default: if a service never reaches a terminal health state (starting that never resolves, or a container repeatedly restarting) the job blocks until the GitHub Actions limit instead of failing, which again loses the logs. Please add an explicit bound, e.g. --wait --wait-timeout 300.
On CI: all four e2e-storage jobs are red on this PR while the identical jobs are green on #7146 and #7153. Only e2e-postgres-compose produced a real error -
error mounting "/tmp/shenyu-e2e/postgres/schema/create-table.sql" to rootfs at
"/docker-entrypoint-initdb.d": ... not a directory: Are you trying to mount a directory
onto a file (or vice-versa)?
the other three were cancelled by fail-fast. That failure comes from k8s/script/storage/storage_init_postgres.sh (cp db/init/pg/create-table.sql ...), which this PR does not touch, so I am not attributing it to this diff. But since the postgres job died before ever reaching the --wait gate, the new gating logic is currently unverified on any of the four cases - please re-run and confirm all four are green before merge.
Non-blocking nits
k8s/script/healthcheck.shis no longer invoked by the storage compose scripts (it is still used by the other modules, so keep it). Note it also looped overservices-*.list, which nothing exercises for these cases any more.PRGDIR=$(dirname "$curPath")is computed in every script and unused (pre-existing).- Fine detail:
trap 'docker compose -f "$COMPOSE_FILE" down || true' EXITin the four single-storage scripts means the stack is torn down even on success, before the explicit log dumps would run if the test passes - but the logs on success are informational anyway, so this is acceptable; just be aware the trap fires before those lines only on theset -eabort paths.
Fix items 1 and 2 and I will approve.
…mpose-reliability
Fixes #6818
Background
The storage Compose scripts ignored failures, left containers running after failures, and relied on fixed 30-second and 60-second sleeps. This could let storage initialization or service startup fail silently and made the E2E timing dependent on machine speed. The Compose files already define service healthchecks, but the scripts did not wait on them directly.
Changes
Verification