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

sysupdate: Implement dbus service #28134

Closed
wants to merge 10 commits into from

Conversation

AdrianVovk
Copy link
Contributor

@AdrianVovk AdrianVovk commented Jun 22, 2023

Replaces #27737

@AdrianVovk AdrianVovk changed the title sysupdate: Implement dbus service WIP!!! sysupdate: Implement dbus service Jun 22, 2023
@AdrianVovk AdrianVovk force-pushed the sysupdate-dbus branch 2 times, most recently from f310ffd to d8d20e7 Compare July 1, 2023 00:29
@github-actions github-actions bot added the meson label Jul 1, 2023
src/sysupdate/sysupdated.c Fixed Show fixed Hide fixed
src/sysupdate/sysupdated.c Fixed Show fixed Hide fixed
@poettering
Copy link
Member

I think it would make sense to split out the first commit into a separate PR. makes sense on its own, we could merge this ahead of time


(void) sd_event_set_watchdog(m->event, true);

r = sd_event_add_signal(m->event, NULL, SIGINT, NULL, NULL);
Copy link
Member

Choose a reason for hiding this comment

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

in new code please use SD_EVENT_SIGNAL_PROCMASK right from the start, and drop the manual sigprocmask() in the main() func

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like I can't do anything about SIGCHILD, though, for sd_event_add_child? So I cannot drop the manual sigprocmask AFAICT

@github-actions github-actions bot added please-review PR is ready for (re-)review by a maintainer and removed reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks labels Jul 20, 2023
@AdrianVovk AdrianVovk force-pushed the sysupdate-dbus branch 3 times, most recently from bfb0744 to e377e43 Compare July 20, 2023 20:36
@pothos
Copy link
Contributor

pothos commented Jul 21, 2023

I though about running systemd-sysupdate from the initrd to download systemd-sysext images on first boot instead of having them shipped through other means (pre-baked in image, or through credentials/Ignition) on first boot. However, that would require to enumerate the components with systemd-sysupdate components and then call it for each component. With #27737 that would be a bit easier but now I see that it would only be available through D-Bus and not usable from the initrd.
Just giving this as a thought, maybe it's even not a good idea because running the update would have to be conditional to either opt-in or opt-out depending on what makes sense for the system, and in the initrd it's a bit ugly to specify this condition through, e.g., the kernel cmdline. Or maybe specific components could opt-in to be updated from the initrd by being marked as that and the rest is skipped in the enumeration.

@AdrianVovk
Copy link
Contributor Author

AdrianVovk commented Jul 21, 2023

systemd-sysupdate is the wrong tool for the job if you want to just download something. The right tool for the job would be systemd-importd (modified to be able to download & install sysexts/confexts/portable services, instead of just machines).
However that's another dbus service.

BUT, I see nothing preventing you from using a part of importd to download the sysext from the initrd. You can run it in a systemd unit, and configure it from the cmdline if you want. Something along the lines of:

[Unit]
...
ConditionKernelCommandLine=!myproject.disable_extension_download
After=sysroot.mount
...

[Service]
Type=oneshot
ExecStart=/usr/lib/systemd/systemd-pull raw https://example.com/mysysext.raw.xz /sysroot/var/lib/extensions/mysysext.raw

You can always make this a little more sophisticated with, for instance, a generator or a script which scans /sysroot for some config file(s) and downloads things based on that.

Edit: systemd-sysupdate is a version management layer sitting on top of systemd-pull. So if you're just downloading a file, then you don't need the version management and thus you can just do systemd-pull directly. Now if for whatever reason you really do need the version management (maybe you want to autodetect the latest version of your sysext?) you can still fork of systemd-sysupdate --definitions=/path/to/defs and bypass the service entirely

@pothos
Copy link
Contributor

pothos commented Jul 22, 2023

To your edit; yes, when systemd-sysupdate does the update and no file exists, it will download the latest version which is quite handy compared to having to specify the download explicitly because it's not only duplication but the URL also requires a particular version which is probably outdated and then after provisioning an extra reboot (or live reload) is needed.
Anyway, this would need not only the "update-all" command but also a way to control whether this action runs, and since the above workaround is ok for the time being I think it's not too important now but something to consider for the future direction.

@DaanDeMeyer
Copy link
Contributor

(There is a TODO to make dbus work in the initrd btw, maybe it's just time to finally make that a thing)

This lets a caller construct a BusLocator with a variable path, which
can then be used like any other BusLocator. This permits the use of the
convenient bus_call_method/etc. methods in situations where an object
path for some set of objects varies.

This will be used in updatectl.
We set up a NOTIFY_SOCKET to get download progress notifications from
each individual import helper. Along with the number of import jobs we
have to run, this gives an overall progress value which we report using
sd_notify
This prevents sysupdate from going out to the network to enumerate
available instances. When combined with the list command, this lets us
query installed instances
Previously, the JSON output happened mostly as an accident (i.e. just
dumped tables intended for viewing). Now we have more complete JSON
output.
Makes it possible to specify URLs to a changelog and an appstream
catalog XML in the sysupdate.d/*.conf files. This will be passed along
to the clients of systemd-sysupdated, which can then present this data.
This will let us reuse UpdateSetFlags in updatectl
This will let us reuse reboot_now in updatectl
This is the command-line tool to manage systemd-sysudpated
@AdrianVovk
Copy link
Contributor Author

@poettering I know we discussed varlink @ ASG, but do you want this in this PR? I'd think its something we can add later?

@keszybz
Copy link
Member

keszybz commented Nov 17, 2023

Oops, this was forgotten. Please rebase. I think at least the man pages need version info added to pass tests.

@keszybz keszybz added needs-rebase and removed please-review PR is ready for (re-)review by a maintainer labels Nov 17, 2023
@YHNdnzj
Copy link
Member

YHNdnzj commented May 7, 2024

Can this be closed in favor of #32363?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

6 participants