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

Fix snapshot location in Dockerfile #2201

Merged
merged 1 commit into from
May 9, 2022
Merged

Conversation

jonastheis
Copy link
Contributor

Description of change

After changing to WORKDIR /tmp in the Dockerfile, the default location for the snapshot ./snapshot points to an invalid location. Therefore, changing the location to /tmp/snapshot.bin should fix the problem and make it usable without specifying the path to the snapshot when starting the container.

@jonastheis jonastheis requested a review from karimodm May 9, 2022 08:03
Copy link
Contributor

@karimodm karimodm left a comment

Choose a reason for hiding this comment

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

But why was the workdir changed to /tmp ? This might break other things depending on how volumes are configured on the compose files.

Database locations for the resulting image are not copied over from the build enrvironment:

+# Fix permission issue when mounting volumes.
+COPY --chown=nonroot:nonroot --from=build /tmp/ /tmp/mainnetdb/
+COPY --chown=nonroot:nonroot --from=build /tmp/ /tmp/peerdb/
+
+WORKDIR /tmp
+USER nonroot
+

I think this is done simply to have those directories available and owned by nonroot when starting goshimmer.
I don't think it was necessary to do it this way, but at this point I think your change is the simplest to patch the issue.

@karimodm karimodm merged commit 5b77a64 into develop May 9, 2022
@karimodm karimodm deleted the fix/docker-snapshot-location branch May 9, 2022 14:14
This was referenced May 16, 2022
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