root
/
buildah
mirror of https://github.com/containers/buildah.git
1
0
Fork 0
Code Issues Actions 2 Packages Projects Releases Wiki Activity

stage_executor: while mounting stages use freshly built stage 

When using `--mount=` in RUN instruction and source is a stage make sure
that freshly built stage is used if the stage selected in source was
just rebuilt.

Closes: https://github.com/containers/buildah/issues/4522

Signed-off-by: Aditya R <arajan@redhat.com>
This commit is contained in:
Aditya R 2023-01-18 15:51:31 +05:30
parent c541c355c0
commit ac7458e70d
No known key found for this signature in database
GPG Key ID: 8E5A8A19DF7C8673
3 changed files with 84 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

49
imagebuildah/stage_executor.go
View File

@ -70,6 +70,7 @@ type StageExecutor struct {
output string
containerIDs []string
stage *imagebuilder.Stage
didExecute bool
argsFromContainerfile []string
}
@ -516,7 +517,7 @@ func (s *StageExecutor) runStageMountPoints(mountList []string) (map[string]inte
// to `mountPoint` replaced from additional
// build-context. Reason: Parser will use this
// `from` to refer from stageMountPoints map later.
stageMountPoints[from] = internal.StageMountDetails{IsStage: false, MountPoint: mountPoint}
stageMountPoints[from] = internal.StageMountDetails{IsStage: false, DidExecute: true, MountPoint: mountPoint}
break
} else {
// Most likely this points to path on filesystem
@ -548,7 +549,7 @@ func (s *StageExecutor) runStageMountPoints(mountList []string) (map[string]inte
mountPoint = additionalBuildContext.DownloadedCache
}
}
stageMountPoints[from] = internal.StageMountDetails{IsStage: true, MountPoint: mountPoint}
stageMountPoints[from] = internal.StageMountDetails{IsStage: true, DidExecute: true, MountPoint: mountPoint}
break
}
}
@ -559,14 +560,14 @@ func (s *StageExecutor) runStageMountPoints(mountList []string) (map[string]inte
return nil, err
}
if otherStage, ok := s.executor.stages[from]; ok && otherStage.index < s.index {
stageMountPoints[from] = internal.StageMountDetails{IsStage: true, MountPoint: otherStage.mountPoint}
stageMountPoints[from] = internal.StageMountDetails{IsStage: true, DidExecute: otherStage.didExecute, MountPoint: otherStage.mountPoint}
break
} else {
mountPoint, err := s.getImageRootfs(s.ctx, from)
if err != nil {
return nil, fmt.Errorf("%s from=%s: no stage or image found with that name", flag, from)
}
stageMountPoints[from] = internal.StageMountDetails{IsStage: false, MountPoint: mountPoint}
stageMountPoints[from] = internal.StageMountDetails{IsStage: false, DidExecute: true, MountPoint: mountPoint}
break
}
default:
@ -1148,6 +1149,7 @@ func (s *StageExecutor) Execute(ctx context.Context, base string) (imgID string,
// If we're doing a single-layer build, just process the
// instruction.
if !s.executor.layers {
s.didExecute = true
err := ib.Run(step, s, noRunsRemaining)
if err != nil {
logrus.Debugf("Error building at step %+v: %v", *step, err)
@ -1193,6 +1195,7 @@ func (s *StageExecutor) Execute(ctx context.Context, base string) (imgID string,
}
// We're in a multi-layered build.
s.didExecute = false
var (
commitName string
cacheID string
@ -1204,7 +1207,36 @@ func (s *StageExecutor) Execute(ctx context.Context, base string) (imgID string,
canMatchCacheOnlyAfterRun bool
)
needsCacheKey := (s.executor.cacheFrom != nil || s.executor.cacheTo != nil)
// Only attempt to find cache if its needed, this part is needed
// so that if a step is using RUN --mount and mounts content from
// previous stages then it uses the freshly built stage instead
// of re-using the older stage from the store.
avoidLookingCache := false
var mounts []string
for _, a := range node.Flags {
arg, err := imagebuilder.ProcessWord(a, s.stage.Builder.Arguments())
if err != nil {
return "", nil, err
}
switch {
case strings.HasPrefix(arg, "--mount="):
mount := strings.TrimPrefix(arg, "--mount=")
mounts = append(mounts, mount)
default:
continue
}
}
stageMountPoints, err := s.runStageMountPoints(mounts)
if err != nil {
return "", nil, err
}
for _, mountPoint := range stageMountPoints {
if mountPoint.DidExecute {
avoidLookingCache = true
}
}
needsCacheKey := (s.executor.cacheFrom != nil || s.executor.cacheTo != nil) && !avoidLookingCache
// If we have to commit for this instruction, only assign the
// stage's configured output name to the last layer.
@ -1228,13 +1260,14 @@ func (s *StageExecutor) Execute(ctx context.Context, base string) (imgID string,
// we need to call ib.Run() to correctly put the args together before
// determining if a cached layer with the same build args already exists
// and that is done in the if block below.
if checkForLayers && step.Command != "arg" && !(s.executor.squash && lastInstruction && lastStage) {
if checkForLayers && step.Command != "arg" && !(s.executor.squash && lastInstruction && lastStage) && !avoidLookingCache {
// For `COPY` and `ADD`, history entries include digests computed from
// the content that's copied in. We need to compute that information so that
// it can be used to evaluate the cache, which means we need to go ahead
// and copy the content.
canMatchCacheOnlyAfterRun = (step.Command == command.Add || step.Command == command.Copy)
if canMatchCacheOnlyAfterRun {
s.didExecute = true
if err = ib.Run(step, s, noRunsRemaining); err != nil {
logrus.Debugf("Error building at step %+v: %v", *step, err)
return "", nil, fmt.Errorf("building at STEP \"%s\": %w", step.Message, err)
@ -1281,6 +1314,7 @@ func (s *StageExecutor) Execute(ctx context.Context, base string) (imgID string,
// cases above, so we shouldn't do it again.
if cacheID == "" && !canMatchCacheOnlyAfterRun {
// Process the instruction directly.
s.didExecute = true
if err = ib.Run(step, s, noRunsRemaining); err != nil {
logrus.Debugf("Error building at step %+v: %v", *step, err)
return "", nil, fmt.Errorf("building at STEP \"%s\": %w", step.Message, err)
@ -1298,7 +1332,7 @@ func (s *StageExecutor) Execute(ctx context.Context, base string) (imgID string,
// Check if there's already an image based on our parent that
// has the same change that we just made.
if checkForLayers {
if checkForLayers && !avoidLookingCache {
cacheID, err = s.intermediateImageExists(ctx, node, addedContentSummary, s.stepRequiresLayer(step))
if err != nil {
return "", nil, fmt.Errorf("checking if cached image exists from a previous build: %w", err)
@ -1334,6 +1368,7 @@ func (s *StageExecutor) Execute(ctx context.Context, base string) (imgID string,
// still don't want to restart using the image's
// configuration blob.
if !s.stepRequiresLayer(step) {
s.didExecute = true
err := ib.Run(step, s, noRunsRemaining)
if err != nil {
logrus.Debugf("Error building at step %+v: %v", *step, err)

1
internal/types.go
View File

@ -12,6 +12,7 @@ const (
// StageExecutor has ability to mount stages/images in current context and
// automatically clean them up.
type StageMountDetails struct {
DidExecute bool // tells if the stage which is being mounted was freshly executed or was part of older cache
IsStage bool // tells if mountpoint returned from stage executor is stage or image
MountPoint string // mountpoint of stage/image
}

41
tests/bud.bats
View File

@ -550,6 +550,47 @@ _EOF
expect_output --substring "world"
}
@test "build-test do not use mount stage from cache if it was rebuilt" {
local contextdir=${TEST_SCRATCH_DIR}/bud/platform
mkdir -p $contextdir
cat > $contextdir/Dockerfile << _EOF
FROM alpine as dependencies
RUN mkdir /build && echo v1 > /build/version
FROM alpine
RUN --mount=type=bind,source=/build,target=/build,from=dependencies \
cp /build/version /version
RUN cat /version
_EOF
run_buildah build $WITH_POLICY_JSON --layers -t source -f $contextdir/Dockerfile
run_buildah build $WITH_POLICY_JSON --layers -t source2 -f $contextdir/Dockerfile
expect_output --substring "Using cache"
# First stage i.e dependencies is changed so it should not use the steps in second stage from
# cache
cat > $contextdir/Dockerfile << _EOF
FROM alpine as dependencies
RUN mkdir /build && echo v2 > /build/version
FROM alpine
RUN --mount=type=bind,source=/build,target=/build,from=dependencies \
cp /build/version /version
RUN cat /version
_EOF
run_buildah build $WITH_POLICY_JSON --layers -t source3 -f $contextdir/Dockerfile
assert "$output" !~ "Using cache"
}
@test "build-test skipping unwanted stages with --skip-unused-stages=false and --skip-unused-stages=true" {
local contextdir=${TEST_SCRATCH_DIR}/bud/platform
mkdir -p $contextdir