Skip to content

coreos: generalize to any aleph created by bootc#1124

Open
jbtrystram wants to merge 1 commit into
coreos:mainfrom
jbtrystram:generalize-aleph
Open

coreos: generalize to any aleph created by bootc#1124
jbtrystram wants to merge 1 commit into
coreos:mainfrom
jbtrystram:generalize-aleph

Conversation

@jbtrystram

Copy link
Copy Markdown
Member

Rename the coreos module to aleph as bootc install will create an aleph file in all cases except the composefs path.

There is nothing specific to coreOS anymore in this path.

See bootc-dev/bootc@c6112d2 And coreos/fedora-coreos-tracker#2187

Rename the `coreos` module to `aleph` as `bootc install` will create
an aleph file in all cases except the composefs path.

There is nothing specific to coreOS anymore in this path.

See bootc-dev/bootc@c6112d2
And coreos/fedora-coreos-tracker#2187

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request replaces the CoreOS-specific coreos.rs module with a more generic aleph.rs module to support both CoreOS (.coreos-aleph-version.json) and bootc (.bootc-aleph.json) aleph version files. The feedback suggests improving robustness by falling back to file modification time if creation time is unsupported by the filesystem, and adding error context when parsing the JSON files.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/aleph.rs
log::debug!("Found aleph version in {aleph_path}");
return Ok(Some(AlephWithTimestamp {
aleph,
ts: meta.created()?.into(),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

Using metadata.created() can fail on filesystems or operating systems that do not support or expose creation times (returning std::io::ErrorKind::Unsupported or similar). Since this error is propagated with ?, it will cause get_aleph_version to fail entirely, which in turn breaks bootupd status and updates on those systems.

Falling back to metadata.modified() when created() fails makes this much more robust.

Suggested change
ts: meta.created()?.into(),
ts: meta.created().or_else(|_| meta.modified())?.into(),

Comment thread src/aleph.rs
let statusf = File::open(path).with_context(|| format!("Opening {path:?}"))?;
let meta = statusf.metadata()?;
let bufr = std::io::BufReader::new(statusf);
let aleph: Aleph = serde_json::from_reader(bufr)?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

If the JSON file is malformed or empty, serde_json::from_reader will fail. Adding context to the error helps with troubleshooting by identifying which file failed to parse.

Suggested change
let aleph: Aleph = serde_json::from_reader(bufr)?;
let aleph: Aleph = serde_json::from_reader(bufr).with_context(|| format!("Parsing {path:?}"))?;

jbtrystram added a commit to jbtrystram/fedora-coreos-config that referenced this pull request Jul 16, 2026
Disk images created by `bootc install` are shipping an aleph file at a
different path than what existed historically.
Update our tests to look for both locations.

See coreos/fedora-coreos-tracker#2187
And coreos/bootupd#1124
jbtrystram added a commit to jbtrystram/fedora-coreos-config that referenced this pull request Jul 16, 2026
Disk images created by `bootc install` are shipping an aleph file at a
different path than what existed historically.
Update our tests to look for both locations.

See coreos/fedora-coreos-tracker#2187
And coreos/bootupd#1124
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.

1 participant