Skip to content

Conversation

@hokadiri
Copy link
Contributor

@hokadiri hokadiri commented May 19, 2016

I ran into an issue lately where I was doing parallel executions on a lot of nodes. By default, in paramiko, ssh-agent is used during the authentication process. I kept getting the lost ssh-agent error below because ssh-agent couldn't handle the load

paramiko.SSHException: lost ssh-agent

To reproduce the issue, try running this ensure you have ssh-agent running. Run the below command

from paramiko import Agent
for i in range(1000):
    agent0=Agent()
    agent1=Agent()
    agent2=Agent()
    ...
    agent98=Agent()
    agent99=Agent()
    agent100=Agent()

Get PID for the ssh-agent socket from the SSH_AUTH_SOCK environment variable

$ echo $SSH_AUTH_SOCK
/tmp/ssh-RFhnS12345/agent.12345

Watch the file descriptors count increase!

$ sudo strace -p 12345 -eselect

And eventually crash with the Exception

SSHException: lost ssh-agent

I want to be able to use username/password OR priv key authentication and I don't want ssh-agent used for authentication. Setting the allow_agent variable in paramiko.SSHClient.connect function to False allows me to disable ssh-agent from being used during authentication.

This PR is to allow us set the allow_agent variable in pssh's ParallelSSHClient & SSHClient and pass it down to paramiko.SSHClient.connect function where its being used

…iko client) in ParallelSSHClient & SSHClient
@coveralls
Copy link

coveralls commented May 19, 2016

Coverage Status

Coverage increased (+3.3%) to 86.744% when pulling bb9de60 on hokadiri:master into 4356eb1 on pkittenis:master.

therefor any connection and/or authentication exceptions will happen on the
call to ``run_command`` and need to be caught.
"""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These indentations are there purposefully as they are used when generating docstrings.

Please revert all indentation changes in this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pkittenis, i've reverted the indentations.

@coveralls
Copy link

coveralls commented May 19, 2016

Coverage Status

Coverage increased (+4.4%) to 87.896% when pulling eb83981 on hokadiri:master into 4356eb1 on pkittenis:master.

@pkittenis
Copy link
Member

pkittenis commented May 19, 2016

Hi there,

Thanks for the interest and the PR - great investigation.

Made some comments on the diff for review. The new flag functionality also needs a short unittest. Looks great other than that!

Out of interest, what is the pool_size being used and how was the performance?

@hokadiri
Copy link
Contributor Author

@pkittenis I've added a test case for the allow_agent variable. My pool size was 150. Performance was ok, no noticeable slowness

@coveralls
Copy link

coveralls commented May 19, 2016

Coverage Status

Coverage increased (+1.8%) to 85.303% when pulling 7c83a08 on hokadiri:master into 4356eb1 on pkittenis:master.

"""Wrapper class over paramiko.SSHClient with sane defaults
Honours ~/.ssh/config and /etc/ssh/ssh_config entries for host username \
overrides"""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please revert indentation changes in this file as well - thanks.

@coveralls
Copy link

coveralls commented May 20, 2016

Coverage Status

Coverage decreased (-1.6%) to 83.573% when pulling bdccc58 on hokadiri:master into 4356eb1 on pkittenis:master.

@coveralls
Copy link

coveralls commented May 20, 2016

Coverage Status

Coverage increased (+3.0%) to 88.184% when pulling b36f468 on hokadiri:master into 4356eb1 on pkittenis:master.

@pkittenis pkittenis merged commit e57462a into ParallelSSH:master May 20, 2016
@pkittenis
Copy link
Member

Looks great! Thanks again 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants