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

Enable composefs #3009

Merged
merged 4 commits into from
Aug 30, 2024
Merged

Enable composefs #3009

merged 4 commits into from
Aug 30, 2024

Conversation

jbtrystram
Copy link
Contributor

@jbtrystram jbtrystram commented May 29, 2024

@jbtrystram
Copy link
Contributor Author

Why is it always kdump 😭
I only see May 29 15:31:15.686509 kdump.service[1598]: kdump: mkdumprd: failed to make kdump initrd
in the journal. I don't see any clue about the filesystem being read-only
I'll investigate more locally.

@jbtrystram
Copy link
Contributor Author

After chatting a bit with Coiby Xu I filed https://bugzilla.redhat.com/show_bug.cgi?id=2284097

@jlebon jlebon marked this pull request as draft July 2, 2024 15:28
@jbtrystram
Copy link
Contributor Author

Maybe related to the kdump failure : https://issues.redhat.com/browse/RHEL-35885

@jbtrystram jbtrystram changed the base branch from rawhide to testing-devel July 10, 2024 15:29
@jbtrystram jbtrystram changed the base branch from testing-devel to rawhide July 10, 2024 15:31
@jbtrystram jbtrystram closed this Jul 10, 2024
@jbtrystram jbtrystram reopened this Jul 15, 2024
@jbtrystram
Copy link
Contributor Author

Tested this today locally :

=== RUN   ext.config.kdump.crash
--- PASS: ext.config.kdump.crash (153.11s)

@jbtrystram jbtrystram force-pushed the enable_composefs branch 2 times, most recently from b3d9f6e to 1850843 Compare July 17, 2024 06:57
@jbtrystram
Copy link
Contributor Author

Interesting failures (but making total sense) :
ext.config.root-reprovision.luks.autosave-xfs:

++ xfs_info /
++ grep -o 'agcount=[0-9]*'
xfs_info: cannot open /: Is a directory
 /usr/local/bin/kola-runext-test.sh: line 32: agcount: unbound variable

I suppose this is because / is no longer an XFS filesystem

ext.config.root-reprovision.raid1:

findmnt -nvr / -o SOURCE
srcdev=none

Same here, / is not mounted from the raid device but from the composeFS overlay

@jlebon
Copy link
Member

jlebon commented Jul 18, 2024

Pushed some fixes which should help.

@jbtrystram
Copy link
Contributor Author

Thanks !

@jbtrystram
Copy link
Contributor Author

jbtrystram commented Jul 18, 2024

Live ISO fails to boot with :

[    6.036086] ostree-prepare-root[761]: ostree-prepare-root: sysroot.readonly=true requires /sysroot to be writable at this point, the cmdline should contain rw but not ro, if that is not the case this is likely the issue

From what I understand it's because in the composeFS path, ostree-prepare-root want to mount /etc/ and /var as writeable , and requires sysroot to be writeable to do that ?

See https://github.com/ostreedev/ostree/blob/4b96359e106a832dacdec9f3ef19dc730783e7a3/src/switchroot/ostree-prepare-root.c#L505

I guess the quickfix option here would be to disable composeFS on the live ISO, since it's already a read-only system anyway. We could add the ostree.prepare-root.composefs=0 karg to https://github.com/coreos/fedora-coreos-config/blob/testing-devel/overlay.d/05core/usr/lib/dracut/modules.d/35coreos-live/live-generator

@cgwalters WDYT ?

Let's try 49c6b4a (#3009)

@jbtrystram
Copy link
Contributor Author

jbtrystram commented Jul 18, 2024

This didn't work :

[    5.554586] ostree-prepare-root[775]: composefs: mounted successfully
[    5.555464] ostree-prepare-root[775]: ostree-prepare-root: sysroot.readonly=true requires /sysroot to be writable at this point, the cmdline should contain rw but not ro, if that is not the case this is likely the issue

okay, I just see now that this karg support was merged last week and not released yet

@cgwalters
Copy link
Member

I guess the quickfix option here would be to disable composeFS on the live ISO, since it's already a read-only system anyway. We could add the ostree.prepare-root.composefs=0 karg to https://github.com/coreos/fedora-coreos-config/blob/testing-devel/overlay.d/05core/usr/lib/dracut/modules.d/35coreos-live/live-generator

Yes, or have the liveiso actually be built from a derived image/commit which drops the composefs config.

@jlebon
Copy link
Member

jlebon commented Jul 18, 2024

I think we can use the karg in the short-term, but it's also awkward to work around this as a built-in karg in our official ISOs. Note that ISO kargs are highly visible because there are APIs to query and modify them. (And on that point, nothing actually prevents a user from deleting that karg. We could say "don't do that", but that's awkward UX.)

I think this is basically another instance of ostreedev/ostree#1921 (which I've just retitled); so then ostree-prepare-root would detect that this is a live environment and just not even try to set up composefs there.

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.

Nice, this looks sane to me. Obviously, the ostree-prepare-root cmdline bit is a massive hack, but it's not new either.

Once those changes are done and CI is happy, we can retarget the PR against testing-devel instead of creating a new one.

manifests/fedora-coreos.yaml Outdated Show resolved Hide resolved
kola-denylist.yaml Show resolved Hide resolved
overlay.d/08composefs/README.md Outdated Show resolved Hide resolved
jbtrystram added a commit to jbtrystram/fedora-coreos-config that referenced this pull request Aug 29, 2024
in the composeFS path, ostree-prepare-root want to
mount /etc/ and /var as writeable, which cannot in the live iso
environnement.
Overriding the kernel command line to disable composeFS in that case.

See ostreedev/ostree#1921
And coreos#3009 (comment)
jlebon pushed a commit to jbtrystram/fedora-coreos-config that referenced this pull request Aug 29, 2024
In the composefs path, ostree-prepare-root want to mount /etc/ and /var
as writeable, which cannot in the live iso environnement.

Override the kernel command line to disable composefs in that case.

See ostreedev/ostree#1921
and coreos#3009 (comment)
@jlebon
Copy link
Member

jlebon commented Aug 29, 2024

Sorry, I messed up the suggestion in #3009 (comment). It should've been >= 41. To not waste another round-trip, I just fixed that. While there, I also cleaned up the commits a bit, merging the two commits that were both called e.g. "Enable composefs for 41+", and finally rebased onto the latest rawhide. Once CI is green here, let's retarget testing-devel and get this in.

@jbtrystram
Copy link
Contributor Author

Thanks @jlebon ! I am a bit embarrassed I didn't caught it 😅

@jlebon jlebon changed the base branch from rawhide to testing-devel August 30, 2024 13:38
Enabling composefs allow an increase in security by making the
filesystem truly read-only.

It's also a cornerstone towards a truly sealed system with full
integrity checks at runtime.

It will also allow storage deduplication between the host filesystem
and the containers storage in the long run, which is a huge win: faster
downloads and faster container startup times.

A thing that this is known to break is the "chattr -i" hack for new
toplevel dirs (xref coreos/rpm-ostree#337).

Basically if you want that, you either need to make a derived image,
or enable transient root.

Ref: https://fedoraproject.org/wiki/Changes/ComposefsAtomicCoreOSIoT

Co-authored-by: jbtrystram <[email protected]>
jbtrystram and others added 3 commits August 30, 2024 09:39
We are trying to enable composeFS in rawhide and there is an issue
where kdump fails to generate the initrd from boot.
Manually trigerring the rebuild works but requires the extra manual
step.
Snoozing this test to let some time for the kdump team to
investigate.

Note that the kdump over SSH test works so we still have some
coverage for kdump.
On composefs, / is now an overlay, so some of the commands that query
`/` don't quite work. Tweak them to instead query `/sysroot`, which
should still be the actual storage layer underneath the composefs mount
that we really care about for these tests.
In the composefs path, ostree-prepare-root want to mount /etc/ and /var
as writeable, which cannot in the live iso environnement.

Override the kernel command line to disable composefs in that case.

See ostreedev/ostree#1921
and coreos#3009 (comment)
@jlebon jlebon marked this pull request as ready for review August 30, 2024 13:39
@jlebon
Copy link
Member

jlebon commented Aug 30, 2024

CI happy on rawhide. Retargeted testing-devel and approved. Will let someone else also stamp and merge.

@travier travier enabled auto-merge (rebase) August 30, 2024 14:49
@jbtrystram
Copy link
Contributor Author

🎉

@travier travier merged commit 3ee8fd1 into coreos:testing-devel Aug 30, 2024
3 checks passed
@jbtrystram jbtrystram changed the title Enable composefs for rawhide Enable composefs Sep 3, 2024
@@ -51,3 +51,10 @@
streams:
- rawhide
- branched
- pattern: ext.config.kdump.crash
tracker: https://bugzilla.redhat.com/show_bug.cgi?id=2284097
Copy link
Member

Choose a reason for hiding this comment

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

can you open an FCOS issue tracker ticket for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

jbtrystram added a commit to jbtrystram/fedora-coreos-docs that referenced this pull request Sep 4, 2024
We added composeFS starting in f41. Since it comes with a couple of
drawbacks let's document it and explain how to disable it.

coreos/fedora-coreos-tracker#1718 (comment)
coreos/fedora-coreos-config#3009
jbtrystram added a commit to jbtrystram/fedora-coreos-docs that referenced this pull request Sep 4, 2024
We added composeFS starting in f41. Since it comes with a couple of
drawbacks let's document it and explain how to disable it.

coreos/fedora-coreos-tracker#1718 (comment)
coreos/fedora-coreos-config#3009
jbtrystram added a commit to jbtrystram/fedora-coreos-docs that referenced this pull request Sep 5, 2024
We added composeFS starting in f41. Since it comes with a couple of
drawbacks let's document it and explain how to disable it.

coreos/fedora-coreos-tracker#1718 (comment)
coreos/fedora-coreos-config#3009
jbtrystram added a commit to jbtrystram/fedora-coreos-docs that referenced this pull request Sep 6, 2024
We added composeFS starting in f41. Since it comes with a couple of
drawbacks let's document it and explain how to disable it.

coreos/fedora-coreos-tracker#1718 (comment)
coreos/fedora-coreos-config#3009
jbtrystram added a commit to coreos/fedora-coreos-docs that referenced this pull request Sep 6, 2024
We added composeFS starting in f41. Since it comes with a couple of
drawbacks let's document it and explain how to disable it.

coreos/fedora-coreos-tracker#1718 (comment)
coreos/fedora-coreos-config#3009
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants