Skip to content

ci: ensure bootstrap node is ready before starting other nodes #7321

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
steviez merged 1 commit into from
Aug 5, 2025
Merged

ci: ensure bootstrap node is ready before starting other nodes #7321

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
25 changes: 12 additions & 13 deletions ci/localnet-sanity.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,19 +156,7 @@ startNodes() {
initCompleteFiles=()
maybeExpectedGenesisHash=
for i in $(seq 0 $((${#nodes[@]} - 1))); do
declare cmd=${nodes[$i]}

declare initCompleteFile="init-complete-node$i.log"
rm -f "$initCompleteFile"
initCompleteFiles+=("$initCompleteFile")

startNode "$i" "$cmd $maybeExpectedGenesisHash"
if $addLogs; then
logs+=("$(getNodeLogFile "$i" "$cmd")")
fi

# 1 == bootstrap validator, wait until it boots before starting
# other validators
# wait for bootstrap validator to boot before starting other validators
Copy link
Copy Markdown

@steviez steviez Aug 5, 2025

Choose a reason for hiding this comment

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

nit: Maybe explicitly mention that bootstrap == index 0.

Also, this might have "worked" at one point. Passing initCompleteFile to waitForNodToInit is tricky because the value (if what integer is attached to the log file) is getting updated in the loop

Maybe we add a new variable to make the intent a little more clear. Something like:

if [[ "$i" -eq 1 ]]; then
    declare bootstrapInitCompleteFile="init-complete-node0.log"
    waitForNodeToInit "$bootstrapInitCompleteFile"
...

if [[ "$i" -eq 1 ]]; then
Copy link
Copy Markdown

@alexpyattaev alexpyattaev Aug 5, 2025

Choose a reason for hiding this comment

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

Maybe move this whole if block above the loop?

Copy link
Copy Markdown
Member Author

@yihau yihau Aug 5, 2025

Choose a reason for hiding this comment

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

would you mind explaining that in a bit more detail?

Copy link
Copy Markdown

@steviez steviez Aug 5, 2025

Choose a reason for hiding this comment

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

I'm not sure I follow either - the loop initializes all validators; we just need to do different stuff for validators > 0

Copy link
Copy Markdown

@roryharr roryharr Aug 5, 2025

Choose a reason for hiding this comment

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

I don't want to speak for what Alex was thinking, but if he means start the bootstrap validator first, separately and then start the rest, I agree with him.

Copy link
Copy Markdown

@roryharr roryharr Aug 5, 2025

Choose a reason for hiding this comment

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

I also don't want to bikeshed this while I'm seeing failures on 50% of my runs though. And would prefer we got something that worked in first.

Copy link
Copy Markdown

@alexpyattaev alexpyattaev Aug 5, 2025

Choose a reason for hiding this comment

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

yes i have meant exactly what Rory mentioned - that we set up bootstrap first to avoid specialcasing based on index. but it is good to have CI fixed thank you!

SECONDS=
waitForNodeToInit "$initCompleteFile"
Expand All @@ -180,6 +168,17 @@ startNodes() {
) | tee genesis-hash.log
maybeExpectedGenesisHash="--expected-genesis-hash $(tail -n1 genesis-hash.log)"
fi

declare cmd=${nodes[$i]}

declare initCompleteFile="init-complete-node$i.log"
rm -f "$initCompleteFile"
initCompleteFiles+=("$initCompleteFile")

startNode "$i" "$cmd $maybeExpectedGenesisHash"
if $addLogs; then
logs+=("$(getNodeLogFile "$i" "$cmd")")
fi
done

waitForAllNodesToInit
Expand Down