systemd-time-wait-sync.service seems to be failing to start inside user namespace containers

[evverx]

I haven't read #8494 where systemd-time-wait-sync.service was introduced yet, so I'm not sure whether it is a bug or not:

bash-4.4# systemd-detect-virt
systemd-nspawn

bash-4.4# systemd-detect-virt --private-users
bash-4.4# echo $?
0

bash-4.4# systemctl --failed --no-pager
  UNIT                           LOAD   ACTIVE SUB    DESCRIPTION
● systemd-time-wait-sync.service loaded failed failed Wait Until Kernel Time Synchronized

LOAD   = Reflects whether the unit definition was properly loaded.
ACTIVE = The high-level unit activation state, i.e. generalization of SUB.
SUB    = The low-level unit activation state, values depend on unit type.

1 loaded units listed. Pass --all to see loaded but inactive units, too.
To show all installed unit files use 'systemctl list-unit-files'.

bash-4.4# journalctl -b | grep -i time-wait-sync
Mar 20 13:34:27 systemd-testsuite systemd[23]: systemd-time-wait-sync.service: Executing: /usr/lib/systemd/systemd-time-wait-sync
Mar 20 13:34:27 systemd-testsuite systemd-time-wait-sync[23]: Failed to read adjtimex state: Operation not permitted

[poettering]

Hmm, I figure adjitemex() will fail in various container setups. Let's handle that gracefully, and simply assume that if we get EPERM or EACCES on adjtimex() we run inside a container, and the clock is set correctly anyway.

@pabigot any chance you can hack up a quick patch for this?

Maybe add a brief log_debug() message explaining the assumption when we exit early if adjtimex() fails like this.

[pabigot]

Would disabling it in a container as is done with timesyncd be the right solution? I'll take a look...

[poettering]

@pabigot we generally want that our images likely boot as well on are metal as in VMs or containers. And that means for auxiliary stuff like this we should handle things gracefully. Of course if some major part of a system can't work in a container, by all means it should fail, but this isn't like that. It's more auxiliary.

[pabigot]

From memory, it is in fact EPERM that shows up in this case.

I'm very naive in system.unit configuration, but I'm looking at the various extra stuff that timesyncd has in its unit, and should be able to make something work.

[poettering]

That said, maybe we should not solve this in code but simply via a condition:

ConditionCapability=CAP_SYS_TIME
ConditionVirtualization=!container

The same two conditions we already have in timesyncd. I think we should add this here too. It would mean the service is skipped silently if CAP_SYS_TIME is missing or we run in a container, which really is the behaviour we want here.

Fixing the unit file like this should be trivial.

[pabigot]

Great; that's exactly what I was looking at.

[poettering]

From memory, it is in fact EPERM that shows up in this case.

Well, container managers might block adjtimes via various different ways, for example via seccomp. If they do, then all bets are off, they might as well use EACCES… But yes you are right the caps/userns based blocking would mean we'd get EPERM.

but anyway, this is moot, I am pretty sure we should go the condition way.

[pabigot]

FWIW, I don't believe CAP_SYS_TIME should be necessary for this service. adjtimex(2) is never invoked by it with any flags that attempt to change state; the way it's invoked is normally allowed for unprivileged processes.

[poettering]

FWIW, I don't believe CAP_SYS_TIME should be necessary for this service. adjtimex(2) is never invoked by it with any flags that attempt to change state; the way it's invoked is normally allowed for unprivileged processes.

It's not necessary. However, given that the ConditionCapability checks PID1's caps, and given that timesyncd has the same condition, we know that there's no point in waiting for an NTP sync if the NTP sync can't happen anyway. Hence, even though this service won't need CAP_SYS_TIME itself, it's still a good check