redhat-openstack / infrared

Plugin based framework that aims to provide an easy-to-use CLI for Ansible based projects
https://infrared.readthedocs.io/en/latest/index.html
Apache License 2.0
99 stars 97 forks source link

infrared ssh should not use -t ssh option by default #324

Open FedericoRessi opened 6 years ago

FedericoRessi commented 6 years ago

-t flag option makes ssh to behave a bit weird at the moment of capturing output produced by executed commands. It looks like it add '\r' character at every line. This is the cause of Issue #315

Is any special reason -t flag is used by default? Shouldn't the user be allowed to specify it manually instead when needed like below:

infrared ssh undercloud-0 -t <some command that requires a terminal>
yfried commented 6 years ago

According to ssh man: https://docs.oracle.com/cd/E36784_01/html/E36870/ssh-1.html

Forces pseudo-tty allocation. This can be used to execute arbitrary screen-based programs on a remote machine, which can be very useful, for example, when implementing menu services. Multiple –t options force allocation, even if ssh has no local tty.

On Thu, Apr 26, 2018, 20:44 Federico Ressi notifications@github.com wrote:

-t flag option makes ssh to behave a bit weird at the moment of capturing output produced by executed commands. It looks like it add '\r' character at every line. This is the cause of Issue #315 https://github.com/redhat-openstack/infrared/issues/315

Is any special reason -t flag is used by default? Shouldn't the user be allowed to specify it manually instead when needed like below:

infrared ssh undercloud-0 -t

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/redhat-openstack/infrared/issues/324, or mute the thread https://github.com/notifications/unsubscribe-auth/AFEIPTso-4iD67ypU6OwHTbxBrAX5YC0ks5tsgeLgaJpZM4TmWbk .

FedericoRessi commented 6 years ago

@yfried -t is indeed useful in some cases, but it should be an optional flag as it is for openssh client. The reason it is an optional parameter is that alternates stdout and stderr streams making it non optimal for piping commands inside bash.

ssbarnea commented 6 years ago

@FedericoRessi Can you please make an example that replicates the bug? As far as I know ssh option was implemented as an interactive option, more of "console", and thus is why -t was added.

Still if you can make an example that replicates the bug we should be able to find a way to fix it without breaking current users.

atalmor commented 6 years ago

@ssbarnea sorry for not point that out - the main issue with -t is that it changes the files when you try things like "ir ssh host "cat file" > localfile" it adds stuff to the file definition and then you must run "dos2unix" command or any equivalent in order to fix the file

for example, you can compare the outcome of: from your infrared dir: ir ssh undercloud-0 "cat overcloudrc" > overcloudrc ; file overcloudrc from hypervisor: ssh undercloud-0 "cat overcloudrc" > overcloudrc ; file overcloudrc