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.

Sign up for GitHub

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

Jump to bottom

Drop systemd service #663

Merged
merged 1 commit into from
May 29, 2024
Merged

Drop systemd service #663

merged 1 commit into from
May 29, 2024

Conversation

HuijingHei
Copy link
Member

Fixes #551

cgwalters
cgwalters reviewed May 22, 2024
View reviewed changes
Copy link
Member

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this looks like a good start! But per my comment in the other thread we do need to at least maintain some mechanism for "locking".

Ostree for example does this https://github.com/ostreedev/ostree/blob/64a09da0eb5ca4dade83125a2ebc47b52c60c5a5/src/libostree/ostree-sysroot.c#L1684
with a classic unix lockfile.

I would lean towards continuing to run under systemd under a well-known unit name, which also acts as a form of "lock" because only one instance a unit can be active at a time.

@HuijingHei HuijingHei force-pushed the drop-systemd-service branch 2 times, most recently from b741034 to 8269c57 Compare May 23, 2024 08:03
@HuijingHei
Copy link
Member Author

Thanks Colin a lot for the detailed info, copy the comment here:

Basically we detect if we're running in systemd; if we're not, we re-exec ourselves via systemd-run. Then we can just directly run code in what is now the daemon.

I think an important aspect of this is that we retain something like --unit bootupd which acts as a lock - only one unit with that name can run at a time to avoid two concurrent invocations breaking things.

cgwalters
cgwalters approved these changes May 23, 2024
View reviewed changes
Copy link
Member

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! This looks good to me.

@HuijingHei HuijingHei force-pushed the drop-systemd-service branch 2 times, most recently from 2727f83 to 8885b6d Compare May 24, 2024 11:03
cgwalters
cgwalters requested changes May 24, 2024
View reviewed changes
Copy link
Member

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much for working on this!

src/cli/bootupctl.rs Outdated Show resolved Hide resolved
src/cli/bootupctl.rs Show resolved Hide resolved
cgwalters
cgwalters reviewed May 24, 2024
View reviewed changes
src/cli/bootupctl.rs Outdated Show resolved Hide resolved
@HuijingHei HuijingHei force-pushed the drop-systemd-service branch 4 times, most recently from 6d217ac to f241cec Compare May 27, 2024 08:00
@HuijingHei
Copy link
Member Author

Seems the bootupd.service is not removed when building with override/rootfs, which might be the limitation of cosa:

[coreos-assembler]$ cosa run -m 4096
[core@cosa-devsh ~]$ systemctl status bootupd
○ bootupd.service - bootloader update daemon
     Loaded: loaded (/usr/lib/systemd/system/bootupd.service; static)
    Drop-In: /usr/lib/systemd/system/service.d
             └─10-timeout-abort.conf
     Active: inactive (dead)
TriggeredBy: ● bootupd.socket
       Docs: https://github.com/coreos/bootupd

[core@cosa-devsh ~]$ sudo -i
[root@cosa-devsh ~]# bootupctl status
Failed to start transient service unit: Unit bootupd.service was already loaded or has a fragment file.

Change to temp bootupd1.service to do some testing, find 2 issues:

  1. run sudo bootupctl status will not output the result directly, this is different from before, should change this
[core@cosa-devsh ~]$ sudo bootupctl status
Running as unit: bootupd1.service; invocation ID: e24cb1628aab4611839f9842373a7387
[core@cosa-devsh ~]$ journalctl -u bootupd1.service
May 27 10:07:40 cosa-devsh systemd[1]: Started bootupd1.service - /usr/bin/bootupctl status.
May 27 10:07:40 cosa-devsh bootupctl[2085]: Component BIOS
May 27 10:07:40 cosa-devsh bootupctl[2085]:   Installed: grub2-tools-1:2.06-121.fc40.x86_64
May 27 10:07:40 cosa-devsh bootupctl[2085]:   Update: At latest version
May 27 10:07:40 cosa-devsh bootupctl[2085]: Component EFI
May 27 10:07:40 cosa-devsh bootupctl[2085]:   Installed: grub2-efi-x64-1:2.06-121.fc40.x86_64,shim-x64-15.8-3.x86_64
May 27 10:07:40 cosa-devsh bootupctl[2085]:   Update: At latest version
May 27 10:07:40 cosa-devsh bootupctl[2085]: No components are adoptable.
May 27 10:07:40 cosa-devsh bootupctl[2085]: CoreOS aleph version: 40.20240527.dev.1
May 27 10:07:40 cosa-devsh bootupctl[2085]: Boot method: BIOS
May 27 10:07:40 cosa-devsh systemd[1]: bootupd1.service: Deactivated successfully.
  1. The options can not be passed for status, for example --print-if-available and --json, should fix this.
[core@cosa-devsh ~]$ sudo bootupctl status --print-if-available
Running as unit: bootupd1.service; invocation ID: 6bc7f05562b3446f9bfc98eef8e57044

[core@cosa-devsh ~]$ journalctl -u bootupd1.service
May 27 10:44:59 cosa-devsh systemd[1]: Started bootupd1.service - /usr/bin/bootupctl status.
May 27 10:45:00 cosa-devsh bootupctl[2300]: Component BIOS
May 27 10:45:00 cosa-devsh bootupctl[2300]:   Installed: grub2-tools-1:2.06-121.fc40.x86_64
May 27 10:45:00 cosa-devsh bootupctl[2300]:   Update: At latest version
May 27 10:45:00 cosa-devsh bootupctl[2300]: Component EFI
May 27 10:45:00 cosa-devsh bootupctl[2300]:   Installed: grub2-efi-x64-1:2.06-121.fc40.x86_64,shim-x64-15.8-3.x86_64
May 27 10:45:00 cosa-devsh bootupctl[2300]:   Update: At latest version
May 27 10:45:00 cosa-devsh bootupctl[2300]: No components are adoptable.
May 27 10:45:00 cosa-devsh bootupctl[2300]: CoreOS aleph version: 40.20240527.dev.1
May 27 10:45:00 cosa-devsh bootupctl[2300]: Boot method: BIOS
May 27 10:45:00 cosa-devsh systemd[1]: bootupd1.service: Deactivated successfully.

@cgwalters
Copy link
Member

Seems the bootupd.service is not removed when building with override/rootfs,

Yeah, I don't think we need to worry about it.

run sudo bootupctl status will not output the result directly, this is different from before, should change this

Right, need to pass -P to systemd-run

The options can not be passed for status, for example --print-if-available and --json, should fix this.

Right, this one is tricky; what we probably need to do is detect the need to re-exec before parsing the args with clap, then we can directly pass through std::env::args() to the exec() (and not parse the options twice).

@HuijingHei HuijingHei force-pushed the drop-systemd-service branch 3 times, most recently from 259bc30 to 0065833 Compare May 28, 2024 06:44
@HuijingHei
Copy link
Member Author

HuijingHei commented May 28, 2024 •
edited

Seems the bootupd.service is not removed when building with override/rootfs,

Yeah, I don't think we need to worry about it.

Agree, it will not be the problem if using rpm. @ravanelli has the workaround to add it in fedora-coreos.yaml.

run sudo bootupctl status will not output the result directly, this is different from before, should change this

Right, need to pass -P to systemd-run

-P works, thanks!

The options can not be passed for status, for example --print-if-available and --json, should fix this.

Right, this one is tricky; what we probably need to do is detect the need to re-exec before parsing the args with clap, then we can directly pass through std::env::args() to the exec() (and not parse the options twice).

SGTM, thanks!

@HuijingHei
Copy link
Member Author

HuijingHei commented May 28, 2024 •
edited

Also check the selinux avc denied logs, should we revert the bootupd policy fedora-selinux/selinux-policy#1598?

avc logs 
----
time->Tue May 28 07:14:21 2024
type=AVC msg=audit(1716880461.014:159): avc:  denied  { search } for  pid=2118 comm="bootupctl" name="/" dev="vda4" ino=128 scontext=system_u:system_r:bootupd_t:s0 tcontext=system_u:object_r:unlabeled_t:s0 tclass=dir permissive=1
----
time->Tue May 28 07:14:21 2024
type=AVC msg=audit(1716880461.014:160): avc:  denied  { read } for  pid=2118 comm="bootupctl" name=".coreos-aleph-version.json" dev="vda4" ino=133 scontext=system_u:system_r:bootupd_t:s0 tcontext=system_u:object_r:unlabeled_t:s0 tclass=lnk_file permissive=1
----
time->Tue May 28 07:14:21 2024
type=AVC msg=audit(1716880461.014:161): avc:  denied  { getattr } for  pid=2118 comm="bootupctl" path="/sysroot/.aleph-version.json" dev="vda4" ino=132 scontext=system_u:system_r:bootupd_t:s0 tcontext=system_u:object_r:unlabeled_t:s0 tclass=file permissive=1
----
time->Tue May 28 07:14:21 2024
type=AVC msg=audit(1716880461.014:162): avc:  denied  { read } for  pid=2118 comm="bootupctl" name=".aleph-version.json" dev="vda4" ino=132 scontext=system_u:system_r:bootupd_t:s0 tcontext=system_u:object_r:unlabeled_t:s0 tclass=file permissive=1
----
time->Tue May 28 07:14:21 2024
type=AVC msg=audit(1716880461.014:163): avc:  denied  { open } for  pid=2118 comm="bootupctl" path="/sysroot/.aleph-version.json" dev="vda4" ino=132 scontext=system_u:system_r:bootupd_t:s0 tcontext=system_u:object_r:unlabeled_t:s0 tclass=file permissive=1
----
time->Tue May 28 07:16:38 2024
type=AVC msg=audit(1716880598.115:173): avc:  denied  { getattr } for  pid=2137 comm="bootupctl" path="/boot/efi" dev="vda3" ino=65537 scontext=system_u:system_r:bootupd_t:s0 tcontext=system_u:object_r:unlabeled_t:s0 tclass=dir permissive=1
----
time->Tue May 28 07:16:38 2024
type=AVC msg=audit(1716880598.115:174): avc:  denied  { getattr } for  pid=2137 comm="bootupctl" path="/dev/vda2" dev="devtmpfs" ino=487 scontext=system_u:system_r:bootupd_t:s0 tcontext=system_u:object_r:fixed_disk_device_t:s0 tclass=blk_file permissive=1
----
time->Tue May 28 07:16:38 2024
type=AVC msg=audit(1716880598.116:175): avc:  denied  { execute } for  pid=2138 comm="bootupctl" name="mount" dev="vda4" ino=1347395 scontext=system_u:system_r:bootupd_t:s0 tcontext=system_u:object_r:mount_exec_t:s0 tclass=file permissive=1
----
time->Tue May 28 07:16:38 2024
type=AVC msg=audit(1716880598.116:176): avc:  denied  { read open } for  pid=2138 comm="bootupctl" path="/usr/bin/mount" dev="vda4" ino=1347395 scontext=system_u:system_r:bootupd_t:s0 tcontext=system_u:object_r:mount_exec_t:s0 tclass=file permissive=1
----
time->Tue May 28 07:16:38 2024
type=AVC msg=audit(1716880598.116:177): avc:  denied  { execute_no_trans } for  pid=2138 comm="bootupctl" path="/usr/bin/mount" dev="vda4" ino=1347395 scontext=system_u:system_r:bootupd_t:s0 tcontext=system_u:object_r:mount_exec_t:s0 tclass=file permissive=1
----
time->Tue May 28 07:16:38 2024
type=AVC msg=audit(1716880598.116:178): avc:  denied  { map } for  pid=2138 comm="mount" path="/usr/bin/mount" dev="vda4" ino=1347395 scontext=system_u:system_r:bootupd_t:s0 tcontext=system_u:object_r:mount_exec_t:s0 tclass=file permissive=1
----
time->Tue May 28 07:16:38 2024
type=AVC msg=audit(1716880598.118:179): avc:  denied  { read } for  pid=2138 comm="mount" name="vda2" dev="devtmpfs" ino=487 scontext=system_u:system_r:bootupd_t:s0 tcontext=system_u:object_r:fixed_disk_device_t:s0 tclass=blk_file permissive=1
----
time->Tue May 28 07:16:38 2024
type=AVC msg=audit(1716880598.118:180): avc:  denied  { open } for  pid=2138 comm="mount" path="/dev/vda2" dev="devtmpfs" ino=487 scontext=system_u:system_r:bootupd_t:s0 tcontext=system_u:object_r:fixed_disk_device_t:s0 tclass=blk_file permissive=1
----
time->Tue May 28 07:16:38 2024
type=AVC msg=audit(1716880598.118:181): avc:  denied  { ioctl } for  pid=2138 comm="mount" path="/dev/vda2" dev="devtmpfs" ino=487 ioctlcmd=0x1272 scontext=system_u:system_r:bootupd_t:s0 tcontext=system_u:object_r:fixed_disk_device_t:s0 tclass=blk_file permissive=1
----
time->Tue May 28 07:16:38 2024
type=AVC msg=audit(1716880598.118:182): avc:  denied  { read } for  pid=2138 comm="mount" name="252:2" dev="sysfs" ino=30332 scontext=system_u:system_r:bootupd_t:s0 tcontext=system_u:object_r:sysfs_t:s0 tclass=lnk_file permissive=1
----
time->Tue May 28 07:16:38 2024
type=AVC msg=audit(1716880598.118:183): avc:  denied  { read } for  pid=2138 comm="mount" name="vda2" dev="sysfs" ino=30307 scontext=system_u:system_r:bootupd_t:s0 tcontext=system_u:object_r:sysfs_t:s0 tclass=dir permissive=1
----
time->Tue May 28 07:16:38 2024
type=AVC msg=audit(1716880598.118:184): avc:  denied  { read } for  pid=2138 comm="mount" name="dev" dev="sysfs" ino=29325 scontext=system_u:system_r:bootupd_t:s0 tcontext=system_u:object_r:sysfs_t:s0 tclass=file permissive=1
----
time->Tue May 28 07:16:38 2024
type=AVC msg=audit(1716880598.118:185): avc:  denied  { open } for  pid=2138 comm="mount" path="/sys/devices/pci0000:00/0000:00:04.0/virtio2/block/vda/dev" dev="sysfs" ino=29325 scontext=system_u:system_r:bootupd_t:s0 tcontext=system_u:object_r:sysfs_t:s0 tclass=file permissive=1
----
time->Tue May 28 07:16:38 2024
type=AVC msg=audit(1716880598.118:186): avc:  denied  { getattr } for  pid=2138 comm="mount" path="/sys/devices/pci0000:00/0000:00:04.0/virtio2/block/vda/dev" dev="sysfs" ino=29325 scontext=system_u:system_r:bootupd_t:s0 tcontext=system_u:object_r:sysfs_t:s0 tclass=file permissive=1
----
time->Tue May 28 07:16:38 2024
type=AVC msg=audit(1716880598.126:187): avc:  denied  { search } for  pid=2138 comm="mount" name="mount" dev="tmpfs" ino=382 scontext=system_u:system_r:bootupd_t:s0 tcontext=system_u:object_r:mount_var_run_t:s0 tclass=dir permissive=1
----
time->Tue May 28 07:16:38 2024
type=AVC msg=audit(1716880598.126:188): avc:  denied  { getattr } for  pid=2138 comm="mount" path="/run/mount" dev="tmpfs" ino=382 scontext=system_u:system_r:bootupd_t:s0 tcontext=system_u:object_r:mount_var_run_t:s0 tclass=dir permissive=1
----
time->Tue May 28 07:16:38 2024
type=AVC msg=audit(1716880598.126:189): avc:  denied  { read write } for  pid=2138 comm="mount" name="mount" dev="tmpfs" ino=382 scontext=system_u:system_r:bootupd_t:s0 tcontext=system_u:object_r:mount_var_run_t:s0 tclass=dir permissive=1
----
time->Tue May 28 07:16:38 2024
type=AVC msg=audit(1716880598.130:190): avc:  denied  { mount } for  pid=2138 comm="mount" name="/" dev="vda2" ino=1 scontext=system_u:system_r:bootupd_t:s0 tcontext=system_u:object_r:dosfs_t:s0 tclass=filesystem permissive=1
----
time->Tue May 28 07:16:38 2024
type=AVC msg=audit(1716880598.130:191): avc:  denied  { mounton } for  pid=2138 comm="mount" path="/boot/efi" dev="vda3" ino=65537 scontext=system_u:system_r:bootupd_t:s0 tcontext=system_u:object_r:unlabeled_t:s0 tclass=dir permissive=1
----
time->Tue May 28 07:16:38 2024
type=AVC msg=audit(1716880598.131:192): avc:  denied  { getattr } for  pid=2138 comm="mount" path="/run/systemd/systemd-units-load" dev="tmpfs" ino=49 scontext=system_u:system_r:bootupd_t:s0 tcontext=system_u:object_r:init_var_run_t:s0 tclass=file permissive=1
----
time->Tue May 28 07:16:38 2024
type=AVC msg=audit(1716880598.132:193): avc:  denied  { getattr } for  pid=2137 comm="bootupctl" path="/boot/efi/EFI/BOOT/BOOTX64.EFI" dev="vda2" ino=115 scontext=system_u:system_r:bootupd_t:s0 tcontext=system_u:object_r:dosfs_t:s0 tclass=file permissive=1
----
time->Tue May 28 07:16:38 2024
type=AVC msg=audit(1716880598.132:194): avc:  denied  { read } for  pid=2137 comm="bootupctl" name="BOOTX64.EFI" dev="vda2" ino=115 scontext=system_u:system_r:bootupd_t:s0 tcontext=system_u:object_r:dosfs_t:s0 tclass=file permissive=1
----
time->Tue May 28 07:16:38 2024
type=AVC msg=audit(1716880598.132:195): avc:  denied  { open } for  pid=2137 comm="bootupctl" path="/boot/efi/EFI/BOOT/BOOTX64.EFI" dev="vda2" ino=115 scontext=system_u:system_r:bootupd_t:s0 tcontext=system_u:object_r:dosfs_t:s0 tclass=file permissive=1
----
time->Tue May 28 07:16:38 2024
type=AVC msg=audit(1716880598.133:196): avc:  denied  { search } for  pid=2137 comm="bootupctl" name="pki" dev="vda4" ino=696603 scontext=system_u:system_r:bootupd_t:s0 tcontext=system_u:object_r:cert_t:s0 tclass=dir permissive=1
----
time->Tue May 28 07:16:38 2024
type=AVC msg=audit(1716880598.133:197): avc:  denied  { read } for  pid=2137 comm="bootupctl" name="openssl.cnf" dev="vda4" ino=696617 scontext=system_u:system_r:bootupd_t:s0 tcontext=system_u:object_r:cert_t:s0 tclass=file permissive=1
----
time->Tue May 28 07:16:38 2024
type=AVC msg=audit(1716880598.133:198): avc:  denied  { open } for  pid=2137 comm="bootupctl" path="/etc/pki/tls/openssl.cnf" dev="vda4" ino=696617 scontext=system_u:system_r:bootupd_t:s0 tcontext=system_u:object_r:cert_t:s0 tclass=file permissive=1
----
time->Tue May 28 07:16:38 2024
type=AVC msg=audit(1716880598.133:199): avc:  denied  { getattr } for  pid=2137 comm="bootupctl" path="/etc/pki/tls/openssl.cnf" dev="vda4" ino=696617 scontext=system_u:system_r:bootupd_t:s0 tcontext=system_u:object_r:cert_t:s0 tclass=file permissive=1
----
time->Tue May 28 07:16:38 2024
type=AVC msg=audit(1716880598.138:200): avc:  denied  { getattr } for  pid=2137 comm="bootupctl" path="/boot/efi/EFI/BOOT/fbx64.efi" dev="vda2" ino=116 scontext=system_u:system_r:bootupd_t:s0 tcontext=system_u:object_r:dosfs_t:s0 tclass=file permissive=1
----
time->Tue May 28 07:16:38 2024
type=AVC msg=audit(1716880598.138:201): avc:  denied  { read } for  pid=2137 comm="bootupctl" name="fbx64.efi" dev="vda2" ino=116 scontext=system_u:system_r:bootupd_t:s0 tcontext=system_u:object_r:dosfs_t:s0 tclass=file permissive=1
----
time->Tue May 28 07:16:38 2024
type=AVC msg=audit(1716880598.138:202): avc:  denied  { open } for  pid=2137 comm="bootupctl" path="/boot/efi/EFI/BOOT/fbx64.efi" dev="vda2" ino=116 scontext=system_u:system_r:bootupd_t:s0 tcontext=system_u:object_r:dosfs_t:s0 tclass=file permissive=1
----
time->Tue May 28 07:16:38 2024
type=AVC msg=audit(1716880598.170:203): avc:  denied  { unmount } for  pid=2139 comm="umount" scontext=system_u:system_r:bootupd_t:s0 tcontext=system_u:object_r:dosfs_t:s0 tclass=filesystem permissive=1

@cgwalters
Copy link
Member

Also check the selinux avc denied logs, should we revert the bootupd policy fedora-selinux/selinux-policy#1598?

I think so yes. It was clearly never tested.

@HuijingHei
Drop systemd service
261fb5e 
Fixes coreos#551

Get hints by coreos#551 (comment),
and copy the comment here:
Basically we detect if we're running in systemd; if we're not,
we re-exec ourselves via systemd-run. Then we can just directly
run code in what is now the daemon.

I think an important aspect of this is that we retain something
like `--unit bootupd` which acts as a lock - only one unit with
that name can run at a time to avoid two concurrent invocations
breaking things.
@travier
Copy link
Member

travier commented May 29, 2024

Thanks! I've not tested the PR but looks good overall.

@cgwalters cgwalters merged commit 19610be into coreos:main May 29, 2024
9 checks passed
@HuijingHei HuijingHei deleted the drop-systemd-service branch May 30, 2024 01:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cgwalters cgwalters cgwalters approved these changes

@travier travier travier left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

drop systemd service
3 participants
@HuijingHei @cgwalters @travier