Add integration test for minikube ssh: stdin input

[joshmue]

Steps to reproduce the issue:

1. Download Minikube v1.11.0 as minikube_v1.11.0 (and v1.10.1 as minikube_v1.10.1 for reference).
2. Do ./minikube_v1.11.0 start (VirtualBox installed).
3. Then do:

prompt$ echo 'some test data' | ./minikube_v1.11.0 ssh -- cat -
some test data
some test data

The data is handled (twice?), but the process does not terminate.

For reference, the same command using minikube_v1.10.1 is working as expected:

prompt$ echo 'some test data' | ./minikube_v1.10.1 ssh -- cat -
some test data
prompt$

I assume this is a regression? Please ask if you need any more information.

[afbjorklund]

Seems to be related to 7224064, @sharifelgamal please take a look

 echo "hello world" | minikube ssh -- cat -
hello world
hello world
^C

When you revert that commit, it goes back to normal:

 echo "hello world" | ./out/minikube ssh -- cat -
hello world

However, the value of the --native-ssh doesn't seem to have any effect.

[afbjorklund]

The difference that was made, was the order between the calls:

ssh.SetDefaultClient

host.CreateSSHClient

That would be from github.com/docker/machine/libmachine/ssh

[afbjorklund]

However, the value of the --native-ssh doesn't seem to have any effect.

Basically the old code always used external, the new code always native.

--native-ssh=false
libmachine: Using SSH client type: external
--native-ssh=true
libmachine: Using SSH client type: external

--native-ssh=true
libmachine: Using SSH client type: native
--native-ssh=false
libmachine: Using SSH client type: native

And the native implementation seems to be broken (doesn't work).

[afbjorklund]

Possibly related to this bug: docker/machine#4806

hello world^M
hello world^M

Yup, looks familiar:

 echo hello world | docker-machine ssh default cat -
hello world
 echo hello world | docker-machine --native-ssh ssh default cat -
hello world
hello world
^C

[sharifelgamal]

So you're exactly right, there are 2 separate issues here:

• We're not honoring the native-ssh parameter so it's native no matter what
• The native ssh implementation end lines with windows style carriage returns for reasons I don't quite understand.

The first one is easy enough to fix, it was just a regression I introduced. The second one should be able to be fixed, at least on our fork of libmachine.

[afbjorklund]

Looks like some kind of bottomless hole, regarding MakeRaw and \r\n in terminals.

Like https://golang.org/cl/33902 and friends, and termios stuff like OPOST and ONLCR

[joshmue]

So you're exactly right, there are 2 separate issues here:

* We're not honoring the `native-ssh` parameter so it's native no matter what

* The native ssh implementation end lines with windows style carriage returns for reasons I don't quite understand.

The first one is easy enough to fix, it was just a regression I introduced. The second one should be able to be fixed, at least on our fork of libmachine.

Maybe it would be possible to fix the regression separate from the more complicated issues with native SSH?

Just asking because of the change of priority from "important-soon" to "important-longterm", while the effort required to fix just the regression appears to be quite small, as far as I understand. If I may be of any assistance (Open a separate issue; Open a PR attempting to fix the regression or similar), please say so.

[priyawadhwa]

Hey @joshmue if you would be interested in opening a PR that would be great!

Info about contributing: https://minikube.sigs.k8s.io/docs/contrib/

[joshmue]

Alright, I will take a look into it!

[medyagh]

this is still a bug,

I tried myself

medya@~/workspace/minikube (master) $ echo 'some test data' | minikube ssh --native-ssh=false -- cat -
some test data
some test data

^C
medya@~/workspace/minikube (master) $ echo 'some test data' | minikube ssh --native-ssh=true -- cat -
some test data
some test data

I am not sure what is the source of the bug, but if someone finds out why this is happening, we should add integration test so we never break this again !

( I suspect this is something about native ssh flag not being set correctly, once we tried to fix this and windows was broken)

[joshmue]

#9417 should fix the underlying bug. It does not contain an integration test, though.

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten