Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stage_executor: re-use all the layers from cache for squashed case and commit as late as possible. #3674

Merged
merged 1 commit into from
Dec 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 25 additions & 1 deletion imagebuildah/stage_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
9 changes: 9 additions & 0 deletions tests/bud/layers-squash/Dockerfile.multi-stage
Original file line number Diff line number Diff line change
@@ -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
15 changes: 15 additions & 0 deletions tests/squash.bats
Original file line number Diff line number Diff line change
Expand Up @@ -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
}