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

05core/*: display Ignition warnings on the console #1621

Merged
merged 1 commit into from
Apr 6, 2022
Merged

05core/*: display Ignition warnings on the console #1621

merged 1 commit into from
Apr 6, 2022

Conversation

sohankunkerkar
Copy link
Member

@sohankunkerkar sohankunkerkar commented Mar 23, 2022 •
edited
Loading

Fixes coreos/fedora-coreos-tracker#1125

This change displays all Ignition warnings on the console. Also, renamed the existing Ignition service to make it more generalized for handling status information about the Ignition run

@sohankunkerkar
Copy link
Member Author 

Fedora CoreOS 35.20220323.dev.1
Kernel 5.16.15-201.fc35.x86_64 on an x86_64 (ttyS0)

SSH host key: SHA256:G++MZpyPpblUMxpFpRLIrnMVKOQSkd0gw7cYz0zgnIE (ECDSA)
SSH host key: SHA256:sRpzjXdiaZ23ls72glVqpFYsFLcOBl+1xub7bZptFSw (ED25519)
SSH host key: SHA256:j0JPMupDzva4j76t9MrLDlQtlB64RentiC8TW0Jio9k (RSA)
ens6: 10.0.2.15 fe80::1bdc:6145:315a:4f00
Ignition: ran on 2022/03/23 15:24:07 UTC (this boot)
Ignition: user-provided config was applied
Ignition: warning at $.storage.directories.0.mode, line 1 col 233: setuid/setgid/sticky bits are not supported in spec versions older than 3.4.0
Ignition: warning at $.storage.files.0.mode, line 1 col 368: setuid/setgid/sticky bits are not supported in spec versions older than 3.4.0
No SSH authorized keys provided by Ignition or Afterburn
cosa-devsh login: core (automatic login)

bgilbert
bgilbert reviewed Mar 25, 2022
View reviewed changes
Copy link
Contributor

@bgilbert bgilbert left a comment

Choose a reason for hiding this comment

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

The commit message mentions the serial console but this is misleading; the MOTD is shown on the graphical console too.

overlay.d/05core/usr/libexec/coreos-ignition-display-debug-info Outdated Show resolved Hide resolved
overlay.d/05core/usr/libexec/coreos-ignition-display-debug-info Outdated Show resolved Hide resolved
overlay.d/05core/usr/libexec/coreos-ignition-display-debug-info Outdated Show resolved Hide resolved
overlay.d/05core/usr/libexec/coreos-ignition-display-debug-info Outdated Show resolved Hide resolved
overlay.d/05core/usr/libexec/coreos-ignition-display-debug-info Outdated Show resolved Hide resolved
overlay.d/05core/usr/lib/systemd/system/coreos-ignition-display-debug-info.service Outdated Show resolved Hide resolved
@sohankunkerkar sohankunkerkar force-pushed the ignition-warnings branch 3 times, most recently from 4a90aaa to 16f2a39 Compare March 29, 2022 21:36
bgilbert
bgilbert reviewed Apr 2, 2022
View reviewed changes
Copy link
Contributor

@bgilbert bgilbert left a comment

Choose a reason for hiding this comment

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

Looks good generally. A couple bugs, and a couple things I hadn't noticed earlier.

overlay.d/05core/usr/lib/systemd/system-preset/40-coreos.preset Outdated Show resolved Hide resolved
overlay.d/05core/usr/libexec/coreos-ignition-write-issue Outdated Show resolved Hide resolved
overlay.d/05core/usr/libexec/coreos-ignition-write-issue Outdated Show resolved Hide resolved
overlay.d/05core/usr/libexec/coreos-ignition-write-issue Outdated Show resolved Hide resolved
@sohankunkerkar sohankunkerkar force-pushed the ignition-warnings branch 3 times, most recently from 7b14e35 to aef2d54 Compare April 4, 2022 13:59
jlebon
jlebon previously approved these changes Apr 4, 2022
View reviewed changes
Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

Some comments but LGTM as is too. Nice work!

I think it wouldn't be too hard to write an external test for this, right? E.g. a test with an Ignition config that we know generates a warning and verify that an issue file with the expected content is created.

overlay.d/05core/usr/libexec/coreos-ignition-write-issue Outdated Show resolved Hide resolved
overlay.d/05core/usr/lib/systemd/system-preset/40-coreos.preset Outdated Show resolved Hide resolved
@sohankunkerkar
Copy link
Member Author

Some comments but LGTM as is too. Nice work!

I think it wouldn't be too hard to write an external test for this, right? E.g. a test with an Ignition config that we know generates a warning and verify that an issue file with the expected content is created.

Yeah, I will do this as a follow-up PR, sounds good?

jlebon
jlebon previously approved these changes Apr 5, 2022
View reviewed changes
Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

Sure, SGTM!

@sohankunkerkar
05core/*: display Ignition warnings on the console
9b35aab 
Fixes coreos/fedora-coreos-tracker#1125

This change displays all Ignition warnings on the console. Also, renamed
the existing Ignition service to make it more generalized for handling
status information about the Ignition run.
@sohankunkerkar sohankunkerkar merged commit 6dc91d2 into coreos:testing-devel Apr 6, 2022
@sohankunkerkar sohankunkerkar deleted the ignition-warnings branch April 6, 2022 17:41
@dustymabe
Copy link
Member

Nice work @sohankunkerkar

sohankunkerkar added a commit to sohankunkerkar/fedora-coreos-config that referenced this pull request Apr 13, 2022
@sohankunkerkar
tests: ensure that Ignition warnings are displayed on the console
231df44 
Follow up to coreos#1621 (review)
This adds kola test coverage for displaying Ignition warnings
on the console.
sohankunkerkar added a commit to sohankunkerkar/fedora-coreos-config that referenced this pull request Apr 13, 2022
@sohankunkerkar
tests: ensure that Ignition warnings are displayed on the console
47d8458 
Follow up to coreos#1621 (review)
This adds kola test coverage for displaying Ignition warnings
on the console.
sohankunkerkar added a commit to sohankunkerkar/fedora-coreos-config that referenced this pull request Apr 13, 2022
@sohankunkerkar
tests: ensure that Ignition warnings are displayed on the console
24ceddd 
Follow up to coreos#1621 (review)
This adds kola test coverage for displaying Ignition warnings
on the console.
sohankunkerkar added a commit to sohankunkerkar/fedora-coreos-config that referenced this pull request Apr 13, 2022
@sohankunkerkar
tests: ensure that Ignition warnings are displayed on the console
140ec0d 
Follow up to coreos#1621 (review)
This adds kola test coverage for displaying Ignition warnings
on the console.
@sohankunkerkar sohankunkerkar mentioned this pull request Apr 13, 2022
Merged
sohankunkerkar added a commit to sohankunkerkar/fedora-coreos-config that referenced this pull request Apr 19, 2022
@sohankunkerkar
tests: ensure that Ignition warnings are displayed on the console
ecfd4ee 
Follow up to coreos#1621 (review)
This adds kola test coverage for displaying Ignition warnings
on the console.
sohankunkerkar added a commit to sohankunkerkar/fedora-coreos-config that referenced this pull request Apr 19, 2022
@sohankunkerkar
tests: ensure that Ignition warnings are displayed on the console
c888e1b 
Follow up to coreos#1621 (review)
This adds kola test coverage for displaying Ignition warnings
on the console.
sohankunkerkar added a commit to sohankunkerkar/fedora-coreos-config that referenced this pull request Apr 19, 2022
@sohankunkerkar
tests: ensure that Ignition warnings are displayed on the console
bc525c0 
Follow up to coreos#1621 (review)
This adds kola test coverage for displaying Ignition warnings
on the console.
sohankunkerkar added a commit to sohankunkerkar/fedora-coreos-config that referenced this pull request Apr 20, 2022
@sohankunkerkar
tests: ensure that Ignition warnings are displayed on the console
13cd94d 
Follow up to coreos#1621 (review)
This adds kola test coverage for displaying Ignition warnings
on the console.
sohankunkerkar added a commit that referenced this pull request Apr 20, 2022
@sohankunkerkar
tests: ensure that Ignition warnings are displayed on the console
5685ae6 
Follow up to #1621 (review)
This adds kola test coverage for displaying Ignition warnings
on the console.
@dustymabe dustymabe mentioned this pull request Apr 22, 2022
Closed
@prestist prestist mentioned this pull request Jul 29, 2022
Open
HuijingHei pushed a commit to HuijingHei/fedora-coreos-config that referenced this pull request Oct 10, 2023
@sohankunkerkar @HuijingHei
tests: ensure that Ignition warnings are displayed on the console
822e703 
Follow up to coreos#1621 (review)
This adds kola test coverage for displaying Ignition warnings
on the console.
HuijingHei pushed a commit to HuijingHei/fedora-coreos-config that referenced this pull request Oct 10, 2023
@sohankunkerkar @HuijingHei
tests: ensure that Ignition warnings are displayed on the console
6bae746 
Follow up to coreos#1621 (review)
This adds kola test coverage for displaying Ignition warnings
on the console.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bgilbert bgilbert bgilbert left review comments

@jlebon jlebon jlebon approved these changes

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

Successfully merging this pull request may close these issues.

Use MOTD to display Ignition warnings on the console
4 participants
@sohankunkerkar @dustymabe @bgilbert @jlebon