Skip to content

[Bug] Unable to catch signals using Docker #1741

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

Closed
1 task
DRoet opened this issue Aug 23, 2020 · 12 comments · Fixed by #2347
Closed
1 task

[Bug] Unable to catch signals using Docker #1741

DRoet opened this issue Aug 23, 2020 · 12 comments · Fixed by #2347
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@DRoet
Copy link
Contributor

DRoet commented Aug 23, 2020

  • I'd be willing to implement a fix

Describe the bug

To perform a graceful shutdown of containers we need a way to catch the signals (SIGTERM/SIGINT) that Docker sends when shutting down a container. In yarn v1 the workaround was to use node <script> directly instead of yarn run. However I can't seem to get this to work using yarn node <script> or yarn run in v2 with PnP enabled.

To Reproduce

Use a minimal Dockerfile:

FROM node:12.18.3

WORKDIR /opt/app

COPY . /opt/app

RUN yarn install

CMD ["yarn", "node", "./src"]

Inside src/index.js, listen for signals

const signals = [SIGHUP, SIGINT, SIGTERM]
for (const signal of signals) {
   process.on(signal, () => {
      console.log(`process received a ${signal} signal`)
   })
}

Then when the container kill it using:

docker kill -s SIGTERM <container name>

Environment if relevant (please complete the following information):

  • OS: Docker
  • Node version 12.18.3
  • Yarn version 2.1.1

Additional context

I also tried using docker-init/tini to play around the PID 1 restriction after reading #991 but that doesn't seem to work either. It would be nice if there is some way to forward signals to the child processes so that yarn v2 works nicely with Docker.

@DRoet DRoet added the bug Something isn't working label Aug 23, 2020
@merceyz
Copy link
Member

merceyz commented Aug 23, 2020

As a workaround you can use

node -r ./.pnp.js ./src

@arcanis
Copy link
Member

arcanis commented Aug 23, 2020

Can you try master (yarn set version from sources)? Should be fixed with #1622

@DRoet
Copy link
Contributor Author

DRoet commented Aug 23, 2020

@arcanis hmm no unfortunately it still doesn't work using master (2.1.1-git.20200822.133f4025)

@arcanis
Copy link
Member

arcanis commented Aug 23, 2020

I confused it with #991, which you mentioned in your post, sorry. I think we intended to forward the signals, but it slipped past our radar after the issue got closed. If you're interested to add this logic, I'd be happy to review that. It shouldn't be too hard, there are only two places where we spawn child processes:

https://github.com/yarnpkg/berry/search?q=siginthandler&unscoped_q=siginthandler

@kevin-lindsay-1
Copy link

Was this ever fixed? I'm using yarn node ./script.js and I'm not seeing SIGTERM, but when I run node -r ./.pnp.cjs ./script.js it works. Wish the docs would have mentioned this if it was never fixed.

@merceyz
Copy link
Member

merceyz commented Oct 6, 2021

Yes, see #2347 (comment)

@kevin-lindsay-1
Copy link

@merceyz so then, no, it wasn't fixed? That, or it's not planned to be fixed.

@merceyz
Copy link
Member

merceyz commented Oct 6, 2021

Not sure how you interpreted my "Yes" to mean "no" but to be specific:
The issue was fixed and released in 3.0.0 and 1.22.11, your global and local Yarn version must satisfy ^1.22.11 || >= 3.0.0

@kevin-lindsay-1
Copy link

@merceyz I interpreted it as "no" because you pointed me to a specific comment that doesn't say it's fixed, it calls out a workaround, and the comments afterwards state they're still having issues. /shrug

@merceyz
Copy link
Member

merceyz commented Oct 6, 2021

It was this part of that comment that was of interest

If you use yarn 1 to spawn yarn 2, note that bumping just v2 to sources isn't enough, because v1 has the same problem:
yarnpkg/yarn#8543

Following that leads you to yarnpkg/yarn#8543 (comment)

@kevin-lindsay-1
Copy link

@merceyz ah, I see. I looked at the docker image from node, and yeah it uses v1.22.5, so that was my issue. Thanks!

@bertho-zero
Copy link

bertho-zero commented Oct 26, 2023

@merceyz I have this issue with Yarn v4.0.0 + Yarn 1.22.19

edit: I'm using yarn workspace <package> <script> and I'm not getting the signal

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants