-
Notifications
You must be signed in to change notification settings - Fork 6
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
fix: clean up ssh agent after ourselves #51
base: main
Are you sure you want to change the base?
Conversation
Is this something you have observed in practice? |
Yes, this had reportedly happend in some scenarios. Although, now, on second thought, I doubt it since the discovery is the only persistent runner and it regularly would not set up a ssh connection to itself. Workers are regularly not persistent so they shouldn't suffer from dangling ssh agents. Only in the scenario that a worker is serviced by a hosted runner can those be left dangling. @michalrus do you happen to have more context on what we had observed, in this context? |
I don’t think this is the case, since if an
Yes, so we’re using NixOS on the discovery host, and the standard GitHub Runner service ( We’re not setting the So the But now that I think of it, I could just set |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it works, then it’s looking good =) Thanks!
@@ -31,6 +31,8 @@ if ssh-keygen -y -f "$ssh_key_file" &>/dev/null; then | |||
ssh-add -q "$ssh_key_file" && rm "$ssh_key_file" | |||
# Auth agent socket to ssh config | |||
echo "IdentityAgent $SSH_AUTH_SOCK" >> "$SSH_CONFIG_FILE" | |||
# Save pid to cleanup in post step | |||
echo "SSH_AGENT_PID=$SSH_AGENT_PID" >> "$GITHUB_STATE" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So later GHA export that automatically?
Context
The ssh connection to a discovery host leaves a dangling ssh agent.
If the workflow runner itself runs on a persistent runner, that process might just accumulate.
Solution
Clean up after ourselves. Unfortunately, only js actions allow to trigger a post action which is why a slight refactor needed to be done to wrap the bash action inside a js action.