feat: push final image to cache repo #197
Conversation
This commit enables pushing the final image to the given cache repo.a As part of #185, the goal is to allow for faster startup when the image has already been built.
| // Boilerplate! | ||
| CustomPlatform: platforms.Format(platforms.Normalize(platforms.DefaultSpec())), | ||
| SnapshotMode: "redo", | ||
| RunV2: true, | ||
| RunStdout: stdoutWriter, | ||
| RunStderr: stderrWriter, | ||
| Destinations: []string{"local"}, |
There was a problem hiding this comment.
This causes problems when DoPush is called. I'm not sure what the original intent was.
There was a problem hiding this comment.
Can you elaborate on what the problems are? Are you referring to #185 (comment)?
There was a problem hiding this comment.
I'm pretty sure this was simply a bug that was present because DoPush wasn't being used previously. Kaniko expects you to either set it or use --no-push.
error: failed to push to destination local: HEAD https://index.docker.io/v2/library/local/manifests/latest: unexpected status code 401 Unauthorized (HEAD responses have no body, use GET for details)
error: running command "envbuilder": HEAD https://index.docker.io/v2/library/local/manifests/latest: unexpected status code 401 Unauthorized (HEAD responses have no body, use GET for details)
failed to push to destination local
There was a problem hiding this comment.
Should we make it optional too? If platform engineers are concerned about envbuilder speed, they can --enable-do-push
There was a problem hiding this comment.
It depends, do we want to expose multiple knobs, one for caching and one for pushing final image/build optimization, or do we want these to be behind one knob? I'm open to splitting it up but could use some thought. Just because it's two different moments in Kaniko, doesn't mean we have to follow that here.
| // Boilerplate! | ||
| CustomPlatform: platforms.Format(platforms.Normalize(platforms.DefaultSpec())), | ||
| SnapshotMode: "redo", | ||
| RunV2: true, | ||
| RunStdout: stdoutWriter, | ||
| RunStderr: stderrWriter, | ||
| Destinations: []string{"local"}, |
There was a problem hiding this comment.
Can you elaborate on what the problems are? Are you referring to #185 (comment)?
mafredri
force-pushed
the
mafredri/envbuilder-push-image
branch
from
c044d2d
to
0765f69
Compare
| @@ -518,11 +523,16 @@ func Run(ctx context.Context, options Options) error { | |||
| RegistryMirrors: registryMirror, | |||
| }, | |||
| SrcContext: buildParams.BuildContext, | |||
| }) | |||
| } | |||
| image, err := executor.DoBuild(opts) | |||
There was a problem hiding this comment.
I'd like to see more debugging/trace logging around this step and DoPush.
There was a problem hiding this comment.
I'm not sure what the ask is here, I didn't change any of this code so seems out of scope?
There was a problem hiding this comment.
Didn't you add image, err := executor.DoBuild(opts)?
There was a problem hiding this comment.
No, I just moved opts to a variable.
There was a problem hiding this comment.
I missed L483. Anyway, I wanted to improve logging around this, but if you think this is useless here, I'm cool with leaving it as is.
| // Boilerplate! | ||
| CustomPlatform: platforms.Format(platforms.Normalize(platforms.DefaultSpec())), | ||
| SnapshotMode: "redo", | ||
| RunV2: true, | ||
| RunStdout: stdoutWriter, | ||
| RunStderr: stderrWriter, | ||
| Destinations: []string{"local"}, |
There was a problem hiding this comment.
Should we make it optional too? If platform engineers are concerned about envbuilder speed, they can --enable-do-push
Let's call this a blocker for merging this PR for now. It's a bit soon to decide how we want to expose this as new configuration options. I had anyway planned to hold off on a decision here until #186 is complete and we have a better picture of how we want to implement #128. |
|
@dannykopping @mtojek I think this one is orthogonal to #213 but I just want to make sure before closing. I have some tests added just in case. |
|
Closing this as a duplicate of #213 |
This commit enables pushing the final image to the given cache repo.a As
part of #185, the goal is to allow for faster startup when the image has
already been built.