From 84519cf7ccb9f82201f37730ca100dd4b977fca5 Mon Sep 17 00:00:00 2001 From: Maxim Shulga Date: Sun, 2 Aug 2026 19:37:26 -0700 Subject: [PATCH 1/2] Run a container smoke test against the real seafile daemon Startup, shutdown and secret handling were only ever checked by hand, and the unit tests cannot check them: they stub seaf-cli, so nothing exercised the daemon that actually ships. The GitHub runner is x86_64, which matches the AppImage, so the image build job can run the real thing. tests/smoke.sh starts and stops the real daemon, sends SIGTERM through docker stop and asserts the container exits 0 within the grace period with the daemon shut down, checks that a configuration error exits 2 naming the variable, that a secret mounted under /run/secrets is read and stays out of the environment, and that a default container gets exactly the documented defaults. The pinned client version is exposed as SEAFILE_CLI_VERSION in the image, since seaf-cli has no --version flag and there is no package manager entry for an AppImage. --- .github/workflows/docker-publish.yml | 5 + .gitignore | 1 + Dockerfile | 4 + IMPROVEMENTS.md | 16 ++- tests/smoke.sh | 151 +++++++++++++++++++++++++++ 5 files changed, 172 insertions(+), 5 deletions(-) create mode 100755 tests/smoke.sh diff --git a/.github/workflows/docker-publish.yml b/.github/workflows/docker-publish.yml index b62c574..80737f5 100644 --- a/.github/workflows/docker-publish.yml +++ b/.github/workflows/docker-publish.yml @@ -52,6 +52,11 @@ jobs: load: true tags: ${{ env.IMAGE }}:ci + # Exercises the real seaf-cli and seaf-daemon in the built image, which + # the unit tests cannot do. The runner is x86_64, matching the AppImage. + - name: Container smoke test + run: tests/smoke.sh ${{ env.IMAGE }}:ci + push: needs: [unit-test, test] diff --git a/.gitignore b/.gitignore index 81799a8..605ef28 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,4 @@ __pycache__/ .python-version docker-compose.yml +.smoke-*/ diff --git a/Dockerfile b/Dockerfile index 3646f90..5943cb8 100644 --- a/Dockerfile +++ b/Dockerfile @@ -72,5 +72,9 @@ RUN seaf-cli --help | grep -q 'download-by-name' && \ seaf-cli list --help | grep -q '\--json' && \ python3 -c "import seafile, pysearpc, dsc.client, dsc.misc" +# Record the pinned client version in the image so it can be checked at +# runtime; seaf-cli itself has no --version flag. +ENV SEAFILE_CLI_VERSION=${SEAFILE_CLI_VERSION} + VOLUME /dsc/seafile-data CMD ["./start.py"] diff --git a/IMPROVEMENTS.md b/IMPROVEMENTS.md index 948e5de..b8b94c8 100644 --- a/IMPROVEMENTS.md +++ b/IMPROVEMENTS.md @@ -134,10 +134,12 @@ Acceptance criteria: parsing, retries, and status parsing are not. - [ ] Add regression tests for externally reported password and deletion threshold issues. -- [ ] Add a container smoke test for imports, installed CLI version, startup, - and shutdown using controlled fakes. Graceful shutdown was verified by hand - in the built image (`docker stop` returns immediately with exit code 0 - instead of being killed after 10 seconds); it is not yet automated in CI. +- [x] Add a container smoke test for imports, installed CLI version, startup, + and shutdown using controlled fakes. See `tests/smoke.sh`, run by the image + build job. It exercises the real `seaf-cli` and `seaf-daemon`, which the unit + tests cannot: the runner is x86_64, matching the AppImage. The pinned client + version is exposed as `SEAFILE_CLI_VERSION` in the image so it can be + asserted at runtime. - [x] Keep image construction as a separate CI job so packaging failures are distinguishable from application failures. The `unit-test` job does not depend on the image build. @@ -160,7 +162,11 @@ Acceptance criteria: ### Networking and protocol behavior - [ ] Add explicit connect and read timeouts to all HTTP requests. -- [ ] Define a bounded retry budget with backoff and jitter. +- [ ] Define a bounded retry budget with backoff and jitter. Measured: the + current `Retry(total=30, backoff_factor=2, backoff_max=60)` with no request + timeout waits about 26 minutes on a single unreachable server before giving + up, which is indistinguishable from a hang. Observed while writing the smoke + test, when a probe pointed at an unroutable host. - [ ] Handle authentication, rate limits, malformed responses, and retryable server errors separately. - [ ] Make TLS configuration consistent between Python requests and Seafile. diff --git a/tests/smoke.sh b/tests/smoke.sh new file mode 100755 index 0000000..a36d9ab --- /dev/null +++ b/tests/smoke.sh @@ -0,0 +1,151 @@ +#!/usr/bin/env bash +# +# Container smoke test: exercises the built image, not the source tree. +# +# Everything here needs the real seaf-cli and seaf-daemon from the image, so +# it cannot run in the unit test job. Usage: +# +# tests/smoke.sh [image] # native architecture +# PLATFORM=linux/amd64 tests/smoke.sh # the AppImage is x86_64 only +# +set -uo pipefail + +IMAGE="${1:-snegov/seafile-client:ci}" +PLATFORM_ARG="" +[ -n "${PLATFORM:-}" ] && PLATFORM_ARG="--platform ${PLATFORM}" + +WORK=$(mktemp -d "${PWD}/.smoke-XXXXXX") +trap 'rm -rf "$WORK"; docker rm -f dsc-smoke >/dev/null 2>&1 || true' EXIT + +failures=0 + +pass() { printf ' ok %s\n' "$1"; } +fail() { printf ' FAIL %s\n' "$1"; failures=$((failures + 1)); } + +check() { # check + if [ "$2" = "$3" ]; then pass "$1"; else + fail "$1"; printf ' expected %s, got %s\n' "$2" "$3" + fi +} + +run() { docker run --rm $PLATFORM_ARG "$@"; } + +echo "== imports and pinned client version" +out=$(run "$IMAGE" python3 -c " +import os, seafile, pysearpc, dsc.client, dsc.misc, dsc.paths, dsc.secrets, dsc.config +print(os.environ.get('SEAFILE_CLI_VERSION', 'unset'))" 2>&1 | tail -1) +if [ -n "$out" ] && [ "$out" != "unset" ]; then + pass "imports resolve, seafile client version is $out" +else + fail "imports or SEAFILE_CLI_VERSION missing: $out" +fi + +echo "== the real daemon starts, reports ready, and stops" +out=$(run "$IMAGE" python3 -c " +from dsc.client import SeafileClient +c = SeafileClient('example.invalid', 'u', 'pw', '/dsc', token='tok') +c.init_config() +c.start_daemon() +ready = c.daemon_ready +c.stop_daemon() +print('READY', ready, 'STOPPED', not c.daemon_ready)" 2>&1 | tail -1) +check "daemon lifecycle" "READY True STOPPED True" "$out" + +echo "== docker stop shuts the daemon down gracefully" +docker rm -f dsc-smoke >/dev/null 2>&1 || true +docker run -d $PLATFORM_ARG --name dsc-smoke "$IMAGE" python3 -c " +import argparse, logging, time +import start +logging.basicConfig(format='%(asctime)s %(message)s', level=logging.INFO) +from dsc.client import SeafileClient + +def watch(): + print('WATCHING', flush=True) + while True: + time.sleep(0.2) + +c = SeafileClient('example.invalid', 'u', 'pw', '/dsc', token='tok') +# The daemon is real; the server is not. Nothing here may reach the network, +# so library lookup is stubbed and only the lifecycle is exercised. +c.watch_status = watch +c.get_library_id = lambda name: None +c.get_local_libraries = lambda: set() + +start.install_signal_handlers() +raise SystemExit( + start.run(c, argparse.Namespace(libraries='none', server='s'), '/dsc/seafile') +) +" >/dev/null + +# Wait for the daemon to be up and the process to be in its watch loop. +for _ in $(seq 1 60); do + docker logs dsc-smoke 2>&1 | grep -q WATCHING && break + sleep 1 +done + +started=$(date +%s) +docker stop dsc-smoke >/dev/null +elapsed=$(( $(date +%s) - started )) +code=$(docker inspect -f '{{.State.ExitCode}}' dsc-smoke) + +check "graceful shutdown exit code" "0" "$code" +if [ "$elapsed" -lt 10 ]; then + pass "docker stop returned in ${elapsed}s, inside the 10s grace period" +else + fail "docker stop took ${elapsed}s, the process was killed" +fi +if docker logs dsc-smoke 2>&1 | grep -q "Seafile daemon is stopped"; then + pass "the daemon was stopped on the way out" +else + fail "the daemon was not stopped on the way out" +fi +docker rm -f dsc-smoke >/dev/null + +echo "== configuration errors stop the container with a useful message" +out=$(docker run --rm $PLATFORM_ARG -e SERVER_HOST=h -e USERNAME=u -e LIBRARY_ID=x "$IMAGE" 2>&1; echo "rc=$?") +check "no credentials exits 2" "rc=2" "$(echo "$out" | tail -1)" + +out=$(docker run --rm $PLATFORM_ARG -e SERVER_HOST=h -e USERNAME=u -e LIBRARY_ID=x \ + -e PASSWORD=p -e PASSWORD_FILE=/run/secrets/pw "$IMAGE" 2>&1) +if echo "$out" | grep -q "PASSWORD_FILE are both set"; then + pass "conflicting secret sources are rejected by name" +else + fail "conflicting secret sources not reported: $(echo "$out" | tail -1)" +fi + +out=$(docker run --rm $PLATFORM_ARG -e SERVER_HOST=h -e USERNAME=u -e PASSWORD=p \ + -e LIBRARY_ID=x -e UPLOAD_LIMIT=-5 "$IMAGE" 2>&1) +if echo "$out" | grep -q "UPLOAD_LIMIT must be 0 or greater"; then + pass "a negative limit is rejected by name" +else + fail "negative limit not reported: $(echo "$out" | tail -1)" +fi + +echo "== a mounted secret is read and stays out of the environment" +printf 's3cret-from-file\n' > "$WORK/pw" +out=$(docker run --rm $PLATFORM_ARG -v "$WORK/pw:/run/secrets/pw:ro" \ + -e PASSWORD_FILE=/run/secrets/pw "$IMAGE" python3 -c " +import os +from dsc.secrets import resolve_secret +secret = resolve_secret('PASSWORD', os.getenv('PASSWORD'), os.getenv('PASSWORD_FILE')) +print(secret, 'PASSWORD' in os.environ)" 2>&1 | tail -1) +check "secret file read, password absent from the environment" \ + "s3cret-from-file False" "$out" + +echo "== a default container gets the documented defaults" +out=$(run "$IMAGE" python3 -c " +import start +from dsc.misc import config_value +d = start.defaults_from_env() +print(*(config_value(d[k]) for k in ( + 'upload_limit', 'download_limit', 'delete_confirm_threshold', + 'uid', 'gid', 'disable_verify_certificate')))" 2>&1 | tail -1) +check "documented defaults" "0 0 500 1000 1000 false" "$out" + +echo +if [ "$failures" -eq 0 ]; then + echo "smoke test passed" +else + echo "smoke test failed: $failures check(s)" +fi +exit $((failures > 0)) From f535e8b7d0a6b33d13c7e4b7ac650275c674081d Mon Sep 17 00:00:00 2001 From: Maxim Shulga Date: Sun, 2 Aug 2026 19:46:27 -0700 Subject: [PATCH 2/2] Stop the smoke test from passing when it measured nothing Two checks could report success without having verified anything. The version check read the last line of output without looking at the exit status, so an import failure printed a traceback whose last line was accepted as a version. The shutdown check waited for the process to reach its watch loop but carried on regardless, so if the container had already exited it measured the shutdown of something that was not running. The version check now requires a zero exit and a version shaped like one. The shutdown check fails and prints the container log when the watch loop is never reached. Both were confirmed against an image with a deliberately removed module: each reports a failure where it previously reported success. --- tests/smoke.sh | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/tests/smoke.sh b/tests/smoke.sh index a36d9ab..5918cc9 100755 --- a/tests/smoke.sh +++ b/tests/smoke.sh @@ -31,13 +31,18 @@ check() { # check run() { docker run --rm $PLATFORM_ARG "$@"; } echo "== imports and pinned client version" -out=$(run "$IMAGE" python3 -c " +# The exit status matters as much as the output: a traceback also prints a +# non-empty last line, and must not be mistaken for a version. +if out=$(run "$IMAGE" python3 -c " import os, seafile, pysearpc, dsc.client, dsc.misc, dsc.paths, dsc.secrets, dsc.config -print(os.environ.get('SEAFILE_CLI_VERSION', 'unset'))" 2>&1 | tail -1) -if [ -n "$out" ] && [ "$out" != "unset" ]; then - pass "imports resolve, seafile client version is $out" +print('VERSION', os.environ['SEAFILE_CLI_VERSION'])" 2>&1); then + version=$(printf '%s\n' "$out" | sed -n 's/^VERSION //p') + case "$version" in + [0-9]*.[0-9]*) pass "imports resolve, seafile client version is $version" ;; + *) fail "no usable version reported, output was: $out" ;; + esac else - fail "imports or SEAFILE_CLI_VERSION missing: $out" + fail "imports or SEAFILE_CLI_VERSION failed: $out" fi echo "== the real daemon starts, reports ready, and stops" @@ -77,12 +82,25 @@ raise SystemExit( ) " >/dev/null -# Wait for the daemon to be up and the process to be in its watch loop. +# Wait for the daemon to be up and the process to be in its watch loop. If it +# never gets there the shutdown assertions mean nothing, so say so loudly +# instead of measuring the shutdown of something that already exited. +watching=0 for _ in $(seq 1 60); do - docker logs dsc-smoke 2>&1 | grep -q WATCHING && break + if docker logs dsc-smoke 2>&1 | grep -q WATCHING; then watching=1; break; fi sleep 1 done +if [ "$watching" -eq 0 ]; then + fail "the container never reached its watch loop, shutdown not measured" + printf ' container log:\n' + docker logs dsc-smoke 2>&1 | sed 's/^/ /' | tail -20 + docker rm -f dsc-smoke >/dev/null + echo + echo "smoke test failed: $failures check(s)" + exit 1 +fi + started=$(date +%s) docker stop dsc-smoke >/dev/null elapsed=$(( $(date +%s) - started ))