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

imagebuildah: wait for stages that might not have even started yet 

When checking if a name specified for a FROM instruction, or a source
for a COPY instruction, was a stage for which it needed to wait,
StageExecutor was checking the Executor's stages list, to which stages
are added only as they're started.  This could cause it to mistakenly
write off a name if the corresponding stage hadn't yet been started, and
assume that the named stage was actually an image.

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
This commit is contained in:
Nalin Dahyabhai 2020-07-14 17:20:39 -04:00
parent 215536fb4b
commit ef24bbbbe7
2 changed files with 40 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
imagebuildah/executor.go
View File

@ -251,8 +251,9 @@ func NewExecutor(store storage.Store, options BuildOptions, mainNode *parser.Nod
// startStage creates a new stage executor that will be referenced whenever a
// COPY or ADD statement uses a --from=NAME flag.
func (b *Executor) startStage(stage *imagebuilder.Stage, stages imagebuilder.Stages, output string) *StageExecutor {
func (b *Executor) startStage(ctx context.Context, stage *imagebuilder.Stage, stages imagebuilder.Stages, output string) *StageExecutor {
stageExec := &StageExecutor{
ctx: ctx,
executor: b,
index: stage.Position,
stages: stages,
@ -289,17 +290,24 @@ func (b *Executor) resolveNameToImageRef(output string) (types.ImageReference, e
return imageRef, nil
}
func (b *Executor) waitForStage(ctx context.Context, name string) error {
stage := b.stages[name]
if stage == nil {
return errors.Errorf("unknown stage %q", name)
// waitForStage waits for an entry to be added to terminatedStage indicating
// that the specified stage has finished. If there is no stage defined by that
// name, then it will return (false, nil). If there is a stage defined by that
// name, it will return true along with any error it encounters.
func (b *Executor) waitForStage(ctx context.Context, name string, stages imagebuilder.Stages) (bool, error) {
found := false
for _, otherStage := range stages {
if otherStage.Name == name || fmt.Sprintf("%d", otherStage.Position) == name {
found = true
break
}
}
if !found {
return false, nil
}
for {
if b.lastError != nil {
return b.lastError
}
if stage.stage == nil {
return nil
return true, b.lastError
}
b.stagesLock.Lock()
@ -307,13 +315,13 @@ func (b *Executor) waitForStage(ctx context.Context, name string) error {
b.stagesLock.Unlock()
if terminated {
return nil
return true, nil
}
b.stagesSemaphore.Release(1)
time.Sleep(time.Millisecond * 10)
if err := b.stagesSemaphore.Acquire(ctx, 1); err != nil {
return errors.Wrapf(err, "error reacquiring job semaphore")
return true, errors.Wrapf(err, "error reacquiring job semaphore")
}
}
}
@ -355,7 +363,7 @@ func (b *Executor) buildStage(ctx context.Context, cleanupStages map[int]*StageE
}
b.stagesLock.Lock()
stageExecutor := b.startStage(&stage, stages, output)
stageExecutor := b.startStage(ctx, &stage, stages, output)
b.stagesLock.Unlock()
// If this a single-layer build, or if it's a multi-layered
@ -559,6 +567,7 @@ func (b *Executor) Build(ctx context.Context, stages imagebuilder.Stages) (image
b.stagesLock.Lock()
b.terminatedStage[stage.Name] = struct{}{}
b.terminatedStage[fmt.Sprintf("%d", stage.Position)] = struct{}{}
b.stagesLock.Unlock()
if r.Error != nil {

32
imagebuildah/stage_executor.go
View File

@ -44,6 +44,7 @@ import (
// If we're naming the result of the build, only the last stage will apply that
// name to the image that it produces.
type StageExecutor struct {
ctx context.Context
executor *Executor
index int
stages imagebuilder.Stages
@ -262,7 +263,7 @@ func (s *StageExecutor) volumeCacheRestore() error {
// don't care about the details of where in the filesystem the content actually
// goes, because we're not actually going to add it here, so this is less
// involved than Copy().
func (s *StageExecutor) digestSpecifiedContent(node *parser.Node, argValues []string, envValues []string) (string, error) {
func (s *StageExecutor) digestSpecifiedContent(ctx context.Context, node *parser.Node, argValues []string, envValues []string) (string, error) {
// No instruction: done.
if node == nil {
return "", nil
@ -295,6 +296,9 @@ func (s *StageExecutor) digestSpecifiedContent(node *parser.Node, argValues []st
// container. Update the ID mappings and
// all-content-comes-from-below-this-directory value.
from := strings.TrimPrefix(flag, "--from=")
if isStage, err := s.executor.waitForStage(ctx, from, s.stages[:s.index]); isStage && err != nil {
return "", err
}
if other, ok := s.executor.stages[from]; ok && other.index < s.index {
contextDir = other.mountPoint
idMappingOptions = &other.builder.IDMappingOptions
@ -422,6 +426,9 @@ func (s *StageExecutor) Copy(excludes []string, copies ...imagebuilder.Copy) err
var copyExcludes []string
contextDir := s.executor.contextDir
if len(copy.From) > 0 {
if isStage, err := s.executor.waitForStage(s.ctx, copy.From, s.stages[:s.index]); isStage && err != nil {
return err
}
if other, ok := s.executor.stages[copy.From]; ok && other.index < s.index {
contextDir = other.mountPoint
idMappingOptions = &other.builder.IDMappingOptions
@ -763,12 +770,8 @@ func (s *StageExecutor) Execute(ctx context.Context, base string) (imgID string,
// substitute that image's ID for the base image's name here. If not,
// then go on assuming that it's just a regular image that's either in
// local storage, or one that we have to pull from a registry.
for _, previousStage := range s.stages[:s.index] {
if previousStage.Name == base {
if err := s.executor.waitForStage(ctx, previousStage.Name); err != nil {
return "", nil, err
}
}
if isStage, err := s.executor.waitForStage(ctx, base, s.stages[:s.index]); isStage && err != nil {
return "", nil, err
}
if stageImage, isPreviousStage := s.executor.imageMap[base]; isPreviousStage {
base = stageImage
@ -876,10 +879,13 @@ func (s *StageExecutor) Execute(ctx context.Context, base string) (imgID string,
if len(arr) != 2 {
return "", nil, errors.Errorf("%s: invalid --from flag, should be --from=<name|stage>", command)
}
// If the source's name corresponds to the
// result of an earlier stage, wait for that
// stage to finish being built.
if isStage, err := s.executor.waitForStage(ctx, arr[1], s.stages[:s.index]); isStage && err != nil {
return "", nil, err
}
if otherStage, ok := s.executor.stages[arr[1]]; ok && otherStage.index < s.index {
if err := s.executor.waitForStage(ctx, otherStage.name); err != nil {
return "", nil, err
}
mountPoint = otherStage.mountPoint
} else if mountPoint, err = s.getImageRootfs(ctx, arr[1]); err != nil {
return "", nil, errors.Errorf("%s --from=%s: no stage or image found with that name", command, arr[1])
@ -907,7 +913,7 @@ func (s *StageExecutor) Execute(ctx context.Context, base string) (imgID string,
return "", nil, errors.Wrapf(err, "error building at STEP \"%s\"", step.Message)
}
// In case we added content, retrieve its digest.
addedContentDigest, err := s.digestSpecifiedContent(node, ib.Arguments(), ib.Config().Env)
addedContentDigest, err := s.digestSpecifiedContent(ctx, node, ib.Arguments(), ib.Config().Env)
if err != nil {
return "", nil, err
}
@ -956,7 +962,7 @@ func (s *StageExecutor) Execute(ctx context.Context, base string) (imgID string,
// cached images so far, look for one that matches what we
// expect to produce for this instruction.
if checkForLayers && !(s.executor.squash && lastInstruction && lastStage) {
addedContentDigest, err := s.digestSpecifiedContent(node, ib.Arguments(), ib.Config().Env)
addedContentDigest, err := s.digestSpecifiedContent(ctx, node, ib.Arguments(), ib.Config().Env)
if err != nil {
return "", nil, err
}
@ -1014,7 +1020,7 @@ func (s *StageExecutor) Execute(ctx context.Context, base string) (imgID string,
return "", nil, errors.Wrapf(err, "error building at STEP \"%s\"", step.Message)
}
// In case we added content, retrieve its digest.
addedContentDigest, err := s.digestSpecifiedContent(node, ib.Arguments(), ib.Config().Env)
addedContentDigest, err := s.digestSpecifiedContent(ctx, node, ib.Arguments(), ib.Config().Env)
if err != nil {
return "", nil, err
}