timedatectl set-timezone fails on UC16

Bug #1650688 reported by Tony Espy on 2016-12-16
30
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Snappy
High
Oliver Grawert

Bug Description

On a system running UC16, the file /etc/localtime is a link that points to /etc/writable/localtime.

On a freshly installed system, /etc/writable/localtime is a fully-qualified link that points at /usr/share/zoneinfo/UTC.

If timedatectl is used to set the timezone to something else, timedated updates the localtime symbolic link with a relative path to the zoneinfo directory, which results in an invalid link.

$ sudo timedatectl set-timezone America/Detroit

$ sudo timedatectl
      Local time: Fri 2016-12-16 18:18:49 EST
  Universal time: Fri 2016-12-16 23:18:49 UTC
        RTC time: Fri 2016-12-16 23:18:49
       Time zone: America/Detroit (EST, -0500)
 Network time on: yes
NTP synchronized: yes
 RTC in local TZ: no

$ ls -l /etc/writable/localtime
/etc/writable/localtime --> ../usr/share/zoneinfo/America/Detroit

admin@localhost:/etc/writable$ date
Fri Dec 16 23:20:07 UTC 2016

I'm running the latest core snap from the candidate channel which contains snapd 2.18.1. Hardware details and/or more debug information can be supplied on request.

See 'man 3 timezone' and 'man timedatectl' for more details.

Tags: hwe Edit Tag help
Simon Fels (morphis) wrote :

I can reproduce this on core revision 1083 (armhf).

@ogra any idea how we can easily fix this? Whoever updates the symlink at /etc/localtime needs to respect that /etc/localtime is also now just a symlink to /etc/writable/localtime and that the proper target for the symlink needs to be ../../usr/share/zoneinfo/America/Detroit.

All this causes timedated to end up in UTC state.

test@localhost:/etc$ sudo timedatectl
      Local time: Thu 2017-02-09 06:21:21 UTC
  Universal time: Thu 2017-02-09 06:21:21 UTC
        RTC time: n/a
       Time zone: n/a (UTC, +0000)
 Network time on: yes
NTP synchronized: yes
 RTC in local TZ: no

Oliver Grawert (ogra) wrote :

well, timedatectl comes from systemd ...
i'm not sure we actually want to patch it ...
what i was thinking about was to have a timezone option in the core snap config instead, then we are free to implement the link farm as we like via some scriptery or go binary

Tony Espy (awe) on 2017-02-09
Changed in snappy:
status: New → Confirmed
Renat (renat2017) wrote :

I can confirm that our system is affected too.

Oliver Grawert (ogra) wrote :

https://github.com/snapcore/core/pull/8 would be a possible fix (waiting for reviews from the team)

Pat McGowan (pat-mcgowan) wrote :

I have the wrapper script on my dragonboard and the date/time still resets as described in the duplicate bug.

snap 2.23.1
snapd 2.23.1
series 16
kernel 4.4.0-1030-snapdragon

Changed in snappy:
assignee: nobody → Oliver Grawert (ogra)
importance: Undecided → High
Pat McGowan (pat-mcgowan) wrote :

From the log when it reverts to utc

Mar 16 11:07:14 patsdragon dbus[1683]: [system] Activating via systemd: service name='org.freedesktop.timedate1' unit='dbus-org.freedesktop.timedate1.service'
Mar 16 11:07:14 patsdragon systemd[1]: Starting Time & Date Service...
Mar 16 11:07:14 patsdragon dbus[1683]: [system] Successfully activated service 'org.freedesktop.timedate1'
Mar 16 11:07:14 patsdragon systemd-timedated[10615]: /etc/localtime should be a symbolic link to a time zone data file in /usr/share/zoneinfo/.
Mar 16 11:07:14 patsdragon systemd[1]: Started Time & Date Service.

pat-mcgowan@patsdragon:~$ ls -l /etc/localtime
lrwxrwxrwx 1 root root 18 Mar 8 10:03 /etc/localtime -> writable/localtime
pat-mcgowan@patsdragon:~$ ls -l /etc/writable/localtime
lrwxrwxrwx 1 root root 35 Mar 16 10:57 /etc/writable/localtime -> /usr/share/zoneinfo/America/Detroit
pat-mcgowan@patsdragon:~$

Pat McGowan (pat-mcgowan) wrote :

systemd-timedated at
https://github.com/systemd/systemd/blob/master/src/timedate/timedated.c#L71
calls get_timezone() at
https://github.com/systemd/systemd/blob/master/src/basic/time-util.c#L1292
which explicitly checks the value of the linked file name to "/usr/share/zoneinfo"
which fails and logs the error and doesn't return a valid timezone.

Not sure how to fix short of modifying the readlink util to follow multiple links
https://github.com/systemd/systemd/blob/master/src/basic/fs-util.c#L147

On the other hand, why doesn't the get_timezone code just read the value of /etc/timezone rather than parsing the file pathnames all over again?

Oliver Grawert (ogra) wrote :

your comment #6 clearly shows that the script did not intercept the set-timezone call:

pat-mcgowan@patsdragon:~$ ls -l /etc/localtime
lrwxrwxrwx 1 root root 18 Mar 8 10:03 /etc/localtime -> writable/localtime

thats a relative link again, the get_timezone() call should be fine if it is an absolute link (which the script cares for)

Pat McGowan (pat-mcgowan) wrote :

I reflashed the dragon board to retest

once a timezone is set the date command works, however timedatectl does not and the daemon does reset the TZ to nothing
~$ timedatectl
      Local time: Mon 2017-03-20 17:39:23 UTC
  Universal time: Mon 2017-03-20 17:39:23 UTC
        RTC time: Thu 1970-01-01 00:48:33
       Time zone: n/a (UTC, +0000)
 Network time on: yes
NTP synchronized: yes
 RTC in local TZ: no
pat-mcgowan@localhost:~$ date
Mon Mar 20 13:39:57 EDT 2017

The snapweb UI reflects what timedatectl sees i.e. the wrong timezone.

So this bug seems to be "fixed", will open another on snapweb not to rely on timedatectl.

Changed in snappy:
status: Confirmed → Fix Released
Tony Espy (awe) wrote :

How can this be fixed if timedatectl is still wrong?

If the timezone is changed via a DBus request to systemd-timedated, will the date command be correct?

Oliver Grawert (ogra) wrote :

if systemd-timedated invokes timedatectl your TZ should be set correctly for date.

Tony Espy (awe) wrote :

Thanks Ogra!

Could you explain what actually was patched and when? We have a customer image due this Thu, and I'm wondering at what point new images will include this fix.

Oliver Grawert (ogra) wrote :

@tony https://github.com/snapcore/core/pull/8 moves timedatectl to timedatectl.real and intercepts the setting of the symlink in /etc/writable to be an absolute one (so it isnt dangling anymore but pointing to an actual timezone file).

Simon Fels (morphis) wrote :

@ogra Actually that doesn't sound like a good solution. Timedated offers a DBus-API (https://www.freedesktop.org/wiki/Software/systemd/timedated/) which allows you to set the timezone. So timedatectl talks to timedated over dbus, isn't it? With that we have to fix timedated as otherwise we still have a open hole in our system. Everyone who will use the timezone-control interface will run into this issue again. See https://github.com/snapcore/snapd/blob/master/interfaces/builtin/timezone_control.go

Tony Espy (awe) wrote :

@Ogra

So is there logic in both timedatectl *and* timedated to update the symlink? I'd actually be surprised if that was the case.

Simon is correct in that we have a snapd interface which allows snaps using it to configure the timezone via DBus requests to timedated. I'd made the assumption in my original bug report that timedatectl delegated the work to timedated.

Simon Fels (morphis) wrote :

@Tony: timedatectl calls timedated over dbus. Same API as we offer with the timezone-control interface. See https://www.freedesktop.org/wiki/Software/systemd/timedated/

Oliver Grawert (ogra) wrote :

@simon, that interface doesnt even mark /etc/localtime as writable ...

/etc/{,writable/}timezone rw,

Oliver Grawert (ogra) wrote :

looking at the code of timedated it doesnt seem like we can teach it easily about teh two levels of symlinks we use ... it hardcodes nearly everything, including to enforce a relative symlink "../usr/share/zoneinfo/" ... not sure how we could fix this without harming the rest of the world.

Simon Fels (morphis) wrote :

@ogra: I think we have to. There needs to be some smart detection code if we're running in a core environment or not.

Pat McGowan (pat-mcgowan) wrote :

The old patch (comment #9) introduced a check for writable_filename then modified the calls to something like
r = readlink_malloc(writable_filename("/etc/localtime"), &t);

We could do that same hacks. It seems this code was refactored a good bit since then.

Tony Espy (awe) wrote :

Can someone please change the status back to 'Confirmed'?

Changed in snappy:
status: Fix Released → Confirmed
Oliver Grawert (ogra) wrote :

@pat the prob is as you say ... it is a hack ... we need to get this proper into systemd.

while we have a PPA for a few image build bits we can not carry a patch for systemd in that PPA if there is no chance it gets merged either as a distro patch or upstream.

i think we should involve foundations in this discussion as they are maintaining the ubuntu systemd package and would have to carry that patch.

Tony Espy (awe) on 2017-03-28
tags: added: hwe
Michael Vogt (mvo) wrote :

This is really messy. As Pat pointed out, one option is to teach systemd to understand that there might be /etc/writable/localtime. I started with such a patch but its more involved (we need to tweak context_write_data_timezone() and get_timezone() to make it understand the different layout) and also very unlikely be taken by upstream (and a high likelyhood for conflicts).

A cheap(er) alternative might be to simply makee /etc/localtime writable and stop it being a symlink to writable/localtime. Then we patch systemd to retry the symlinking of that file in a non-atomic way if the atomic one failed. This patch is much more isolated and easier to carry. *If* all apps are using systemd to write this symlink we should be fine. It is still non-atomic but its just the timezone that would be wrong if in the middle of unlink/symlink() things go bad.

Oliver Grawert (ogra) wrote :

we can not make single files writeable because our writable-paths only act on directories using bind mounts, this is the main reason for the whole /etc/writable mess to exist in the first place.

Michael Vogt (mvo) wrote :

@ogra: check /etc/default/keyboard. The fact that we have /etc/writable is only because things want to write atomic with a pattern like write("/etc/foo.tmp"), rename("/etc/foo.tmp", "/etc/foo")

Renat (renat2017) wrote :

Guys, can't we have a symlink like /etc/usr -> /usr to resolve this? I tried to patch the image but, seems, /etc is not changeable from the image file.

Kyle Nitzsche (knitzsche) wrote :

This is a critical issue for Renat and is blocking their project. Can it be fixed quickly?

Simon Fels (morphis) wrote :

According to https://forum.snapcraft.io/t/next-snapd-2-25/197 this is set to be fixed with the 2.25 release.

Tony Espy (awe) wrote :

Doesn't look like it made the cut per the release announcement:

https://github.com/snapcore/snapd/releases/tag/2.25

Oliver Grawert (ogra) on 2017-05-12
Changed in snappy:
status: Confirmed → In Progress
Oliver Grawert (ogra) wrote :

the following changes were recently added in edge, do we have any snap or a simplified test case to test the dbus way of setting the timezone to verify the changes ?

https://github.com/snapcore/core/pull/40
https://github.com/snapcore/snapd/pull/3316

Michael Vogt (mvo) wrote :

Using the core from edge and the command in the bug description this works now. This will be part of the 2.26.3 stable core to be released in ~2 weeks.

Changed in snappy:
status: In Progress → Fix Committed
Oliver Grawert (ogra) wrote :

@mvo the command in the bug description works since march when this bug was originally closed ... what doesnt work is the additional method used via the dbus interface (the original command just runs timedatectl directly, no dbus interface involved).

what we are missing is a verification for the dbus method to set the timezone for which no test case was added to this bug ...

Tony Espy (awe) wrote :

@mvo, @ogra

I'll test the DBus case this afternoon, and add my details to the bug. Sorry for the delay in reviewing this.

Tony Espy (awe) wrote :

I'd originally assumed that timedatectl used DBus to do the heavy lifting, and had asked the question of whether there was duplicate code to handle the symlink in both timedatectl and timedated in comment #17.

Anyways, the command to set the timezone to "Detroit" via DBus is simply:

sudo dbus-send --system --print-reply \
     --dest=org.freedesktop.timedate1 \
     /org/freedesktop/timedate1 \
     org.freedesktop.timedate1.SetTimezone \
     string:"America/Detroit" boolean:false

After installing core from edge (16-2.26.3+git204.1bc8375/r2013), and running this command, it fails and I end up with the same broken link as originally reported in the bug:

$ ls -l /etc/writable/localtime
lrwxrwxrwx 1 root root 37 May 23 20:44 /etc/writable/localtime -> ../usr/share/zoneinfo/America/Detroit

That said, using timedatectl to change the timezone does work, and results in the following symbolic link:

lrwxrwxrwx 1 root root 35 May 23 16:51 /etc/writable/localtime -> /usr/share/zoneinfo/America/Detroit

I'm confused, it appears that we still haven't touched timedate1 itself yet?

Tony Espy (awe) wrote :

Looks like Tyler want ahead and modified timezone-control, so that snaps using this interface can execute timedatectl directly, so at least there's a way for a snap to actually control the timezone programmatically.

Perhaps we should file a new bug to track that the DBus interface way of doing this is still broken?

Kyle Nitzsche (knitzsche) wrote :

@Tony. When is Tyler's landing going to reach snapd stable? This is a blocker for a customer going to beta, which they hope to do next week.

Kyle Nitzsche (knitzsche) wrote :

Sorry, I missed mvo's comment 33 " This will be part of the 2.26.3 stable core to be released in ~2 weeks."

Tony Espy (awe) wrote :

One of our commercial customers just reported an issue about timedatectl not working properly on Ubuntu Core 16.

The short summary:

1. sudo timedatectl set-timezone TZ works:
  - /etc/timezone is set correctly
  - the date command displays the new TZ correctly
  - timedatectl displays the new TZ

2. after a reboot, /etc/timezone is still set to the new TZ, date still displays the new TZ, however timedatectl shows the timezone as unset

3. Using DBus to set the TZ as described in comment #36 still fails

Zygmunt Krynicki (zyga) wrote :

Reopened based on last comment.

Changed in snappy:
status: Fix Committed → New
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Duplicates of this bug

Other bug subscribers