systemd: add scripts and systemd units for running at boot #119

dustymabe · 2019-12-21T19:58:10Z

This builds on #100. It fixes up the units with changes from testing and adds a coreos-installer-noreboot.service that is used when the user requests no reboot.

jlebon · 2020-01-06T20:25:53Z

systemd/coreos-installer-generator

+
+    # Create precondition for coreos-installer-reboot.service if requested
+    if ! karg_bool coreos.inst.skip_reboot; then
+        touch /run/coreos-installer-reboot


Hmm, I'm confused why we need the reboot to happen in a separate service entirely. Can't we just do it from the installer directly after a successful install? E.g. we add a --reboot flag and have coreos-installer-service just pass that in if the karg is specified.

I think @bgilbert was trying to keep the utility separate from the service, but I'm not opposed to adding a --reboot flag to the utility. Though maybe we can open a separate issue and do that in a followup so we don't block?

jlebon · 2020-01-06T20:31:08Z

systemd/coreos-installer-reboot.service

@@ -2,7 +2,7 @@
 Description=Reboot after CoreOS Installer
 After=coreos-installer.service
 OnFailure=emergency.target
-OnFailureJobMode=isolate
+OnFailureJobMode=replace-irreversibly


Huh weird, replace-irreversibly didn't work for me in the past (hence why the ignition-dracut services use isolate: https://github.com/coreos/ignition-dracut/blob/c4790bc283c8cc5318019ef8d5ad3929c8259d0c/dracut/30ignition/ignition-complete.target#L12). Though maybe it's time to retest this again and clean things up if it works again.

Let me know what you find

jlebon · 2020-01-06T20:37:29Z

systemd/coreos-installer-noreboot.service

+StandardOutput=inherit
+StandardError=inherit
+ExecStartPre=/usr/bin/echo -e "\nCoreOS install complete. Starting login shell\n"
+ExecStart=/usr/sbin/sulogin --force


Ahh, interesting. I think this overlaps with my previous question re. the reboot service.

So I guess the question is: do we want this boot to be special or just like any other boot (but with code that runs the installer in the background). IOW, if users want to use a shell after installation, they could just provision the system as usual via Ignition and login, right?

I guess, I'm trying to wrap my head around whether we want to do this, when IIUC part of the idea was to move towards coreos-installer just be a regular binary you can run from a normal FCOS/RHCOS boot (or script to run via Ignition), and leave the kargs approach as a "legacy" thing.

and leave the kargs approach as a "legacy" thing.

Though I notice Benjamin's patch adds more kargs to the list for e.g. FCOS streams, so maybe I'm mistaken here.

And OK yeah, I totally missed that we're still overriding default.target here. So this WFM! (I thought we were just running this in the background of an otherwise normal boot.)

OK, I've split out some thoughts on this in #124. But anyway, it doesn't have to block on this, though... can we not add new kargs for the time being?

can we not add new kargs for the time being?

it looks like the ones he added were coreos.inst.insecure coreos.inst.stream coreos.inst.stream_base_url. Those were mostly convenience based on the new functionality within the rust installer. I'd prefer not to rip them out, but also want to get this merged ASAP, so I'll go either way.

Note that the tool defaults to the stable stream. If we don't provide any way to change the stream then anyone trying the installer today (when no stable stream exists) and not providing their own disk image url will experience a failure.

and I think we need coreos.inst.insecure too because otherwise you can't even locally test the kargs approach right now without it.

Hmm, at least coreos.inst.stream_base_url, it seems like we can drop, right? If you're targeting your own image, you can just use coreos.inst.image_url for now.

After that, yeah let me finish testing and then get this in!

yes, that one option seems to be more useful for us testing out the other parts of the metadata we publish. If you'd like I can try to drop that option.

jlebon · 2020-01-06T20:45:13Z

systemd/coreos-installer-reboot.service

@@ -8,3 +8,5 @@ ConditionPathExists=/run/coreos-installer-reboot
 [Service]
 Type=simple
 ExecStart=/usr/bin/systemctl --no-block reboot
+StandardOutput=kmsg+console
+StandardError=kmsg+console


Is this just to be consistent, or did you actually encounter an issue with the service?

I think there was a case where something wasn't working and I had to add this to find out why

ahh yes. I think it was fab2fcc

They were for the old shell script installer, running in the initramfs.

Should be compatible with the legacy kargs syntax, with the following adjustments: - Drop coreos.inst=yes, since it's redundant. coreos.inst.install_dev is mandatory, so run only if that karg is specified. - Expect full device paths in coreos.inst.install_dev. For compatibility, prepend "/dev/" if a bare device name is specified. - Make coreos.inst.ignition_url optional. For compatibility, ignore the karg if "skip" is specified as the value. - Add kargs for Fedora CoreOS stream, stream base URL, and to enable insecure mode.

It looks like getarg() and cmdline_arg() were ditched in favor of karg().

This was renamed in the service so let's rename it in the generator for consistency.

Needed if we're going to be downloading images over the network.

It's at /usr/bin/systemctl.

We can't wait until the service runs to create the flag file (`/run/coreos-installer-reboot`) because systemd unit conditions are evaluated way before the coreos-installer.service runs. Let's do it in the generator instead.

We make the reboot service as After=coreos-installer.service and we mark coreos-installer.serivce as `oneshot` that will make systemd not start any other units until the service has completed.

Add stderr to the installer and stdout/stderr to the reboot service.

isolate wasn't working for me so I checked other units on the system that have OnFailure=emergency.target and noticed almost all of them use OnFailureJobMode=replace-irreversibly. It works for me so let's go with it.

Let's drop to a shell if the user doesn't want to reboot for whatever reason. Otherwise we just idle in the coreos-installer.target but you can't do anything because you have no shell.

On error of the installer we invoke emergency.target. The executable that gets called in emergency.service will won't give us a shell if the root account is locked. We'll get a message like this: ``` Cannot open access to console, the root account is locked. See sulogin(8) man page for more details. ``` We can force to login by setting SYSTEMD_SULOGIN_FORCE=1.

jlebon · 2020-01-07T16:12:25Z

OK, tested this locally both in the interactive mode and via kargs. ✔️ (I fell right in #118 too. Will address that one as a follow-up.)

Anyway, mind addressing #119 (comment) before we merge this? I really don't want to add more kargs than necessary until we sort out #124.

jlebon

LGTM!

Re.

Anyway, mind addressing #119 (comment) before we merge this? I really don't want to add more kargs than necessary until we sort out #124.

Dusty confirmed he'll address it in a follow-up.

dustymabe · 2020-01-07T16:31:37Z

Rebased on top of latest master. Will merge shortly

Pending the discussion in coreos#124 let's minimize the number of kargs for now. coreos.inst.stream_base_url was identified as one that wasn't absolutely necessary. This is a followup to coreos#119 to address this discussion: coreos#119 (comment)

dustymabe · 2020-01-09T02:11:25Z

Dusty confirmed he'll address it in a follow-up.

this was addressed in #125

bgilbert · 2020-01-13T06:56:14Z

From cb6dc61:

We can't wait until the service runs to create the flag file (/run/coreos-installer-reboot) because systemd unit conditions are evaluated way before the coreos-installer.service runs.

They're evaluated at the moment a service is about to be started. Did the original code not work for you in testing?

bgilbert · 2020-01-13T07:00:21Z

This looks great. Thanks for getting it to the finish line!

dustymabe · 2020-01-13T15:08:26Z

From cb6dc61:

We can't wait until the service runs to create the flag file (/run/coreos-installer-reboot) because systemd unit conditions are evaluated way before the coreos-installer.service runs.

They're evaluated at the moment a service is about to be started. Did the original code not work for you in testing?

Nope. It didn't work for me but now that I look at the commits I suspect that the very next commit (87c8c31) is related to why it wasn't working for me.

systemd: service: Make reboot service run after installer

We make the reboot service as After=coreos-installer.service and we
mark coreos-installer.serivce as oneshot that will make systemd not
start any other units until the service has completed.

dustymabe · 2020-01-13T15:08:56Z

@bgilbert Care to post a follow-up to fix that?

This was referenced Dec 21, 2019

can't see image download progress when run via systemd service on boot #120

Closed

systemd: add scripts and systemd units for running at boot #100

Closed

dustymabe force-pushed the dusty branch from 8b21025 to 2c0bf45 Compare December 22, 2019 16:24

jlebon mentioned this pull request Jan 6, 2020

Add Makefile #123

Merged

jlebon reviewed Jan 6, 2020

View reviewed changes

jlebon mentioned this pull request Jan 6, 2020

Configurability for automated installs without kargs #124

Closed

bgilbert and others added 12 commits January 6, 2020 17:23

dracut: drop dracut modules

da95cd2

They were for the old shell script installer, running in the initramfs.

systemd: service: fix calls to get commandline arg values

cdb76d0

It looks like getarg() and cmdline_arg() were ditched in favor of karg().

systemd: generator: rename cmdline_arg() to karg()

8841fb8

This was renamed in the service so let's rename it in the generator for consistency.

systemd: installer: indicate we want the network

7e115d0

Needed if we're going to be downloading images over the network.

systemd: reboot service: fix path to systemctl

2d61ae3

It's at /usr/bin/systemctl.

systemd: generator: mv reboot flag file creation to generator

cb6dc61

We can't wait until the service runs to create the flag file (`/run/coreos-installer-reboot`) because systemd unit conditions are evaluated way before the coreos-installer.service runs. Let's do it in the generator instead.

systemd: service: Make reboot service run after installer

87c8c31

We make the reboot service as After=coreos-installer.service and we mark coreos-installer.serivce as `oneshot` that will make systemd not start any other units until the service has completed.

systemd: services: log more to the console

1dffdf9

Add stderr to the installer and stdout/stderr to the reboot service.

systemd: use OnFailureJobMode=replace-irreversibly

352af2d

isolate wasn't working for me so I checked other units on the system that have OnFailure=emergency.target and noticed almost all of them use OnFailureJobMode=replace-irreversibly. It works for me so let's go with it.

systemd: add coreos-installer-noreboot.service

dd3aa1c

Let's drop to a shell if the user doesn't want to reboot for whatever reason. Otherwise we just idle in the coreos-installer.target but you can't do anything because you have no shell.

jlebon approved these changes Jan 7, 2020

View reviewed changes

dustymabe force-pushed the dusty branch from 2c0bf45 to 15a7926 Compare January 7, 2020 16:30

dustymabe merged commit 0868004 into coreos:master Jan 7, 2020

dustymabe mentioned this pull request Jan 7, 2020

Remove coreos.inst.stream_base_url karg #125

Merged

dustymabe mentioned this pull request Jan 9, 2020

Move coreos-installer out of initramfs coreos/fedora-coreos-tracker#203

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

systemd: add scripts and systemd units for running at boot #119

systemd: add scripts and systemd units for running at boot #119

dustymabe commented Dec 21, 2019

jlebon Jan 6, 2020

dustymabe Jan 6, 2020

jlebon Jan 6, 2020

dustymabe Jan 7, 2020

jlebon Jan 6, 2020

jlebon Jan 6, 2020

jlebon Jan 6, 2020

dustymabe Jan 6, 2020

dustymabe Jan 6, 2020

dustymabe Jan 7, 2020

jlebon Jan 7, 2020

dustymabe Jan 7, 2020 •

edited

jlebon Jan 6, 2020

dustymabe Jan 6, 2020

dustymabe Jan 6, 2020

jlebon commented Jan 7, 2020

jlebon left a comment

dustymabe commented Jan 7, 2020

dustymabe commented Jan 9, 2020

bgilbert commented Jan 13, 2020

bgilbert commented Jan 13, 2020

dustymabe commented Jan 13, 2020

dustymabe commented Jan 13, 2020

systemd: add scripts and systemd units for running at boot #119

systemd: add scripts and systemd units for running at boot #119

Conversation

dustymabe commented Dec 21, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dustymabe Jan 7, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlebon commented Jan 7, 2020

jlebon left a comment

Choose a reason for hiding this comment

dustymabe commented Jan 7, 2020

dustymabe commented Jan 9, 2020

bgilbert commented Jan 13, 2020

bgilbert commented Jan 13, 2020

dustymabe commented Jan 13, 2020

dustymabe commented Jan 13, 2020

dustymabe Jan 7, 2020 •

edited