Fix issue with ssh flags used with scp. #505

jraygauthier · 2016-09-08T03:56:10Z

As the '-p' flag attempt to preserve file attributes
with scp, it was making the scp command fail in
contexts where preserving attributes is not possible
(this is without mentionning the '22' argument that
followed and which seems to be accepted silently).

Here's the error it fixes:

afrontend> uploading key ‘my_secret.key’...
afrontend> scp: /run/keys/my_secret.key: No such file or directory
afrontend> error: command ‘['scp', '-P', '22', '-i', '/run/user/1000/nixops-tmp4MXxgW/id_nixops-afrontend', '-p', '22', '-oControlPath=/run/user/1000/nixops-ssh-tmpakFvsQ/master-socket', '/run/user/1000/nixops-tmp4MXxgW/key-afrontend', u'root@10.233.1.2:/run/my_secret.key']’ failed on machine ‘afrontend’ (exit code 1)
error: activation of 1 of 1 machines failed (namely on ‘afrontend’)

See the ['-P', '22'] part followed not long after by wrong ['-p', '22'].

Note that this is a prerequisite for #300

domenkozar · 2016-09-19T17:03:13Z

This doesn't call get_ssh_flags anymore, which is needed sometimes to pass the private ssh key

As the '-p' flag attempt to preserve file attributes with scp, it was making the scp command fail in contexts where preserving attributes is not possible (this is without mentionning the '22' argument that followed and which seems to be accepted silently).

jraygauthier · 2016-09-23T02:28:59Z

Good point. It seems that default options were working fine for the localhost case. Bringing back
get_ssh_flags uncovered supplementary arguments propagation errors in the container backend.
This one should do the trick.

domenkozar · 2016-12-06T14:52:11Z

nixops/backends/container.py

@@ -63,9 +63,9 @@ def get_ssh_flags(self, *args, **kwargs):
        flags = super(ContainerState, self).get_ssh_flags(*args, **kwargs)
        flags += ["-i", self.get_ssh_private_key_file()]
        if self.host == "localhost":
-            flags.extend(MachineState.get_ssh_flags(self))
+            flags.extend(MachineState.get_ssh_flags(self, *args, **kwargs))


Why is this needed?

so that MachineState.get_ssh_flags gets its expected scp flag as specified by the caller. Otherwise it will default to False.

I honestly find these var args parameters confusing, I'm not sure why there was a need for it in the first place but I tried to make do with it.

domenkozar · 2017-12-09T15:22:47Z

@jraygauthier could you rebase? I think we can merge this.

jraygauthier · 2017-12-12T02:16:02Z

@domenkozar This PR is no longer relevant now that #804 fixes the problem.

jraygauthier force-pushed the jrg/fix_issue_with_scp_flags branch from 605c09f to aa96792 Compare September 23, 2016 02:25

domenkozar reviewed Dec 6, 2016

View reviewed changes

AmineChikhaoui closed this Sep 3, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix issue with ssh flags used with scp. #505

Fix issue with ssh flags used with scp. #505

jraygauthier commented Sep 8, 2016

domenkozar commented Sep 19, 2016

jraygauthier commented Sep 23, 2016

domenkozar Dec 6, 2016

jraygauthier Dec 6, 2016

domenkozar commented Dec 9, 2017

jraygauthier commented Dec 12, 2017

Fix issue with ssh flags used with scp. #505

Fix issue with ssh flags used with scp. #505

Conversation

jraygauthier commented Sep 8, 2016

domenkozar commented Sep 19, 2016

jraygauthier commented Sep 23, 2016

domenkozar Dec 6, 2016

Choose a reason for hiding this comment

jraygauthier Dec 6, 2016

Choose a reason for hiding this comment

domenkozar commented Dec 9, 2017

jraygauthier commented Dec 12, 2017