Skip to content

Commit

Permalink
upgrade: Check imgref digest before pulling image
Browse files Browse the repository at this point in the history
Previously, `rpm-ostree upgrade` was re-pulling a container image
each time, and throwing it away if it realizes that the commit is
the same as the currently booted deployment much later on.
Be smarter about this by checking the image reference digest, and
don't bother pulling the container image if the digest is the same.
  • Loading branch information
kelvinfan001 committed Jul 15, 2021
1 parent 6bd2c25 commit ff130aa
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 8 deletions.
15 changes: 14 additions & 1 deletion rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,22 @@ pub mod ffi {
fn cliwrap_destdir() -> String;
}

/// `ContainerImport` is currently identical to ostree-rs-ext's `Import` struct, because
/// cxx.rs currently requires types used as extern Rust types to be defined by the same crate
/// that contains the bridge using them, so we redefine an `ContainerImport` struct here.
pub(crate) struct ContainerImport {
pub ostree_commit: String,
pub image_digest: String,
}

// sysroot_upgrade.rs
extern "Rust" {
fn import_container(sysroot: Pin<&mut OstreeSysroot>, imgref: String) -> Result<String>;
fn import_container(
sysroot: Pin<&mut OstreeSysroot>,
imgref: String,
) -> Result<Box<ContainerImport>>;

fn fetch_digest(imgref: String) -> Result<String>;
}

// core.rs
Expand Down
20 changes: 16 additions & 4 deletions rust/src/sysroot_upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,34 @@
// SPDX-License-Identifier: Apache-2.0 OR MIT

use crate::cxxrsutil::*;
use crate::ffi::ContainerImport;
use anyhow::{Context, Result};
use std::convert::TryInto;
use std::pin::Pin;

/// Import ostree commit in container image using ostree-rs-ext's API.
pub fn import_container(
pub(crate) fn import_container(
mut sysroot: Pin<&mut crate::FFIOstreeSysroot>,
imgref: String,
) -> CxxResult<String> {
) -> CxxResult<Box<ContainerImport>> {
// TODO: take a GCancellable and monitor it, and drop the import task (which is how async cancellation works in Rust).
let sysroot = &sysroot.gobj_wrap();
let repo = &sysroot.get_repo(gio::NONE_CANCELLABLE)?;
let imgref = imgref.as_str().try_into()?;
let commit = build_runtime()?
let imported = build_runtime()?
.block_on(async { ostree_ext::container::import(&repo, &imgref, None).await })?;
Ok(commit.ostree_commit)
Ok(Box::new(ContainerImport {
ostree_commit: imported.ostree_commit,
image_digest: imported.image_digest,
}))
}

/// Fetch the image digest for `imgref` using ostree-rs-ext's API.
pub(crate) fn fetch_digest(imgref: String) -> CxxResult<String> {
let imgref = imgref.as_str().try_into()?;
let digest = build_runtime()?
.block_on(async { ostree_ext::container::fetch_manifest_info(&imgref).await })?;
Ok(digest.manifest_digest)
}

fn build_runtime() -> Result<tokio::runtime::Runtime> {
Expand Down
21 changes: 19 additions & 2 deletions src/daemon/rpmostree-sysroot-upgrader.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -448,9 +448,26 @@ rpmostree_sysroot_upgrader_pull_base (RpmOstreeSysrootUpgrader *self,
if (override_commit)
return glnx_throw (error, "Specifying commit overrides for container-image-reference type refspecs is not supported");

auto commit = rpmostreecxx::import_container(*self->sysroot, std::string(refspec));
if (strstr (refspec, "@"))
return glnx_throw (error, "Pinned to specific container image digest, cannot upgrade");

new_base_rev = strdup (commit.c_str());
/* Check to see if the digest that `refspec` points to is the same before pulling a new container image. */
const char *cur_digest = rpmostree_origin_get_container_image_reference_digest (self->original_origin);
if (cur_digest)
{
auto new_digest = rpmostreecxx::fetch_digest(std::string(refspec));
if (strcmp (new_digest.c_str(), cur_digest) == 0)
{
/* No new digest. */
*out_changed = FALSE;
return TRUE;
}
}

auto import = rpmostreecxx::import_container(*self->sysroot, std::string(refspec));

rpmostree_origin_set_container_image_reference_digest (self->original_origin, import->image_digest.c_str());
new_base_rev = strdup (import->ostree_commit.c_str());
break;
}
case RPMOSTREE_REFSPEC_TYPE_CHECKSUM:
Expand Down
31 changes: 30 additions & 1 deletion src/libpriv/rpmostree-origin.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ struct RpmOstreeOrigin {
RpmOstreeRefspecType refspec_type;
char *cached_refspec;

/* Container image digest, if tracking a container image reference */
char *cached_digest;

/* Version data that goes along with the refspec */
char *cached_override_commit;

Expand Down Expand Up @@ -115,7 +118,7 @@ rpmostree_origin_parse_keyfile (GKeyFile *origin,

ret->cached_unconfigured_state = g_key_file_get_string (ret->kf, "origin", "unconfigured-state", NULL);

/* Note that the refspec type can be inferred from the key in the orgin file, where
/* Note that the refspec type can be inferred from the key in the origin file, where
* the `RPMOSTREE_REFSPEC_OSTREE_ORIGIN_KEY` and `RPMOSTREE_REFSPEC_OSTREE_BASE_ORIGIN_KEY` keys
* may correspond to either TYPE_OSTREE or TYPE_CHECKSUM (further classification is required), and
* `RPMOSTREE_REFSPEC_CONTAINER_ORIGIN_KEY` always only corresponds to TYPE_CONTAINER. */
Expand Down Expand Up @@ -151,6 +154,8 @@ rpmostree_origin_parse_keyfile (GKeyFile *origin,
{
ret->refspec_type = RPMOSTREE_REFSPEC_TYPE_CONTAINER;
ret->cached_refspec = util::move_nullify (imgref);

ret->cached_digest = g_key_file_get_string (ret->kf, "origin", "container-image-reference-digest", NULL);
}
else
{
Expand Down Expand Up @@ -210,6 +215,12 @@ rpmostree_origin_remove_transient_state (RpmOstreeOrigin *origin)
rpmostree_origin_set_override_commit (origin, NULL, NULL);
}

const char *
rpmostree_origin_get_container_image_reference_digest (RpmOstreeOrigin *origin)
{
return origin->cached_digest;
}

const char *
rpmostree_origin_get_refspec (RpmOstreeOrigin *origin)
{
Expand Down Expand Up @@ -465,6 +476,23 @@ rpmostree_origin_set_regenerate_initramfs (RpmOstreeOrigin *origin,
g_key_file_get_string_list (origin->kf, "rpmostree", "initramfs-args", NULL, NULL);
}

void
rpmostree_origin_set_container_image_reference_digest (RpmOstreeOrigin *origin,
const char *digest)
{
if (digest != NULL)
{
g_key_file_set_string (origin->kf, "origin", "container-image-reference-digest", digest);
}
else
{
g_key_file_remove_key (origin->kf, "origin", "container-image-reference-digest", NULL);
}

g_free (origin->cached_digest);
origin->cached_digest = g_strdup (digest);
}

void
rpmostree_origin_set_override_commit (RpmOstreeOrigin *origin,
const char *checksum,
Expand Down Expand Up @@ -543,6 +571,7 @@ rpmostree_origin_set_rebase_custom (RpmOstreeOrigin *origin,
{
/* Remove `TYPE_CONTAINER`-related keys */
g_key_file_remove_key (origin->kf, "origin", RPMOSTREE_REFSPEC_CONTAINER_ORIGIN_KEY, NULL);
g_key_file_remove_key (origin->kf, "origin", "container-image-reference-digest", NULL);

const char *refspec_key =
g_key_file_has_key (origin->kf, "origin", RPMOSTREE_REFSPEC_OSTREE_BASE_ORIGIN_KEY, NULL) ?
Expand Down
7 changes: 7 additions & 0 deletions src/libpriv/rpmostree-origin.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ rpmostree_origin_dup (RpmOstreeOrigin *origin);
void
rpmostree_origin_remove_transient_state (RpmOstreeOrigin *origin);

const char *
rpmostree_origin_get_container_image_reference_digest (RpmOstreeOrigin *origin);

const char *
rpmostree_origin_get_refspec (RpmOstreeOrigin *origin);

Expand Down Expand Up @@ -134,6 +137,10 @@ rpmostree_origin_set_regenerate_initramfs (RpmOstreeOrigin *origin,
gboolean regenerate,
char **args);

void
rpmostree_origin_set_container_image_reference_digest (RpmOstreeOrigin *origin,
const char *digest);

void
rpmostree_origin_set_override_commit (RpmOstreeOrigin *origin,
const char *checksum,
Expand Down

0 comments on commit ff130aa

Please sign in to comment.