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

WIP: Use podman pull to fetch containers #215

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion lib/src/blockdev.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use nix::errno::Errno;
use regex::Regex;
use serde::Deserialize;

use crate::install::run_in_host_mountns;
use crate::hostexec::run_in_host_mountns;
use crate::task::Task;

#[derive(Debug, Deserialize)]
Expand Down
41 changes: 33 additions & 8 deletions lib/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,10 @@ pub(crate) struct SwitchOpts {

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

/// The storage backend
#[clap(long, hide = true)]
pub(crate) backend: Option<crate::spec::Backend>,
}

/// Options controlling rollback
Expand Down Expand Up @@ -251,6 +255,14 @@ impl InternalsOpts {
const GENERATOR_BIN: &'static str = "bootc-systemd-generator";
}

#[derive(Debug, clap::Parser, PartialEq, Eq)]
pub(crate) struct InternalPodmanOpts {
#[clap(long, value_parser, default_value = "/")]
root: Utf8PathBuf,
#[clap(trailing_var_arg = true, allow_hyphen_values = true)]
args: Vec<std::ffi::OsString>,
}

/// Deploy and transactionally in-place with bootable container images.
///
/// The `bootc` project currently uses ostree-containers as a backend
Expand Down Expand Up @@ -379,6 +391,9 @@ pub(crate) enum Opt {
#[clap(subcommand)]
#[clap(hide = true)]
Internals(InternalsOpts),
/// Execute podman in our internal configuration
#[clap(hide = true)]
InternalPodman(InternalPodmanOpts),
#[clap(hide(true))]
#[cfg(feature = "docgen")]
Man(ManOpts),
Expand Down Expand Up @@ -482,7 +497,7 @@ async fn upgrade(opts: UpgradeOpts) -> Result<()> {
let sysroot = &get_locked_sysroot().await?;
let repo = &sysroot.repo();
let (booted_deployment, _deployments, host) =
crate::status::get_status_require_booted(sysroot)?;
crate::status::get_status_require_booted(sysroot).await?;
let imgref = host.spec.image.as_ref();
// If there's no specified image, let's be nice and check if the booted system is using rpm-ostree
if imgref.is_none() {
Expand Down Expand Up @@ -540,7 +555,7 @@ async fn upgrade(opts: UpgradeOpts) -> Result<()> {
}
}
} else {
let fetched = crate::deploy::pull(sysroot, imgref, opts.quiet).await?;
let fetched = crate::deploy::pull(sysroot, spec.backend, imgref, opts.quiet).await?;
Copy link
Contributor

Choose a reason for hiding this comment

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

In the branch above here where we handle --check, it doesn't know how to store cached updates properly for the podman backend. This in turn means we don't have a good way to represent that in bootc status (it will always just be None currently)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We have a lot of choice for how to represent this; it strongly relates to the question of image GC though. I think today, podman and c/storage generally handle this by creating containers which hold references to images.

Copy link
Contributor

Choose a reason for hiding this comment

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

This also feels like more reason re: your previous comment to switch to using skopeo instead of podman, since we could use skopeo inspect to only fetch the metadata in the --check case. If I'm following correctly from the way ostree does it today it's similar in that it's just saving the manifest/config.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep

let kargs = crate::kargs::get_kargs(repo, &booted_deployment, fetched.as_ref())?;
let staged_digest = staged_image.as_ref().map(|s| s.image_digest.as_str());
let fetched_digest = fetched.manifest_digest.as_str();
Expand Down Expand Up @@ -603,14 +618,16 @@ async fn switch(opts: SwitchOpts) -> Result<()> {
let target = ostree_container::OstreeImageReference { sigverify, imgref };
let target = ImageReference::from(target);

let backend = opts.backend.unwrap_or_default();

// 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)
crate::deploy::switch_origin_inplace(&root, &target, backend)
})
.await??
};
Expand All @@ -623,11 +640,12 @@ async fn switch(opts: SwitchOpts) -> Result<()> {
let sysroot = &get_locked_sysroot().await?;
let repo = &sysroot.repo();
let (booted_deployment, _deployments, host) =
crate::status::get_status_require_booted(sysroot)?;
crate::status::get_status_require_booted(sysroot).await?;

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

Expand All @@ -637,7 +655,7 @@ async fn switch(opts: SwitchOpts) -> Result<()> {
}
let new_spec = RequiredHostSpec::from_spec(&new_spec)?;

let fetched = crate::deploy::pull(sysroot, &target, opts.quiet).await?;
let fetched = crate::deploy::pull(sysroot, new_spec.backend, &target, opts.quiet).await?;
let kargs = crate::kargs::get_kargs(repo, &booted_deployment, fetched.as_ref())?;

if !opts.retain {
Expand Down Expand Up @@ -672,7 +690,7 @@ async fn rollback(_opts: RollbackOpts) -> Result<()> {
async fn edit(opts: EditOpts) -> Result<()> {
let sysroot = &get_locked_sysroot().await?;
let (booted_deployment, _deployments, host) =
crate::status::get_status_require_booted(sysroot)?;
crate::status::get_status_require_booted(sysroot).await?;
let new_host: Host = if let Some(filename) = opts.filename {
let mut r = std::io::BufReader::new(std::fs::File::open(filename)?);
serde_yaml::from_reader(&mut r)?
Expand All @@ -697,7 +715,8 @@ async fn edit(opts: EditOpts) -> Result<()> {
return crate::deploy::rollback(sysroot).await;
}

let fetched = crate::deploy::pull(sysroot, new_spec.image, opts.quiet).await?;
let fetched =
crate::deploy::pull(sysroot, new_spec.backend, new_spec.image, opts.quiet).await?;
let repo = &sysroot.repo();
let kargs = crate::kargs::get_kargs(repo, &booted_deployment, fetched.as_ref())?;

Expand Down Expand Up @@ -799,7 +818,7 @@ async fn run_from_opt(opt: Opt) -> Result<()> {
},
#[cfg(feature = "install")]
Opt::ExecInHostMountNamespace { args } => {
crate::install::exec_in_host_mountns(args.as_slice())
crate::hostexec::exec_in_host_mountns(args.as_slice())
}
Opt::Status(opts) => super::status::status(opts).await,
Opt::Internals(opts) => match opts {
Expand All @@ -813,6 +832,12 @@ async fn run_from_opt(opt: Opt) -> Result<()> {
}
InternalsOpts::FixupEtcFstab => crate::deploy::fixup_etc_fstab(&root),
},
Opt::InternalPodman(args) => {
prepare_for_write()?;
// This also remounts writable
let _sysroot = get_locked_sysroot().await?;
crate::podman::exec(args.root.as_path(), args.args.as_slice())
}
#[cfg(feature = "docgen")]
Opt::Man(manopts) => crate::docgen::generate_manpages(&manopts.directory),
}
Expand Down
93 changes: 82 additions & 11 deletions lib/src/deploy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,19 @@ use anyhow::{anyhow, Context, Result};
use cap_std::fs::{Dir, MetadataExt};
use cap_std_ext::cap_std;
use cap_std_ext::dirext::CapStdExtDirExt;
use chrono::DateTime;
use fn_error_context::context;
use ostree::{gio, glib};
use ostree_container::OstreeImageReference;
use ostree_ext::container as ostree_container;
use ostree_ext::container::store::PrepareResult;
use ostree_ext::oci_spec;
use ostree_ext::ostree;
use ostree_ext::ostree::Deployment;
use ostree_ext::sysroot::SysrootLock;

use crate::spec::ImageReference;
use crate::spec::{BootOrder, HostSpec};
use crate::spec::{Backend, BootOrder, HostSpec};
use crate::status::labels_of_config;

// TODO use https://github.com/ostreedev/ostree-rs-ext/pull/493/commits/afc1837ff383681b947de30c0cefc70080a4f87a
Expand All @@ -31,11 +33,14 @@ const BOOTC_DERIVED_KEY: &str = "bootc.derived";
/// Variant of HostSpec but required to be filled out
pub(crate) struct RequiredHostSpec<'a> {
pub(crate) image: &'a ImageReference,
pub(crate) backend: Backend,
}

/// State of a locally fetched image
pub(crate) struct ImageState {
pub(crate) backend: Backend,
pub(crate) manifest_digest: String,
pub(crate) created: Option<DateTime<chrono::Utc>>,
pub(crate) version: Option<String>,
pub(crate) ostree_commit: String,
}
Expand All @@ -48,16 +53,43 @@ impl<'a> RequiredHostSpec<'a> {
.image
.as_ref()
.ok_or_else(|| anyhow::anyhow!("Missing image in specification"))?;
Ok(Self { image })
Ok(Self {
image,
backend: spec.backend,
})
}
}

impl From<ostree_container::store::LayeredImageState> for ImageState {
fn from(value: ostree_container::store::LayeredImageState) -> Self {
let version = value.version().map(|v| v.to_owned());
let ostree_commit = value.get_commit().to_owned();
let labels = crate::status::labels_of_config(&value.configuration);
let created = labels
.and_then(|l| {
l.get(oci_spec::image::ANNOTATION_CREATED)
.map(|s| s.as_str())
})
.and_then(crate::status::try_deserialize_timestamp);
Self {
backend: Backend::OstreeContainer,
manifest_digest: value.manifest_digest,
created,
version,
ostree_commit,
}
}
}

impl From<crate::podman::PodmanInspect> for ImageState {
fn from(value: crate::podman::PodmanInspect) -> Self {
let version = None;
let ostree_commit = "".to_owned();
let created = value.created;
Self {
backend: Backend::Container,
manifest_digest: value.digest,
created,
version,
ostree_commit,
}
Expand All @@ -70,8 +102,14 @@ impl ImageState {
&self,
repo: &ostree::Repo,
) -> Result<Option<ostree_ext::oci_spec::image::ImageManifest>> {
ostree_container::store::query_image_commit(repo, &self.ostree_commit)
.map(|v| Some(v.manifest))
match self.backend {
Backend::OstreeContainer => {
ostree_container::store::query_image_commit(repo, &self.ostree_commit)
.map(|v| Some(v.manifest))
}
// TODO: Figure out if we can get the OCI manifest from podman
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it is possible to fetch a manifest with a podman command; you'd need to fall back to skopeo inspect or something else

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We can continue to use https://github.com/containers/containers-image-proxy-rs/ for this stuff, see this API which is what's used by the ostree-container code today.

We can consider extending the proxy API too with other things we need, as it's a reliable and tested bridge between the Rust and Go sides.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually also, we could fork skopeo copy and not podman pull too. That may help keep things clearer even in the short term.

Backend::Container => Ok(None),
}
}
}

Expand Down Expand Up @@ -164,6 +202,31 @@ async fn handle_layer_progress_print(
/// Wrapper for pulling a container image, wiring up status output.
#[context("Pulling")]
pub(crate) async fn pull(
sysroot: &SysrootLock,
backend: Backend,
imgref: &ImageReference,
quiet: bool,
) -> Result<Box<ImageState>> {
match backend {
Backend::OstreeContainer => pull_via_ostree(sysroot, imgref, quiet).await,
Backend::Container => pull_via_podman(sysroot, imgref, quiet).await,
}
}

/// Wrapper for pulling a container image, wiring up status output.
async fn pull_via_podman(
sysroot: &SysrootLock,
imgref: &ImageReference,
quiet: bool,
) -> Result<Box<ImageState>> {
let rootfs = &Dir::reopen_dir(&crate::utils::sysroot_fd_borrowed(sysroot))?;
let fetched_imageid = crate::podman::podman_pull(rootfs, imgref, quiet).await?;
crate::podman_ostree::commit_image_to_ostree(sysroot, &fetched_imageid)
.await
.map(Box::new)
}

async fn pull_via_ostree(
sysroot: &SysrootLock,
imgref: &ImageReference,
quiet: bool,
Expand Down Expand Up @@ -295,14 +358,17 @@ async fn deploy(
}

#[context("Generating origin")]
fn origin_from_imageref(imgref: &ImageReference) -> Result<glib::KeyFile> {
fn origin_from_imageref(imgref: &ImageReference, backend: Backend) -> Result<glib::KeyFile> {
let origin = glib::KeyFile::new();
let imgref = OstreeImageReference::from(imgref.clone());
origin.set_string(
"origin",
ostree_container::deploy::ORIGIN_CONTAINER,
imgref.to_string().as_str(),
);
if backend == Backend::Container {
origin.set_string("bootc", "backend", "container");
}
Ok(origin)
}

Expand All @@ -316,7 +382,7 @@ pub(crate) async fn stage(
opts: Option<ostree::SysrootDeployTreeOpts<'_>>,
) -> Result<()> {
let merge_deployment = sysroot.merge_deployment(Some(stateroot));
let origin = origin_from_imageref(spec.image)?;
let origin = origin_from_imageref(spec.image, image.backend)?;
crate::deploy::deploy(
sysroot,
merge_deployment.as_ref(),
Expand All @@ -340,7 +406,8 @@ pub(crate) async fn stage(
pub(crate) async fn rollback(sysroot: &SysrootLock) -> Result<()> {
const ROLLBACK_JOURNAL_ID: &str = "26f3b1eb24464d12aa5e7b544a6b5468";
let repo = &sysroot.repo();
let (booted_deployment, deployments, host) = crate::status::get_status_require_booted(sysroot)?;
let (booted_deployment, deployments, host) =
crate::status::get_status_require_booted(sysroot).await?;

let new_spec = {
let mut new_spec = host.spec.clone();
Expand Down Expand Up @@ -415,9 +482,13 @@ fn find_newest_deployment_name(deploysdir: &Dir) -> Result<String> {
}

// Implementation of `bootc switch --in-place`
pub(crate) fn switch_origin_inplace(root: &Dir, imgref: &ImageReference) -> Result<String> {
pub(crate) fn switch_origin_inplace(
root: &Dir,
imgref: &ImageReference,
backend: Backend,
) -> Result<String> {
// First, just create the new origin file
let origin = origin_from_imageref(imgref)?;
let origin = origin_from_imageref(imgref, backend)?;
let serialized_origin = origin.to_data();

// Now, we can't rely on being officially booted (e.g. with the `ostree=` karg)
Expand Down Expand Up @@ -477,7 +548,7 @@ fn test_switch_inplace() -> Result<()> {
signature: None,
};
{
let origin = origin_from_imageref(&orig_imgref)?;
let origin = origin_from_imageref(&orig_imgref, Backend::OstreeContainer)?;
deploydir.atomic_write(
format!("{target_deployment}.origin"),
origin.to_data().as_bytes(),
Expand All @@ -490,7 +561,7 @@ fn test_switch_inplace() -> Result<()> {
signature: None,
};

let replaced = switch_origin_inplace(&td, &target_imgref).unwrap();
let replaced = switch_origin_inplace(&td, &target_imgref, Backend::OstreeContainer).unwrap();
assert_eq!(replaced, target_deployment);
Ok(())
}
Expand Down
38 changes: 38 additions & 0 deletions lib/src/hostexec.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
//! Run a command in the host mount namespace

use std::os::fd::AsFd;
use std::os::unix::process::CommandExt;
use std::process::Command;

use anyhow::{Context, Result};
use camino::Utf8Path;
use fn_error_context::context;

/// Run a command in the host mount namespace
pub(crate) fn run_in_host_mountns(cmd: &str) -> Command {
let mut c = Command::new("/proc/self/exe");
c.args(["exec-in-host-mount-namespace", cmd]);
c
}

#[context("Re-exec in host mountns")]
pub(crate) fn exec_in_host_mountns(args: &[std::ffi::OsString]) -> Result<()> {
let (cmd, args) = args
.split_first()
.ok_or_else(|| anyhow::anyhow!("Missing command"))?;
tracing::trace!("{cmd:?} {args:?}");
let pid1mountns = std::fs::File::open("/proc/1/ns/mnt").context("open pid1 mountns")?;
nix::sched::setns(pid1mountns.as_fd(), nix::sched::CloneFlags::CLONE_NEWNS).context("setns")?;
rustix::process::chdir("/").context("chdir")?;
// Work around supermin doing chroot() and not pivot_root
// https://github.com/libguestfs/supermin/blob/5230e2c3cd07e82bd6431e871e239f7056bf25ad/init/init.c#L288
if !Utf8Path::new("/usr").try_exists().context("/usr")?
&& Utf8Path::new("/root/usr")
.try_exists()
.context("/root/usr")?
{
tracing::debug!("Using supermin workaround");
rustix::process::chroot("/root").context("chroot")?;
}
Err(Command::new(cmd).args(args).exec()).context("exec")?
}
2 changes: 1 addition & 1 deletion lib/src/image.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ pub(crate) async fn push_entrypoint(source: Option<&str>, target: Option<&str>)
let source = if let Some(source) = source {
ImageReference::try_from(source).context("Parsing source image")?
} else {
let status = crate::status::get_status_require_booted(&sysroot)?;
let status = crate::status::get_status_require_booted(&sysroot).await?;
// SAFETY: We know it's booted
let booted = status.2.status.booted.unwrap();
let booted_image = booted.image.unwrap().image;
Expand Down
Loading
Loading