Repository navigation
docs: Describe systemd-sysext and confext as distribution-dependent - #21
jmarrero-bot wants to merge 1 commit into
Conversation
jmarrero-bot
left a comment
There was a problem hiding this comment.
Review guide for head 778f8f838c: where to look closely and what is safe to skim. It is advice from the bot's reviewer and doesn't replace reading the diff; the review app walks it.
Docs-only, one commit, one section of docs/src/bootc-filesystem.7.md (also rendered as bootc-filesystem(7)). It replaces the 'not currently supported / not recommended / has not validated' language from bootc-dev#2436 with: bootc aims to work together with sysext and confext, support depends mostly on the distribution (systemd version, SELinux labeling), and bootc#7 discusses a more integrated approach. The overlayfs stacking-depth caveat stays as 'one thing to be aware of', and the per-host configuration pointer changes from 'instead' to 'see also'. All bullets of the tracker#88 docs spec are covered. The only technical claim worth a second look is the /etc remedy sentence: [etc] mount = "none" avoids the extra overlay only when transient root is off, and the default bind mode does not stack either. Rebuilt mdbook and the man page on the devspace at this head; both render cleanly.
Hotspots
- note · api —
docs/src/bootc-filesystem.7.md:116-125(778f8f8): This is the operator-specified positioning (aim to work with sysext, distribution-dependent, link to #7). Worth reading once for wording, since it becomes the project's public stance in the docs and man page. - look-closely · logic —
docs/src/bootc-filesystem.7.md:133-136(778f8f8): Reads as if mount = "none" is the fix for the depth limit, but with [root] transient = true (the pairing the setup-root-conf man page recommends for none) /etc sits in the transient overlay and is still at depth 2. The default bind mode also adds no overlay; overlay and transient /etc are the modes that stack.
778f8f8 to
3042e00
Compare
jmarrero-bot
left a comment
There was a problem hiding this comment.
Review guide for head 3042e00bb2: where to look closely and what is safe to skim. It is advice from the bot's reviewer and doesn't replace reading the diff; the review app walks it.
Docs-only, one commit, one section of docs/src/bootc-filesystem.7.md (also rendered as bootc-filesystem(7)). It replaces the 'not currently supported / not recommended / has not validated' language from bootc-dev#2436 with: bootc aims to work together with sysext and confext, support depends mostly on the distribution (systemd version, SELinux labeling), and bootc#7 discusses a more integrated approach. The overlayfs stacking-depth caveat stays as 'one thing to be aware of'. Since the previous review the /etc sentence no longer implies mount = "none" is a fix on its own: it says bind and none add no overlay and adds a transient-root caveat. That caveat is strictly needed only for none (bind covers /etc with the state dir even under transient root), so it is conservative rather than wrong. All bullets of the tracker#88 docs spec are covered. Rebuilt mdbook and the man page on the devspace at this head; both render cleanly.
Hotspots
- note · api —
docs/src/bootc-filesystem.7.md:116-125(3042e00): This is the operator-specified positioning (aim to work with sysext, distribution-dependent, link to #7). Worth reading once for wording, since it becomes the project's public stance in the docs and man page. - note · logic —
docs/src/bootc-filesystem.7.md:133-138(3042e00): The transient-root caveat is applied to both modes, but per mount_subdir in crates/initramfs/src/lib.rs the default bind mount puts the state dir over /etc even under transient root, so only mount = "none" is affected. Conservative, optional to tighten.
jmarrero-bot
left a comment
There was a problem hiding this comment.
Verdict: minor fixes at 3042e00bb25e20f25155df796562e097829f25c4.
One low-severity accuracy fix in docs/src/bootc-filesystem.7.md:134–136: apply the transient-root restriction only to mount = "none". With the default bind, bootc mounts the deployment state directory over /etc even when transient root is enabled (crates/initramfs/src/lib.rs:664–684); confext therefore does not inherit the root overlay's depth. Suggested wording: “The default bind mount leaves room for confext even with transient root enabled. [etc] mount = "none" also leaves room when transient root is disabled.”
All bullets of the operator's tracker bootc-dev#88 docs spec are covered. The kernel's depth limit of 2 and the composefs/transient-overlay interaction are accurate; [etc] mount names and defaults match the implementation. Both systemd links and bootc#7 return HTTP 200; the local page links and anchors exist. bot-git check BASE..HEAD and git diff --check pass locally; the worktree is clean. The PR reports successful mdbook/check-docs, linkcheck, and man-page builds on RHEL 10; I could not independently rerun them because the existing docs devspace hostname did not resolve.
Review guide: focus on lines 134–136; no skim ranges for this small prose change.
3042e00 to
fbbab76
Compare
jmarrero-bot
left a comment
There was a problem hiding this comment.
Review guide for head fbbab76323: where to look closely and what is safe to skim. It is advice from the bot's reviewer and doesn't replace reading the diff; the review app walks it.
Docs-only, one commit, one section of docs/src/bootc-filesystem.7.md (also rendered as bootc-filesystem(7)). It replaces the 'not currently supported / not recommended / has not validated' language from bootc-dev#2436 with: bootc aims to work together with sysext and confext, support depends mostly on the distribution (systemd version, SELinux labeling), and bootc#7 discusses a more integrated approach. The overlayfs stacking-depth caveat stays as 'one thing to be aware of'. The /etc sentence now matches mount_subdir in crates/initramfs/src/lib.rs: the default bind mount covers /etc with the state dir even under transient root, while mount = "none" leaves /etc inside the transient root overlay. All bullets of the tracker#88 docs spec are covered. Rebuilt mdbook and the man page on the devspace at this head; both render cleanly.
Hotspots
- note · api —
docs/src/bootc-filesystem.7.md:116-125(fbbab76): This is the operator-specified positioning (aim to work with sysext, distribution-dependent, link to #7). Worth reading once for wording, since it becomes the project's public stance in the docs and man page. - note · logic —
docs/src/bootc-filesystem.7.md:133-139(fbbab76): The only technical claim that changed during review: how bind and none interact with transient root. It matches the initramfs mount code, but it is the sentence most likely to draw upstream comment.
jmarrero-bot
left a comment
There was a problem hiding this comment.
Verdict: ship at fbbab763230651df79a4f6e0654ac52e0323cff0. No remaining findings.
docs/src/bootc-filesystem.7.md:133–137 now correctly limits the transient-root caveat to [etc] mount = "none". mount_subdir returns without remounting for none, whereas bind mounts the deployment state directory over /etc (crates/initramfs/src/lib.rs:505–511); its caller uses visible_root even with transient root enabled (lines 664–684). The default bind therefore leaves stacking room for confext in both root modes.
The full diff still meets every docs bullet in the operator's tracker bootc-dev#88 spec: positive project aim, distribution/systemd/SELinux dependencies, bootc#7 integration discussion, and the useful overlay-depth caveat without unsupported/not-recommended framing. The fix is squashed into the original docs commit; no unrelated changes.
Re-ran bot-git check BASE..HEAD, git diff --check, and external-link HTTP checks locally: all pass; local targets/anchors exist and the worktree is clean. The PR reports mdbook/check-docs and man-page builds on RHEL 10 at this head, with linkcheck errors confined to existing rustdoc links. I did not independently rerun those builds; the docs devspace is no longer listed.
Review guide: look closely at lines 133–137 for the mount distinction; no skim ranges for this small prose diff.
| composefs backend with `etc = none` (there is not yet a corresponding ostree | ||
| knob) leaves `/etc` free for confext to manage. | ||
|
|
||
| #### OS-specific integration concerns |
There was a problem hiding this comment.
We should probably not remove this section...
There was a problem hiding this comment.
Restored in 37ccf9f: "OS-specific integration concerns" is back word for word. Below it, the per-host configuration paragraph keeps one change, "use … instead" → "see also", because "instead" steers readers away from confext.
| systems. Using them on a bootc host is not recommended. There are two categories | ||
| of concern to be aware of. | ||
|
|
||
| #### Conflicts with transient overlays |
There was a problem hiding this comment.
Why not keep the heading here when we start talking about the overlay limitations?
There was a problem hiding this comment.
Done in 37ccf9f: the "Conflicts with transient overlays" heading is back, with its original first paragraph, and the intro again ends with "There are two categories of concern to be aware of." One paragraph in that section still differs from upstream: the /etc note (lines 137-142) replaces the nonexistent etc = none with the real [etc] mount options and says when each leaves room for confext.
fbbab76 to
37ccf9f
Compare
jmarrero-bot
left a comment
There was a problem hiding this comment.
Review guide for head 37ccf9fe01: where to look closely and what is safe to skim. It is advice from the bot's reviewer and doesn't replace reading the diff; the review app walks it.
Docs-only, one commit, one section of docs/src/bootc-filesystem.7.md (also rendered as bootc-filesystem(7)). Per the operator's review at fbbab76, both #### subsections from bootc-dev#2436 are restored: 'OS-specific integration concerns' verbatim, and 'Conflicts with transient overlays' with its original first paragraph. Only three things differ from upstream. The intro drops 'not currently supported / not validated / not recommended' and says bootc aims to work with sysext and confext, links bootc#7, and says support depends mostly on the distribution (systemd version, SELinux). The /etc paragraph replaces the non-existent 'etc = none' spelling with an accurate bind-vs-none description that matches mount_subdir in crates/initramfs/src/lib.rs. The per-host pointer says 'see also' instead of 'use ... instead'. The bot's thread replies describe the restored sections as having their original text but do not say that the /etc paragraph inside 'Conflicts with transient overlays' is still rewritten, so read that paragraph. Rebuilt mdbook and the man page on the devspace at this head; both headings render.
Hotspots
- note · api —
docs/src/bootc-filesystem.7.md:116-124(37ccf9f): This is the operator-specified positioning (aim to work with sysext, link to #7, distribution-dependent). It now leads into the original 'two categories of concern' framing, so check that the two read well together. - look-closely · logic —
docs/src/bootc-filesystem.7.md:137-142(37ccf9f): This is the one rewritten paragraph inside the restored 'Conflicts with transient overlays' section, and the thread replies don't mention it. The claim about bind vs none under transient root matches the initramfs code, but it goes beyond the tracker spec, which justifies it as fixing the old, inaccurate 'etc = none' wording. - note · api —
docs/src/bootc-filesystem.7.md:151-152(37ccf9f): 'use ... instead' becomes 'see also'. The spec does not ask for this explicitly, but 'instead' steers readers away from sysext. Keep or revert as you prefer.
jmarrero-bot
left a comment
There was a problem hiding this comment.
Verdict: ship at 37ccf9fe01f4911d2a898538110553945ce0ba7f. No remaining findings.
Both operator comments are addressed in docs/src/bootc-filesystem.7.md: “Conflicts with transient overlays” is restored at line 126, with its first paragraph verbatim from upstream main; “OS-specific integration concerns” is restored at line 144, with its integration paragraph verbatim. The current bot replies accurately disclose the retained /etc correction (137–142) and “use … instead” → “see also” change (151–152). The earlier review guide’s claim that the replies omit these details is stale.
The tracker bootc-dev#88 docs spec still holds: no unsupported, not-recommended, or “has not validated” framing in this section; a positive aim to work together with sysext; distribution-dependent support mentioning systemd and SELinux; bootc#7 linked as an integration discussion; and the useful overlay-depth caveat preserved. The /etc distinction matches mount_subdir and its visible_root caller.
Re-ran exact upstream-restoration comparisons, wording assertions, git diff --check, bot-git check BASE..HEAD, and external-link HTTP checks locally: all pass. Local link targets/anchors exist; the worktree is clean and the fixes remain squashed into one correctly attributed AI-tagged commit. The PR reports mdbook/check-docs and man-page builds at this head, with linkcheck errors confined to existing rustdoc links. I did not independently rerun those builds; no devspace is currently listed.
Review guide: read lines 137–142 for the retained mount distinction; no skim ranges for this small prose diff.
|
/promote |
The section added in bootc-dev#2436 says sysext and confext are not supported and not recommended on bootc hosts. That overstates it: the bootc project wants to work together with sysext, and whether it works well today is mostly down to the distribution (its systemd version, SELinux labeling and the like), not bootc itself. Say that instead, and point at #7, where a more integrated approach is being discussed. The transient overlay and OS-specific sections stay, as things to be aware of. The /etc note now names the real setup-root-conf.toml option and says when /etc actually leaves room for confext: the old wording suggested etc = none avoids the limit, but under transient root /etc is still inside that overlay. Generated-by: AI Signed-off-by: Joseph Marrero Corchado <jmarrero@redhat.com>
|
Signed off 1 commit(s) with |
37ccf9f to
6036e6a
Compare
|
Opened upstream as bootc-dev#2649. Closing this review draft. |
bootc-dev#2436 documented systemd-sysext and confext as not supported and not recommended on bootc hosts. This replaces that with: bootc aims to work together with sysext, support today depends mostly on the distribution (systemd version, SELinux and the like), and #7 is where a more integrated approach is being discussed. The transient overlay and OS-specific integration sections stay, as things to be aware of.
Tracker: jmarrero-forge/tracker#88
Testing, on a 16-core RHEL 10 devspace at 37ccf9f:
just build-mdbook-topasses, includingxtask check-docs("Validated 81 canonical pages");mdbook-linkcheck --standalonereports nothing for this page (its only errors are the existinginternals/rustdoc links, which are copied in after the build);make manpages-no-syncrenders the section cleanly in bootc-filesystem(7).Generated-by: https://github.com/jmarrero-bot#llms
Review draft in jmarrero-forge, not upstream yet. This section is removed when the PR is opened upstream.
bootc-dev/bootc, basemainPVTI_lADOFBIZ7c4BlZPrzhH73icTo review:
/promoteon a line of its own, to open it upstream, ready for review. Either covers only the commits pushed so far.Signed-off-by: Joseph Marrero Corchado <jmarrero@redhat.com>to the commits lacking it (the bot's and yours; anyone else's only if you ask), with you as committer./draftline (in the same comment or before) to open it upstream as a draft (/readyundoes that).