-
Notifications
You must be signed in to change notification settings - Fork 174
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
Lightnode docker #726
Lightnode docker #726
Conversation
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
lightnode/docker/default-args.sh
Outdated
export GIT_URL=https://github.com/Layr-Labs/eigenda.git | ||
|
||
# The name of the branch or the commit sha to clone. | ||
export BRANCH_OR_COMMIT=master |
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.
Would it make sense to use the commit that is currently checked out? Curious about the thought process 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.
I'm open to that idea, but we should weigh pros and cons.
This build process clones the repo into the docker image, as opposed to using the copy on the host file system. This adds extra overhead. The reason I took this step was that I wanted to avoid uncommitted local changes accidentally making their way into the resulting docker file, as well as requiring an explicit choice when it comes to choosing which version to build. But there is certainly additional friction to the developer in doing it this way. Let's discuss.
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.
Per our discussion, I've changed this so that it instead builds against the code that is checked out locally (as opposed to cloning the code inside the docker file).
Signed-off-by: Cody Littley <[email protected]>
In general, are we going to publish these docker builds as part of releases? If so, we should consider signing the containers and GHCR does enable this. |
I'm guessing the answer is "yes", although I haven't spent much time yet thinking about releases. Would it be ok with you if we addressed this as a stand alone task? This docker image will be useful during testing as we do development work. |
Yep thats fine. |
lightnode/docker/Dockerfile
Outdated
|
||
# Download all go dependencies and build the binary. | ||
WORKDIR /home/lnode/eigenda/lightnode | ||
RUN go mod download |
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.
I think a worthwhile optimization would be the following sequence:
- copy go.mod
- Run
go mod download
- Copy the remainder of source
- Run build
This would prevent having to redownload the go modules every time the source changes (as long as modules don't change)
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.
Very good suggestion. Change made.
Signed-off-by: Cody Littley <[email protected]>
Why are these changes needed?
Create a docker image suitable for running the light node on a raspberry pi.
Checks