diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5d8c3df..8a94485 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -114,44 +114,95 @@ jobs: chmod -R a+rX /var/www/html/project2 curl -fsS -o /dev/null -w 'GET /project2/ : %{http_code}\n' http://localhost/project2/ - # Restarting the GraphQL API is M2 work. Until `server/` exists in this - # repository there is nothing to deploy, and restarting the unit would - # just bring back whatever was last installed on the VM by hand. + # Deploy the API. The unit runs `node dist/index.js` out of + # ~/T19-Project-2/server, so that directory has to hold the build of the + # commit we just tested. It is a git clone of main (see #44), so point it + # at $GITHUB_SHA rather than copying files around, then install and build + # there. # - # Two things here are deliberately not what the first version did: + # Deliberately not what earlier versions did: # # - The unit check is `[ -f ... ]`, not `systemctl list-unit-files | grep`. # `grep -q` exits on the first match and closes the pipe, `systemctl` # gets SIGPIPE and dies, and with `pipefail` the whole pipeline reports # failure. So the `if` took the else branch and skipped a service that - # was definitely there. The log showed "Failed to print table: Broken + # was definitely there. The log said "Failed to print table: Broken # pipe". Reported by rachelks. # - # - The restart needs `sudo`, and the runner has no TTY. That needs a + # - The restart needs `sudo` and the runner has no TTY. That needs a # NOPASSWD rule scoped to exactly these commands. Without it the step # dies with "sudo: A terminal is required to authenticate". - - name: Restart the GraphQL API if the server is deployed + # + # - No `sleep`. `Type=simple` reports active before Node binds the port. + # Measured at about 1850 ms, so a fixed `sleep 2` has already produced a + # 503 through Apache. Poll instead. + - name: Deploy the API and restart crate-api run: | set -euo pipefail + DEPLOY_ROOT="${HOME}/T19-Project-2" + + # #42 has landed, so `server/` is expected and the unit is expected + # to exist. These used to skip, which meant the deploy job could go + # green without deploying anything. rachelks on #45: that is the + # wrong failure mode. if [ ! -f /etc/systemd/system/crate-api.service ]; then - echo "crate-api.service is not installed. Skipping API restart." - exit 0 + echo "::error::crate-api.service is not installed on this machine." + exit 1 fi if [ ! -d server ]; then - echo "No server/ in this repository yet (arrives in M2). Skipping API restart." - exit 0 + echo "::error::server/ is missing from the checkout." + exit 1 fi - if [ ! -f server/dist/index.js ]; then - echo "::error::server/ exists but server/dist/index.js does not. Was 'pnpm build' run?" + if [ ! -d "${DEPLOY_ROOT}/.git" ]; then + echo "::error::${DEPLOY_ROOT} is not a git clone. See #44." + exit 1 + fi + + echo "Deploying ${GITHUB_SHA} to ${DEPLOY_ROOT}" + git -C "${DEPLOY_ROOT}" fetch --quiet origin + # --force so a dirty tree cannot block the deploy. .env is gitignored + # and so survives the checkout; it holds DATABASE_URL. + git -C "${DEPLOY_ROOT}" checkout --force "${GITHUB_SHA}" + echo " at $(git -C "${DEPLOY_ROOT}" rev-parse --short HEAD)" + + if [ ! -f "${DEPLOY_ROOT}/.env" ]; then + echo "::error::${DEPLOY_ROOT}/.env is missing. crate-api will not start without DATABASE_URL." + exit 1 + fi + + pnpm install --dir "${DEPLOY_ROOT}" --frozen-lockfile + pnpm --dir "${DEPLOY_ROOT}" --filter @t19/server build + + if [ ! -f "${DEPLOY_ROOT}/server/dist/index.js" ]; then + echo "::error::server built but server/dist/index.js is missing." + exit 1 + fi + + # #5 landed migrations for tracks, simulated users, reviews and crate + # items. A deploy that lands new migration files and never applies + # them is the same gap as #44, one layer down: the code is current + # and the schema is not. Migrate before the restart so the new build + # is never pointed at an old database. + # + # `db:migrate` is idempotent (verified on #46), so this runs on every + # deploy and not only when the migration list changes. + # + # DATABASE_URL comes from the same .env the systemd unit loads. The + # server package scripts look for `server/.env` instead, which may + # not exist here, so export it explicitly rather than assuming the + # two agree. Node's --env-file does not override a variable already + # in the environment, so this wins even if server/.env is present. + echo + echo "Applying database migrations" + db_url="$(sed -n 's/^DATABASE_URL=//p' "${DEPLOY_ROOT}/.env" | tail -n 1)" + if [ -z "${db_url}" ]; then + echo "::error::DATABASE_URL is not set in ${DEPLOY_ROOT}/.env, so migrations cannot run." exit 1 fi + DATABASE_URL="${db_url}" pnpm --dir "${DEPLOY_ROOT}" --filter @t19/server db:migrate + sudo /usr/bin/systemctl restart crate-api - # `Type=simple` makes systemd report active as soon as the process is - # spawned, well before Node has bound the port. Measured on the VM, - # the API answers again after about 1850 ms, so a fixed `sleep 2` - # leaves a margin of roughly 120 ms. That was enough to return 503 - # through the Apache proxy on at least one run. Poll instead. printf 'Waiting for the API' for _ in $(seq 1 30); do code=$(curl -s -o /dev/null -w '%{http_code}' --max-time 2 \ @@ -161,11 +212,72 @@ jobs: if [ "$code" = "200" ]; then echo echo "POST /graphql : 200" - exit 0 + break fi printf '.' sleep 1 done + if [ "${code:-000}" != "200" ]; then + echo + echo "::error::crate-api restarted but /graphql never returned 200 (last code ${code})." + exit 1 + fi + + # Smoke suite, against the real port and through the same Apache + # proxy the browser uses. This is the acceptance line of #6. + # + # GraphQL answers HTTP 200 even when the payload carries errors, so + # the status code alone proves nothing (rachelks on #45). The body is + # parsed as JSON and rejected unless it has a non-null `data` and no + # `errors`, rather than grepping for the word "errors", which would + # also fire on an album called "Errors". + # + # The validators live in shell variables on one line. A multi-line + # python -c heredoc cannot be used here: the YAML block scalar would + # end at the first unindented line and the workflow would not parse. echo - echo "::error::crate-api restarted but /graphql never returned 200 (last code ${code})." - exit 1 + echo "Smoke suite:" + CHECK_BODY='import json,sys; p=json.load(sys.stdin); e=p.get("errors"); n=p.get("data"); print(f"errors: {e}" if e else (f"no data: {p}" if n is None else "")); sys.exit(1 if (e or n is None) else 0)' + check() { + local label="$1" body="$2" resp code json + resp=$(curl -s -X POST http://localhost/graphql \ + -H 'content-type: application/json' \ + -d "${body}" -w '\n%{http_code}') + code="${resp##*$'\n'}" + json="${resp%$'\n'*}" + if [ "${code}" != "200" ]; then + echo "::error::${label}: HTTP ${code} body: ${json}" + return 1 + fi + detail=$(printf '%s' "${json}" | python3 -c "${CHECK_BODY}" 2>&1) || { + echo "::error::${label}: ${detail:-body check failed} body: ${json}" + return 1 + } + printf ' %-22s %s ok\n' "${label}" "${code}" + } + + check '{ __typename }' '{"query":"{ __typename }"}' + # `sort` is `AlbumSort!` in AlbumSearchInput, so leaving it out makes + # the query fail validation. That was a real bug in the first version + # of this check. + check 'searchAlbums' '{"query":"{ searchAlbums(input:{query:\"\",sort:TITLE_ASC},first:1){totalCount pageInfo{hasNextPage}} }"}' + + # One assertion on the data itself, not just its presence. + CHECK_TOTAL='import json,sys; p=json.load(sys.stdin); t=p["data"]["searchAlbums"]["totalCount"]; print(t) if isinstance(t,int) and t>=0 else sys.exit(f"totalCount is not a non-negative integer: {t!r}")' + totals=$(curl -s -X POST http://localhost/graphql \ + -H 'content-type: application/json' \ + -d '{"query":"{ searchAlbums(input:{query:\"\",sort:TITLE_ASC},first:1){totalCount} }"}') + if total=$(printf '%s' "${totals}" | python3 -c "${CHECK_TOTAL}" 2>&1); then + echo " totalCount ${total} albums" + else + echo "::error::totalCount assertion failed: ${total} body: ${totals}" + exit 1 + fi + + + if curl -fsS -o /dev/null http://localhost/project2/; then + echo " GET /project2/ 200 ok" + else + echo "::error::GET /project2/ failed" + exit 1 + fi