Skip to content

Commit

Permalink
Use --pull-policy if-not-present when running pack build (#373)
Browse files Browse the repository at this point in the history
Since it:
1. Saves ~2 seconds per integration test re-pulling an image that's already up to date.
2. Helps prevent hitting Docker Hub or ECR rate limits from duplicate (and redundant)
   image pulling (that counts agains the rate limit even when it's a no-op).

Longer term, if Pack CLI supports a periodic pulling mode (buildpacks/pack#1368), we
can switch to that, however for now this is the lesser of two evils - and in most cases
Pack usage outside of `libcnb-test` will ensure that newer builder images are pulled
from time to time.

See #306 for more details.

See also:
https://buildpacks.io/docs/tools/pack/cli/pack_config_pull-policy/

Fixes #306.
GUS-W-10799284.
  • Loading branch information
edmorley authored Mar 7, 2022
1 parent c83fefb commit 95feb4c
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 17 deletions.
9 changes: 2 additions & 7 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -60,17 +60,12 @@ jobs:
uses: Swatinem/[email protected]
- name: Install Pack CLI
uses: buildpacks/github-actions/[email protected]
- name: Configure Pack CLI
# Default to a small non-libc image for faster CI + to validate the static musl cross-compilation.
# Adjust pull-policy to prevent redundant image-pulling, which slows CI and risks hitting registry rate limits.
run: |
pack config default-builder cnbs/sample-builder:alpine
pack config pull-policy if-not-present
- name: Run integration tests
# Runs any tests annotated with the `#[ignore]` attribute (which in this repo, are all of the integration tests).
run: cargo test -- --ignored
- name: Compile and package examples/basics
run: cargo run --package libcnb-cargo -- libcnb package
working-directory: ./examples/basics
- name: Pack build using examples/basics
run: pack build example-basics --buildpack target/buildpack/debug/libcnb-examples_basics --path examples/
# Uses a non-libc image to validate the static musl cross-compilation.
run: pack build example-basics --builder cnbs/sample-builder:alpine --buildpack target/buildpack/debug/libcnb-examples_basics --path examples/
1 change: 1 addition & 0 deletions libcnb-test/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
- Replaced `IntegrationTestContext::start_container` with `IntegrationTestContext::prepare_container`, allowing users to configure the container before starting it. Ports can now be exposed via `PrepareContainerContext::expose_port`. ([#346](https://github.com/Malax/libcnb.rs/pull/346))
- Added the ability to set environment variables for the container via `PrepareContainerContext::env` and `PrepareContainerContext::envs`. ([#346](https://github.com/Malax/libcnb.rs/pull/346))
- Switch from `libcnb-cargo` to the new `libcnb-package` crate for buildpack packaging, which improves compile times due to it not including CLI-related dependencies ([#362](https://github.com/Malax/libcnb.rs/pull/362)).
- Use `--pull-policy if-not-present` when running `pack build` ([#373](https://github.com/Malax/libcnb.rs/pull/373)).

## [0.2.0] 2022-02-28

Expand Down
11 changes: 8 additions & 3 deletions libcnb-test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ mod util;
pub use crate::container_context::{
ContainerContext, ContainerExecResult, PrepareContainerContext,
};
use crate::pack::PackBuildCommand;
use crate::pack::{PackBuildCommand, PullPolicy};
use bollard::image::RemoveImageOptions;
use bollard::Docker;
use std::collections::HashMap;
Expand Down Expand Up @@ -233,8 +233,13 @@ impl IntegrationTest {

let image_name = util::random_docker_identifier();

let mut pack_command =
PackBuildCommand::new(&self.builder_name, temp_app_dir.path(), &image_name);
let mut pack_command = PackBuildCommand::new(
&self.builder_name,
temp_app_dir.path(),
&image_name,
// Prevent redundant image-pulling, which slows tests and risks hitting registry rate limits.
PullPolicy::IfNotPresent,
);

self.env.iter().for_each(|(key, value)| {
pack_command.env(key, value);
Expand Down
38 changes: 31 additions & 7 deletions libcnb-test/src/pack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@ use tempfile::TempDir;
#[derive(Clone, Debug)]
pub(crate) struct PackBuildCommand {
builder: String,
path: PathBuf,
image_name: String,
buildpacks: Vec<BuildpackReference>,
env: BTreeMap<String, String>,
image_name: String,
path: PathBuf,
pull_policy: PullPolicy,
verbose: bool,
}

Expand Down Expand Up @@ -38,18 +39,32 @@ impl From<String> for BuildpackReference {
}
}

#[derive(Clone, Debug)]
/// Controls whether Pack should pull images.
#[allow(dead_code)]
pub(crate) enum PullPolicy {
/// Always pull images.
Always,
/// Use local images if they are already present, rather than pulling updated images.
IfNotPresent,
/// Never pull images. If the required images are not already available locally the pack command will fail.
Never,
}

impl PackBuildCommand {
pub fn new(
builder: impl Into<String>,
path: impl Into<PathBuf>,
image_name: impl Into<String>,
pull_policy: PullPolicy,
) -> PackBuildCommand {
PackBuildCommand {
builder: builder.into(),
path: path.into(),
image_name: image_name.into(),
buildpacks: vec![],
buildpacks: Vec::new(),
env: BTreeMap::new(),
image_name: image_name.into(),
path: path.into(),
pull_policy,
verbose: false,
}
}
Expand All @@ -76,6 +91,12 @@ impl From<PackBuildCommand> for Command {
pack_build_command.builder,
String::from("--path"),
pack_build_command.path.to_string_lossy().to_string(),
String::from("--pull-policy"),
match pack_build_command.pull_policy {
PullPolicy::Always => "always".to_string(),
PullPolicy::IfNotPresent => "if-not-present".to_string(),
PullPolicy::Never => "never".to_string(),
},
];

for buildpack in pack_build_command.buildpacks {
Expand Down Expand Up @@ -115,8 +136,6 @@ mod tests {
fn from_pack_build_command_to_command() {
let mut input = PackBuildCommand {
builder: String::from("builder:20"),
path: PathBuf::from("/tmp/foo/bar"),
image_name: String::from("my-image"),
buildpacks: vec![
BuildpackReference::Id(String::from("libcnb/buildpack1")),
BuildpackReference::Path(PathBuf::from("/tmp/buildpack2")),
Expand All @@ -125,6 +144,9 @@ mod tests {
(String::from("ENV_FOO"), String::from("FOO_VALUE")),
(String::from("ENV_BAR"), String::from("WHITESPACE VALUE")),
]),
image_name: String::from("my-image"),
path: PathBuf::from("/tmp/foo/bar"),
pull_policy: PullPolicy::IfNotPresent,
verbose: true,
};

Expand All @@ -141,6 +163,8 @@ mod tests {
"builder:20",
"--path",
"/tmp/foo/bar",
"--pull-policy",
"if-not-present",
"--buildpack",
"libcnb/buildpack1",
"--buildpack",
Expand Down

0 comments on commit 95feb4c

Please sign in to comment.