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

Set $NOTIFY_SOCKET for ExecStop= processes, and process notify messages from them #4212

Closed
1 of 2 tasks
joukewitteveen opened this issue Sep 25, 2016 · 5 comments
Closed
1 of 2 tasks
Labels
pid1 RFE 🎁 Request for Enhancement, i.e. a feature request

Comments

@joukewitteveen
Copy link
Contributor

joukewitteveen commented Sep 25, 2016

Submission type

  • Bug report
  • Request for enhancement (RFE)

NOTE: Do not submit anything other than bug reports or RFEs via the issue tracker!

systemd version the issue has been seen with

231

NOTE: Do not submit bug reports about anything but the two most recently released systemd versions upstream!

Used distribution

Arch Linux

In case of bug report: Expected behaviour you didn't see

Updating the unit status string through systemd-notify --pid --status="this service is no longer running so the old status should be invalidated" should update the unit status string.

In case of bug report: Unexpected behaviour you saw

No update could be performed as $NOTIFY_SOCKET was not set for the 'ExecStop' process. This happens when the service type is 'notify' and 'NotifyAccess' is not set (implicitly set to 'main'). I feel that the ExecStop process should behave as the main process.

In case of bug report: Steps to reproduce the problem

Try to update the process status from a program run via 'ExecStop'.

@joukewitteveen
Copy link
Contributor Author

joukewitteveen commented Sep 25, 2016

I think the offending piece of code is the presence of EXEC_IS_CONTROL in src/core/service.c:service_enter_stop. Is it very unreasonable to expect the ExecStop process to behave as the main process?

@davidstrauss
Copy link
Member

I'm not sure I understand the use case. An ExecStop= should perform actions to shut down the daemon, which should cause the daemon tell the notify socket that it's stopping.

@joukewitteveen
Copy link
Contributor Author

For a RemainAfterExit= daemon, all stopping functionality takes place in the ExecStop= process. Consider the following:

[Service]
Type=notify
RemainAfterExit=yes
ExecStart=systemd-notify --pid --status="Going strong" --ready
ExecStop=systemd-notify --pid --status=""

This does not work as desired because the ExecStop= process is not allowed to notify. Of course, this can be solved by opening up NotifyAccess=, but this feels like overkill. Maybe make ExecStop= the main process if the ExecStart= process has already exited? Or include more granularity (NotifyAccess=toplevel for access only to processes started by systemd itself)?

@poettering
Copy link
Member

Hmm, I figure we could also set the env var for ExecStop=, at least depending on some setting in NotifyAccess= as proposed.

@poettering poettering added RFE 🎁 Request for Enhancement, i.e. a feature request pid1 labels Oct 7, 2016
@poettering poettering changed the title NOTIFY_SOCKET not set when service is stopped Set $NOTIFY_SOCKET for ExecStop= processes, and process notify messages from them Oct 7, 2016
joukewitteveen added a commit to joukewitteveen/systemd that referenced this issue Nov 26, 2016
Setting NotifyAccess=exec allows notifications coming directly from any
control process.
joukewitteveen added a commit to joukewitteveen/systemd that referenced this issue Nov 28, 2016
Setting NotifyAccess=exec allows notifications coming directly from any
control process.
joukewitteveen added a commit to joukewitteveen/systemd that referenced this issue Nov 29, 2016
Setting NotifyAccess=exec allows notifications coming directly from any
control process.
joukewitteveen added a commit to joukewitteveen/systemd that referenced this issue Nov 29, 2016
Setting NotifyAccess=exec allows notifications coming directly from any
control process.
joukewitteveen added a commit to joukewitteveen/systemd that referenced this issue Nov 29, 2016
Setting NotifyAccess=exec allows notifications coming directly from any
control process.
evverx added a commit that referenced this issue Nov 30, 2016
Improvements for notify services (including #4212)
@evverx
Copy link
Member

evverx commented Nov 30, 2016

Closed by #4745

@evverx evverx closed this as completed Nov 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pid1 RFE 🎁 Request for Enhancement, i.e. a feature request
Development

No branches or pull requests

4 participants