-
Notifications
You must be signed in to change notification settings - Fork 51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve handling of ssh-agent process #87
base: master
Are you sure you want to change the base?
Improve handling of ssh-agent process #87
Conversation
essentially attempt to spawn ssh agent independently and kill it when the action is over
doesn't seem like eval persists the environment variables
chore: add comment explaining why we regex output
Thanks for PR. Let's merge it but with little bit changes. |
Thanks for the consideration, what changes did you have in mind here? |
Different naming and default. Not running ssh=agent instead of killing. |
Do you have any suggestions for these?
The main reason I'm still running ssh-agent in my scenario is because deployerphp itself cannot read in a private key from a string and requires it to be a file on the system. I could put the "cleanup" step behind an additional flag so it only kills off the ssh-agent process if you pass through another option telling it to. I should note killing the ssh-agent process does cleanup the socket file as well. |
I'm just circling back to this PR after a bit, is this still something you'd be interesting in merging @antonmedv? |
Fixes #22
Currently I have the following added to all of my workflow files using the deployerphp action for our self hosted runners:
This works fine, however recently we had started having multiple runner instances on each of our runner servers and conflicts began to arise with my solution due to the usage of
killall
While I could have improved killing off the ssh-agent, I've decided to take a crack at implement a way of handling the
ssh-agent
process a little more nicely within the action iteself and have ended up with this.There is probably a slightly better way of doing this via a proper setting, however I was aiming for backwards compatibility where possible.
What this PR does:
Why make these changes: