Skip to content
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.

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

docker: Do not catchup if already initialized. #5756

Merged
merged 4 commits into from
Sep 28, 2023

Conversation

winder
Copy link
Contributor

@winder winder commented Sep 26, 2023

Summary

Avoid running fast catchup in docker containers which have already been initialized.

Test Plan

  1. Build docker container with these changes:
docker build \
           -t wwinder/algod:fast-catchup-fix  \
           --build-arg TARGETARCH=amd64 --no-cache \
           --build-arg BRANCH=will/docker-catchup-init \
           --build-arg URL=https://github.com/winder/go-algorand \
           .
  1. Run a container such that it initialize a local data directory
docker run --rm -it -p 8080:8080 --name algod-test-run \
           -e ADMIN_TOKEN=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa \
           -e TOKEN=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa \
           -e PROFILE=development \
           -e NETWORK=betanet \
           -e FAST_CATCHUP=1 \
           -v (pwd)/docker_fast:/algod/data \
           wwinder/algod:fast-catchup-fix
  1. Observe that fast catchup runs, and completes:
curl "localhost:8080/v2/status?pretty" -H "Authorization: bearer aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
  1. Shutdown the container, and start it back up again with the same command from step 2.
  2. Monitor status with command from step 3, it should not run fast-catchup.
  3. Shutdown the container, start it back up again with algorand/algod:3.18.0-stable.
  4. Monitor status with command from step 3, it should start fast-catchup (the bug).

@winder winder added the Bug-Fix label Sep 26, 2023
@winder winder force-pushed the will/docker-catchup-init branch from 8147317 to 91b5f58 Compare September 26, 2023 17:58
@codecov
Copy link

codecov bot commented Sep 26, 2023

Codecov Report

Merging #5756 (a7dd889) into master (ff82655) will decrease coverage by 0.02%.
Report is 3 commits behind head on master.
The diff coverage is 10.00%.

@@            Coverage Diff             @@
##           master    #5756      +/-   ##
==========================================
- Coverage   55.47%   55.46%   -0.02%     
==========================================
  Files         473      473              
  Lines       66682    66682              
==========================================
- Hits        36995    36984      -11     
- Misses      27175    27182       +7     
- Partials     2512     2516       +4     
Files Coverage Δ
cmd/goal/node.go 12.65% <50.00%> (ø)
libgoal/libgoal.go 2.44% <0.00%> (ø)
daemon/algod/api/server/v2/handlers.go 0.79% <0.00%> (ø)

... and 17 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@winder winder force-pushed the will/docker-catchup-init branch from 91b5f58 to 55538e6 Compare September 26, 2023 19:28
@winder winder marked this pull request as ready for review September 26, 2023 20:29
cmd/goal/node.go Outdated Show resolved Hide resolved
@winder winder requested a review from bbroder-uji September 27, 2023 13:32
@bbroder-uji
Copy link
Contributor

LGTM

algorandskiy
algorandskiy previously approved these changes Sep 27, 2023
cmd/goal/node.go Show resolved Hide resolved
Copy link
Contributor

@jannotti jannotti left a comment

Choose a reason for hiding this comment

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

How do we manage the releases so this doesn't get run against old algods?

@winder
Copy link
Contributor Author

winder commented Sep 27, 2023

@jannotti the docker image and algod are released and packaged together, so it isn't an issue.

algorandskiy
algorandskiy previously approved these changes Sep 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants