Update docker-entrypoint template to avoid failing container restarts

[hschne]

Motivation / Background

Rails provides great templates for running applications with Docker. However, when running Rails apps with the default docker-entrypoint, containers may fail to restart if they crashed (e.g. because of OOM). This is becuase the /rails/tmp/pids/server.pid file is already present.

Such failed containers will require manual intervention (e.g. remove the faulty container) to get up-and-running again. To allow containers to restart the server.pid file must be removed on container start.

This Pull Request has been created to incorporate this improvement into the default docker-entrypoint template.

Detail

This Pull Request changes the docker-entrypoint template used when generating a new Rails application. A new line to remove any existing server PID files has been added.

rm -f /rails/tmp/pids/server.pid

The -f flag is required to avoid failing the command if the file is not present (as should be the case for any new container).

Additional information

The abovementioned issue can be reproduced by running a Rails application container, killing it, and trying to start the same container again. I'd be happy to create a separate bug report if additional details or reproduction steps are required.

# Start container
docker run -p 3000:3000 --name rails-demo -e RAILS_ENV=production -e RAILS_MASTER_KEY=key rails-demo:latest
# Kill from separate terminal
docker kill --signal=9 rails-demo
# Try restarting the container
docker start -a rails-demo
W, [2024-01-07T13:22:14.105941 #7]  WARN -- : You are running SQLite in production, this is generally not recommended. You can disable this warning by setting "config.active_record.sqlite3_production_warning=false".
=> Booting Puma
=> Rails 7.1.2 application starting in production
=> Run `bin/rails server --help` for more startup options
Exiting
A server is already running (pid: 1, file: /rails/tmp/pids/server.pid).

There is no alternative solution that I know of. I can't think of any side effects caused by removing the server.pid file if its present either. After all, containers are supposed to be ephermal, and the file should actually never be there in the first place.

The fix proposed in this PR is commonplace see for example GitHub discussion or the Entrypoint for httpd default image.

[dhh]

In our new docker container world, I'm actually wondering whether we should be generating these pid files by default at all? It seems like they're more trouble than they're worth, and I don't believe we have anything relying on them at the moment? At least in production, it seems to me like they should not be generated at all.

cc @rafaelfranca

[byroot]

In our new docker container world, I'm actually wondering whether we should be generating these pid files by default at all?

Yes, there is no point because Puma is PID 1 anyway.

[dhh]

@hschne If you want to pivot this to stopping the puma pid’ing instead, let’s roll with that.

[hschne]

@dhh Sounds good, will look into it. I'll close this PR in the meantime.