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

Add dockerfile #6

Merged
merged 8 commits into from
Mar 19, 2024
Merged

Add dockerfile #6

merged 8 commits into from
Mar 19, 2024

Conversation

smk762
Copy link
Contributor

@smk762 smk762 commented Mar 16, 2024

Explorer is currently mostly functional, though it appears to be not sunning on the same chain as the existing nodes, even with the addrbook.json file. I think this might be by design - VCS stamping possibly enforces unmodified binaries for joining the network. If so, we probably need to set an update height using whatever mechanism is used here.

The explorer is otherwise functional.

image

If it is preferable to not update the version yet, I'll create a new branch of explorer without the nucleus container and deploy/attach on an existing nucleus server to serve in the meantime anyway.

@smk762 smk762 marked this pull request as draft March 16, 2024 09:51
@smk762 smk762 requested a review from onur-ozkan March 18, 2024 18:40
@smk762 smk762 marked this pull request as ready for review March 18, 2024 18:40
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated
Comment on lines 9 to 10
RUN python3 ./init.py -b /nucleus/build/nucleusd -c /root/.nucleus --overwrite
COPY addrbook.json /root/.nucleus/config/addrbook.json
Copy link
Member

Choose a reason for hiding this comment

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

Not every use case requires init.py to be run. We should provide a plain nucleus daemon and leave the rest to the users. How about simply copying init.py (without running) into the container?

Also addrbook.json file can be moved into https://github.com/KomodoPlatform/nucleus-assets repository.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

copying init.py into container will mean needing to add some extra py related deps, and it wont work unless we also copy the app.toml template.

Copy link
Member

Choose a reason for hiding this comment

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

Right, maybe just copy the nucleusd binary and leave rest to the users. You can easily mount files/directories inside the container anyway.

As we providing the bare container for nucleus, I would pick one of debian images (like 1.22.1-bookworm) just to make install/configure parts of custom workflows easier for people.

Dockerfile Outdated
Comment on lines 14 to 16
COPY --from=builder /nucleus/build/nucleusd /usr/bin/nucleusd
COPY --from=builder /root/.nucleus /root/.nucleus
# COPY --from=builder /nucleus/nucleus_conf.yaml /root/.nucleus/config.yaml
Copy link
Member

Choose a reason for hiding this comment

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

We just need nucleusd and init.py from the builder.

Dockerfile Outdated
RUN apk update && apk add --no-cache make git gcc libc-dev linux-headers python3-dev
WORKDIR /nucleus
COPY . /nucleus
RUN rm -rf .git && git init
Copy link
Member

Choose a reason for hiding this comment

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

Why is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

VCS stamping fails if there is no .git folder, this was a workaround. It is because the .git is a file, not a folder when its a submodule.

Copy link
Member

Choose a reason for hiding this comment

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

Setting GOFLAGS=-buildvcs=false env should disable VCS.

Copy link
Contributor Author

@smk762 smk762 Mar 19, 2024

Choose a reason for hiding this comment

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

it does, but then later when it looks for the version stamp which is required, it fails. I think this is part of the "daemon version validation" consensus

Copy link
Member

Choose a reason for hiding this comment

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

I think this is part of the "daemon version validation" consensus

I wonder if initializing fresh git context will cause any problem. Maybe there's a better solution instead of removing the actual git state?

Dockerfile Outdated Show resolved Hide resolved
smk762 and others added 2 commits March 19, 2024 16:30
Co-authored-by: Onur Özkan <[email protected]>
Co-authored-by: Onur Özkan <[email protected]>
@smk762
Copy link
Contributor Author

smk762 commented Mar 19, 2024

I've removed the dockerfile and the addrbook.json files. These are only needed for a specific use case and can be dealt with or accommodated as required elsewhere.

The other branch I am working on does not include nucleus as a submodule https://github.com/smk762/nucleus-explorer/tree/v0.46.7-local

@onur-ozkan
Copy link
Member

I've removed the dockerfile and the addrbook.json files. These are only needed for a specific use case and can be dealt with or accommodated as required elsewhere.

I am happy to merge this PR without them, did you push the changes (they still exists in the PR)?

@onur-ozkan onur-ozkan merged commit d6f4c54 into main Mar 19, 2024
6 checks passed
@onur-ozkan onur-ozkan deleted the add-dockerfile branch March 19, 2024 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants