Delegate checking of remote image existence to the daemon #1145
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
the
type/enhancement
aemengo
force-pushed
the
fix-unable-to-pull-insecure-registry
branch
2 times, most recently
from
f7628bf to
0c911b7
Compare
|
@aemengo this looks like a great fix. Any progress on getting the tests to pass? |
jromero
changed the title
aemengo
force-pushed
the
fix-unable-to-pull-insecure-registry
branch
2 times, most recently
from
f62f64e to
6313d36
Compare
github-actions
bot
added
the
type/chore
When Docker Daemon is available, defer to Docker Daemon when pulling remote images. The motivation is that the Docker Daemon already handles more edge cases that we do. Signed-off-by: Anthony Emengo <aemengo@vmware.com>
aemengo
force-pushed
the
fix-unable-to-pull-insecure-registry
branch
from
6313d36 to
cf1156c
Compare
github-actions
bot
removed
the
type/chore
Codecov Report
@@ Coverage Diff @@
## main #1145 +/- ##
==========================================
+ Coverage 79.74% 80.86% +1.13%
==========================================
Files 136 136
Lines 8294 8296 +2
==========================================
+ Hits 6613 6708 +95
+ Misses 1254 1160 -94
- Partials 427 428 +1
Flags with carried forward coverage won't be shown. Click here to find out more. |
aemengo
force-pushed
the
fix-unable-to-pull-insecure-registry
branch
from
cf1156c to
0423545
Compare
|
@jromero @natalieparellano I'm really sorry for the delay on this. The PR should now be ready to look at. |
dfreilich
approved these changes
May 19, 2021
Comment on lines
+10
to
14
| "github.com/buildpacks/imgutil/remote" | ||
|
|
||
| "github.com/buildpacks/imgutil" | ||
| "github.com/buildpacks/imgutil/local" | ||
| "github.com/buildpacks/imgutil/remote" | ||
| "github.com/buildpacks/lifecycle/auth" |
There was a problem hiding this comment.
As Javier mentioned #1187 (comment), there should be 3 import groups, I think.
Comment on lines
+48
to
+49
| img, err := f.fetchDaemonImage(name) | ||
| return img, err |
There was a problem hiding this comment.
Suggested change
| img, err := f.fetchDaemonImage(name) | |
| return img, err | |
| return f.fetchDaemonImage(name) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
When Docker Daemon is available, defer to Docker Daemon when pulling remote images.
The motivation is that the Docker Daemon already handles more edge cases that we do.
The biggest difference to the end user is the error message bubbled from the Daemon, in the case of
config.PullPolicy == AlwaysOutput
Before
After
Documentation
Related
Resolves #771