Skip to content
This repository has been archived by the owner on Nov 29, 2023. It is now read-only.

Override Docker ENTRYPOINT so that PID 1 inside the container is correctly assigned #571

Merged
merged 1 commit into from
Aug 4, 2023

Conversation

jnm
Copy link
Member

@jnm jnm commented Jul 12, 2023

pm2-runtime will now run as PID 1 as intended in #533. See also enketo/enketo#960.

I have verified this PR works with

  • Online form submission
  • Offline form submission
  • Saving offline drafts
  • Loading offline drafts
  • Editing submissions
  • Form preview
  • None of the above
    (…because no application code has changed. If anyone feels they're warranted, however, I'm happy to carry out the tests.)

What else has been done to verify that this works as intended?

Verified pm2-runtime as PID 1 within the container using the following method:

john@world$ git show --oneline --no-patch
9f4d9aa0 (HEAD -> override-docker-entrypoint, origin/override-docker-entrypoint) Override Docker ENTRYPOINT so that PID 1 inside…
john@world$ docker build -t ee-test-entrypoint .
<snip>
Successfully tagged ee-test-entrypoint:latest

john@world$ docker run --rm --name ee-test-entrypoint ee-test-entrypoint &
[1] 428179
john@world$ 2023-07-12T22:55:19: PM2 log: Launching in no daemon mode
2023-07-12T22:55:19: PM2 log: App [enketo:0] starting in -fork mode-
2023-07-12T22:55:19: PM2 log: App [enketo:0] online
Worker 1 ready for duty at port 8005! (environment: production)
Worker 2 ready for duty at port 8005! (environment: production)

john@world$ docker exec -it ee-test-entrypoint ps x
    PID TTY      STAT   TIME COMMAND
      1 ?        Ssl    0:00 node /usr/local/bin/pm2-runtime app.js -n enketo
     19 ?        Ssl    0:00 node /srv/src/enketo_express/app.js
     72 ?        Sl     0:00 node /srv/src/enketo_express/app.js
     73 ?        Sl     0:00 node /srv/src/enketo_express/app.js
    116 pts/0    Rs+    0:00 ps x

Why is this the best possible solution? Were any other approaches considered?

The CMD instruction could've been modified instead of switching to ENTRYPOINT, but even if that did work, it would rely on a convoluted execution path that depends on some peculiar behavior of the node:14 base image.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

Systems administrators should notice that Enketo Docker containers exit cleanly and more quickly. I can't foresee any negative consequences. The change should have no impact on end users.

Do we need any specific form for testing your changes? If so, please attach one.

No.

@lognaturel
Copy link
Contributor

Thanks! I see this and will aim for a deeper review soon. I'd like to understand entrypoint vs cmd a little bit better.

Copy link
Contributor

@eyelidlessness eyelidlessness left a comment

Choose a reason for hiding this comment

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

Thanks! I'm pleased that this finally allowed me to interrupt the container with ^C 🙃

@eyelidlessness eyelidlessness merged commit df8057b into master Aug 4, 2023
5 checks passed
@jnm jnm deleted the override-docker-entrypoint branch August 7, 2023 16:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants