Skip to content

fix: Fix develop script pid tracking, improve logging and interrupt #5186

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

Merged
merged 2 commits into from
Nov 29, 2022
Merged

fix: Fix develop script pid tracking, improve logging and interrupt #5186

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ab6823e
fix: Fix develop script pid tracking, improve logging and interrupt
mafredri Nov 29, 2022
08df5a2
Remove superfluous redirection
mafredri Nov 29, 2022
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
Next Next commit
fix: Fix develop script pid tracking, improve logging and interrupt
  • Loading branch information
@mafredri
mafredri committed Nov 29, 2022
commit ab6823e16973a6e7267c3131e655237298d2401b
7 changes: 5 additions & 2 deletions cli/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,11 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co
defer func() {
cmd.Printf("Stopping built-in PostgreSQL...\n")
// Gracefully shut PostgreSQL down!
_ = closeFunc()
cmd.Printf("Stopped built-in PostgreSQL\n")
if err := closeFunc(); err != nil {
cmd.Printf("Failed to stop built-in PostgreSQL: %v\n", err)
Copy link
Member Author

Choose a reason for hiding this comment

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

Review: Didn't catch this issue before now because we were ignoring errors.

} else {
cmd.Printf("Stopped built-in PostgreSQL\n")
}
}()
}

Expand Down
57 changes: 42 additions & 15 deletions scripts/develop.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,53 +51,84 @@ make -j "build/coder_${GOOS}_${GOARCH}"
# Use the coder dev shim so we don't overwrite the user's existing Coder config.
CODER_DEV_SHIM="${PROJECT_ROOT}/scripts/coder-dev.sh"

# Stores the pid of the subshell that runs our main routine.
ppid=0
# Tracks pids of commands we've started.
pids=()
exit_cleanup() {
set +e
# Set empty interrupt handler so cleanup isn't interrupted.
trap '' INT
# Send interrupt to the entire process group to start shutdown procedures.
kill -INT -$$
trap '' INT TERM
# Remove exit trap to avoid infinite loop.
trap - EXIT

# Just in case, send interrupts to our children.
# Send interrupts to the processes we started. Note that we do not
# (yet) want to send a kill signal to the entire process group as
# this can halt processes started by graceful shutdown.
kill -INT "${pids[@]}" >/dev/null 2>&1
# Use the hammer if things take too long.
{ sleep 5 && kill -TERM -$$ >/dev/null 2>&1; } &
{ sleep 5 && kill -TERM "${pids[@]}" >/dev/null 2>&1; } &

# Wait for all children to exit (this can be aborted by hammer).
wait_cmds

# Just in case, send termination to the entire process group
# in case the children left something behind.
kill -TERM -"${ppid}" >/dev/null 2>&1

exit 1
}
start_cmd() {
name=$1
prefix=$2
shift 2

echo "== CMD: $*" >&2
"$@" 2>&1 || fatal "CMD: $*" &

FORCE_COLOR=1 "$@" 3>&2 > >(
# Ignore interrupt, read will keep reading until stdin is gone.
trap '' INT

while read -r line; do
if [[ $prefix == date ]]; then
echo "[$name] $(date '+%Y-%m-%d %H:%M:%S') $line"
else
echo "[$name] $line"
fi
done
echo "== CMD EXIT: $*" >&2
# Let parent know the command exited.
kill -INT $ppid >/dev/null 2>&1
) 2>&1 &
pids+=("$!")
}
wait_cmds() {
wait "${pids[@]}" >/dev/null 2>&1
}
fatal() {
echo "== FAIL: $*" >&2
exit_cleanup
kill -INT $ppid >/dev/null 2>&1
}

# This is a way to run multiple processes in parallel, and have Ctrl-C work correctly
# to kill both at the same time. For more details, see:
# https://stackoverflow.com/questions/3004811/how-do-you-run-multiple-programs-in-parallel-from-a-bash-script
(
ppid=$BASHPID
# If something goes wrong, just bail and tear everything down
# rather than leaving things in an inconsistent state.
trap 'exit_cleanup' INT EXIT
trap 'exit_cleanup' INT TERM EXIT
trap 'fatal "Script encountered an error"' ERR

cdroot
start_cmd "${CODER_DEV_SHIM}" server --address 0.0.0.0:3000
start_cmd API "" "${CODER_DEV_SHIM}" server --address 0.0.0.0:3000

echo '== Waiting for Coder to become ready'
# Start the timeout in the background so interrupting this script
# doesn't hang for 60s.
timeout 60s bash -c 'until curl -s --fail http://localhost:3000/healthz > /dev/null 2>&1; do sleep 0.5; done' ||
fatal 'Coder did not become ready in time'
fatal 'Coder did not become ready in time' &
wait $!

# Check if credentials are already set up to avoid setting up again.
"${CODER_DEV_SHIM}" list >/dev/null 2>&1 && touch "${PROJECT_ROOT}/.coderv2/developsh-did-first-setup"
Expand Down Expand Up @@ -138,11 +169,7 @@ fatal() {
fi

# Start the frontend once we have a template up and running
CODER_HOST=http://127.0.0.1:3000 start_cmd yarn --cwd=./site dev --host > >(
while read -r line; do
echo "[SITE] $(date -Iseconds): $line"
done
)
CODER_HOST=http://127.0.0.1:3000 start_cmd SITE date yarn --cwd=./site dev --host

interfaces=(localhost)
if which ip >/dev/null 2>&1; then
Expand Down