Fix #803 containers keys upload #804
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
…S#803. This defaulted to True even though the nixops changelog for version 1.2 says that it was changed to False for security reasons.
In the same fashion as in commit 985886c, container.py did not call `set_common_state()`, so keys remained set to `{}` (and other attributes to their default values likely as well). `get_ssh_flags()` was incorrectly called twice, and the second time with missing `**kwargs` thus leading to `scp` getting incorrect parameters, such as `-p 22`, which is not a valid flag for scp, which made uploading keys fail. The failure error message was also very bad, I filed an OpenSSH issue for scp at https://bugzilla.mindrot.org/show_bug.cgi?id=2809. Finally, the code that set the home directory in container.py's `run_command()` assumed that the passed command would be a single command that can be prefixed like `HOME=/root ...command...` which is not the case; the command may as well be a `(subshell)`, in which case this failed (nixops uses a subshell for keys permission settings to save roundtrips). Fixed with an `export` and semicolon.
This has the same effect, but makes it be printed more obviously when debugging.
nh2
force-pushed
the
fix-803-containers-keys-upload
branch
from
1b65a9f
to
e041873
Compare
kevincox
approved these changes
Dec 8, 2017
|
Overall looks good. Pinging @johbo about the |
|
Let's merge this, we can add send_keys() also with another PR. This chunk looks good to me. @mbld ping |
This was referenced Dec 9, 2017
@domenkozar @johbo @kevincox Hmm, for me it seems to have worked without an extra call to |
|
If I remember correctly, then the call to |
Ah, I see. I haven't tried |
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.
This fixes the multiple issues I've found in #803 about stuff being broken in
containers.py.See commit messages for details.