Skip to content

install.sh: Post-verification cleanup (terminate gpg-agent)#1030

Closed
jayaddison wants to merge 5 commits into
yarnpkg:masterfrom
jayaddison:gpg-verification-cleanup
Closed

install.sh: Post-verification cleanup (terminate gpg-agent)#1030
jayaddison wants to merge 5 commits into
yarnpkg:masterfrom
jayaddison:gpg-verification-cleanup

Conversation

@jayaddison

@jayaddison jayaddison commented Dec 6, 2019

Copy link
Copy Markdown

This changeset terminates any gpg-agent processes created as side-effects during the install.sh script.

If existing gpg-agent processes were found before GPG functionality is invoked in the script, those existing processes will be left untouched so as not to affect unrelated system/user functionality.

Fixes #1029

@DanBuild

DanBuild commented Dec 6, 2019

Copy link
Copy Markdown
Contributor

Deploy preview for yarnpkg failed.

Built with commit 4463c83

https://app.netlify.com/sites/yarnpkg/deploys/5dee5a3904b544000807d163

@jayaddison

Copy link
Copy Markdown
Author

cc @BanzaiMan - this may help with the issues re: GPG subprocesses you've seen user feedback about (ref: travis-ci/travis-ci#8082 (comment)). It's important to make sure this PR doesn't cause side-effects elsewhere though.

@jayaddison jayaddison closed this Dec 6, 2019
@jayaddison jayaddison reopened this Dec 6, 2019
@jayaddison

Copy link
Copy Markdown
Author

gpg-connect-agent --no-autostart also looked like a possibility to determine if the agent is current running.

However, trying that with version 2.2.12 of gpg-connect-agent, the agent is still started by a reset process (confirmed via strace), so it may not be reliable.

Comment thread install.sh Outdated
@Daniel15

Daniel15 commented Dec 6, 2019

Copy link
Copy Markdown
Member

Seems reasonable to me. Does pgrep come preinstalled on minimal installs of popular Linux distros (Debian, Ubuntu, CentOS)?

@jayaddison

Copy link
Copy Markdown
Author

@Daniel15 Good question, perhaps there's an authority / comparison on which Linux distros it's available under; I'm taking a bit of a look around just now.

@jayaddison

jayaddison commented Dec 6, 2019

Copy link
Copy Markdown
Author

While searching around elsewhere, here's whether pgrep is available in each of the latest Docker Hub official images for Centos, Debian and Ubuntu:

Alpine

/ # cat /etc/alpine-release 
3.10.3
/ # which pgrep
/usr/bin/pgrep
/ #

CentOS

sh-4.4# cat /etc/centos-release
CentOS Linux release 8.0.1905 (Core) 
sh-4.4# whereis pgrep
pgrep: /usr/bin/pgrep

Debian:

# cat /etc/debian_version
10.2
# which pgrep
#

Ubuntu

# cat /etc/debian_version
buster/sid
# which pgrep
/usr/bin/pgrep
#

So Debian does not seem to have pgrep by default.

Edit - add sources:

@jayaddison

Copy link
Copy Markdown
Author

@Daniel15 Hrm. I'm not finding many resources to compare command-line tooling per-distro; it'd probably take a bit more research. Something like caniuse for command-line tools would be wonderful.. 🙏

One option to handle cases like Debian could be to fall-back to alternatives like the killall command. It doesn't look like yarnpkg currently supports Solaris or other non-Linuxes where that command can be a bit more dangerous, so perhaps that's fine (or even a better alternative than using pkill in the first place).

Whatever approach, I'm keen to be a bit cautious, since this script seems fairly important infrastructure-wise :)

@jayaddison

Copy link
Copy Markdown
Author

@Daniel15 @BanzaiMan Just for transparency; it looks likely that this wasn't the true root cause of Travis CI timeout issues seen in lighthouse during Linux builds.

It still seems useful to clean-up non-install side-effects of the script, so there may still be some value here, but I think the importance & impact of this PR are low.

Glad for any thoughts, and I'll gradually continue on to see if there are more cross-platform alternatives to pgrep and pkill, but there's no urgency (and I don't think it's a magic solution to any other customer issue reports).

@jayaddison

Copy link
Copy Markdown
Author

Investigating pidof and killall as alternatives to pgrep and pkill, respectively:

Alpine

/ # cat /etc/alpine-release 
3.10.3
/ # command -v pidof
/bin/pidof
/ # command -v killall
/usr/bin/killall
/ #

Centos

sh-4.4# cat /etc/centos-release
CentOS Linux release 8.0.1905 (Core) 
sh-4.4# command -v pidof
/usr/sbin/pidof
sh-4.4# command -v killall
sh-4.4#

Debian

# cat /etc/debian_version
10.2
# command -v pidof
/bin/pidof
# command -v killall
# 

Ubuntu

# cat /etc/debian_version
buster/sid
# command -v pidof
/bin/pidof
# command -v killall
# 

Findings:

  • pidof is a widely available alternative as a fallback from pgrep. Note that pidof is not available on Mac OS X, so we should still try both.
  • killall is not available widely; it may be better to rely on gpgconf --kill gpg-agent for gpg-agent termination since it is bundled with the gpg toolset

@jayaddison

Copy link
Copy Markdown
Author

@Daniel15 This turned out not to be the cause of the Travis build failures I was seeing; instead it looks like that was caused by slow cache restoration - so I'll go ahead and close this PR.

@jayaddison jayaddison closed this Jan 15, 2020
@Daniel15

Copy link
Copy Markdown
Member

Thanks for the update... Feel free to ping me again if you'd like to reopen this at some point.

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.

install.sh script may not clean up gpg-agent processes

3 participants