Skip to content

Commit

Permalink
Fix a race we hit during conformance tests
Browse files Browse the repository at this point in the history
The imagebuilder.Executor.imageMap map is written in one goroutine which
waits on StageExecutors, and is read from the goroutines that run the
StageExecutors, so we need to synchronize access to the map.

Signed-off-by: Nalin Dahyabhai <[email protected]>
  • Loading branch information
nalind committed Jul 23, 2020
1 parent 187e697 commit aaeadb6
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 0 deletions.
2 changes: 2 additions & 0 deletions imagebuildah/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,9 @@ func (b *Executor) Build(ctx context.Context, stages imagebuilder.Stages) (image
// If this is an intermediate stage, make a note of the ID, so
// that we can look it up later.
if r.Index < len(stages)-1 && r.ImageID != "" {
b.stagesLock.Lock()
b.imageMap[stage.Name] = r.ImageID
b.stagesLock.Unlock()
// We're not populating the cache with intermediate
// images, so add this one to the list of images that
// we'll remove later.
Expand Down
4 changes: 4 additions & 0 deletions imagebuildah/stage_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -645,9 +645,11 @@ func (s *StageExecutor) prepare(ctx context.Context, from string, initializeIBCo

// Check and see if the image is a pseudonym for the end result of a
// previous stage, named by an AS clause in the Dockerfile.
s.executor.stagesLock.Lock()
if asImageFound, ok := s.executor.imageMap[from]; ok {
builderOptions.FromImage = asImageFound
}
s.executor.stagesLock.Unlock()
builder, err = buildah.NewBuilder(ctx, s.executor.store, builderOptions)
if err != nil {
return nil, errors.Wrapf(err, "error creating build container")
Expand Down Expand Up @@ -773,9 +775,11 @@ func (s *StageExecutor) Execute(ctx context.Context, base string) (imgID string,
if isStage, err := s.executor.waitForStage(ctx, base, s.stages[:s.index]); isStage && err != nil {
return "", nil, err
}
s.executor.stagesLock.Lock()
if stageImage, isPreviousStage := s.executor.imageMap[base]; isPreviousStage {
base = stageImage
}
s.executor.stagesLock.Unlock()

// Create the (first) working container for this stage. Reinitializing
// the imagebuilder configuration may alter the list of steps we have,
Expand Down

0 comments on commit aaeadb6

Please sign in to comment.