Skip to content

fix: stop the right containers for minimize-downtime flag #4454

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
aldy505 wants to merge 3 commits into
base: master
Choose a base branch
from
Open

fix: stop the right containers for minimize-downtime flag #4454

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5b08f48
fix: stop the right containers for minimize-downtime flag
aldy505 Aug 7, 2026
0dff550
fix: remove invalid flags for `docker compose down`
aldy505 Aug 12, 2026
36b2b11
fix: revert wrap-up change
aldy505 Aug 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions install/turn-things-off.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
echo "${_group}Turning things off ..."

if [[ -n "$MINIMIZE_DOWNTIME" ]]; then
# Stop everything but relay and nginx
$dc rm -fsv $($dc config --services | grep -v -E '^(nginx|relay)$')
# Stop everything unless databases, relay, web, and nginx
$dc down $($dc config --services | grep -v -E '^(nginx|web|relay|smtp|memcached|redis|postgres|pgbouncer|kafka|clickhouse|seaweedfs)$')

@aminvakil aminvakil Aug 15, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

podman down, stops and removes dependencies like postgres as well.

@aldy505 aldy505 Aug 15, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sigh...

else
# Clean up old stuff and ensure nothing is working while we install/update
if [ "$CONTAINER_ENGINE" = "podman" ]; then
Expand Down
8 changes: 1 addition & 7 deletions install/wrap-up.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,9 @@
if [[ "$MINIMIZE_DOWNTIME" ]]; then
echo "${_group}Waiting for Sentry to start ..."

# Start the whole setup, except nginx and relay.
# Start the whole setup, except nginx, relay, and web.
start_service_and_wait_ready --remove-orphans $($dc config --services | grep -v -E '^(nginx|relay)$')

@sentry sentry Bot Aug 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The comment in wrap-up.sh states the web service should be excluded from the initial start, but the grep regex on the next line does not exclude it.
Severity: MEDIUM

Suggested Fix

Update the grep regular expression on line 5 of install/wrap-up.sh to include web in the list of excluded services. The regex should be changed from '^(nginx|relay)$' to '^(nginx|web|relay)$' to match the intent described in the comment.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: install/wrap-up.sh#L5

Potential issue: In the `--minimize-downtime` upgrade path, the script
`install/wrap-up.sh` is intended to perform a staged service startup. A comment on line
4 indicates that the `web` service should be excluded from the initial
`start_service_and_wait_ready` call. However, the `grep` command's regex on line 5
(`'^(nginx|relay)$'`) fails to exclude `web`. This causes the `web` service to be
started and force-recreated prematurely, which contradicts the intended staged startup
logic designed to minimize downtime. While this may not cause a complete failure, it
deviates from the expected upgrade process.


if [ -n "$($dc ps -q relay 2>/dev/null)" ]; then
$dc restart relay
else
echo "Relay container not found, skipping restart."
fi

if [ -n "$($dc ps -q nginx 2>/dev/null)" ]; then
$dc exec -T nginx nginx -s reload || true
else
Expand Down
Loading