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

"enable boot-efi.mount" make ppc64le platform fail #115

Closed
menantea opened this issue Jul 11, 2019 · 16 comments · Fixed by #155
Closed

"enable boot-efi.mount" make ppc64le platform fail #115

menantea opened this issue Jul 11, 2019 · 16 comments · Fixed by #155

Comments

@menantea
Copy link

cosa run failed on ppc64le platform.
ppc64le qcow2 generated image does not end its boot and get stuck with no access to login.

The problem is due to the "enable boot-efi.mount" added in
overlay/usr/lib/systemd/system-preset/42-coreos.preset for all platforms.
see commit cb3f240
and also #105 and #106 pull requests

I didn't find a fair way to avoid it, please @ajeddeloh could you have a look ?

@ajeddeloh
Copy link
Contributor

I'll take a look at this. I'm not sure why the ConditionPathExists= isn't preventing it from running.

@menantea
Copy link
Author

It's working fine for ppc64le when I replace the "RequirdBy=local-fs.target" by "Before=local-fs.target" in boot-efi.mount.
I didn't test for x86-64.
I am not sure to understand very well how work, systemd work and boot sequence process, then I can say if it could trig to side effects but I guess the problem is coming from this "RequiredBy" property.

@jlebon
Copy link
Member

jlebon commented Aug 2, 2019

Hmm yeah, I had noticed the same issue re. the conditional not being respected in #105 (comment).

Related: #119 -- systemd was mounting things automatically. Might be worth rebuilding on the latest FCOS config with that?

@alicefr
Copy link

alicefr commented Aug 8, 2019

hi, I've the same issue on s390x. Would be possible to have some sort of parametrization for some of the config? Like arch specific. This could also solve some other issues I have on s390x

@jcajka
Copy link
Contributor

jcajka commented Aug 12, 2019

I have been investigating this for awhile as this has appeared as unit file dependency issue, looking for cycles. I have spoken about it with @msekletar (thanks !!) and he pointed out to me that actually the RequriedBy is incorrect(too strong dependency, i.e. if the dependent unit is not executed for any reason, including the conditional in the unit, it will be marked as failed) and it it should be rather WantedBy=local-fs-pre.target. PR in works.

@jlebon
Copy link
Member

jlebon commented Aug 12, 2019

actually the RequriedBy is incorrect(too strong dependency, i.e. if the dependent unit is not executed for any reason, including the conditional in the unit, it will be marked as failed)

Hmm odd, are we sure about this? systemd.unit(5) says:

Requires=
...
Note that this dependency type does not imply that the other unit always has to be in active state when this unit is running. Specifically: failing condition checks (such as ConditionPathExists=, ConditionPathIsSymbolicLink=, ... — see below) do not cause the start job of a unit with a Requires= dependency on it to fail.

(RequiredBy is the "reverse" property of Requires= so I would expect it to follow the same semantics there. If it doesn't, that might be a systemd bug?).

it should be rather WantedBy=local-fs-pre.target

local-fs-pre.target is meant to run units which need to run before mounting any filesystems.


Does a run with systemd.log_level=debug print anything insightful here about why the unit is failing/not silently being skipped?

@jlebon
Copy link
Member

jlebon commented Aug 12, 2019

systemd.unit(5) says:

Seems to work as documented in a quick test on FCOS itself:

[root@localhost ~]# cat /run/systemd/system/foo.service
[Unit]
Description=Foo service
ConditionPathExists=/enoent

[Service]
Type=oneshot
ExecStart=/bin/echo 'foo'

[Install]
RequiredBy=bar.service
[root@localhost ~]# cat /run/systemd/system/bar.service
[Unit]
Description=Bar service

[Service]
Type=oneshot
ExecStart=/bin/echo 'bar'
[root@localhost ~]# systemctl enable foo.service
Created symlink /etc/systemd/system/bar.service.requires/foo.service → /run/systemd/system/foo.service.
[root@localhost ~]# systemctl start bar.service
[root@localhost ~]# systemctl status foo.service
● foo.service - Foo service
   Loaded: loaded (/run/systemd/system/foo.service; enabled; vendor preset: disabled)
   Active: inactive (dead)

Aug 12 22:30:45 localhost systemd[1]: Condition check resulted in Foo service being skipped.
[root@localhost ~]# journalctl -u bar
-- Logs begin at Mon 2019-08-12 22:25:42 UTC, end at Mon 2019-08-12 22:30:45 UTC. --
Aug 12 22:30:45 localhost systemd[1]: Starting Bar service...
Aug 12 22:30:45 localhost echo[1234]: bar
Aug 12 22:30:45 localhost systemd[1]: bar.service: Succeeded.
Aug 12 22:30:45 localhost systemd[1]: Started Bar service.

@jlebon
Copy link
Member

jlebon commented Aug 12, 2019

Can we sanity check BTW that Anaconda didn't create /boot/efi on s390x/ppc64le?

I have been investigating this for awhile as this has appeared as unit file dependency issue, looking for cycles.

Ahh didn't catch right away that you were referring to cycles in the dependency graph. Can you post the cycle systemd logs out?

@alicefr
Copy link

alicefr commented Aug 13, 2019

@jlebon anaconda didn't create the efi partition on s390x

@jlebon
Copy link
Member

jlebon commented Aug 13, 2019

@alicefr Not the partition, but the /boot/efi directory itself. If so, then we might need to use a better conditional as suggested in #105 (comment) (at least until we switch all architectures to create_disk.sh... which actually I see is already underway in coreos/coreos-assembler#701 for s390x).

@tuan-hoang1
Copy link
Contributor

tuan-hoang1 commented Aug 13, 2019

We are planning to add masking efi-mount in image-base.ks for anaconda installation in rhcos-4.2 branch but I think ConditionPathExists is the best way. For non-anaconda master branch, just like Jonathan said, we are good with /boot/efi directory.

@dustymabe
Copy link
Member

dustymabe commented Aug 16, 2019

Where do we stand on this issue?

@msekletar
Copy link

I think the condition on /boot/efi is not the best option. If you really want mount to trigger only on UEFI enabled systems you should check for /sys/firmware/efi/ instead (that is what systemd does to determine if we are running on UEFI capable system). You can also have more than one condition in the unit file.

Also WantedBy=local-fs-pre.target is incorrect, you should enable the unit under local-fs.target.

@msekletar
Copy link

msekletar commented Aug 26, 2019

he pointed out to me that actually the RequriedBy is incorrect(too strong dependency, i.e. if the dependent unit is not executed for any reason, including the conditional in the unit, it will be marked as failed)

That is what I've probably said but as @jlebon explained, conditions are an exception to this rule. Sorry for the confusion.

At any rate, you really don't want RequiredBy. In order to make sure that the mount is pulled into boot transaction by the respective target you want Wants/WantedBy.

jcajka added a commit to jcajka/fedora-coreos-config that referenced this issue Aug 27, 2019
@jcajka
Copy link
Contributor

jcajka commented Aug 27, 2019

I think the condition on /boot/efi is not the best option. If you really want mount to trigger only on UEFI enabled systems you should check for /sys/firmware/efi/ instead (that is what systemd does to determine if we are running on UEFI capable system). You can also have more than one condition in the unit file.

Also WantedBy=local-fs-pre.target is incorrect, you should enable the unit under local-fs.target.

Thanks for the feedback. I have incorporated the changes in to the PR. Working on testing it atm. @jlebon what do you think about it?

@bgilbert
Copy link
Contributor

bgilbert commented Sep 5, 2019

Conditions are evaluated only after a unit has pulled in its dependencies, so they won't prevent e.g. waiting for a device unit to activate. Avoiding this generally requires a systemd generator.

#155 moves boot-efi.mount to a generator and enables it only on platforms with UEFI support. I didn't add the /sys/firmware/efi check because IMO we should still mount /boot/efi on x86 BIOS systems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
9 participants