Skip to content

Commit

Permalink
Exploded layer proof of concept
Browse files Browse the repository at this point in the history
The current design of the Layer trait encourages developers to put lots of logic into their layer. One impact of this, is it forces stateful logging to jump through hoops that I would call less ergonomic heroku/libcnb.rs#669.

This proof of concept is a re-imagining of how we might want to interact with a layer using only the existing layer primitives from libcnb. It encourages operations to be performed outside of a layer, because it does not require the developer to implement Layer on their own structs. Instead it treats metadata as a primitive and makes "read" and "write" layer information their own discrete actions.

At a high level the flow of interacting with a layer now becomes:

- Read cached data (get LayerData)
- Do stuff (modify path, log, etc.)
- Write new data (write LayerResult)

In this process I reimagined the flow for cache invalidation to fit within a `MetadataDiff` enum. This essentially encourages a set of "reasonable defaults" for equality based metadata, but does not restrict to a specific use cases.

Worth noting that I've got an idea of a way to better handle metadata that cannot be deserialized, but it would be distracting from the primary purpose of this PoC.

## Known

The code in `main.rs` is quite long and would likely be moved to its own function if we continued down this path.

## Discussion/Feedback

The purpose of this PR is to get feedback on this approach, both high level (concept) and low level (implementation).
  • Loading branch information
schneems committed Sep 19, 2023
1 parent 2b56182 commit 4974006
Show file tree
Hide file tree
Showing 9 changed files with 505 additions and 118 deletions.
12 changes: 11 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

127 changes: 31 additions & 96 deletions buildpacks/ruby/src/layers/ruby_install_layer.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,10 @@
#[allow(clippy::wildcard_imports)]
use commons::output::{
fmt::{self},
section_log::*,
};

use crate::{RubyBuildpack, RubyBuildpackError};
use crate::RubyBuildpackError;
use commons::gemfile_lock::ResolvedRubyVersion;
use commons::output::section_log::SectionLogger;
use flate2::read::GzDecoder;
use libcnb::build::BuildContext;
use libcnb::data::buildpack::StackId;
use libcnb::data::layer_content_metadata::LayerTypes;
use libcnb::layer::{ExistingLayerStrategy, Layer, LayerData, LayerResult, LayerResultBuilder};
use libcnb::layer::{LayerResult, LayerResultBuilder};
use serde::{Deserialize, Serialize};
use std::io;
use std::path::Path;
Expand All @@ -31,99 +25,40 @@ use url::Url;
///
/// When the Ruby version changes, invalidate and re-run.
///
pub(crate) struct RubyInstallLayer<'a> {
pub _in_section: &'a dyn SectionLogger, // force the layer to be called within a Section logging context, not necessary but it's safer
pub metadata: RubyInstallLayerMetadata,
}
#[derive(Deserialize, Serialize, Debug, Clone)]
#[derive(Deserialize, Serialize, Debug, Clone, Eq, PartialEq)]
pub(crate) struct RubyInstallLayerMetadata {
pub stack: StackId,
pub version: ResolvedRubyVersion,
}

impl<'a> Layer for RubyInstallLayer<'a> {
type Buildpack = RubyBuildpack;
type Metadata = RubyInstallLayerMetadata;

fn types(&self) -> LayerTypes {
LayerTypes {
build: true,
launch: true,
cache: true,
}
}

fn create(
&self,
_context: &BuildContext<Self::Buildpack>,
layer_path: &Path,
) -> Result<LayerResult<Self::Metadata>, RubyBuildpackError> {
log_step_timed("Installing", || {
let tmp_ruby_tgz = NamedTempFile::new()
.map_err(RubyInstallError::CouldNotCreateDestinationFile)
.map_err(RubyBuildpackError::RubyInstallError)?;

let url = download_url(&self.metadata.stack, &self.metadata.version)
.map_err(RubyBuildpackError::RubyInstallError)?;

download(url.as_ref(), tmp_ruby_tgz.path())
.map_err(RubyBuildpackError::RubyInstallError)?;

untar(tmp_ruby_tgz.path(), layer_path).map_err(RubyBuildpackError::RubyInstallError)?;

LayerResultBuilder::new(self.metadata.clone()).build()
})
}

fn existing_layer_strategy(
&self,
_context: &BuildContext<Self::Buildpack>,
layer_data: &LayerData<Self::Metadata>,
) -> Result<ExistingLayerStrategy, RubyBuildpackError> {
let old = &layer_data.content_metadata.metadata;
let now = self.metadata.clone();

match cache_state(old.clone(), now) {
Changed::Nothing(_version) => {
log_step("Using cached version");

Ok(ExistingLayerStrategy::Keep)
}
Changed::Stack(_old, _now) => {
log_step(format!("Clearing cache {}", fmt::details("stack changed")));

Ok(ExistingLayerStrategy::Recreate)
}
Changed::RubyVersion(_old, _now) => {
log_step(format!(
"Clearing cache {}",
fmt::details("ruby version changed")
));

Ok(ExistingLayerStrategy::Recreate)
}
}
}
}

fn cache_state(old: RubyInstallLayerMetadata, now: RubyInstallLayerMetadata) -> Changed {
let RubyInstallLayerMetadata { stack, version } = now;

if old.stack != stack {
Changed::Stack(old.stack, stack)
} else if old.version != version {
Changed::RubyVersion(old.version, version)
} else {
Changed::Nothing(version)
}
}

#[derive(Debug)]
enum Changed {
Nothing(ResolvedRubyVersion),
Stack(StackId, StackId),
RubyVersion(ResolvedRubyVersion, ResolvedRubyVersion),
pub(crate) fn install(
log: Box<dyn SectionLogger>,
path: &Path,
metadata: RubyInstallLayerMetadata,
) -> Result<
(
Box<dyn SectionLogger>,
LayerResult<RubyInstallLayerMetadata>,
),
RubyBuildpackError,
> {
let timer = log.step_timed("Installing");
let tmp_ruby_tgz = NamedTempFile::new()
.map_err(RubyInstallError::CouldNotCreateDestinationFile)
.map_err(RubyBuildpackError::RubyInstallError)?;

let url = download_url(&metadata.stack, &metadata.version)
.map_err(RubyBuildpackError::RubyInstallError)?;

download(url.as_ref(), tmp_ruby_tgz.path()).map_err(RubyBuildpackError::RubyInstallError)?;

untar(tmp_ruby_tgz.path(), path).map_err(RubyBuildpackError::RubyInstallError)?;
let log = timer.finish_timed_step();

LayerResultBuilder::new(metadata)
.build()
.map(|result| (log, result))
}

fn download_url(stack: &StackId, version: impl std::fmt::Display) -> Result<Url, RubyInstallError> {
Expand Down
108 changes: 87 additions & 21 deletions buildpacks/ruby/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#![warn(unused_crate_dependencies)]
#![warn(clippy::pedantic)]
#![allow(clippy::module_name_repetitions)]
use commons::cache::CacheError;
use commons::cache::{toml_delta, CacheError, CachedLayer, CachedLayerData, MetadataDiff};
use commons::fun_run::CmdError;
use commons::gemfile_lock::GemfileLock;
use commons::metadata_digest::MetadataDigest;
Expand All @@ -10,15 +10,15 @@ use core::str::FromStr;
use layers::{
bundle_download_layer::BundleDownloadLayer, bundle_download_layer::BundleDownloadLayerMetadata,
bundle_install_layer::BundleInstallLayer, bundle_install_layer::BundleInstallLayerMetadata,
ruby_install_layer::RubyInstallError, ruby_install_layer::RubyInstallLayer,
ruby_install_layer::RubyInstallLayerMetadata,
ruby_install_layer::RubyInstallError, ruby_install_layer::RubyInstallLayerMetadata,
};
use libcnb::build::{BuildContext, BuildResult, BuildResultBuilder};
use libcnb::data::build_plan::BuildPlanBuilder;
use libcnb::data::launch::LaunchBuilder;
use libcnb::data::layer_name;
use libcnb::detect::{DetectContext, DetectResult, DetectResultBuilder};
use libcnb::generic::{GenericMetadata, GenericPlatform};
use libcnb::layer::LayerResultBuilder;
use libcnb::layer_env::Scope;
use libcnb::Platform;
use libcnb::{buildpack_main, Buildpack};
Expand Down Expand Up @@ -88,26 +88,81 @@ impl Buildpack for RubyBuildpack {
let ruby_version = gemfile_lock.resolve_ruby("3.1.3");

// ## Install executable ruby version

(logger, env) = {
let section = logger.section(&format!(
"Ruby version {} from {}",
fmt::value(ruby_version.to_string()),
fmt::value(gemfile_lock.ruby_source())
let metadata = RubyInstallLayerMetadata {
stack: context.stack_id.clone(),
version: ruby_version.clone(),
};

let log_section = logger.section(&format!(
"Ruby version {value} from {source}",
value = fmt::value(metadata.version.to_string()),
source = fmt::value(gemfile_lock.ruby_source())
));
let ruby_layer = context //
.handle_layer(
layer_name!("ruby"),
RubyInstallLayer {
_in_section: section.as_ref(),
metadata: RubyInstallLayerMetadata {
stack: context.stack_id.clone(),
version: ruby_version.clone(),
},
},
)?;
let env = ruby_layer.env.apply(Scope::Build, &env);
(section.end_section(), env)

let cached_layer = CachedLayer {
name: layer_name!("ruby"),
build: true,
launch: true,
metadata,
}
.read(&context)?;

let (log_section, result) = match cached_layer.diff() {
MetadataDiff::Same(metadata) => (
log_section.step("Using cache"),
LayerResultBuilder::new(metadata.clone()).build_unwrapped(),
),
MetadataDiff::Different { old, now } => {
let reason = if old.version == now.version {
format!(
"{differences} changed",
differences = commons::display::SentenceList::new(
&toml_delta(&old, &now)
.into_iter()
.map(|diff| diff.key)
.collect::<Vec<_>>()
)
)
} else {
format!(
"Ruby version changed from {old} to {now}",
old = fmt::value(old.version.to_string()),
now = fmt::value(now.version.to_string())
)
};

crate::layers::ruby_install_layer::install(
clear_cache_with_reason(log_section, &cached_layer, reason).unwrap(),
&cached_layer.path,
now.clone(),
)?
}
MetadataDiff::CannotDeserialize { old: _old, now } => {
crate::layers::ruby_install_layer::install(
clear_cache_with_reason(
log_section,
&cached_layer,
"cannot deserialize metadata",
)
.unwrap(),
&cached_layer.path,
now.clone(),
)?
}
MetadataDiff::NoCache(metadata) => crate::layers::ruby_install_layer::install(
log_section,
&cached_layer.path,
metadata.clone(),
)?,
};

let env = cached_layer
.write(&context, result)?
.env
.apply(Scope::Build, &env);

(log_section.end_section(), env)
};

// ## Setup bundler
Expand Down Expand Up @@ -212,6 +267,17 @@ fn needs_java(gemfile_lock: &str) -> bool {
java_regex.is_match(gemfile_lock)
}

fn clear_cache_with_reason<B, M>(
log: Box<dyn SectionLogger>,
layer_data: &CachedLayerData<B, M>,
reason: impl AsRef<str>,
) -> Result<Box<dyn SectionLogger>, std::io::Error> {
let log = log.step(&format!("Clearing cache ({})", reason.as_ref()));
layer_data.clear_path_contents()?;

Ok(log)
}

#[derive(Debug)]
pub(crate) enum RubyBuildpackError {
RakeDetectError(CmdError),
Expand Down
2 changes: 2 additions & 0 deletions commons/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ thiserror = "1"
walkdir = "2"
which_problem = "0.1"
ascii_table = { version = "4.0.2", features = ["color_codes"] }
toml = "0.7"
itertools = "0.11.0"

[dev-dependencies]
indoc = "2.0.1"
Expand Down
4 changes: 4 additions & 0 deletions commons/src/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ mod clean;
mod config;
mod error;
mod in_app_dir_cache_layer;
mod read_write_layer;

pub use self::app_cache::{build, PathState};
pub use self::app_cache::{AppCache, CacheState};
Expand All @@ -13,3 +14,6 @@ pub use self::clean::FilesWithSize;
pub use self::config::CacheConfig;
pub use self::config::{mib, KeepPath};
pub use self::error::CacheError;
pub use self::read_write_layer::{
metadata_diff, toml_delta, CachedLayer, CachedLayerData, MetadataDiff,
};
Loading

0 comments on commit 4974006

Please sign in to comment.