Skip to content

Commit

Permalink
Merge pull request #2127 from buildpacks/fix/run-image-pull
Browse files Browse the repository at this point in the history
Pull run image using digest reference in analyzed.toml (not image name from extensions)
  • Loading branch information
jjbustamante authored May 20, 2024
2 parents 884dd18 + 6405ff4 commit c1676bb
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 20 deletions.
7 changes: 3 additions & 4 deletions acceptance/acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -941,7 +941,7 @@ func testAcceptance(
if imageManager.HostOS() != "windows" {
// Linux containers (including Linux containers on Windows)
extSimpleLayersDiffID := "sha256:d24758b8b75b13292746fe7a06666f28a9499da31826a60afe6ee6b8cba29b73"
extReadEnvDiffID := "sha256:4490d78f2b056cdb99ad9cd3892f3c0617c5a485fb300dd90c572ce375ee45b2"
extReadEnvDiffID := "sha256:43072b16e96564a4dd6bd2e74c55c3c94af78cf99d869cab1e62c873e1fa6780"
bpSimpleLayersDiffID := "sha256:ade9da86859fa4ea50a513757f9b242bf1038667abf92dad3d018974a17f0ea7"
bpReadEnvDiffID := "sha256:db0797077ba8deff7054ab5578133b8f0206b6393de34b5bfd795cf50f6afdbd"
// extensions
Expand Down Expand Up @@ -1012,16 +1012,15 @@ func testAcceptance(
h.SkipIf(t, !lifecycle.SupportsFeature(config.RunImageExtensions), "")
})

it("uses the 5 phases, and tries to pull the new run image before restore", func() {
it("uses the 5 phases, and tries to pull the new run image by name before restore, and by identifier after restore", func() {
output, _ := pack.Run(
"build", repoName,
"-p", filepath.Join("testdata", "mock_app"),
"--network", "host",
"-B", builderName,
"--env", "EXT_RUN_SWITCH=1",
)
h.AssertContains(t, output, "ERROR: failed to build: executing lifecycle: resolve auth for ref some-not-exist-run-image!")
h.AssertNotContains(t, output, "RESTORING")
h.AssertContains(t, output, "Pulling image 'busybox:latest'")
})
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,6 @@ if [[ -z "$EXT_RUN_SWITCH" ]]; then
else
echo "Generating run.Dockerfile for run image switch..."
cat >>"${output_dir}/run.Dockerfile" <<EOL
FROM some-not-exist-run-image!
FROM busybox:latest
EOL
fi
46 changes: 36 additions & 10 deletions internal/build/lifecycle_execution.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,12 +244,9 @@ func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseF
ephemeralRunImage string
err error
)
currentRunImage := l.runImageAfterExtensions()
if l.runImageChanged() || l.hasExtensionsForRun() {
if currentRunImage == "" { // sanity check
return nil
}
if ephemeralRunImage, err = l.opts.FetchRunImageWithLifecycleLayer(currentRunImage); err != nil {
// Pull the run image by name in case we fail to pull it by identifier later.
if ephemeralRunImage, err = l.opts.FetchRunImageWithLifecycleLayer(l.runImageNameAfterExtensions()); err != nil {
return err
}
}
Expand All @@ -261,6 +258,16 @@ func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseF
return err
}

if l.runImageChanged() || l.hasExtensionsForRun() {
if newEphemeralRunImage, err := l.opts.FetchRunImageWithLifecycleLayer(l.runImageIdentifierAfterExtensions()); err == nil {
// If the run image was switched by extensions, the run image reference as written by the __restorer__ will be a digest reference
// that is pullable from a registry.
// However, if the run image is only extended (not switched), the run image reference as written by the __analyzer__ may be an image identifier
// (in the daemon case), and will not be pullable.
ephemeralRunImage = newEphemeralRunImage
}
}

group, _ := errgroup.WithContext(context.TODO())
if l.platformAPI.AtLeast("0.10") && l.hasExtensionsForBuild() {
group.Go(func() error {
Expand Down Expand Up @@ -494,7 +501,7 @@ func (l *LifecycleExecution) Restore(ctx context.Context, buildCache Cache, kani
registryImages = append(registryImages, l.opts.BuilderImage)
}
if l.runImageChanged() || l.hasExtensionsForRun() {
registryImages = append(registryImages, l.runImageAfterExtensions())
registryImages = append(registryImages, l.runImageNameAfterExtensions())
}
if l.hasExtensionsForBuild() || l.hasExtensionsForRun() {
kanikoCacheBindOp = WithBinds(fmt.Sprintf("%s:%s", kanikoCache.Name(), l.mountPaths.kanikoCacheDir()))
Expand Down Expand Up @@ -545,6 +552,8 @@ func (l *LifecycleExecution) Restore(ctx context.Context, buildCache Cache, kani
registryOp,
layoutOp,
layoutBindOp,
If(l.hasExtensions(), WithPostContainerRunOperations(
CopyOutToMaybe(filepath.Join(l.mountPaths.layersDir(), "analyzed.toml"), l.tmpDir))),
)

restore := phaseFactory.New(configProvider)
Expand Down Expand Up @@ -932,25 +941,42 @@ func (l *LifecycleExecution) hasExtensionsForRun() bool {
return amd.RunImage.Extend
}

func (l *LifecycleExecution) runImageAfterExtensions() string {
func (l *LifecycleExecution) runImageIdentifierAfterExtensions() string {
if !l.hasExtensions() {
return l.opts.RunImage
}
var amd files.Analyzed
if _, err := toml.DecodeFile(filepath.Join(l.tmpDir, "analyzed.toml"), &amd); err != nil {
l.logger.Warnf("failed to parse analyzed.toml file, assuming run image identifier did not change: %s", err)
return l.opts.RunImage
}
if amd.RunImage == nil || amd.RunImage.Reference == "" {
// this shouldn't be reachable
l.logger.Warnf("found no run image in analyzed.toml file, assuming run image identifier did not change...")
return l.opts.RunImage
}
return amd.RunImage.Reference
}

func (l *LifecycleExecution) runImageNameAfterExtensions() string {
if !l.hasExtensions() {
return l.opts.RunImage
}
var amd files.Analyzed
if _, err := toml.DecodeFile(filepath.Join(l.tmpDir, "analyzed.toml"), &amd); err != nil {
l.logger.Warnf("failed to parse analyzed.toml file, assuming run image did not change: %s", err)
l.logger.Warnf("failed to parse analyzed.toml file, assuming run image name did not change: %s", err)
return l.opts.RunImage
}
if amd.RunImage == nil || amd.RunImage.Image == "" {
// this shouldn't be reachable
l.logger.Warnf("found no run image in analyzed.toml file, assuming run image did not change...")
l.logger.Warnf("found no run image in analyzed.toml file, assuming run image name did not change...")
return l.opts.RunImage
}
return amd.RunImage.Image
}

func (l *LifecycleExecution) runImageChanged() bool {
currentRunImage := l.runImageAfterExtensions()
currentRunImage := l.runImageNameAfterExtensions()
return currentRunImage != "" && currentRunImage != l.opts.RunImage
}

Expand Down
16 changes: 11 additions & 5 deletions internal/build/lifecycle_execution_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) {
configProvider *build.PhaseConfigProvider

extensionsForBuild, extensionsForRun bool
extensionsRunImage string
extensionsRunImageName string
extensionsRunImageIdentifier string
useCreatorWithExtensions bool
)

Expand Down Expand Up @@ -153,8 +154,11 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) {
if extensionsForRun {
amd.RunImage.Extend = true
}
if extensionsRunImage != "" {
amd.RunImage.Image = extensionsRunImage
if extensionsRunImageName != "" {
amd.RunImage.Image = extensionsRunImageName
}
if extensionsRunImageIdentifier != "" {
amd.RunImage.Reference = extensionsRunImageIdentifier
}
f, err := os.Create(filepath.Join(tmpDir, "analyzed.toml"))
h.AssertNil(t, err)
Expand Down Expand Up @@ -689,15 +693,17 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) {
})

when("does not match provided run image", func() {
extensionsRunImage = "some-new-run-image"
extensionsRunImageName = "some-new-run-image"
extensionsRunImageIdentifier = "some-new-run-image-identifier"

it("pulls the new run image", func() {
err := lifecycle.Run(context.Background(), func(execution *build.LifecycleExecution) build.PhaseFactory {
return fakePhaseFactory
})
h.AssertNil(t, err)
h.AssertEq(t, fakeFetcher.callCount, 2)
h.AssertEq(t, fakeFetcher.calledWithArgAtCall[0], "some-new-run-image")
h.AssertEq(t, fakeFetcher.callCount, 1)
h.AssertEq(t, fakeFetcher.calledWithArgAtCall[1], "some-new-run-image-identifier")
})
})
})
Expand Down

0 comments on commit c1676bb

Please sign in to comment.