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

RFE: systemd.exec Type=Notify do not work with RootDirectory #3544

Closed
1 of 2 tasks
alepuccetti opened this issue Jun 15, 2016 · 7 comments
Closed
1 of 2 tasks

RFE: systemd.exec Type=Notify do not work with RootDirectory #3544

alepuccetti opened this issue Jun 15, 2016 · 7 comments
Labels
RFE 🎁 Request for Enhancement, i.e. a feature request

Comments

@alepuccetti
Copy link

alepuccetti commented Jun 15, 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

systemd v230

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

Used distribution

Arch

When a service set Type=notify and RootDirectory=, then the notification mechanism does not work, the serivice is marked "activating", and eventually killed by systemd because of timeout.

How to reproduce the issue:

  • Copy the following unit file in your /etc/systemd/system/chroot-test.service
[Unit]
Description=chroot-test

[Service]
Type=notify
NotifyAccess=all
RootDirectory=/distros/arch-base/
ExecStart=@/usr/bin/sh "/usr/bin/sh" "-c" "sleep 5 ; echo \"READY=1\"; systemd-notify --ready; sleep 300"

[Install]
WantedBy=multi-user.target
  • Run sudo systemctl start chroot-test.service
  • Run sudo systemctl status chroot-test.service

Proposed solution

When both Type=notify and RootDirectory= are present, systemd will bind mount the /run/systemd/notify on the host into the service filesystem in the same location.

If /run/systemd/notify is not present in the service filesystem, systemd could create it by default or exit with an error. Ask to the user to set it up force them to understand better what is happening but will break service that now are working. Do not do anything will be more back compatible but it is still a wrong behavior. Any thoughts about it?

@alban alban added the RFE 🎁 Request for Enhancement, i.e. a feature request label Jun 15, 2016
@poettering
Copy link
Member

I figure if Type=notify or NotifyAccess= are explicitly set we indeed should just bind mount the socket into the chroot. I mean, If people mix RootDirectory= and any of those two settings, then they basically ask for it...

@poettering
Copy link
Member

or hmmm, bind mounting sucks here, because after all RootDirectory= doesn't imply fs namespacing...

Maybe we should instead simply add BindPath= or so as generic way to bind mount something somewhere for a specific service, and then simply document that people should use it for /run/systemd/notify if they use RootDirectory=...

@zdzichu
Copy link
Contributor

zdzichu commented Jun 16, 2016

Moving notify socket to abstract namespace would solve it.

@poettering
Copy link
Member

@zdzichu well, we used to have it in the abstract namespace, but doing this is nasty for containers as abstract AF_UNIX sockets are only namespaced if network namespaces are used for a container (which often is not), unlike in-file-system AF_UNIX sockets which are used for file system namespaces (which is definitely namespaced for containers).

@alban
Copy link
Member

alban commented Jun 16, 2016

or hmmm, bind mounting sucks here, because after all RootDirectory= doesn't imply fs namespacing...

We already force mnt namespacing implicitely on private_tmp, read_write_dirs, etc.:
https://github.com/systemd/systemd/blob/master/src/core/execute.c#L1432

We could document that in RootDirectory= that "root_directory != "" && type==notify" implies "MountFlags=slave".

I would also prefer not to use abstract sockets for the notify socket: file sockets across mnt namespaces can be bypassed with bind mounts, but Abstract sockets across net namespaces cannot.

@alepuccetti
Copy link
Author

Maybe we should instead simply add BindPath= or so as generic way to bind mount something somewhere for a specific service, and then simply document that people should use it for /run/systemd/notify if they use RootDirectory=...

I think it will much better if this will be done automatically if root_directory != "" && type==notify, this will preserve the notification functionality already implemented. Forcing users to add every time BindPath=/run/systemd/notify it seems unnecessary to me. Honestly, I think in this way is cleaner.
If comes out some use cases where people do not want to have the socket we could add a switch to suppress it (or we could add it anyway, but I don't think it will very useful).

I would like to have this in the next systemd release, so let;s come out with a design decision in time to write the patch for the v231.

We could document that in RootDirectory= that "root_directory != "" && type==notify" implies "MountFlags=slave".

About this I think is a good idea, but if it is only to hide /run/systemd/notify we might want to leave this decision to the user instead force MountFlags=slave on everything.

alban referenced this issue in kinvolk/rkt Jun 22, 2016
… to the host

After this patch, systemd on the host marks the container as "active"
when the init systemd in the container sends the ready message
instead of doing it after the container is created.
After this patch rkt sets `--service-type=notify`.

Also appc/spec in godeps is patched to expose `SupportNotify`.

Fixes: rkt#1464

Warning:
If the application launched by rkt does not notify (with sd_notify())
that it is ready, then the application may be killed after a timeout.

Note: Requires systemd v231 in stage1.
@poettering
Copy link
Member

Fixed with 3bdc25a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFE 🎁 Request for Enhancement, i.e. a feature request
Development

No branches or pull requests

4 participants