*: drop dialog #16

arithx · 2019-03-21T15:46:31Z

Switches to echo "" >&2 and updates the StandardOutput of the
coreos-installer.service to kmsg+console. This makes the installer
be non-interactive. Later patches will drop into a shell in the
initramfs if all required parameters are not given.

dustymabe

a few comments and some follow up comments about behavior in a minute.

dustymabe · 2019-05-15T19:38:21Z

coreos-installer

@@ -1,7 +1,5 @@
 #!/bin/bash

-DEFAULT_IMAGE_URL='https://example.com/fedora-coreos.raw.gz'
-


FYI @yuqi-zhang ^^. If we merge this we'll be dropping the need for any patching here.

dustymabe · 2019-05-15T19:39:35Z

coreos-installer

        sleep 5
        reboot
    fi
 }

+USAGE="Usage: $0 [options]


thoughts on making this block into a function and calling it from main() ?

I initially had it in a function and it seemed to be breaking getopts

Did you try calling the function with $@ as an argument. i.e. process_args $@

dustymabe · 2019-05-15T19:41:23Z

dracut/30coreos-installer/coreos-installer.service

@@ -7,6 +7,7 @@ Type=simple
 ExecStart=/usr/libexec/coreos-installer
 #StandardInput=tty-force
 StandardInput=tty
+StandardOutput=journal+console


the PR description says kmsg+console - did that change in an update after the PR was created?

It was lost in a rebase and I just picked journal+console, I'll update back to kmsg

dustymabe · 2019-05-15T22:28:15Z

On a successful install I see:

[  OK  ] Reached target CoreOS Installer Target.
[  OK  ] Started CoreOS Installer.                               
[  OK  ] Reached target Remote File Systems (Pre).   
[    7.522620] coreos-installer[683]:   PID TTY          TIME CMD          
[    7.524060] coreos-installer[683]:   698 tty2     00:00:00 curl
[    8.546166] coreos-installer[683]:   PID TTY          TIME CMD
[    8.547657] coreos-installer[683]:   698 tty2     00:00:00 curl
[    8.589230] coreos-installer[683]: 24
[    9.620445] coreos-installer[683]:   PID TTY          TIME CMD 
[    9.621858] coreos-installer[683]:   698 tty2     00:00:00 curl          
[    9.658000] coreos-installer[683]: 52         
[   10.694455] coreos-installer[683]:   PID TTY          TIME CMD
[   10.696422] coreos-installer[683]:   698 tty2     00:00:01 curl
[   10.743443] coreos-installer[683]: 83                                  
[   11.764424] coreos-installer[683]:   PID TTY          TIME CMD
[   11.767527] coreos-installer[683]: Wiping /dev/sda
[   39.659360] coreos-installer[683]: embedding provided networking options
[   39.706754] coreos-installer[683]: Install complete
[   44.727146] coreos-installer[683]: umount:
         Stopping CoreOS Installer...

The ps output is confusing - maybe we silence that
can we get a few more lines of info output about different steps of the install? at least one for downloading image would be nice

On an unsucessfull install: I provided a bogus URL and my system just stayed at:

[  OK  ] Started CoreOS Installer.
[  OK  ] Reached target Remote File Systems (Pre).
[   12.545760] coreos-installer[682]: failed fetching image headers from http://192.168.1.101:8000/fedora-coreos-30.5-metal-bios.raw.gzz: 22

So it doesn't look like it is hitting the emergency target ?

arithx · 2019-05-15T22:55:26Z

Hmm, my local builds are hitting the emergency shell, I'll go back through and make sure I don't have something out of date that's causing a discrepancy.

arithx · 2019-05-16T15:44:06Z

Updated.

Looks like the OnFailure was moved from coreos-installer.service to coreos-installer.target in #23 but the service runs After= to the target so it didn't trigger through. I've re-added it to coreos-installer.service

Other notable changes in this last patch:

Switched output lines to use a log function which both writes to /tmp/debug as well as writing to stderr
Added -f to the Ignition curl, which as a by-product will now properly error when receiving 404's.
Moved argument parsing into a parse_args function.
Switched back to kmsg+console for StandardOutput in coreos-installer.service

dustymabe · 2019-05-16T18:44:36Z

coreos-installer

        # If the image hasn't show up yet then wait a sec and loop
        if [ ! -f /mnt/dl/imagefile.gz ]; then
            sleep 1
            continue
        fi
        PART_FILE_SIZE=$(ls -l /mnt/dl/imagefile.gz | awk '{print $5}') 2>/dev/null
        PCT=$(dc -e"2 k $PART_FILE_SIZE $IMAGE_SIZE / 100 * p" | sed -e"s/\..*$//" 2>/dev/null)
-        echo $PCT
+        echo "$PCT%" >&2


this still uses echo.. did you mean to point to stderr and not stdout?

also we might want to do "${PCT}%"

I intentionally didn't log this to /tmp/debug as I didn't think it was useful outside of watching it on the console. I can switch it to the log method if you think it has value.

As far as writing to stderr, everything being printed on the serial console is being written to stderr (log is doing it as well).

Will update the variable wrapping.

dustymabe · 2019-05-16T19:30:56Z

Emergency.target seems to work now and the output from a successful install looks better:

coreos-installer[683]: Image size is 666989080                             
coreos-installer[683]: tmpfs sized to 686 MB                               
coreos-installer[683]: IGNITION_URL IS http://192.168.1.101:8000/config.ign
coreos-installer[683]: Selected device is /dev/sda                         
coreos-installer[683]: Mounting tmpfs                                      
coreos-installer[683]: Downloading install image                           
coreos-installer[683]: 25%                                                 
coreos-installer[683]: 52%                                                 
coreos-installer[683]: 79%                                                 
coreos-installer[683]: Wiping /dev/sda
coreos-installer[683]: Writing disk image
coreos-installer[683]: embedding provided networking options
coreos-installer[683]: Install complete
coreos-installer[683]: umount: /mnt/boot_partition: not mounted.

One nit might be to change embedding to Embedding to match most of the other log statements capitalization.

coreos-installer

Drops dialog and switches to text based output. Now exits to the emergency shell and allows /usr/libexec/coreos-installer to be ran as a script.

arithx · 2019-05-16T20:09:39Z

Update pushed.

Changed Embedding capitalization
Image download percent output string format
Help message character line count restricted to 72 per line.

dustymabe · 2019-05-17T15:00:33Z

LGTM

With coreos#16 we no longer need `chvt`, `lsblk` or `tee`.

*: drop dialog

With #16 we no longer need `chvt`, `lsblk` or `tee`.

arithx force-pushed the drop_dialog branch from dc29030 to 7af9e1a Compare April 2, 2019 17:21

arithx force-pushed the drop_dialog branch 4 times, most recently from 973ed3e to 5f5dbce Compare May 13, 2019 19:31

dustymabe self-requested a review May 14, 2019 19:05

dustymabe reviewed May 15, 2019

View reviewed changes

arithx force-pushed the drop_dialog branch from 5f5dbce to 3fdea14 Compare May 16, 2019 15:38

arithx marked this pull request as ready for review May 16, 2019 15:50

dustymabe reviewed May 16, 2019

View reviewed changes

coreos-installer Outdated Show resolved Hide resolved

*: drop dialog

7cba048

Drops dialog and switches to text based output. Now exits to the emergency shell and allows /usr/libexec/coreos-installer to be ran as a script.

arithx force-pushed the drop_dialog branch from 3fdea14 to 7cba048 Compare May 16, 2019 20:07

dustymabe merged commit 13f0de2 into coreos:master May 17, 2019

This was referenced May 17, 2019

support running via CLI invocation #14

Closed

support install through serial console #5

Closed

arithx added a commit to arithx/coreos-installer that referenced this pull request May 17, 2019

30coreos-installer: drop unused inst_modules

e5d01c7

With coreos#16 we no longer need `chvt`, `lsblk` or `tee`.

arithx mentioned this pull request May 17, 2019

30coreos-installer: drop unused inst_modules #32

Merged

dustymabe mentioned this pull request May 20, 2019

various CLI invocation fixups #35

Merged

dustymabe mentioned this pull request Jun 7, 2019

"writing to disk" percentage is off #45

Closed

glennswest referenced this pull request in glennswest/coreos-installer Mar 31, 2020

Merge pull request #16 from arithx/drop_dialog

1ea47a9

*: drop dialog

glennswest referenced this pull request in glennswest/coreos-installer Mar 31, 2020

30coreos-installer: drop unused inst_modules

63250c8

With #16 we no longer need `chvt`, `lsblk` or `tee`.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: drop dialog #16

*: drop dialog #16

arithx commented Mar 21, 2019

dustymabe left a comment

dustymabe May 15, 2019

dustymabe May 15, 2019

arithx May 15, 2019

dustymabe May 16, 2019

dustymabe May 15, 2019

arithx May 15, 2019

dustymabe commented May 15, 2019 •

edited

Loading

arithx commented May 15, 2019

arithx commented May 16, 2019

dustymabe May 16, 2019

arithx May 16, 2019

dustymabe commented May 16, 2019

arithx commented May 16, 2019

dustymabe commented May 17, 2019

		@@ -1,7 +1,5 @@
		#!/bin/bash

		DEFAULT_IMAGE_URL='https://example.com/fedora-coreos.raw.gz'

*: drop dialog #16

*: drop dialog #16

Conversation

arithx commented Mar 21, 2019

dustymabe left a 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 commented May 15, 2019 • edited Loading

arithx commented May 15, 2019

arithx commented May 16, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dustymabe commented May 16, 2019

arithx commented May 16, 2019

dustymabe commented May 17, 2019

dustymabe commented May 15, 2019 •

edited

Loading