diff --git a/TODO.AI.md b/TODO.AI.md index e6358dd..5694af5 100644 --- a/TODO.AI.md +++ b/TODO.AI.md @@ -78,6 +78,216 @@ Needs syncing back to the upstream template per AI.md's runbook. populates the (previously missing) `org.opencontainers.image.licenses` label per AI.md's OCI label standard (lines 58-87), and `source` keeps the single github.com URL. +## App-breaking bug fixed — act_runner init.d hang + duplicate runner registration (zz-act_runner.sh) + +Needs syncing back to the upstream template in `casjay-dotfiles/scripts` per the Docker Template +Update Runbook in AI.md — the outer hooks (`__post_execute`, `__run_start_script`) are normally +generated, not hand-edited, but this repo's app-specific runner logic already lives inline in +them rather than in the `*_local()` stubs, so the fix was applied in place. + +- Root cause of the hang: because `EXEC_CMD_BIN=''` for this service, `__run_start_script` + (called at the script's tail) internally calls `__post_execute` synchronously through + `__post_execute | tee -p -a "/data/logs/init.txt"`. Inside `__post_execute`'s own backgrounded + subshell, `/usr/local/bin/start-runners &` was launched without redirecting its stdout/stderr, + so it (and the long-running `act_runner daemon` processes it `exec`s, which never exit) + inherited the write end of that `tee` pipe. Since that write end never closed, `tee` never saw + EOF, so it never exited, so `__run_start_script` never returned, so `__start_init_scripts`'s + `( source "$init" )` for this script never completed — the init.d loop hung forever after all + services were actually up, and `/data/logs/start.log`'s final `printf` (the completion marker) + was never reached. Fixed by redirecting `start-runners`'s stdout/stderr to + `"$LOG_DIR/runners.log"` at its invocation site instead of inheriting the pipe. +- The script also called `__post_execute` a second time explicitly + (`__post_execute ... | tee ... &` near the end of the script), duplicating the call + `__run_start_script` already makes via its empty-`EXEC_CMD_BIN` branch. Removed the redundant + explicit call. +- The script additionally registered and started a hardcoded, always-on single runner named + `"gitea"` (in `__run_pre_execute_checks` and `__post_execute`) independent of and in addition + to the `RUNNERS_START`-driven runners from `start-runners` — so `RUNNERS_START=2` produced 3 + registered runners (`gitea`, `runner-1`, `runner-2`) instead of 2. Removed both the legacy + registration block and its matching daemon-start block; `start-runners` (already idempotent + per-runner via its `.runner`-file check) is now the sole runner registration/startup path. +- Verified with `bash -n` and a live rebuild/retest: the init.d loop now completes and + `/data/logs/start.log` gets its completion marker; only `runner-1`/`runner-2` are registered + (no `gitea` runner). + +## App-breaking bug fixed — act_runner registered against wrong/unreachable IP (zz-act_runner.sh) + +Needs syncing back to the upstream template per AI.md's runbook (same file/hooks as above). + +- `RUNNER_IP_ADDRESS` defaulted to `$IP4_ADDRESS` (the container's externally-detected IP, + captured once very early in `bin/entrypoint.sh` via `__get_ip4`). In live testing this value + did not match the container's actual `eth0` address by the time `act_runner daemon` tried to + connect, causing every runner to fail its first RPC with `dial tcp :80: connect: no route + to host` and exit immediately (act_runner does not retry a failed initial connection). Since + act_runner registers against gitea running in the very same container/network namespace, the + detected external IP was never the right thing to use. Fixed by defaulting `RUNNER_IP_ADDRESS` + to `127.0.0.1` instead (still overridable via the `RUNNER_IP_ADDRESS` env var). +- Verified live: after the fix, `curl http://127.0.0.1/` inside the container succeeds + immediately. A first retest still showed runners connecting to `172.17.0.2:80`, but this + was a false alarm caused by stale test volumes: a previous test run's `.runner` + registration files (with the old baked-in address) were still present, and + `__register_runner`'s idempotency check correctly skipped re-registering over them. After + fully removing the test volumes and starting a genuinely fresh container, runner + registration and connectivity succeeded (`Runner registered successfully.` / + `declare successfully` for both `runner-1` and `runner-2`, no connection errors). + +## App-breaking bug fixed — cache-server fd leak hangs init.d loop (zz-act_runner.sh) + +Needs syncing back to the upstream template per AI.md's runbook (same file/hooks as above). + +- Same fd-leak class as the `start-runners` hang fixed above, but for + `act_runner cache-server`: `act_runner cache-server --config "$CACHE_CONFIG_FILE" + 2>>/dev/stderr >>"$CACHE_LOG_FILE" &` used `2>>/dev/stderr`, which duplicates the + process's *current* stderr fd — at that point in `__post_execute`'s backgrounded + subshell, still the write end of the `tee -p -a "/data/logs/init.txt"` pipe used by the + synchronous `__post_execute` call in `__run_start_script`. Since `cache-server` is a + long-running daemon that never exits, that pipe's write end never closed, so `tee` never + saw EOF, hanging `__run_start_script` (and therefore the whole init.d loop) forever even + though dockerd/gitea/act_runner/cache-server were all actually up and working. Fixed by + redirecting to `"$CACHE_LOG_FILE" 2>&1` instead of `/dev/stderr`. +- The redirect fix alone did not fully resolve the hang: the subshell launching + `start-runners`/`cache-server` (the left side of `__post_execute`'s pipe to + `tee -p -a /data/logs/init.txt`) was still observed blocked in `do_wait` on its + long-running child even with output redirected to a real file, preventing the pipe from + ever seeing EOF. Added an explicit `disown "$!"` immediately after each background launch + (`act_runner cache-server` and `/usr/local/bin/start-runners`) to fully detach them from + the subshell's job table. +- Verified live on a genuinely fresh container (volumes removed first): `/data/logs/start.log` + now gets its completion marker, no orphaned `tee -p -a /data/logs/init.txt` processes + remain, and both `runner-1`/`runner-2` register and declare successfully against gitea + (`runners.log` shows `Runner registered successfully.` and `declare successfully` for + both, no connection errors). + +## App-breaking bug fixed — dockerd fails to restart under rapid restart cycling (05-dockerd.sh) + +Found while directly testing the user's "restart container many times" requirement with a rapid +4x-restart loop. + +- `docker restart` reuses the same container filesystem (unlike a fresh `run`), so + `/tmp/docker.pid` from the previous dockerd instance survives the restart. The PID namespace + itself resets on every restart, so the low PID number written into that file (e.g. `461`) can + coincidentally be reused by an unrelated early-boot process in the new namespace. dockerd's own + startup check saw `/proc/` exist and refused to start: `failed to start daemon, ensure + docker is not running or delete /tmp/docker.pid: process with PID 461 is still running`. +- Observed impact: `05-dockerd.sh` logged `❌ Service dockerd failed to start - check logs`, but + gitea and act_runner still reported starting successfully; the entrypoint logged + `⚠️ Warning: 1 critical service(s) reported failures` / `ℹ️ Continuing with 1 failure(s) - + container may still be functional` and kept going in a degraded state, then the whole container + crashed anyway roughly 2 minutes later (`ExitCode=1`). +- Fixed by removing `/tmp/docker.pid` unconditionally at the top of `__run_pre_execute_checks` in + `05-dockerd.sh`, before dockerd is started. This init script is the sole owner of the dockerd + lifecycle (a separate real overlap-guard already exists via `SERVICE_PID_FILE`), so clearing + docker's own pidfile before every start attempt is safe. +- Verified live on a rebuilt image: no `failed to start daemon` / stale-pidfile error appeared in + the logs across either a 3x normally-paced restart loop or a 4x rapid back-to-back restart burst + (see the verification note on the PID-reuse fix below — both fixes were tested together in the + same runs). + +## App-breaking bug fixed — PID-reuse false positive kills container after restart (entrypoint.sh, functions/entrypoint.sh) + +Found and fixed immediately after the dockerd stale-pidfile fix above, while retesting restart +robustness on the rebuilt image. Same root bug class (a PID recorded in a file that persists +across `docker restart` gets coincidentally reused by an unrelated process once the PID namespace +resets), but hitting two different guards this time, one of which actively took down the whole +container: + +- `functions/entrypoint.sh`'s `__no_exit()`: guarded re-entry with `[ -f /run/.no_exit.pid ] && + kill -0 "$no_exit_pid"`. After a restart, an unrelated early-boot process could reuse that old + PID number, making the check wrongly believe the monitor loop was already running. `__no_exit` + then `return`ed 0 instead of `exec`ing the actual monitor loop, so `bin/entrypoint.sh` fell + straight through to `exit $?` — the whole entrypoint process exited cleanly (`ExitCode=0`), + killing the container roughly 2 minutes after a normal, correctly-paced restart, well after + dockerd/gitea/act_runner had all logged successful starts. This is almost certainly the actual + root cause behind earlier restart-loop crashes previously attributed only to the dockerd + stale-pidfile bug. +- `bin/entrypoint.sh`'s `ENTRYPOINT_PID_FILE` check (~line 433) has the identical hazard in the + opposite direction: a false-positive "still alive" match sets `START_SERVICES=no`, which would + silently skip `__start_init_scripts` entirely on a genuine restart (not observed in this test run, + but reachable by the same mechanism). +- Fixed both by requiring the live PID's own `/proc//cmdline` to actually match the expected + process, not just `kill -0` succeeding: `__no_exit`'s exec'd monitor loop now embeds a + `__no_exit_monitor_loop` marker comment in its own `bash -c` command text (visible in its own + cmdline), and the re-check greps for it; `ENTRYPOINT_PID_FILE`'s check greps the candidate PID's + cmdline for `entrypoint.sh`. +- Verified live on a rebuilt image (fresh volumes): a 3x normally-paced restart loop (each + followed by a 90s post-init settle, well past the previously-observed ~2min crash window) and a + 4x rapid back-to-back restart burst (3s apart, no waiting for init between them) both completed + with the container remaining `running`/`ExitCode=0` throughout — no clean-exit crash, and no + `failed to start daemon ... still running` dockerd error in the logs. Runner UUIDs for all 5 + registered runners were identical before and after both test sequences (`/config/act_runner/reg/` + still holds exactly 5 directories, no duplicates), confirming the "same id, not new" requirement + holds even under rapid restart cycling. + +## Config cleanup done — deprecated `[webhook].ALLOWED_HOST_LIST` moved to `[security]` (rootfs/tmp/etc/gitea/app.ini) + +- Every `gitea admin user create` (and, by extension, every gitea startup) logged: + `[E] Deprecation: config option [webhook].ALLOWED_HOST_LIST present, please use + [security].ALLOWED_HOST_LIST instead because this fallback will be/has been removed in v28.0.0`. +- Root cause: the shipped `app.ini` template set `ALLOWED_HOST_LIST = *` under `[webhook]` only; no + `[security]` equivalent existed, so gitea used the deprecated fallback on every run. +- Fix: added `ALLOWED_HOST_LIST = *` to `[security]` and removed it from `[webhook]`. +- `app.ini` is a staged config file (`rootfs/tmp/etc/gitea/app.ini` → `/config/gitea/app.ini`), not a + `gen-dockerfile`-generated file, so this is a normal in-repo edit, not a template/generation issue. +- Verified live on a rebuilt image: `gitea admin user create` no longer emits the deprecation warning, + and admin login/API access still works normally after the move. +- Re-ran the full functional + restart-stability regression suite against this rebuilt image on fresh + volumes: admin user creation, non-admin user creation, repo creation, fork (`forker/testrepo`), and + GitHub mirror creation + manual `mirror-sync` trigger (`mirror_last_sync_at` moved off epoch) all + passed. A 3x paced restart loop (90s settle each) and a 4x rapid back-to-back restart burst (3s + apart) both left the container `running`/`ExitCode=0` throughout, with dockerd, `gitea web`, and all + 5 `act_runner` daemons (+ cache-server) alive afterward, and all 5 runner UUIDs byte-for-byte + identical before and after (`/config/act_runner/reg/` still exactly 5 directories, no duplicates). + +## App-breaking bug fixed — act_runner CI jobs cannot start: nested overlayfs mount failure (05-dockerd.sh) + +- Every Gitea Actions job failed within ~13s with, both via act_runner and via a plain manual + `docker run` inside the container: `failed to mount ... fstype: overlay ... err: invalid argument`. + Reproduced with `docker exec gitea-test docker run --rm ...` — 100% failure rate, not + specific to any one image or to act_runner's job-container logic. +- Root cause: the inner (DinD) dockerd defaulted to the `overlayfs` storage driver, same as the outer + host/container's own root filesystem. Nesting an `overlay2`-driver dockerd inside a container whose + own root is itself an overlay filesystem is a well-known Docker-in-Docker failure mode — the kernel + rejects the resulting overlay-on-overlay mount with `invalid argument` on this kernel/host + combination, even with `index=off` already set. +- Impact: **the entire act_runner CI subsystem was non-functional** — runners registered fine and + showed as online, but every single job would fail immediately at the "Starting job container" step, + regardless of workflow, label, or target image. This would not have been caught by + container-startup/registration testing alone; it required actually running a real workflow to + surface. +- Fix: added `"storage-driver": "fuse-overlayfs"` to both branches of the `/config/docker/daemon.json` + generation in `__run_pre_execute_checks_local` (with-registry and without-registry cases). + `fuse-overlayfs` is already bundled in the image (`/usr/bin/fuse-overlayfs`, `/dev/fuse` present) and + avoids the kernel-level overlay-on-overlay conflict by mounting entirely in userspace via FUSE. The + existing `--privileged`/`SYS_ADMIN` requirement (already documented in README.md as required for + Docker-in-Docker) covers fuse-overlayfs's own requirements — no new run-flag requirement introduced. +- Verified live: manually patched `/config/docker/daemon.json` on the running container, restarted the + inner dockerd, and confirmed `docker info` reports `Storage Driver: fuse-overlayfs` and `docker run` + (both `casjaysdev/alpine:latest` and `ubuntu:latest`) now creates and starts containers successfully + with no mount error, where it previously failed 100% of the time. +- **Follow-up root cause found**: the initial `05-dockerd.sh` heredoc fix alone did not take effect on + a fresh container/fresh volumes. Traced to `__run_precopy`'s baked-`/etc`-seeding step: on first run + it copies the whole baked `/etc/docker` directory (staged from `rootfs/tmp/etc/docker/daemon.json` via + `03-files.sh`) into `/config/docker/` BEFORE `__run_pre_execute_checks_local`'s + `[ ! -f "/config/docker/daemon.json" ]` guard ever runs — so the guard always saw the file already + present and the heredoc (containing the fix) never executed. Fixed by adding + `"storage-driver": "fuse-overlayfs"` to the actual baked source file, + `rootfs/tmp/etc/docker/daemon.json`, in addition to keeping the `05-dockerd.sh` heredoc fix as + defense-in-depth for any case where the baked `/etc/docker` dir is absent. +- Rebuilt the image and recreated the container on completely fresh volumes: confirmed + `/config/docker/daemon.json` now contains `"storage-driver": "fuse-overlayfs"` and + `docker info` reports `Storage Driver: fuse-overlayfs` via the normal init path (not a manual patch). + Confirmed `docker exec gitea-test docker run --rm ubuntu:latest echo ...` succeeds with no mount + error. Pushed a real `runs-on: alpine` Gitea Actions workflow via the contents API and confirmed the + job reached `"status":"success"` in ~3 seconds via `GET .../actions/tasks` — full end-to-end + confirmation, not just container-creation succeeding. **This bug is now fully resolved and verified.** +- Re-ran the full restart-stability + functional regression suite on this fix, on the same fresh + container: 3x paced restarts (90s settle each) + 4x rapid-burst restarts (3s apart) — container + stayed `running`/`ExitCode:0`/`healthy` throughout, all 5 runner UUIDs identical before/after (no + duplicate registrations), and `Storage Driver: fuse-overlayfs` + prior admin user/repo state all + survived the restarts intact. Also verified admin user creation, repo creation, fork (into a second + `forker` user account), and mirror+sync (`mirror_updated` timestamp advanced from mirror-registration + time to a fresh sync time) all still work with no regressions from this fix. + ## Non-issue — confirmed intentional (`.gitea/workflows/docker.yaml`) - Uses a stale/unpinned action pattern (`@v2`-`@v4`, DockerHub-only, `catthehacker/ubuntu:act-latest`).