Installs python-fixtures in Whonix VMs only when necessary #190
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
added 3 commits
November 5, 2018 20:51
The Whonix 14 templates lack `python-futures`, due to their dependency on an older version of the debian-9-minimal Template, so Salt cannot be used against them out of of the box. We must ensure the package is present in order to use Salt. Since updating apt lists can a while, especially over Tor, let's first check whether the package is present by trying to import it, and skip if so. Otherwise, we'll proceed with the apt list update and install command. The other significant change here is more explicit dependency declaration, so that the python-futures task is guaranteed to finish before any Salt-related tasks are run against the child VMs.
The proper syntax for Salt dependencies is:
require:
- <pkg>: <name>
Omitting the Salt package name was causing failures in setting up the
apt repo in the securedrop-workstation template. Resolved.
The default behavior of `qubesctl` is to mask details of command execution, for brevity. That masking makes debugging quite difficult, so let's display the output by default to developers.
emkll
approved these changes
Nov 6, 2018
There was a problem hiding this comment.
I have tried several times to reproduce the error described in #184 but was unable to.
Soon qubes 4.0.1 will be released that will likely solve this problem as whonix-14 templates are included by default[0], and we will be able to use those as a base.
make all and make test do indeed complete successfully, and a visual review of this diff does indeed make the install logic more verbose, robust and less failure-prone. Unfortunately I cannot validate the fix functionally since I cannot reproduce the error, but will not immediately merge so that others which were experiencing the issue can chime in.
6 tasks
|
I just experienced this issue right now, and can confirm this fixes the issue. Thanks @conorsch, merging! |
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.
The
qvm-runcommand ensuring thatpython-fixtureswas present in the Whonix templates was flakey, causing several developers (myself included) to omit it when testing locally to avoid problems. Made it more stable by checking whether the library is available before running the apt commands.Also finetuned the dependency declaration in the related Salt commands, so that python-fixtures is always present before Salt commands are run against the associated VMs. Increased verbosity of the Salt output, as well, since the default behavior was masking highly informative error output.
Closes #184.
Testing
make allin dom0, confirm no errorsmake testin dom0, confirm no errors.If you see failing tests in step 2 regarding uninstalled updates, run
sudo securedrop-update, then re-runmake test.