Skip to content
This repository has been archived by the owner on Aug 25, 2021. It is now read-only.

99emergency-timeout: improvements to failure message #181

Merged
merged 2 commits into from
May 4, 2020
Merged

99emergency-timeout: improvements to failure message #181

merged 2 commits into from
May 4, 2020

Conversation

bgilbert
Copy link
Contributor

@bgilbert bgilbert commented May 4, 2020

Fix line wrapping and add line breaks. Also remember to check ignition-fetch.service for failure.

See also #168; cc @cgwalters.

If the config is invalid, the failure will show up here.
Line-wrap to 80 characters and add some line breaks.
@cgwalters
Copy link
Member

LGTM, though I would like to solve the more general problem in #168 at some point.

@dustymabe
Copy link
Member

I imagine this was inspired partly because coreos/fedora-coreos-tracker#471 - any ideas on how we could make all of this information get printed out to all consoles specified via console= on the kernel command line ?

@dustymabe
Copy link
Member

ok with this change I now see this on the console instead of what I saw in coreos/fedora-coreos-tracker#471 (comment)

-------------------------------------------------------------------------------
Ignition has failed. Please ensure your config is valid. Note that only
Ignition spec v3.0.0+ configs are accepted.

A CLI validation tool to check this called ignition-validate can be
downloaded from GitHub:
    https://github.com/coreos/ignition/releases

Here are the Ignition logs:
Ignition 2.2.1
Stage: fetch
reading system config file "/usr/lib/ignition/base.ign"
parsing config with SHA512: ff6a5153be363997e4d5d3ea8cc4048373a457c48c4a5b134a08a30aacd167c1e0f099f0bdf1e24c99ad180628cd02b767b863b5fe3a8fce3fe1886847eb8e2e
parsed url from cmdline: ""
no config URL provided
reading system config file "/usr/lib/ignition/user.ign"
no config at "/usr/lib/ignition/user.ign"
op(1): [started]  loading QEMU firmware config module
op(1): executing: "modprobe" "qemu_fw_cfg"
op(1): [finished] loading QEMU firmware config module
parsing config with SHA512: c11f32a098f32ee923f298bbd3cea7e9ec8d1b6e48c87227fc0007780d6d867d559a8a64162535c9e0e30831c2f0d79a63a061b4a7ea4dcd23166fc08b9f280e
failed to fetch config: unsupported config version
failed to acquire config: unsupported config version
Ignition failed: unsupported config version

Press Enter for emergency shell or wait 4 minutes 30 seconds for reboot.

Which is much better.

@@ -29,7 +29,7 @@ _prompt_for_timeout() {
if [[ -e /.emergency-shell-confirmed ]]; then
return
fi
ignition_units="ignition-disks.service ignition-files.service ignition-mount.service"
ignition_units="ignition-fetch.service ignition-disks.service ignition-files.service ignition-mount.service"
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we can just glob on ignition-* here so we don't hit this problem in the future if we add more ignition units.

Here are the Ignition logs:
EOF
journalctl -t ignition --no-pager --no-hostname -o cat
echo
fi
Copy link
Member

Choose a reason for hiding this comment

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

should we add an else to this if statement and just have it print out failed systemd unit info (assuming there is a failed systemd unit) in the case it's not related to ignition? This wouldn't have to be done in this PR, but is an idea to improve things for the future.

@bgilbert
Copy link
Contributor Author

bgilbert commented May 4, 2020

Re serial+VGA console, I'd still like to see coreos/fedora-coreos-tracker#136 happen.

@bgilbert
Copy link
Contributor Author

bgilbert commented May 4, 2020

Would it make sense to defer the other suggested improvements to #168 and get this in as an incremental improvement?

@dustymabe
Copy link
Member

Re serial+VGA console, I'd still like to see coreos/fedora-coreos-tracker#136 happen.

Agree.

Would it make sense to defer the other suggested improvements to #168 and get this in as an incremental improvement?

I think so. Just gave #168 a look and you're right it pretty much implements exactly what I suggested.

Copy link
Member

@dustymabe dustymabe left a comment

Choose a reason for hiding this comment

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

LGTM

@bgilbert bgilbert merged commit 7ff38d9 into coreos:master May 4, 2020
@bgilbert bgilbert deleted the linewrap branch May 4, 2020 23:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants