Merge ~rbalint/ubuntu-release-upgrader:ubuntu/master into ubuntu-release-upgrader:ubuntu/master

[Balint Reczey]

[Julian Andres Klode (juliank)]

Would feel better with `and any(p.is_installed for p in self.get_providing_packages("time-daemon"))` added to the if.

[Julian Andres Klode (juliank)]

The reason being that we do want to install systemd-timesyncd if no time-daemon was previously installed, and not by accident some other time-daemon because something somewhere has a depends wrong.

[Julian Andres Klode (juliank)]

review: Needs Fixing

[Dimitri John Ledkov (xnox)]

"""
Would feel better with `and any(p.is_installed for p in self.get_providing_packages("time-daemon"))` added to the if.
"""

I wouldn't.

I older releases systemd provided timesyncd without declaring time-daemon, other packages that are installed may have been doing the same. However in the next series, they might start providing "time-daemon" to correctly resolve the conflict. Thus one cannot assert that already installed packages provide "time-daemon" here.

If some other package will provide "time-daemon" in the next series, the total transaction will be calculated correctly. If there are none, due to how systemd depends are declared, timesyncd will be picked as the first alternate.

Thus the proposed fix, seems correct.

review: Approve

[Balint Reczey (rbalint)]

@juliank ntp is a package that did not have Provides: time-daemon in Bionic and Eoan, thus those would still be removed with the added check.

I also don't agree with the concept that a package picking up dependency in focal on chrony but not any other time daemon should be removed on upgrade.

[Julian Andres Klode (juliank)]

@xnox This looks at the candidate's provides (or if you want, any version, candidate_only=False), hence the installed one not providing it is not a concern.

I don't want to argue about what the behavior should be wrt a package picking up a dependency, but this changes the behavior from what it used to be, and is for other packages in similar situations - they get removed.

Hence this changes more than necessary to resolve the bug, which might have broader implications as to how systems diverge from our expectations after an upgrade, which I don't want to get into days before the release.

[Julian Andres Klode (juliank)]

Oh I missed an @rbalint for the second paragraph.

Optimally we'd also replace any other automatically installed time-daemon (whether the provides are pre-focal or not) with systemd-timesyncd, because if the user did not explictly chose one of them, they should get the official standard supported one.

[Dimitri John Ledkov (xnox)]

On Fri, 17 Apr 2020, 09:51 Julian Andres Klode, <email address hidden>
wrote:

> Oh I missed an @rbalint for the second paragraph.
>
> Optimally we'd also replace any other automatically installed time-daemon
> (whether the provides are pre-focal or not) with systemd-timesyncd, because
> if the user did not explictly chose one of them, they should get the
> official standard supported one.
>

Timesyncd is the fallback time-daemon. Any other time-daemon is better and
preferred. We do not not want to install timesyncd, if any other
time-daemon is available. Chrony is probably the best one, that's why we
promoted it to main. Timesyncd is simply the smallest /minimal one. Hence
we use it in Ubuntu Core.

This is atypical dependency declaration. Previously timesyncd was always
co-installed, but neutralised with systemd unit conflicts, which sometimes
didn't work right. Thus timesyncd was never intended to be running, if any
other time-daemon was installed. If Focal, one even saves disk space, by
having only one time-daemon on disk.

This issue is a regression in release-upgrader. On prior dist-upgrades
(e.g. to Eoan), chosen time-daemon was kept. And yes this is new, due to
package split landing only recently.

We should not enable Eoan -> Focal upgrades until this is fixed.

>

[Dimitri John Ledkov (xnox)]

On Fri, 17 Apr 2020, 09:51 Julian Andres Klode, <email address hidden>
wrote:

> Oh I missed an @rbalint for the second paragraph.
>
> Optimally we'd also replace any other automatically installed time-daemon
> (whether the provides are pre-focal or not) with systemd-timesyncd, because
> if the user did not explictly chose one of them, they should get the
> official standard supported one.
>

Should we somehow remove Task: minimal from timesyncd then? To make the
intentions clearer here.

Cause I only want that task on the virtual "time-daemon", rather than on
packages that provide it. Which is impossible.

>

[Balint Reczey (rbalint)]

@juliank I agree with @xnon's reasoning but I also agree that we don't have enough time to discuss all aspects before the release.

People not wanting to install systemd-timesyncd can pin it to -1 before do-release-upgrade and both of the fix variants work AFAICT, so I'd like to have one one of them in.

[Julian Andres Klode (juliank)]

review: Abstain

[Dimitri John Ledkov (xnox)]

I wonder if we can make it a quirk, and on upgrades to focal only.

[Balint Reczey (rbalint)]

@xnox Why should u-r-u force-install systemd-timesyncd removing chrony in 20.04 -> 20.10 upgrades?

[Michael Vogt (mvo)]

Thanks for this PR. I think it's the right way to fix this issue. It's a bit annoying this has to be special cased in the generic installTasks() code, normally there is the "quirks" system for this. But given the nature of the issue it's the best place.

Fwiw, I wonder if removing systemd-timesyncd from the minimal task would be more appropriate, I think this should fix the issue as well because then the systemd-timesyscnd install would be handled via a dependency of systemd (wich is also task minimal) and there apt should DTRT because there is already a or-dependency. So with that change this PR would not be needed and I don't see any downside - but I also have not tested it (but that should be easy but just stopping the upgrade and cowboying /var/lib/apt/lists/*focal*Packages into the right shape and then starting the upgrade again without running a apt.update() in the release upgrader).

[Brian Murray (brian-murray)]

Michael and I both tested moving "systemd-timesyncd | time-daemon" from a Depends to a Recommends of systemd and doing so made ubuntu-release-upgrader keep chrony installed.

[Balint Reczey (rbalint)]

@brian-murray @mvo While it is probably correct to on recommend ntp-server rather than depending on it because the system can still be functional I believe in Ubuntu the we would like to be fairly sure that the system has a synchronized clock thus Depends: seems to be more appropriate.

[Balint Reczey (rbalint)]

I'd like to converge on fixing this issue soon.

I think moving time-daemon to Recommends: in systemd needs a bit more discussion and is also slow to get through proposed.

I'm OK with fixing germinate? to not have systemd-timesyncd in the seed, but I have not implemented that yet and this would make a confusion exception. I like that this solution does not involve fixing anything in the release.

I'm also OK merging one of the offered solutions of skipping systemd-timesyncd in u-r-u and I would even fix @mvo's nitpick if that's needed.

The quirk does not seem to be any better than the skip since it would be more complex as it must be applied later. Nor I believe that this should be a ->20.04-only quirk.

Could we please agree on one of the solutions and drive it to completion?

[Dimitri John Ledkov (xnox)]

Digging into this, I am opening up a can of warms it seems.

First we try to determine if we should go into "desktop" or "server" mode. This is done by trying to detect "keydependencies" of metapackages.

For example, key dependency for kubuntu-desktop is plasma-desktop. However for ubuntu-desktop it is "lightdm, unity, ubuntu-artwork, ubuntu-sounds" which imho is an out of date list. It should be "gnome-shell, gdm3".

Also I do not see any ubuntu-desktop-minimal handling, of how to upgrade a minimal ubuntu desktop.

Next in server mode, we try to gather a list of all tasks installed... However we do not exclude tasks, that are configured as BaseMetaPkgs "ubuntu-minimal, ubuntu-standard". Those are already installed under keepBaseMetaPkgsInstalled rule anyway.

I also noticed that we have stray tasks. Specifically "ubuntu-core" is still a task, despite the metapackage getting removed. We should ignore this task.

If we fix above, a situation that timesyncd has fallen uder will not materialize, and this special casing then would not be needed.

review: Needs Fixing (can-of-warms)

Unmerged commits

56a245e... by Balint Reczey on 2020-04-16

Don't force installation of systemd-timesyncd just because it is in minimal seed

LP: #1872902

diff --git a/DistUpgrade/DistUpgradeCache.py b/DistUpgrade/DistUpgradeCache.py
index 3cd8504..0ff82f2 100644
--- a/DistUpgrade/DistUpgradeCache.py
+++ b/DistUpgrade/DistUpgradeCache.py
@@ -829,6 +829,13 @@ class MyCache(apt.Cache):
     def installTasks(self, tasks):
         logging.debug("running installTasks")
         for pkg in self:
+            if pkg.name == "systemd-timesyncd" \
+               and any(p.is_installed for p in self.get_providing_packages(
+                   "time-daemon")):
+                # systemd Depends: systemd-timesyncd | time-daemon
+                # Let the installed time-daemon kept installed and don't
+                # replace it with systemd-timesyncd. See LP: #1872902
+                continue
             if pkg.marked_install or pkg.is_installed:
                 continue
             self._lookupPkgRecord(pkg)