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

Shutdown verdi daemon gracefully upon container shutdown #180

Open
danielhollas opened this issue May 22, 2023 · 5 comments
Open

Shutdown verdi daemon gracefully upon container shutdown #180

danielhollas opened this issue May 22, 2023 · 5 comments
Milestone

Comments

@danielhollas
Copy link
Contributor

Due to an change in behaviour of aiida-core==2.3.0 we've discovered that we probably do not shutdown the verdi daemon gracefully and it leaves its PID file behind.

We should try to modify the aiidalab-launch stop command to first execute verdi daemon stop before it shutdowns the container.

@unkcpz
Copy link
Member

unkcpz commented May 27, 2023

Rather than put the elegant shutdown in Aiidalab-launch, it makes more sense to handle this by the image itself. When the container stop, a signal is send to the container and we can use that to stop the daemon. As show https://medium.com/@gchudnov/trapping-signals-in-docker-containers-7a57fdda7d86#.6b04xvnr8, it deserves a try to see if it is easy to add directly.

There is also https://github.com/just-containers/s6-overlay which designed to manage the services for containers and provide exit point for what we need here to elegantly shutdown with stoping the daemon.

@danielhollas
Copy link
Contributor Author

I agree it would be better to handle this at the image level.

When the container stop, a signal is send to the container and we can use that to stop the daemon. As show https://medium.com/@gchudnov/trapping-signals-in-docker-containers-7a57fdda7d86#.6b04xvnr8, it deserves a try to see if it is easy to add directly.

Nice find! I think the first thing here would be to determine if the SIGTERM signal is sent to all processes in the container. If that is the case, we could add the SIGTERM handler to the daemon in aiida-core.

When the container stop, a signal is send to the container and we can use that to stop the daemon. As show https://medium.com/@gchudnov/trapping-signals-in-docker-containers-7a57fdda7d86#.6b04xvnr8, it deserves a try to see if it is easy to add directly.

I would be against this unless we have a very strong reason. Changing the Init process is a non-trivial change and potentially openning a whole new can of worms.

@danielhollas danielhollas changed the title Shutdown verdi daemon gracefull upon container shutdown Shutdown verdi daemon gracefully upon container shutdown Jun 16, 2023
@unkcpz unkcpz added this to the v2023.1020 milestone Jul 10, 2023
@unkcpz
Copy link
Member

unkcpz commented Aug 14, 2023

I'll remove this from milestone, since the better way is to use the new aiida-core image I prepared which handle the gracefully shutdown of services, see aiidateam/aiida-core#6080, using s6-overlay as mentioned in this OP.

@danielhollas
Copy link
Contributor Author

But switching to this new image would mean that we would no longer be based on the Jupyter images?

@unkcpz
Copy link
Member

unkcpz commented Aug 15, 2023

Not explicitly based on it but I actually borrowed > 90% of https://github.com/jupyter/docker-stacks/tree/main/docker-stacks-foundation include this fix-permissions script. The different is I use s6-overlay to manage the services rather than start a jupyter server which is not needed in aiida-core image. When we use it, by using s6-overlay, it is very straightforward to make jupyter server the main longrun service.

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

No branches or pull requests

2 participants