From e7b846e54f3f755a18418b09a1322c46efdcb5d0 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Tue, 14 Nov 2023 14:57:25 -0500 Subject: [PATCH 1/3] Fix imports Signed-off-by: Natalie Arellano --- internal/build/extend_build_test.go | 11 +++-------- internal/build/helper.go | 1 - internal/build/lifecycle_execution.go | 9 ++++----- 3 files changed, 7 insertions(+), 14 deletions(-) diff --git a/internal/build/extend_build_test.go b/internal/build/extend_build_test.go index 391a50e2d7..709e90b5b7 100644 --- a/internal/build/extend_build_test.go +++ b/internal/build/extend_build_test.go @@ -4,7 +4,6 @@ import ( "bytes" "context" "io" - "path/filepath" "runtime" "strconv" @@ -12,22 +11,18 @@ import ( "testing" "github.com/apex/log" - "github.com/buildpacks/lifecycle/buildpack" - - "github.com/buildpacks/pack/internal/build" - "github.com/buildpacks/pack/internal/build/fakes" - "github.com/docker/docker/api/types" "github.com/golang/mock/gomock" "github.com/heroku/color" "github.com/sclevine/spec" "github.com/sclevine/spec/report" + "github.com/buildpacks/pack/internal/build" + "github.com/buildpacks/pack/internal/build/fakes" + mockdocker "github.com/buildpacks/pack/internal/build/mockdocker" "github.com/buildpacks/pack/pkg/logging" h "github.com/buildpacks/pack/testhelpers" - - mockdocker "github.com/buildpacks/pack/internal/build/mockdocker" ) func TestBuildDockerfiles(t *testing.T) { diff --git a/internal/build/helper.go b/internal/build/helper.go index 73e3f8957b..ab2986f919 100644 --- a/internal/build/helper.go +++ b/internal/build/helper.go @@ -14,7 +14,6 @@ import ( "github.com/buildpacks/lifecycle/cmd" "github.com/buildpacks/pack/pkg/archive" - "github.com/buildpacks/pack/pkg/logging" ) diff --git a/internal/build/lifecycle_execution.go b/internal/build/lifecycle_execution.go index 40340fb251..a1aa4b2c15 100644 --- a/internal/build/lifecycle_execution.go +++ b/internal/build/lifecycle_execution.go @@ -10,7 +10,6 @@ import ( "strconv" "github.com/BurntSushi/toml" - "github.com/buildpacks/lifecycle/api" "github.com/buildpacks/lifecycle/auth" "github.com/buildpacks/lifecycle/platform/files" @@ -794,10 +793,10 @@ func (l *LifecycleExecution) ExtendBuild(ctx context.Context, kanikoCache Cache, return extend.Run(ctx) } -/* - Note: - Run Image Extension by docker daemon was much worse than kaniko because of saving layers on disk. -*/ - +// ExtendRun extends the run image by invoking the `extender` in the context of the run image +// to apply the provided Dockerfiles using kaniko - unlike extending the build image, which uses `docker build`. +// Extending the run image with `docker build` degrades performance +// due to the extra time spent exporting the extended layers from the daemon. func (l *LifecycleExecution) ExtendRun(ctx context.Context, kanikoCache Cache, phaseFactory PhaseFactory) error { flags := []string{"-app", l.mountPaths.appDir(), "-kind", "run"} From ca6aa1f53957b558eec3dcc75363b90e3316a89b Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Tue, 14 Nov 2023 15:02:59 -0500 Subject: [PATCH 2/3] When running the restorer, don't provide -build-image arg or /kaniko mount These are only needed when extending the build image with kaniko. Eliminating the -build-image input removes the requirement that the build image must exist in a registry. Signed-off-by: Natalie Arellano --- internal/build/lifecycle_execution.go | 9 ++------- internal/build/lifecycle_execution_test.go | 18 ------------------ 2 files changed, 2 insertions(+), 25 deletions(-) diff --git a/internal/build/lifecycle_execution.go b/internal/build/lifecycle_execution.go index a1aa4b2c15..7ecd6eef6d 100644 --- a/internal/build/lifecycle_execution.go +++ b/internal/build/lifecycle_execution.go @@ -479,16 +479,11 @@ func (l *LifecycleExecution) Restore(ctx context.Context, buildCache Cache, kani // for kaniko kanikoCacheBindOp := NullOp() - if (l.platformAPI.AtLeast("0.10") && l.hasExtensionsForBuild()) || - l.platformAPI.AtLeast("0.12") { - if l.hasExtensionsForBuild() { - flags = append(flags, "-build-image", l.opts.BuilderImage) - registryImages = append(registryImages, l.opts.BuilderImage) - } + if l.platformAPI.AtLeast("0.12") { if l.runImageChanged() || l.hasExtensionsForRun() { registryImages = append(registryImages, l.runImageAfterExtensions()) } - if l.hasExtensionsForBuild() || l.hasExtensionsForRun() { + if l.hasExtensionsForRun() { kanikoCacheBindOp = WithBinds(fmt.Sprintf("%s:%s", kanikoCache.Name(), l.mountPaths.kanikoCacheDir())) } } diff --git a/internal/build/lifecycle_execution_test.go b/internal/build/lifecycle_execution_test.go index 7dbd2c90fb..7591f8b493 100644 --- a/internal/build/lifecycle_execution_test.go +++ b/internal/build/lifecycle_execution_test.go @@ -1707,14 +1707,6 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) { platformAPI = api.MustParse("0.12") providedOrderExt = dist.Order{dist.OrderEntry{Group: []dist.ModuleRef{ /* don't care */ }}} - when("for build", func() { - extensionsForBuild = true - - it("configures the phase with registry access", func() { - h.AssertSliceContains(t, configProvider.ContainerConfig().Env, "CNB_REGISTRY_AUTH={}") - }) - }) - when("for run", func() { extensionsForRun = true @@ -1789,16 +1781,6 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) { h.AssertSliceNotContains(t, configProvider.HostConfig().Binds, "some-kaniko-cache:/kaniko") }) }) - - when("platform >= 0.10", func() { - platformAPI = api.MustParse("0.10") - - it("provides -build-image and /kaniko bind", func() { - h.AssertSliceContainsInOrder(t, configProvider.ContainerConfig().Cmd, "-build-image", providedBuilderImage) - h.AssertSliceContains(t, configProvider.ContainerConfig().Env, "CNB_REGISTRY_AUTH={}") - h.AssertSliceContains(t, configProvider.HostConfig().Binds, "some-kaniko-cache:/kaniko") - }) - }) }) when("not present in /generated/build", func() { From d887335211110f72fd7ec8cfadb698d0c06fd2f4 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Wed, 15 Nov 2023 10:41:30 -0500 Subject: [PATCH 3/3] Fix docs comments Signed-off-by: Natalie Arellano --- internal/build/lifecycle_execution.go | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/internal/build/lifecycle_execution.go b/internal/build/lifecycle_execution.go index 7ecd6eef6d..9cac3ccfab 100644 --- a/internal/build/lifecycle_execution.go +++ b/internal/build/lifecycle_execution.go @@ -706,16 +706,12 @@ const ( argGroupID = "group_id" ) -/* - This implementation of ExtendBuildByDaemon is based on the RFC #0105 which uses docker daemon to extend the build Image instead of kaniko. - * Parsing the `group.toml` from the temp directory of buildpack and set the extensions. - * Reading the dockerfiles that were generated during the `generate` phase and also parsing the Arguments given by the user from - `extend-config.toml`. - * Using ImageBuild method of docker API client to extend the Image and save it to the daemon. - * Invoking Build phase of lifecycle by creating a container from the extended Image and dropping the privileges. - -*/ - +// ExtendBuildByDaemon uses a daemon to extend the build image, instead of kaniko via the lifecycle `extender`. It does this by: +// - Parsing the detected `group.toml` from a temp directory to find the detected extensions. +// - Reading the Dockerfiles that were generated during the `generate` phase and also parsing the arguments +// given by the extension in `extend-config.toml`. +// - Using the `ImageBuild` method of the docker API client to extend the image and save it to the daemon. +// - Invoking the `build` phase of lifecycle by creating a container from the extended image. func (l *LifecycleExecution) ExtendBuildByDaemon(ctx context.Context) error { builderImageName := l.opts.BuilderImage extendedBuilderImageName := l.opts.BuilderImage + "-extended" @@ -763,10 +759,7 @@ func (l *LifecycleExecution) ExtendBuildByDaemon(ctx context.Context) error { return nil } -/* - Deprecated: Check RFC #0105 for the new implementation of ExtendBuild using docker daemon #1623. -*/ - +// Deprecated: ExtendBuild uses kaniko to extend the build image, via the lifecycle `extender`. func (l *LifecycleExecution) ExtendBuild(ctx context.Context, kanikoCache Cache, phaseFactory PhaseFactory) error { flags := []string{"-app", l.mountPaths.appDir()}