Bug 1722950 - Setting of default.target broken for live images
Summary: Setting of default.target broken for live images
Alias: None
Product: Fedora
Classification: Fedora
Component: anaconda
Version: rawhide
Hardware: All
OS: Linux
Target Milestone: ---
Assignee: Anaconda Maintenance Team
QA Contact: Fedora Extras Quality Assurance
Whiteboard: openqa
Depends On:
Blocks: F31BetaFreezeException
TreeView+ depends on / blocked
Reported: 2019-06-21 19:25 UTC by Adam Williamson
Modified: 2019-07-17 18:08 UTC (History)
7 users (show)

Fixed In Version: anaconda-31.17-1
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Last Closed: 2019-07-17 18:08:14 UTC
Type: Bug

Attachments (Terms of Use)

Description Adam Williamson 2019-06-21 19:25:18 UTC
openQA testing of the KDE lives from Fedora-Rawhide-20190621.n.0 failed, because initial-setup (text mode) runs on boot when either nothing (if a user was created during install) or initial-setup-gui (if no user was created during install) ought to run.

Looking at the logs suggests this is something to do with the systemd default target being broken:

Jun 21 12:12:17 localhost.localdomain initial-setup[855]: Initial Setup GUI is installed, but default.target != graphical.target

and indeed, it is:

[root@localhost-live test]# ls -l /etc/systemd/system/default.target 
lrwxrwxrwx. 1 root root 49 Jun 21 12:00 /etc/systemd/system/default.target -> /mnt/sysroot/etc/systemd/system/multi-user.target

that is a broken symlink, obviously. Two things wrong there: it shouldn't have /mnt/sysroot on the front, and since this is a desktop live install, it should be graphical.target , not multi-user.target .

This seems to not technically be a Beta blocker, given the criterion says " A system installed with a release-blocking desktop must boot to a log in screen where it is possible to log in to a working desktop using a user account created during installation or a 'first boot' utility."...well, it *does* do that, it just boots to the wrong 'first boot' utility, and shows it when it shouldn't, but doing that isn't a violation...

Comment 1 Adam Williamson 2019-06-21 19:32:44 UTC
So it seems like the code for configuring the default target explicitly includes the sysroot in the symlink target. That's just wrong, it shouldn't do that. You set symlink targets as they will eventually be correct on the installed system, not as they are correct within the installer environment.

Also, this is wrong, I think:

        selected_target_path = os.path.join(self._sysroot, 'etc/systemd/system', self._default_target)

the stock 'multi-user.target' and 'graphical.target' files are in /usr/lib/systemd/system , they are not in /etc/systemd/system :

[adamw@adam anaconda (master %)]$ ls -l /etc/systemd/system/graphical.target
ls: cannot access '/etc/systemd/system/graphical.target': No such file or directory
[adamw@adam anaconda (master %)]$ ls -l /usr/lib/systemd/system/graphical.target
-rw-r--r--. 1 root root 598 Apr 26 01:48 /usr/lib/systemd/system/graphical.target
[adamw@adam anaconda (master %)]$

The code that decides whether to use graphical.target or multi-user.target must also be incorrect for the live desktop image case...

Comment 2 Adam Williamson 2019-06-21 19:37:11 UTC
Note what the old code did here:

        default_target = util.getSysroot() + '/etc/systemd/system/default.target'
        if os.path.islink(default_target):
        os.symlink('/lib/systemd/system/%s' % self.default_target, default_target)

that was correct in both respects the new code is incorrect: it uses /lib/systemd/system not /etc/systemd/system for the target path, and it doesn't include the sysroot in the target path.

Comment 3 Adam Williamson 2019-06-21 19:55:57 UTC
I *think* what's going on with 'graphical.target' not being picked up as the default target is this. There's a method called `_set_default_boot_target()` in pyanaconda/payload/__init__.py which I think is expected to handle this. It's called in `post_install()`. But I think we're hitting this block of it now:

        # If the target was already set, we don't have to continue.
        services_proxy = SERVICES.get_proxy()
        if services_proxy.DefaultTarget:
            log.debug("The default target is already set.")

and I'm guessing we weren't hitting it before. We're now hitting it I believe because of one of the changes in 91586cc171 , the recent commit that changed all this stuff quite a lot. It did this in class ServicesModule:

@@ -164,13 +170,23 @@ class ServicesModule(KickstartModule):
     def default_target(self):
         """Default target of the installed system."""
-        return self._default_target
+        # if no target has been set, default to the text only target
+        if not self._default_target:
+            return TEXT_ONLY_TARGET
+        else:
+            return self._default_target

i.e. previously the `default_target` property might be "" (the value `self._default_target` is originally initialized to) if no default target has yet been set, but after this change, it will never be "", even if nothing has explicitly set a default target, the property will be TEXT_ONLY_TARGET (so, 'multi-user.target'). That means that _set_default_boot_target() will *never* get past its 'has the default target already been set?' check.

Comment 4 Adam Williamson 2019-06-21 19:58:37 UTC
Heh, then *elsewhere* in services.py you actually had to adjust other code to get around this same problem:

-        if self.default_target == TEXT_ONLY_TARGET:
+        # Use the _default_target attribute directly instead of the
+        # default_target property to differentiate if default
+        # target has been set.
+        if self._default_target == TEXT_ONLY_TARGET:
             data.skipx.skipx = True
-        elif self.default_target == GRAPHICAL_TARGET:
+        elif self._default_target == GRAPHICAL_TARGET:
             data.xconfig.startX = True

but neither the commit message nor anything in the code gives any justification for changing the behaviour of the `default_target` property in the first place, so I can't be sure if it's safe to just turn it back :(

Comment 5 Adam Williamson 2019-06-21 21:41:12 UTC
OK, so I poked more into how this works and I think I fixed it.


Comment 6 Martin Kolman 2019-06-24 20:25:55 UTC
And I've fixed it as well. :)

PR: https://github.com/rhinstaller/anaconda/pull/2023

Comment 7 Adam Williamson 2019-07-17 18:08:14 UTC
Yep, it's definitely fixed.

Note You need to log in before you can comment on or make changes to this bug.