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

Defer to builder run image mirror if not publishing (daemon case) #783

Merged
merged 1 commit into from
Aug 11, 2020

Conversation

dfreilich
Copy link
Member

Signed-off-by: David Freilich dfreilich@vmware.com

Summary

This work attempts to make the selection of run-image-mirrors more intuitive. Previously, our selection of a run image mirror looked like this:

  1. run image provided with pack build --run-image
  2. run image mirror in the same registry as the app you are trying to build (so, for instance, pack build test-app will look for a run image mirror in dockerhub, while pack build gcr.io/test-app will look for a run image mirror in gcr.io)
  3. run image mirrors configured using pack set-run-image-mirror
  4. run image mirrors defined on the builder/stack

In practice, this was confusing to users (see #712, and a conversation on slack here). While the above priorities made sense when thinking about publishing images (if you are publishing, we want to minimize the number of bits you have to stream, so using a run image mirror in that registry would be optimal), they were suboptimal when users were doing local development, where users would have expected the run image to be in the same registry as the builder.

As such, this PR addresses changes priority (2) above, adjusting between selecting a run image mirror:

  • in the same registry as the app, if you are publishing (running pack build --publish, referred to as a non-daemon case)
  • in the same registry as the builder, if you are doing local development (running pack build, referred to as the daemon case)

Output

Before

$ pack build

pack build

$ pack build --publish

pack build --publish

After

$ pack build

pack build

$ pack build --publish

pack build --publish

Documentation

  • Should this change be documented?
    • Yes, in the release notes

Related

Resolves #712

@dfreilich dfreilich requested a review from a team as a code owner August 9, 2020 19:03
@dfreilich dfreilich added this to the 0.13.0 milestone Aug 9, 2020
* Refactor and add to Build tests

Signed-off-by: David Freilich <dfreilich@vmware.com>
@codecov
Copy link

codecov bot commented Aug 10, 2020

Codecov Report

Merging #783 into main will increase coverage by 0.09%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #783      +/-   ##
==========================================
+ Coverage   74.76%   74.85%   +0.09%     
==========================================
  Files          77       77              
  Lines        5174     5180       +6     
==========================================
+ Hits         3868     3877       +9     
+ Misses       1001      999       -2     
+ Partials      305      304       -1     
Flag Coverage Δ
#os_linux 77.33% <100.00%> (+0.08%) ⬆️
#os_macos 73.25% <100.00%> (+0.08%) ⬆️
#os_windows 73.13% <100.00%> (+0.09%) ⬆️
#unit 74.85% <100.00%> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@jromero jromero merged commit a94d2e6 into main Aug 11, 2020
@jromero jromero deleted the fix/712-default-run-images branch August 11, 2020 04:45
@jromero jromero added the type/enhancement Issue that requests a new feature or improvement. label Aug 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement Issue that requests a new feature or improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve UX of run-image-mirrors
2 participants