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

bib: respect the root fs type from the container #272

Closed
wants to merge 5 commits into from

Conversation

ondrejbudai
Copy link
Member

Needs #262 or an alternative, this is mostly a showcase of how bib can be expanded after we can actually run the container.

Prior to this commit, we always assumed that the root fs is ext4. However, it's possible to redefine this in the container, so let's respect this.

I think that pulling always is not the greatest idea since we switched
to a container storage, but it keeps the backward compatibility.

Note that the manifest test now has to be rootful because we run podman
pull.
Prior this commit, we always assumed that the root fs is ext4. However,
it's possible to redefine this in the container, so let's respect this.
Copy link
Member

@achilleas-k achilleas-k left a comment

Choose a reason for hiding this comment

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

🚀 Finally!!

@ondrejbudai
Copy link
Member Author

ftr, this needs tests and I also want to implement this properly in images, this is mostly a showcase for now. :)

// filesystem.root.type is the preferred way instead of the old root-fs-type top-level key.
// See https://github.com/containers/bootc/commit/558cd4b1d242467e0ffec77fb02b35166469dcc7
fsType := bootcConfig.Filesystem.Root.Type
supportedFS := []string{"ext4", "xfs"}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious about the allowlist here; it seems distribution dependent at the minimum right? E.g. some distros/OSes use btrfs, there's also bcachefs coming in, etc.

Maybe it'd be better to have a denylist instead, i.e. not allow doing vfat for example. We could I guess do a sanity check that mkfs.$type exists in the root I guess to give a better error perhaps?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if this is the right place for policy like this (i.e. I would rather just remove it). It seems to me the best place for this would be something like a "bootc lint" or "bootc build" that does checks. If we add the policy here we need to duplicate it accross bootc install to-disk and anaconda so having it in one central place seems more maintainable to me?

@cgwalters
Copy link
Contributor

cgwalters commented Mar 20, 2024

If we don't think we can land this type of thing somewhat in the near future, I think I'd advocate for making the default filesystem choice a configurable option when building the bib container image (i.e. building bib itself), as some OSes/distros will want to pick that default independently of this git repository.

Copy link

This PR is stale because it has been open 30 days with no activity. Remove "Stale" label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the Stale Issue or PR with no activity for extended period of time label Apr 20, 2024
@ondrejbudai
Copy link
Member Author

This was moved to #383.

@ondrejbudai ondrejbudai deleted the bootc-shenanigans branch April 23, 2024 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
😮 IMPORTANT Stale Issue or PR with no activity for extended period of time
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants