Skip to content

Commit

Permalink
Shared layer logic for consistent output and unit testing (#327)
Browse files Browse the repository at this point in the history
* Let rust-analyzer reorder use statements

There was a newline preventing rust-analyzer from ordering the `use` statements. I removed it, and saved and this is the outcome.

* Move metadata diff logic into a shared trait

* Extract invalid_metadata_action to shared module

* Extract restored_layer_action to shared module

* Bundle default cache behavior into shared module

* Ensure we don't forget to write new metdata

* Apply clippy suggestion

* Add a test for metadata differences

* Handle case where metadata is invalid

It might be safe to `expect` in the specific case, but not in the general one. We can return a nicely formatted string with information if it ever happens.

* Assert desired caching behavior

Instead of asserting implementation (that diffing a metadata object returns a vec entry), assert the behavior we want (that it clears the cache).

* Clippy

* Use standardized output

* Add tests to shared layer cache logic

To fully exercise all caching logic in the shared folder, I'm introducing a helper to create a temporary `BuildContext` that can be used for exercising caching logic in test.

This also introduces tests for both `restored_layer_action` states as called through `cached_layer_write_metadata`. Which should address this comment #327 (comment).

* Assert the behavior of invalid_metadata_action

* Exercise same interface used in file

In the ruby_install_layer we're exercising `cached_layer_write_metadata` but we were testing `restored_layer_action` which is called currently but might not be in the future via refactoring. This change asserts the behavior we want while using the exact same interface we are currently using.

* Combine distro name and version in changed output

* Add helper for removing ANSI codes for testing

* Format distribution values as backticks with color

* Assert all difference output

- Ensures each of these cases where the vec is not empty will clear the cache.
- Makes it easier for reviewers to see the output as a user would see it.
  • Loading branch information
schneems authored Oct 2, 2024
1 parent da9c2ff commit ec57995
Show file tree
Hide file tree
Showing 4 changed files with 445 additions and 95 deletions.
1 change: 1 addition & 0 deletions buildpacks/ruby/src/layers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ pub(crate) mod bundle_download_layer;
pub(crate) mod bundle_install_layer;
pub(crate) mod metrics_agent_install;
pub(crate) mod ruby_install_layer;
mod shared;
223 changes: 129 additions & 94 deletions buildpacks/ruby/src/layers/ruby_install_layer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,19 @@
//!
//! When the Ruby version changes, invalidate and re-run.
//!
use bullet_stream::state::SubBullet;
use bullet_stream::{style, Print};
use commons::display::SentenceList;
use libcnb::data::layer_name;
use libcnb::layer::{
CachedLayerDefinition, EmptyLayerCause, InvalidMetadataAction, LayerState, RestoredLayerAction,
};
use libcnb::layer_env::LayerEnv;
use magic_migrate::{try_migrate_deserializer_chain, TryMigrate};

use crate::layers::shared::{cached_layer_write_metadata, MetadataDiff};
use crate::{
target_id::{TargetId, TargetIdError},
RubyBuildpack, RubyBuildpackError,
};
use bullet_stream::state::SubBullet;
use bullet_stream::{style, Print};
use commons::gemfile_lock::ResolvedRubyVersion;
use flate2::read::GzDecoder;
use libcnb::data::layer_name;
use libcnb::layer::{EmptyLayerCause, LayerState};
use libcnb::layer_env::LayerEnv;
use magic_migrate::{try_migrate_deserializer_chain, TryMigrate};
use serde::{Deserialize, Deserializer, Serialize};
use std::convert::Infallible;
use std::io::{self, Stdout};
Expand All @@ -38,67 +35,26 @@ use url::Url;
pub(crate) fn handle(
context: &libcnb::build::BuildContext<RubyBuildpack>,
mut bullet: Print<SubBullet<Stdout>>,
metadata: Metadata,
metadata: &Metadata,
) -> libcnb::Result<(Print<SubBullet<Stdout>>, LayerEnv), RubyBuildpackError> {
let layer_ref = context.cached_layer(
layer_name!("ruby"),
CachedLayerDefinition {
build: true,
launch: true,
invalid_metadata_action: &|old| match Metadata::try_from_str_migrations(
&toml::to_string(old).expect("TOML deserialization of GenericMetadata"),
) {
Some(Ok(migrated)) => (
InvalidMetadataAction::ReplaceMetadata(migrated),
"replaced metadata".to_string(),
),
Some(Err(error)) => (
InvalidMetadataAction::DeleteLayer,
format!("metadata migration error {error}"),
),
None => (
InvalidMetadataAction::DeleteLayer,
"invalid metadata".to_string(),
),
},
restored_layer_action: &|old: &Metadata, _| {
let diff = metadata_diff(old, &metadata);
if diff.is_empty() {
(
RestoredLayerAction::KeepLayer,
"using cached version".to_string(),
)
} else {
(
RestoredLayerAction::DeleteLayer,
format!(
"due to {changes}: {differences}",
changes = if diff.len() > 1 { "changes" } else { "change" },
differences = SentenceList::new(&diff)
),
)
}
},
},
)?;
let layer_ref = cached_layer_write_metadata(layer_name!("ruby"), context, metadata)?;
match &layer_ref.state {
LayerState::Restored { cause: _ } => {
bullet = bullet.sub_bullet("Using cached Ruby version");
LayerState::Restored { cause } => {
bullet = bullet.sub_bullet(cause);
}
LayerState::Empty { cause } => {
match cause {
EmptyLayerCause::NewlyCreated => {}
EmptyLayerCause::InvalidMetadataAction { cause }
| EmptyLayerCause::RestoredLayerAction { cause } => {
bullet = bullet.sub_bullet(format!("Clearing cache {cause}"));
bullet = bullet.sub_bullet(cause);
}
}
let timer = bullet.start_timer("Installing");
install_ruby(&metadata, &layer_ref.path())?;
install_ruby(metadata, &layer_ref.path())?;
bullet = timer.done();
}
}
layer_ref.write_metadata(metadata)?;
Ok((bullet, layer_ref.read_env()?))
}

Expand Down Expand Up @@ -170,44 +126,39 @@ impl TryFrom<MetadataV1> for MetadataV2 {
}
}

fn metadata_diff(old: &Metadata, metadata: &Metadata) -> Vec<String> {
let mut differences = Vec::new();
let Metadata {
distro_name,
distro_version,
cpu_architecture,
ruby_version,
} = old;
if ruby_version != &metadata.ruby_version {
differences.push(format!(
"Ruby version ({old} to {now})",
old = style::value(ruby_version.to_string()),
now = style::value(metadata.ruby_version.to_string())
));
}
if distro_name != &metadata.distro_name {
differences.push(format!(
"distro name ({old} to {now})",
old = style::value(distro_name),
now = style::value(&metadata.distro_name)
));
}
if distro_version != &metadata.distro_version {
differences.push(format!(
"distro version ({old} to {now})",
old = style::value(distro_version),
now = style::value(&metadata.distro_version)
));
}
if cpu_architecture != &metadata.cpu_architecture {
differences.push(format!(
"CPU architecture ({old} to {now})",
old = style::value(cpu_architecture),
now = style::value(&metadata.cpu_architecture)
));
}
impl MetadataDiff for Metadata {
fn diff(&self, old: &Self) -> Vec<String> {
let mut differences = Vec::new();
let Metadata {
distro_name,
distro_version,
cpu_architecture,
ruby_version,
} = old;
if ruby_version != &self.ruby_version {
differences.push(format!(
"Ruby version ({old} to {now})",
old = style::value(ruby_version.to_string()),
now = style::value(self.ruby_version.to_string())
));
}
if distro_name != &self.distro_name || distro_version != &self.distro_version {
differences.push(format!(
"Distribution ({old} to {now})",
old = style::value(format!("{distro_name} {distro_version}")),
now = style::value(format!("{} {}", self.distro_name, self.distro_version))
));
}
if cpu_architecture != &self.cpu_architecture {
differences.push(format!(
"CPU architecture ({old} to {now})",
old = style::value(cpu_architecture),
now = style::value(&self.cpu_architecture)
));
}

differences
differences
}
}

fn download_url(
Expand Down Expand Up @@ -291,6 +242,8 @@ pub(crate) enum RubyInstallError {

#[cfg(test)]
mod tests {
use crate::layers::shared::{strip_ansi, temp_build_context};

use super::*;

/// If this test fails due to a change you'll need to
Expand Down Expand Up @@ -362,4 +315,86 @@ version = "3.1.3"
"https://heroku-buildpack-ruby.s3.us-east-1.amazonaws.com/heroku-22/ruby-2.7.4.tgz",
);
}

#[test]
fn metadata_diff_messages() {
let old = Metadata {
ruby_version: ResolvedRubyVersion("3.5.3".to_string()),
distro_name: "ubuntu".to_string(),
distro_version: "20.04".to_string(),
cpu_architecture: "amd64".to_string(),
};
assert_eq!(old.diff(&old), Vec::<String>::new());

let diff = Metadata {
ruby_version: ResolvedRubyVersion("3.5.5".to_string()),
distro_name: old.distro_name.clone(),
distro_version: old.distro_version.clone(),
cpu_architecture: old.cpu_architecture.clone(),
}
.diff(&old);
assert_eq!(
diff.iter().map(strip_ansi).collect::<Vec<String>>(),
vec!["Ruby version (`3.5.3` to `3.5.5`)".to_string()]
);

let diff = Metadata {
ruby_version: old.ruby_version.clone(),
distro_name: "alpine".to_string(),
distro_version: "3.20.0".to_string(),
cpu_architecture: old.cpu_architecture.clone(),
}
.diff(&old);

assert_eq!(
diff.iter().map(strip_ansi).collect::<Vec<String>>(),
vec!["Distribution (`ubuntu 20.04` to `alpine 3.20.0`)".to_string()]
);

let diff = Metadata {
ruby_version: old.ruby_version.clone(),
distro_name: old.distro_name.clone(),
distro_version: old.distro_version.clone(),
cpu_architecture: "arm64".to_string(),
}
.diff(&old);
assert_eq!(
diff.iter().map(strip_ansi).collect::<Vec<String>>(),
vec!["CPU architecture (`amd64` to `arm64`)".to_string()]
);
}

#[test]
fn test_ruby_version_difference_clears_cache() {
let temp = tempfile::tempdir().unwrap();
let context = temp_build_context::<RubyBuildpack>(temp.path());
let old = Metadata {
ruby_version: ResolvedRubyVersion("2.7.2".to_string()),
distro_name: "ubuntu".to_string(),
distro_version: "20.04".to_string(),
cpu_architecture: "x86_64".to_string(),
};
let differences = old.diff(&old);
assert_eq!(differences, Vec::<String>::new());

cached_layer_write_metadata(layer_name!("ruby"), &context, &old).unwrap();
let result = cached_layer_write_metadata(layer_name!("ruby"), &context, &old).unwrap();
let actual = result.state;
assert!(matches!(actual, LayerState::Restored { .. }));

let now = Metadata {
ruby_version: ResolvedRubyVersion("3.0.0".to_string()),
..old.clone()
};
let differences = now.diff(&old);
assert_eq!(differences.len(), 1);

let result = cached_layer_write_metadata(layer_name!("ruby"), &context, &now).unwrap();
assert!(matches!(
result.state,
LayerState::Empty {
cause: EmptyLayerCause::RestoredLayerAction { .. }
}
));
}
}
Loading

0 comments on commit ec57995

Please sign in to comment.