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

bootc switch --stateroot flag #617

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

omertuc
Copy link
Contributor

@omertuc omertuc commented Jun 20, 2024

Background

added the stateroot option to the install subcommand

Issue

The stateroot option is not available on the switch subcommand

Solution

Add the stateroot option to the switch subcommand

Implementation

  • If the stateroot is different than the current, we should allow using the same image as the currently booted one

  • Stateroot has to be explicitly created (init_osname binding) if it doesn't exist. If it does, we still call init_osname and simply ignore the error (TODO: only ignore non-already-exists errors)

  • Copy /var from the old stateroot to the new one. I'm doing --reflink but it's still very slow

  • Must use the old stateroot to find the merge_deployment because otherwise we boot without the required kargs (it manifested as a missing root=UUID=... which caused the dracut rootfs-generator to silently fail to create sysroot.mount and so ostree-prepare-root failed due to empty /sysroot)

@github-actions github-actions bot added the area/install Issues related to `bootc install` label Jun 20, 2024
@omertuc
Copy link
Contributor Author

omertuc commented Jun 20, 2024

After shelling into the podman-bootc VM and running:

ostree admin stateroot-init foo
bootc switch --stateroot foo quay.io/fedora/fedora-bootc:40

I've tried to then reboot (simply reboot in the shell) into the new stateroot, but the only grub option is the original CentOS stateroot.

I've also noticed that after rebooting the machine, podman-bootc list shows the machine is not running, so I have to podman-bootc run <machine-id> to get it to run.

I've also observed the following weird behavior:

Ran the above commands and then:

[root@localhost ~]# ostree admin status
  foo 51d7dcab77ab6a4e7235bbdbbe32f70f6f97b6a679f7a359bfea346ae2f18b0e.0 (staged)
    origin: <unknown origin type>
* default f96c05656ae82d9758be08bcf888d8f130fafaf9723967255bd76b51f26bd2e4.0
    origin: <unknown origin type>
[root@localhost ~]# ostree admin set-default 0
  foo 51d7dcab77ab6a4e7235bbdbbe32f70f6f97b6a679f7a359bfea346ae2f18b0e.0 (staged)
    origin: <unknown origin type>
* default f96c05656ae82d9758be08bcf888d8f130fafaf9723967255bd76b51f26bd2e4.0
    origin: <unknown origin type>
[root@localhost ~]# ostree admin set-default 1
[root@localhost ~]# ostree admin status
* default f96c05656ae82d9758be08bcf888d8f130fafaf9723967255bd76b51f26bd2e4.0
    origin: <unknown origin type>

The stateroot just disappears? That's a bit odd

@cgwalters
Copy link
Collaborator

Let's split the install vs switch changes into separate PRs; the install one I think we can trivially land with just a simple integration test.

omertuc added a commit to omertuc/bootc that referenced this pull request Jun 21, 2024
This commit makes it so that the `bootc install` stateroot will be
configurable (it defaults to `default`). For now this is a hidden CLI
option until we decide whether we want to commit to this API.

In the future we also want to make the stateroot of `bootc switch` be
configurable (containers#617) so that
users can install an image to a new stateroot while they already have an
existing stateroot

Also removed the constant `STATEROOT_DEFAULT`, we're now simply taking
it from the `ostree_ext` crate
@omertuc
Copy link
Contributor Author

omertuc commented Jun 21, 2024

Split install CLI into #622

omertuc added a commit to omertuc/bootc that referenced this pull request Jun 25, 2024
This commit makes it so that the `bootc install` stateroot will be
configurable (it defaults to `default`). For now this is a hidden CLI
option until we decide whether we want to commit to this API.

In the future we also want to make the stateroot of `bootc switch` be
configurable (containers#617) so that
users can install an image to a new stateroot while they already have an
existing stateroot

Also removed the constant `STATEROOT_DEFAULT`, we're now simply taking
it from the `ostree_ext` crate
omertuc added a commit to omertuc/bootc that referenced this pull request Jun 25, 2024
This commit makes it so that the `bootc install` stateroot will be
configurable (it defaults to `default`). For now this is a hidden CLI
option until we decide whether we want to commit to this API.

In the future we also want to make the stateroot of `bootc switch` be
configurable (containers#617) so that
users can install an image to a new stateroot while they already have an
existing stateroot

Also removed the constant `STATEROOT_DEFAULT`, we're now simply taking
it from the `ostree_ext` crate
omertuc added a commit to omertuc/bootc that referenced this pull request Jun 25, 2024
This commit makes it so that the `bootc install` stateroot will be
configurable (it defaults to `default`). For now this is a hidden CLI
option until we decide whether we want to commit to this API.

In the future we also want to make the stateroot of `bootc switch` be
configurable (containers#617) so that
users can install an image to a new stateroot while they already have an
existing stateroot

Also removed the constant `STATEROOT_DEFAULT`, we're now simply taking
it from the `ostree_ext` crate
omertuc added a commit to omertuc/bootc that referenced this pull request Jul 3, 2024
This commit makes it so that the `bootc install` stateroot will be
configurable (it defaults to `default`). For now this is a hidden CLI
option until we decide whether we want to commit to this API.

In the future we also want to make the stateroot of `bootc switch` be
configurable (containers#617) so that
users can install an image to a new stateroot while they already have an
existing stateroot

Also removed the constant `STATEROOT_DEFAULT`, we're now simply taking
it from the `ostree_ext` crate
omertuc added a commit to omertuc/bootc that referenced this pull request Sep 3, 2024
This commit makes it so that the `bootc install` stateroot will be
configurable (it defaults to `default`). For now this is a hidden CLI
option until we decide whether we want to commit to this API.

In the future we also want to make the stateroot of `bootc switch` be
configurable (containers#617) so that
users can install an image to a new stateroot while they already have an
existing stateroot

Also removed the constant `STATEROOT_DEFAULT`, we're now simply taking
it from the `ostree_ext` crate
omertuc added a commit to omertuc/bootc that referenced this pull request Sep 3, 2024
This commit makes it so that the `bootc install` stateroot will be
configurable (it defaults to `default`). For now this is a hidden CLI
option until we decide whether we want to commit to this API.

In the future we also want to make the stateroot of `bootc switch` be
configurable (containers#617) so that
users can install an image to a new stateroot while they already have an
existing stateroot

Also removed the constant `STATEROOT_DEFAULT`, we're now simply taking
it from the `ostree_ext` crate
omertuc added a commit to omertuc/bootc that referenced this pull request Sep 3, 2024
This commit makes it so that the `bootc install` stateroot will be
configurable (it defaults to `default`). For now this is a hidden CLI
option until we decide whether we want to commit to this API.

In the future we also want to make the stateroot of `bootc switch` be
configurable (containers#617) so that
users can install an image to a new stateroot while they already have an
existing stateroot

Also removed the constant `STATEROOT_DEFAULT`, we're now simply taking
it from the `ostree_ext` crate
omertuc added a commit to omertuc/bootc that referenced this pull request Sep 3, 2024
This commit makes it so that the `bootc install` stateroot will be
configurable (it defaults to `default`). For now this is a hidden CLI
option until we decide whether we want to commit to this API.

In the future we also want to make the stateroot of `bootc switch` be
configurable (containers#617) so that
users can install an image to a new stateroot while they already have an
existing stateroot

Also removed the constant `STATEROOT_DEFAULT`, we're now simply taking
it from the `ostree_ext` crate
omertuc added a commit to omertuc/bootc that referenced this pull request Sep 3, 2024
This commit makes it so that the `bootc install` stateroot will be
configurable (it defaults to `default`). For now this is a hidden CLI
option until we decide whether we want to commit to this API.

In the future we also want to make the stateroot of `bootc switch` be
configurable (containers#617) so that
users can install an image to a new stateroot while they already have an
existing stateroot

Also removed the constant `STATEROOT_DEFAULT`, we're now simply taking
it from the `ostree_ext` crate
omertuc added a commit to omertuc/bootc that referenced this pull request Sep 3, 2024
This commit makes it so that the `bootc install` stateroot will be
configurable (it defaults to `default`). For now this is a hidden CLI
option until we decide whether we want to commit to this API.

In the future we also want to make the stateroot of `bootc switch` be
configurable (containers#617) so that
users can install an image to a new stateroot while they already have an
existing stateroot

Also removed the constant `STATEROOT_DEFAULT`, we're now simply taking
it from the `ostree_ext` crate
omertuc added a commit to omertuc/bootc that referenced this pull request Sep 3, 2024
This commit makes it so that the `bootc install` stateroot will be
configurable (it defaults to `default`). For now this is a hidden CLI
option until we decide whether we want to commit to this API.

In the future we also want to make the stateroot of `bootc switch` be
configurable (containers#617) so that
users can install an image to a new stateroot while they already have an
existing stateroot

Also removed the constant `STATEROOT_DEFAULT`, we're now simply taking
it from the `ostree_ext` crate
omertuc added a commit to omertuc/bootc that referenced this pull request Sep 3, 2024
This commit makes it so that the `bootc install` stateroot will be
configurable (it defaults to `default`). For now this is a hidden CLI
option until we decide whether we want to commit to this API.

In the future we also want to make the stateroot of `bootc switch` be
configurable (containers#617) so that
users can install an image to a new stateroot while they already have an
existing stateroot

Also removed the constant `STATEROOT_DEFAULT`, we're now simply taking
it from the `ostree_ext` crate
omertuc added a commit to omertuc/bootc that referenced this pull request Sep 3, 2024
This commit makes it so that the `bootc install` stateroot will be
configurable (it defaults to `default`). For now this is a hidden CLI
option until we decide whether we want to commit to this API.

In the future we also want to make the stateroot of `bootc switch` be
configurable (containers#617) so that
users can install an image to a new stateroot while they already have an
existing stateroot

Also removed the constant `STATEROOT_DEFAULT`, we're now simply taking
it from the `ostree_ext` crate
omertuc added a commit to omertuc/bootc that referenced this pull request Sep 3, 2024
This commit makes it so that the `bootc install` stateroot will be
configurable (it defaults to `default`). For now this is a hidden CLI
option until we decide whether we want to commit to this API.

In the future we also want to make the stateroot of `bootc switch` be
configurable (containers#617) so that
users can install an image to a new stateroot while they already have an
existing stateroot

Also removed the constant `STATEROOT_DEFAULT`, we're now simply taking
it from the `ostree_ext` crate
omertuc added a commit to omertuc/bootc that referenced this pull request Sep 3, 2024
This commit makes it so that the `bootc install` stateroot will be
configurable (it defaults to `default`). For now this is a hidden CLI
option until we decide whether we want to commit to this API.

In the future we also want to make the stateroot of `bootc switch` be
configurable (containers#617) so that
users can install an image to a new stateroot while they already have an
existing stateroot

Also removed the constant `STATEROOT_DEFAULT`, we're now simply taking
it from the `ostree_ext` crate
omertuc added a commit to omertuc/bootc that referenced this pull request Sep 3, 2024
This commit makes it so that the `bootc install` stateroot will be
configurable (it defaults to `default`). For now this is a hidden CLI
option until we decide whether we want to commit to this API.

In the future we also want to make the stateroot of `bootc switch` be
configurable (containers#617) so that
users can install an image to a new stateroot while they already have an
existing stateroot

Also removed the constant `STATEROOT_DEFAULT`, we're now simply taking
it from the `ostree_ext` crate
omertuc added a commit to omertuc/bootc that referenced this pull request Sep 3, 2024
This commit makes it so that the `bootc install` stateroot will be
configurable (it defaults to `default`). For now this is a hidden CLI
option until we decide whether we want to commit to this API.

In the future we also want to make the stateroot of `bootc switch` be
configurable (containers#617) so that
users can install an image to a new stateroot while they already have an
existing stateroot

Also removed the constant `STATEROOT_DEFAULT`, we're now simply taking
it from the `ostree_ext` crate
omertuc added a commit to omertuc/bootc that referenced this pull request Sep 3, 2024
This commit makes it so that the `bootc install` stateroot will be
configurable (it defaults to `default`). For now this is a hidden CLI
option until we decide whether we want to commit to this API.

In the future we also want to make the stateroot of `bootc switch` be
configurable (containers#617) so that
users can install an image to a new stateroot while they already have an
existing stateroot

Also removed the constant `STATEROOT_DEFAULT`, we're now simply taking
it from the `ostree_ext` crate
omertuc added a commit to omertuc/bootc that referenced this pull request Sep 3, 2024
This commit makes it so that the `bootc install` stateroot will be
configurable (it defaults to `default`). For now this is a hidden CLI
option until we decide whether we want to commit to this API.

In the future we also want to make the stateroot of `bootc switch` be
configurable (containers#617) so that
users can install an image to a new stateroot while they already have an
existing stateroot

Also removed the constant `STATEROOT_DEFAULT`, we're now simply taking
it from the `ostree_ext` crate
omertuc added a commit to omertuc/bootc that referenced this pull request Sep 3, 2024
This commit makes it so that the `bootc install` stateroot will be
configurable (it defaults to `default`). For now this is a hidden CLI
option until we decide whether we want to commit to this API.

In the future we also want to make the stateroot of `bootc switch` be
configurable (containers#617) so that
users can install an image to a new stateroot while they already have an
existing stateroot

Also removed the constant `STATEROOT_DEFAULT`, we're now simply taking
it from the `ostree_ext` crate

Signed-off-by: Omer Tuchfeld <omer@tuchfeld.dev>
omertuc added a commit to omertuc/bootc that referenced this pull request Sep 3, 2024
This commit makes it so that the `bootc install` stateroot will be
configurable (it defaults to `default`). For now this is a hidden CLI
option until we decide whether we want to commit to this API.

In the future we also want to make the stateroot of `bootc switch` be
configurable (containers#617) so that
users can install an image to a new stateroot while they already have an
existing stateroot

Also removed the constant `STATEROOT_DEFAULT`, we're now simply taking
it from the `ostree_ext` crate

Signed-off-by: Omer Tuchfeld <omer@tuchfeld.dev>
omertuc added a commit to omertuc/bootc that referenced this pull request Sep 6, 2024
This commit makes it so that the `bootc install` stateroot will be
configurable (it defaults to `default`). For now this is a hidden CLI
option until we decide whether we want to commit to this API.

In the future we also want to make the stateroot of `bootc switch` be
configurable (containers#617) so that
users can install an image to a new stateroot while they already have an
existing stateroot

Also removed the constant `STATEROOT_DEFAULT`, we're now simply taking
it from the `ostree_ext` crate

Signed-off-by: Omer Tuchfeld <omer@tuchfeld.dev>
omertuc added a commit to omertuc/bootc that referenced this pull request Sep 7, 2024
This commit makes it so that the `bootc install` stateroot will be
configurable (it defaults to `default`). For now this is a hidden CLI
option until we decide whether we want to commit to this API.

In the future we also want to make the stateroot of `bootc switch` be
configurable (containers#617) so that
users can install an image to a new stateroot while they already have an
existing stateroot

Also removed the constant `STATEROOT_DEFAULT`, we're now simply taking
it from the `ostree_ext` crate

Signed-off-by: Omer Tuchfeld <omer@tuchfeld.dev>
@omertuc omertuc force-pushed the stateroot branch 4 times, most recently from 0fcdc63 to a80b83a Compare September 10, 2024 17:42
# Background

<hash> added the `stateroot` option to the `install` subcommand

# Issue

The `stateroot` option is not available on the `switch` subcommand

# Solution

Add the `stateroot` option to the `switch` subcommand

# Implementation

* If the stateroot is different than the current, we should allow using
  the same image as the currently booted one

* Stateroot has to be explicitly created (`init_osname` binding) if it
  doesn't exist. If it does, we still call `init_osname` and simply
  ignore the error (TODO: only ignore non-already-exists errors)

* Copy `/var` from the old stateroot to the new one. I'm doing `--reflink`
  but it's still very slow

* Must use the old stateroot to find the `merge_deployment` because
  otherwise we boot without the required kargs (it manifested as a
  missing `root=UUID=...` which caused the dracut rootfs-generator to
  silently fail to create `sysroot.mount` and so `ostree-prepare-root`
  failed due to empty `/sysroot`)

Signed-off-by: Omer Tuchfeld <omer@tuchfeld.dev>
@omertuc
Copy link
Contributor Author

omertuc commented Sep 11, 2024

Should we provide users with a CLI option to skip the /var copy? Or maybe skipping should be the default?

@cgwalters
Copy link
Collaborator

This one heavily relates to #404

Task::new("Copying /var to new stateroot", "cp")
.args([
"--recursive",
"--reflink=auto",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Some discussion about hard requiring reflinks by default

Copy link
Collaborator

Choose a reason for hiding this comment

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

One subtle thing about this implementation is that as it is today because we're operating on the stateroot view of /var I don't think we'll see any mounts that happen externally today. For example if someone has /var/lib/postgres on a separate partition - which is a totally sane thing to do.

Speaking of, we almost certainly want to pass --one-file-system here just in case we somehow encounter a mount.

(Also this relates a bit to ostreedev/ostree#3292 )

@cgwalters
Copy link
Collaborator

I think my overall inclination here is to actually make this part of bootc install, and roll up fixing things related to #137 at the same time.

Hence because it's install I think I'd argue we drop all state by default...except let's add a check for "you have root in current kargs but missing in new kargs". That would avoid probably the biggest footgun and help people understand that often they're going to need to allow-list in at least some state from the current root.

Something like bootc install --stateroot=newroot --inherit-karg=root or something?

Or I guess the best place to bikeshed this would be #404

if new_spec == host.spec {
println!("Image specification is unchanged.");
if new_spec == host.spec && old_stateroot == stateroot {
// TODO: Should we really be confusing users with terms like "stateroot"?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, it's a good question. My thinking here is it's going to be hard to hide...and we should lean into exposing it as a more first class verb.

return Ok(());
}
let new_spec = RequiredHostSpec::from_spec(&new_spec)?;

if old_stateroot != stateroot {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Related to the above I'm uncertain about doing this by default. It may make sense to have a bootc stateroot init --clone or so?

Definitely in some cases we explicitly want to ignore previous state.

/// Make the switch into a custom stateroot. If the stateroot doesn't exist, it will be created
/// and `/var` of the current stateroot will be copied (copy-on-write) into it.
#[clap(long)]
pub(crate) stateroot: Option<String>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

In the interest of forward progress, how about adding #[clap(hide = true))] and calling this experimental_clone_into_stateroot so it can be an experimental feature for now while we decide on how we stabilize this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/install Issues related to `bootc install`
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants