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.

Sign up for GitHub

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

bootc switch --stateroot flag #617

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 46 additions & 7 deletions lib/src/cli.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use crate::deploy::RequiredHostSpec;
use crate::lints;
use crate::spec::Host;
use crate::spec::ImageReference;
use crate::task::Task;
use crate::utils::sigpolicy_from_opts;

include!(concat!(env!("OUT_DIR"), "/version.rs"));
Expand Down Expand Up @@ -99,6 +100,11 @@ pub(crate) struct SwitchOpts {

/// Target image to use for the next boot.
pub(crate) target: String,

/// 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?

}

/// Options controlling rollback
Expand Down Expand Up @@ -628,7 +634,7 @@ async fn upgrade(opts: UpgradeOpts) -> Result<()> {
println!("No update available.")
} else {
let osname = booted_deployment.osname();
crate::deploy::stage(sysroot, &osname, &fetched, &spec).await?;
crate::deploy::stage(sysroot, &osname, &osname, &fetched, &spec).await?;
changed = true;
if let Some(prev) = booted_image.as_ref() {
if let Some(fetched_manifest) = fetched.get_manifest(repo)? {
Expand Down Expand Up @@ -664,13 +670,13 @@ async fn switch(opts: SwitchOpts) -> Result<()> {
);
let target = ostree_container::OstreeImageReference { sigverify, imgref };
let target = ImageReference::from(target);
let root = cap_std::fs::Dir::open_ambient_dir("/", cap_std::ambient_authority())?;

// If we're doing an in-place mutation, we shortcut most of the rest of the work here
if opts.mutate_in_place {
let deployid = {
// Clone to pass into helper thread
let target = target.clone();
let root = cap_std::fs::Dir::open_ambient_dir("/", cap_std::ambient_authority())?;
tokio::task::spawn_blocking(move || {
crate::deploy::switch_origin_inplace(&root, &target)
})
Expand All @@ -687,18 +693,52 @@ async fn switch(opts: SwitchOpts) -> Result<()> {
let (booted_deployment, _deployments, host) =
crate::status::get_status_require_booted(sysroot)?;

let (old_stateroot, stateroot) = {
let booted_osname = booted_deployment.osname();
let stateroot = opts
.stateroot
.as_deref()
.unwrap_or_else(|| booted_osname.as_str());

(booted_osname.to_owned(), stateroot.to_owned())
};

let new_spec = {
let mut new_spec = host.spec.clone();
new_spec.image = Some(target.clone());
new_spec
};

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.

println!(
"The currently running deployment in stateroot {stateroot} is already using this image"
);
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.

let init_result = sysroot.init_osname(&stateroot, cancellable);
match init_result {
Ok(_) => {
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 )

"--archive",
format!("/sysroot/ostree/deploy/{old_stateroot}/var").as_str(),
format!("/sysroot/ostree/deploy/{stateroot}/").as_str(),
])
.run()?;
}
Err(err) => {
// TODO: Only ignore non already-exists errors
println!("Ignoring error creating new stateroot: {err}");
}
}
}

let fetched = crate::deploy::pull(repo, &target, None, opts.quiet).await?;

if !opts.retain {
Expand All @@ -712,8 +752,7 @@ async fn switch(opts: SwitchOpts) -> Result<()> {
}
}

let stateroot = booted_deployment.osname();
crate::deploy::stage(sysroot, &stateroot, &fetched, &new_spec).await?;
crate::deploy::stage(sysroot, &old_stateroot, &stateroot, &fetched, &new_spec).await?;

if opts.apply {
crate::reboot::reboot()?;
Expand Down Expand Up @@ -766,7 +805,7 @@ async fn edit(opts: EditOpts) -> Result<()> {
// TODO gc old layers here

let stateroot = booted_deployment.osname();
crate::deploy::stage(sysroot, &stateroot, &fetched, &new_spec).await?;
crate::deploy::stage(sysroot, &stateroot, &stateroot, &fetched, &new_spec).await?;

Ok(())
}
Expand Down
6 changes: 3 additions & 3 deletions lib/src/deploy.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,6 @@ async fn deploy(
image: &ImageState,
origin: &glib::KeyFile,
) -> Result<Deployment> {
let stateroot = Some(stateroot);
let mut opts = ostree::SysrootDeployTreeOpts::default();
// Compute the kernel argument overrides. In practice today this API is always expecting
// a merge deployment. The kargs code also always looks at the booted root (which
Expand All @@ -396,7 +395,7 @@ async fn deploy(
let cancellable = gio::Cancellable::NONE;
return sysroot
.stage_tree_with_options(
stateroot,
Some(stateroot),
image.ostree_commit.as_str(),
Some(origin),
merge_deployment,
Expand All @@ -422,11 +421,12 @@ fn origin_from_imageref(imgref: &ImageReference) -> Result<glib::KeyFile> {
#[context("Staging")]
pub(crate) async fn stage(
sysroot: &Storage,
merge_stateroot: &str,
stateroot: &str,
image: &ImageState,
spec: &RequiredHostSpec<'_>,
) -> Result<()> {
let merge_deployment = sysroot.merge_deployment(Some(stateroot));
let merge_deployment = sysroot.merge_deployment(Some(merge_stateroot));
let origin = origin_from_imageref(spec.image)?;
let deployment = crate::deploy::deploy(
sysroot,
Expand Down