stage_executor: re-use all possible layers from cache for squashed bu…

…ilds

Re-use all the layers from cache for squashed case and commit as late as possible

Current implementation of marking for re-use of cache `commits` on every
`stage` when used with `--squashed` however we should try to re-use as
many layers are possible if `--layers` is specified and `commit` only on
`last instruction` of `last stage` to perform final squash.

Also treat all other layers as if they are being written for regular
build and write them cache.

Signed-off-by: Aditya Rajan <arajan@redhat.com>

imagebuildah/stage_executor.go

@@ -974,7 +974,13 @@ func (s *StageExecutor) Execute(ctx context.Context, base string) (imgID string,
 			}
 		}
 
-		if cacheID != "" && !(s.executor.squash && lastInstruction) {
+		// We want to save history for other layers during a squashed build.
+		// Toggle flag allows executor to treat other instruction and layers
+		// as regular builds and only perform squashing at last
+		squashToggle := false
+		// Note: If the build has squash, we must try to re-use as many layers as possible if cache is found.
+		// So only perform commit if its the lastInstruction of lastStage.
+		if cacheID != "" {
 			logCacheHit(cacheID)
 			// A suitable cached image was found, so we can just
 			// reuse it.  If we need to add a name to the resulting
@@ -988,6 +994,13 @@ func (s *StageExecutor) Execute(ctx context.Context, base string) (imgID string,
 				}
 			}
 		} else {
+			if s.executor.squash {
+				// We want to save history for other layers during a squashed build.
+				// squashToggle flag allows executor to treat other instruction and layers
+				// as regular builds and only perform squashing at last
+				s.executor.squash = false
+				squashToggle = true
+			}
 			// We're not going to find any more cache hits, so we
 			// can stop looking for them.
 			checkForLayers = false
@@ -999,6 +1012,17 @@ func (s *StageExecutor) Execute(ctx context.Context, base string) (imgID string,
 				return "", nil, errors.Wrapf(err, "error committing container for step %+v", *step)
 			}
 		}
+
+		// Perform final squash for this build as we are one the,
+		// last instruction of last stage
+		if (s.executor.squash || squashToggle) && lastInstruction && lastStage {
+			s.executor.squash = true
+			imgID, ref, err = s.commit(ctx, s.getCreatedBy(node, addedContentSummary), !s.stepRequiresLayer(step), commitName)
+			if err != nil {
+				return "", nil, errors.Wrapf(err, "error committing final squash step %+v", *step)
+			}
+		}
+
 		logImageID(imgID)
 
 		// Update our working container to be based off of the cached

tests/bud/layers-squash/Dockerfile.multi-stage

@@ -0,0 +1,9 @@
+# Following stage must be picked from cache
+FROM busybox as one
+RUN echo hello
+RUN echo hello > world
+
+# Following stage must be picked from cache except last instruction
+FROM busybox as two
+RUN echo hello1
+RUN echo helloworld

tests/squash.bats

@@ -126,3 +126,18 @@ function check_lengths() {
 	run_buildah inspect -t image -f '{{.Docker.Parent}}' squashed
         expect_output "" "should have no parent image set"
 }
+
+
+@test "bud-squash-should-use-cache" {
+  _prefetch alpine
+  # populate cache from simple build
+  run_buildah build --layers -t test --signature-policy ${TESTSDIR}/policy.json -f ${TESTSDIR}/bud/layers-squash/Dockerfile.multi-stage
+  # create another squashed build and check if we are using cache for everything.
+  # instead of last instruction in last stage
+  run_buildah build --layers --squash -t testsquash --signature-policy ${TESTSDIR}/policy.json -f ${TESTSDIR}/bud/layers-squash/Dockerfile.multi-stage
+  expect_output --substring "Using cache"
+  run_buildah inspect -t image -f '{{len .Docker.RootFS.DiffIDs}}' testsquash
+  expect_output "1" "should only container 1 diff"
+  run_buildah rmi -f testsquash
+  run_buildah rmi -f test
+}