-
Notifications
You must be signed in to change notification settings - Fork 0
IA1.2 Generate dockers for services #182
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
base: celo-integration-rebase-13.1
Are you sure you want to change the base?
Conversation
@@ -0,0 +1,112 @@ | |||
# OP Stack Dockerfile, simplified from ops/docker/op-stack-go/Dockerfile |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ancient123 Following the pattern in op-stack-go/Dockerfile, I used this file for the OP node, batcher, and proposer, instead of creating one Dockerfile for one service--is this okay?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fine.
Will the other services (Espresso node, Caff node) be implemented in another PR? |
mv "$GETH_DIR/geth" /usr/local/bin/geth && \ | ||
rm -rf geth.tar.gz "$GETH_DIR" && \ | ||
chmod +x /usr/local/bin/geth | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should check the hash GETH_SHA here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, we do! It's located around 10+ lines above, the command starting from GETH_SHA=
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, but this variable GETH_SHA
does not seem to be used anywhere? Should not we compute the hash of the tar.gz file against GETH_SHA
or something similar?
|
||
# Rust builder for Espresso crypto libraries | ||
FROM --platform=$BUILDPLATFORM rust:1.84.1-alpine3.20 AS rust-builder | ||
ARG ESPRESSO_NETWORK_GO_VER=0.0.34 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a TODO to check the hash of the espresso go library when we switch to the new one. cc @dailinsubjam
healthcheck: | ||
test: ["CMD", "curl", "-f", "http://localhost:8545"] | ||
interval: 3s | ||
timeout: 2s | ||
retries: 40 | ||
build: | ||
context: ../ops/docker/deployment-utils | ||
context: ./dockerfiles/l1-geth | ||
image: l1-geth:espresso | ||
volumes: | ||
- ../config/l1-genesis-devnet.json:/l1-genesis-devnet.json:ro |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pasting @Ancient123's comment from Slack:
when doing mounts at runtime don't mount individual files. If the file gets rewritten on the host machine, it won't take affect on the docker, because it can hold onto an older handle for the previous file.
Is this expected? |
@philippecamacho The Espresso dev node uses the Espresso pre-built image, so it doesn't need a Dockerfile. The Caff node uses the same target as the OP node does, so we don't need to specify a Dockerfile for it separately. |
@philippecamacho Not expected! I'll investigate this. |
Made a separate task for this issue because it also occurred on the default branch: https://app.asana.com/1/1208976916964769/project/1209392461754458/task/1210618856384949?focus=true. Will fix it in a separate PR. |
op-geth-init: | ||
image: us-docker.pkg.dev/oplabs-tools-artifacts/images/op-geth:v1.101503.2-rc.3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is helpful!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Volumes will all need to be deployed in EFS, heads up that that's decidedly nontrivial. We can talk about that.
- op-geth-data:/data | ||
entrypoint: "" | ||
command: > | ||
sh -c " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might be really annoying in terraform. I'd recommend checking how arbitrary scripts work in there. Ideally we'd just make this part of the container instead, so we can fork their container and then add this as a setup step (I think).
Closes https://app.asana.com/1/1208976916964769/project/1209392461754458/task/1210546932882749?focus=true.
This PR:
op-geth-init
service to avoid manual initialization with the genesis file.How to test this PR:
docker compose up --build -d
to build.docker compose logs -f
to run all services.Docker Compose
section inREADME_ESPRESSO.md
.