Adds numerous bug fixes for remote cross. #922
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Adds simpler container and volume cleanup, and avoiding keeping anonymous volumes for `/cargo/bin` present after the container exited. It also ensures containers are stopped even if a signal is sent to the `cross` command (except `SIGKILL`), and ensures that containers exit quicker if possible. This is done by using the presence of a TTY, and if it is present, running the default entrypoint, which responds to signals. If we do not have a TTY, the image will not respond to signals, so we send the container a `SIGKILL` via `docker stop` using a an instant timeout (0). This also fixes an issue with stale fingerprint data: if a persistent data volume was deleted and then recreated, previously the project data would not be copied over. This meant that the project could not be found, and it would require users to manually delete the fingerprint data or use single-used volumes. This also checks if the container has been scheduled for deletion (the drop may not have completed), and errors out early if more calls are made to the container. This also properly handles `docker rm` calls if the `docker stop` command worked without needing a `SIGKILL`: it silences the output of the error if the container wasn't found.
|
There's a lot of comments but ~80% of them are comments, just to document clearly the behavior that's going on, the bugs we aim to prevent, etc. |
|
Build succeeded: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds simpler container and volume cleanup, and avoiding keeping anonymous volumes for
/cargo/binpresent after the container exited. It also ensures containers are stopped even if a signal is sent to thecrosscommand (exceptSIGKILL), and ensures that containers exit quicker if possible. This is done by using the presence of a TTY, and if it is present, running the default entrypoint, which responds to signals. If we do not have a TTY, the image will not respond to signals, so we send the container aSIGKILLviadocker stopusing a an instant timeout (0).This also fixes an issue with stale fingerprint data: if a persistent data volume was deleted and then recreated, previously the project data would not be copied over. This meant that the project could not be found, and it would require users to manually delete the fingerprint data or use single-used volumes.
This also checks if the container has been scheduled for deletion (the drop may not have completed), and errors out early if more calls are made to the container.
This also properly handles
docker rmcalls if thedocker stopcommand worked without needing aSIGKILL: it silences the output of the error if the container wasn't found.