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: Fix sapphire-localnet startup on Mac Docker #593

Merged
merged 1 commit into from
Jul 9, 2024

Conversation

abukosek
Copy link
Contributor

@abukosek abukosek commented Jul 9, 2024

This PR fixes sapphire-localnet startup in Docker on Mac, as reported by @lubej.

@lubej
Copy link
Contributor

lubej commented Jul 9, 2024

Closes #592

Copy link
Member

@kostko kostko left a comment

Choose a reason for hiding this comment

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

Does something related to version determination fail? What is the "version line" output on the failing machine?

@lubej
Copy link
Contributor

lubej commented Jul 9, 2024

Does something related to version determination fail? What is the "version line" output on the failing machine?

image

@kostko
Copy link
Member

kostko commented Jul 9, 2024

Thanks, but can you also show the exact output of the "version line" after you run the Docker image with the proposed workaround from this PR.

@lubej
Copy link
Contributor

lubej commented Jul 9, 2024

Thanks, but can you also show the exact output of the "version line" after you run the Docker image with the proposed workaround from this PR.

There was no version line, it failed beforehand.

@kostko kostko added the bug Something isn't working label Jul 9, 2024
@kostko
Copy link
Member

kostko commented Jul 9, 2024

As mentioned in the comment, what is the output with the proposed fix from this PR.

@abukosek abukosek force-pushed the andrej/bugfix/mac-docker-fix branch from e1e29a2 to 7b62c9a Compare July 9, 2024 11:35
@abukosek abukosek force-pushed the andrej/bugfix/mac-docker-fix branch from 7b62c9a to ea76d83 Compare July 9, 2024 11:54
@lubej
Copy link
Contributor

lubej commented Jul 9, 2024

As mentioned in the comment, what is the output with the proposed fix from this PR.

Here is the output(Docker image build by @abukosek, and tested on my machine):

image

@abukosek
Copy link
Contributor Author

abukosek commented Jul 9, 2024

Here is what I think was the problem and why just moving set -euo pipefail a few lines down is a good enough solution.

The initial problem was that when running the image on Docker Mac on a machine with Apple M silicon (with the appropriate --platform linux/x86_64 flag), the startup script (/start.sh) immediately exited with error code 141, which means that it received a SIGPIPE.

We narrowed this down to the following line in the script:

OASIS_WEB3_GATEWAY_VERSION=$(${OASIS_WEB3_GATEWAY} -v | head -n1 | cut -d " " -f 3 | sed -r 's/^v//')

Sometimes it looked like head failed or cut failed or even sed, but it was all random.
It makes no sense that these commands would fail, since the Docker image is the same as on Linux and there it works perfectly.

However, on Apple M silicon, Docker images run in an emulation layer and run more slowly.

Normally, this wouldn't be a problem, but in this case, the output from oasis-web3-gateway -v has multiple lines, but with head -n1, we only take the first one.
If the emulation is slow, the oasis-web3-gateway might still be outputting the version information, while head -n1 has already taken the first line and closed the pipe. Closing the pipe while the writing end isn't done yet results in a broken pipe.
Since we have pipefail enabled, this causes the entire startup script to fail with error 141.

This also explains why the error was so random -- sometimes the process managed to output everything on time and exited cleanly, but sometimes it was interrupted by the broken pipe and brought down the entire script with it.

So, moving the set -euo pipefail command below these initial lines that extract the version etc. is a good enough solution, since we only care about the first line of the version output and don't care if the process isn't done writing the rest of the version information, as we don't need it anyway.

@abukosek abukosek merged commit 9764351 into main Jul 9, 2024
6 checks passed
@abukosek abukosek deleted the andrej/bugfix/mac-docker-fix branch July 9, 2024 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants