Skip to content

Commit d253da8

Browse files
committed
Upgrades wait for running scans and for a slow start
install.sh --upgrade gave up after two minutes of waiting for /healthz and then printed "Upgraded" anyway. A restart that stops scans makes the next start read back what they had finished before the port opens, and that has taken over four minutes, so the warning was wrong and the success line was not earned. The upgrade now waits for any running scan to finish before it restarts, unless --now is given. After the restart it keeps waiting while startup is still logging, and shows progress after two minutes. It gives up when the container stops or restarts, when startup is silent for ten minutes, or after thirty minutes. It prints "Upgraded" only when the app answers, and otherwise says so and exits 1. A first install uses the same wait.
1 parent 899a0c1 commit d253da8

3 files changed

Lines changed: 136 additions & 24 deletions

File tree

‎documentation/CHANGELOG.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,15 @@ moved.
66

77
## 2026-11-04
88

9+
**Upgrades wait for running scans, and for a slow start.** `install.sh
10+
--upgrade` now waits for any scan in progress to finish before it restarts
11+
the app, because a restart stops it. Use `--upgrade --now` to restart
12+
straight away. After the restart it keeps waiting while the app is still
13+
starting, which can take several minutes when stopped scans are being read
14+
back, and shows what it is doing. It used to give up after two minutes and
15+
then print "Upgraded" anyway. Now it says "Upgraded" only when the app
16+
answers, and otherwise says it is not answering and exits with an error.
17+
918
**Change, withdraw or resubmit your own exception request.** The person who
1019
asked can now change the reason, expiry, ticket or kind on the finding page,
1120
and withdraw it while it is still waiting. A rejected or withdrawn request

‎documentation/installation.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,11 @@ A release that adds a column brings it in on the next start. The app compares
123123
what the code expects against what the file has and adds anything missing. It
124124
only ever adds, so an upgrade cannot drop a column or rewrite your data.
125125

126+
`./install.sh --upgrade` waits for running scans to finish before it
127+
restarts, since a restart stops them. `--upgrade --now` restarts anyway.
128+
What a stopped scan had finished is kept and read back on the next start,
129+
before the app opens its port, so that start can take several minutes.
130+
126131
Take a copy first if you want one:
127132

128133
```bash

‎install.sh‎

Lines changed: 122 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -130,10 +130,109 @@ ensure_container_dir() {
130130
warn "could not chown $path to uid 10001, the container may not be able to write"
131131
}
132132

133+
# running_scans
134+
# How many scans the running container is in the middle of, or nothing when
135+
# it cannot be asked (not running yet, or too old to answer). Read from the
136+
# database, because a live app has already marked any dead run interrupted
137+
# on its own start, so a row still marked running is a scan really running.
138+
running_scans() {
139+
"${COMPOSE[@]}" exec -T github-code-review python -c '
140+
from app.core.db import session_scope
141+
from app.core.models import ScanRun
142+
with session_scope() as db:
143+
print(db.query(ScanRun).filter(ScanRun.status == "running").count())
144+
' 2>/dev/null | tail -1 | grep -E '^[0-9]+$' || true
145+
}
146+
147+
# wait_for_scans
148+
# Restarting stops every scan in progress. What each one had finished is
149+
# read back on the next start, but the rest of the run is lost, and on a big
150+
# estate that read back alone holds the app off the network for minutes.
151+
# So wait for them, unless told not to.
152+
wait_for_scans() {
153+
local n
154+
n="$(running_scans)"
155+
[[ -n "$n" && "$n" != 0 ]] || return 0
156+
if (( NOW )); then
157+
warn "$n scan(s) are running and will be stopped (--now)"
158+
hint "What they had finished is kept and read back on start, which takes a while"
159+
return 0
160+
fi
161+
say "Waiting for $n running scan(s) to finish before restarting"
162+
hint "A restart would stop them. Run with --now to restart anyway."
163+
local last="$n"
164+
while [[ -n "$n" && "$n" != 0 ]]; do
165+
if [[ "$n" != "$last" ]]; then
166+
hint "$(date +%H:%M) $n still running"
167+
last="$n"
168+
fi
169+
sleep 30
170+
n="$(running_scans)"
171+
done
172+
ok "no scans running"
173+
}
174+
175+
# wait_for_up
176+
# Sets up=1 once /healthz answers. The app does real work before it opens
177+
# the port: migrations, and reading back what any stopped scan had finished,
178+
# which has taken over four minutes on production. So two minutes is only
179+
# the point to start explaining, not to give up. It gives up when the
180+
# container stops or restarts, when startup has logged nothing new for ten
181+
# minutes, or after thirty minutes in all.
182+
wait_for_up() {
183+
up=0
184+
local cid started waited=0 quiet=0 lines=0 now_lines state said=0
185+
cid="$("${COMPOSE[@]}" ps -q github-code-review 2>/dev/null | head -1)"
186+
[[ -n "$cid" ]] && started="$(docker inspect -f '{{.State.StartedAt}}' "$cid" 2>/dev/null || true)"
187+
[[ -n "$started" ]] || cid=""
188+
while (( waited < 1800 )); do
189+
if curl -fsS "http://${BIND}:${PORT}/healthz" >/dev/null 2>&1; then
190+
up=1; return 0
191+
fi
192+
if [[ -n "$cid" ]]; then
193+
state="$(docker inspect -f '{{.State.Status}} {{.State.StartedAt}}' "$cid" 2>/dev/null || true)"
194+
if [[ "${state%% *}" != running ]]; then
195+
state="${state%% *}"
196+
warn "the container is ${state:-gone}, not running"
197+
return 1
198+
fi
199+
if [[ -n "$started" && "${state#* }" != "$started" ]]; then
200+
warn "the container restarted while starting up"
201+
return 1
202+
fi
203+
now_lines="$(docker logs --since "$started" "$cid" 2>&1 | wc -l)"
204+
if (( now_lines > lines )); then
205+
lines="$now_lines"; quiet=0
206+
else
207+
quiet=$(( quiet + 2 ))
208+
fi
209+
if (( quiet >= 600 )); then
210+
warn "startup has logged nothing for ten minutes"
211+
return 1
212+
fi
213+
fi
214+
if (( waited >= 120 && ! said )); then
215+
hint "Still starting after two minutes. That is normal after scans were stopped:"
216+
hint "it reads back what they had finished before it opens the port."
217+
said=1
218+
fi
219+
if (( said && waited % 60 == 0 )) && [[ -n "$cid" ]]; then
220+
hint "$(date +%H:%M) $(docker logs --since "$started" "$cid" 2>&1 \
221+
| grep -E 'salvaged|startup|interrupted' | tail -1 | cut -c1-110)"
222+
fi
223+
sleep 2
224+
waited=$(( waited + 2 ))
225+
done
226+
warn "it did not answer in thirty minutes"
227+
return 1
228+
}
229+
133230
UPGRADE=0
231+
NOW=0
134232
while (( $# )); do
135233
case "$1" in
136234
--upgrade|-u) UPGRADE=1; shift ;;
235+
--now) NOW=1; shift ;;
137236
-h|--help)
138237
cat <<'__USAGE__'
139238
install.sh - set Git Code Review up, or bring it up to date.
@@ -142,6 +241,10 @@ install.sh - set Git Code Review up, or bring it up to date.
142241
where the reports go, then builds and starts it.
143242
install.sh --upgrade already installed. Pulls the latest, rebuilds and
144243
restarts. Asks nothing and changes no settings.
244+
Waits for any running scan to finish first.
245+
install.sh --upgrade --now
246+
the same, but restarts straight away. Running
247+
scans stop; what they finished is kept.
145248
146249
Everything you answered the first time lives in .env and is left alone either
147250
way, so an upgrade cannot quietly move your database or your reports.
@@ -218,30 +321,32 @@ if (( UPGRADE )); then
218321
"${COMPOSE[@]}" build || die "build failed"
219322
ok "image built"
220323

324+
# Checked after the build, right before the restart, so a scan that a
325+
# schedule started while the image was building is caught too.
326+
wait_for_scans
327+
221328
say "Restarting"
222329
# up -d recreates only what changed, so the database container is left
223330
# alone unless its own definition moved.
224331
"${COMPOSE[@]}" up -d || die "could not start the container"
225332

226333
say "Waiting for it to come up"
227-
up=0
228-
for _ in $(seq 1 60); do
229-
curl -fsS "http://${BIND}:${PORT}/healthz" >/dev/null 2>&1 && { up=1; break; }
230-
sleep 2
231-
done
232-
if (( up )); then
233-
ok "it is up"
234-
else
235-
warn "it did not answer in two minutes. Check: ${COMPOSE[*]} logs"
236-
fi
334+
wait_for_up || true
237335

238336
echo
239-
ok "Upgraded. Running at http://${BIND}:${PORT}"
240-
hint "New columns were added on boot, and any that had outgrown their size"
241-
hint "were widened. There is nothing to run by hand."
242-
hint "Logs: ${COMPOSE[*]} logs -f"
337+
if (( up )); then
338+
ok "Upgraded. Running at http://${BIND}:${PORT}"
339+
hint "New columns were added on boot, and any that had outgrown their size"
340+
hint "were widened. There is nothing to run by hand."
341+
hint "Logs: ${COMPOSE[*]} logs -f"
342+
echo
343+
exit 0
344+
fi
345+
warn "The new image is built and the container was started, but it is not answering."
346+
hint "Look at: ${COMPOSE[*]} logs --tail 100 github-code-review"
347+
hint "Then: curl -fsS http://${BIND}:${PORT}/healthz"
243348
echo
244-
exit 0
349+
exit 1
245350
fi
246351

247352
say "Git Code Review 1.0.1 setup"
@@ -391,17 +496,10 @@ say "Starting it"
391496

392497
# Wait for the health check rather than guessing.
393498
say "Waiting for it to come up"
394-
up=0
395-
for _ in $(seq 1 60); do
396-
if curl -fsS "http://${BIND}:${PORT}/healthz" >/dev/null 2>&1; then
397-
up=1; break
398-
fi
399-
sleep 2
400-
done
401-
if (( up )); then
499+
if wait_for_up; then
402500
ok "it is up"
403501
else
404-
warn "it did not answer in two minutes. Check: ${COMPOSE[*]} logs"
502+
warn "it is not answering. Check: ${COMPOSE[*]} logs --tail 100 github-code-review"
405503
fi
406504

407505
# -----------------------------------------------------------------------------

0 commit comments

Comments
 (0)