Skip to content
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

[#55176] Cleanup Rails's tmp/pid/server.pid file for docker based development #15621

Conversation

wielinde
Copy link
Member

@wielinde wielinde commented May 21, 2024

https://community.openproject.org/work_packages/55176

When developing with Rails it happens frequently that a PID file does not get cleaned up. Debugging why a local dev environment did not boot slows down development and increases developer frustration.

This PR aims to ensure that all former PID files are gone when calling docker compose up -d backend next time.

Implementation details:

There are multiple approaches on how to make that happen.

  • @oliverguenther for example chose a solution that removes such files from within the docker/prod/web file. That was inspired by an existing solution in docker/prod/entrypoint.sh
  • At least for development I prefer the solution promoted in this blog post, as it hands over the complexity of having temporary files to specialized volumes (tmpfs, which lives in memory only), makes it transparent in the docker-compose.yml file and also allows individual developers to easily override it at need.

Copy link
Member

@Kharonus Kharonus left a comment

Choose a reason for hiding this comment

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

Still have some questions open, but I appreciate the approach. removing the server.pid manually is a nuisance.

docker-compose.yml Show resolved Hide resolved
docker-compose.yml Show resolved Hide resolved
@klaustopher
Copy link
Contributor

Really cool approach, learned about tmpfs for Docker compose. 👍

One question, why is this PR against the release branch?

@Kharonus
Copy link
Member

True, right now the target should be dev

@wielinde
Copy link
Member Author

One question, why is this PR against the release branch?

Because we run into that problem also when debugging or bug fixing on the release branch. So my intention is to get rid of the problem as soon as possible. And it only affects the developer setup.

@wielinde
Copy link
Member Author

A question to the Mac users: Does tmpfs work for you? I just read here that this is only available for Linux hosts.

@klaustopher
Copy link
Contributor

Let me check

@klaustopher
Copy link
Contributor

klaustopher commented May 23, 2024

$ docker-compose exec backend sh
WARN[0000] The "OPENPROJECT_DEV_URL" variable is not set. Defaulting to a blank string.

$ echo $PIDFILE
/home/dev/openproject/tmpfs/pids/server.pid

$ cat /home/dev/openproject/tmpfs/pids/server.pid
1

$ ps aux | grep puma
dev          1  2.4  7.3 2853212 589088 pts/0  Ssl+ 13:53   0:11 puma 6.4.2 (tcp://0.0.0.0:3000) [openproject]
dev         61  0.0  0.0   4796  1920 pts/1    S+   14:01   0:00 grep puma

$ kill $(cat $PIDFILE)
$ %

This worked and after stopping with Ctrl+C, I did not have a PIDfile remain


Also checked to run docker directly with --tmpfs option and this also properly worked:

$ docker run -it --tmpfs /foo alpine sh
/ # ls /foo
/ # touch /foo/hello.txt
/ # ls /foo
hello.txt
/ # exit

Copy link
Member

@Kharonus Kharonus left a comment

Choose a reason for hiding this comment

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

LGTM

@wielinde wielinde merged commit a32d4c6 into release/14.1 May 28, 2024
4 checks passed
@wielinde wielinde deleted the implementation/55176-cleanup-railss-tmppidserverpid-file-for-docker-based-development branch May 28, 2024 07:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants