Skip to content

Commit

Permalink
Inherit arguments between stages
Browse files Browse the repository at this point in the history
Signed-off-by: Gregor Riepl <[email protected]>
  • Loading branch information
onitake committed Nov 23, 2024
1 parent 9983122 commit d2f617d
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 10 deletions.
21 changes: 15 additions & 6 deletions imagebuildah/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ type Executor struct {
compatVolumes types.OptionalBool
compatScratchConfig types.OptionalBool
noPivotRoot bool
argResult map[string]map[string]string
}

type imageTypeAndHistoryAndDiffIDs struct {
Expand Down Expand Up @@ -324,6 +325,7 @@ func newExecutor(logger *logrus.Logger, logPrefix string, store storage.Store, o
compatVolumes: options.CompatVolumes,
compatScratchConfig: options.CompatScratchConfig,
noPivotRoot: options.NoPivotRoot,
argResult: make(map[string]map[string]string),
}
if exec.err == nil {
exec.err = os.Stderr
Expand Down Expand Up @@ -413,7 +415,7 @@ func (b *Executor) resolveNameToImageRef(output string) (types.ImageReference, e
// 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) {
func (b *Executor) waitForStage(ctx context.Context, name string, stages imagebuilder.Stages) (bool, map[string]string, error) {
found := false
for _, otherStage := range stages {
if otherStage.Name == name || strconv.Itoa(otherStage.Position) == name {
Expand All @@ -422,32 +424,39 @@ func (b *Executor) waitForStage(ctx context.Context, name string, stages imagebu
}
}
if !found {
return false, nil
return false, nil, nil
}
for {
if b.lastError != nil {
return true, b.lastError
return true, nil, b.lastError
}

b.stagesLock.Lock()
terminationError, terminated := b.terminatedStage[name]
args := b.argResult[name]
b.stagesLock.Unlock()

if terminationError != nil {
return false, terminationError
return false, nil, terminationError
}
if terminated {
return true, nil
return true, nil, nil
}

b.stagesSemaphore.Release(1)
time.Sleep(time.Millisecond * 10)
if err := b.stagesSemaphore.Acquire(ctx, 1); err != nil {
return true, fmt.Errorf("reacquiring job semaphore: %w", err)
return true, args, fmt.Errorf("reacquiring job semaphore: %w", err)
}
}
}

func (b *Executor) freezeArgs(name string, args map[string]string) {
b.stagesLock.Lock()
b.argResult[name] = args
b.stagesLock.Unlock()
}

// getImageTypeAndHistoryAndDiffIDs returns the manifest type, history, and diff IDs list of imageID.
func (b *Executor) getImageTypeAndHistoryAndDiffIDs(ctx context.Context, imageID string) (string, []v1.History, []digest.Digest, error) {
b.imageInfoLock.Lock()
Expand Down
16 changes: 12 additions & 4 deletions imagebuildah/stage_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"errors"
"fmt"
"io"
"maps"
"os"
"path"
"path/filepath"
Expand Down Expand Up @@ -512,7 +513,7 @@ func (s *StageExecutor) performCopy(excludes []string, copies ...imagebuilder.Co
}
}
if additionalBuildContext == nil {
if isStage, err := s.executor.waitForStage(s.ctx, from, s.stages[:s.index]); isStage && err != nil {
if isStage, _, err := s.executor.waitForStage(s.ctx, from, s.stages[:s.index]); isStage && err != nil {
return err
}
if other, ok := s.executor.stages[from]; ok && other.index < s.index {
Expand Down Expand Up @@ -677,7 +678,7 @@ func (s *StageExecutor) runStageMountPoints(mountList []string) (map[string]inte
// 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(s.ctx, from, s.stages[:s.index]); isStage && err != nil {
if isStage, _, err := s.executor.waitForStage(s.ctx, from, s.stages[:s.index]); isStage && err != nil {
return nil, err
}
// If the source's name is a stage, return a
Expand Down Expand Up @@ -1149,8 +1150,13 @@ func (s *StageExecutor) Execute(ctx context.Context, base string) (imgID string,
// 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, subject to the passed-in pull policy.
if isStage, err := s.executor.waitForStage(ctx, base, s.stages[:s.index]); isStage && err != nil {
// This helper will also return the final resolved argument list from
// the parent state.
if isStage, parentArgs, err := s.executor.waitForStage(ctx, base, s.stages[:s.index]); isStage && err != nil {
return "", nil, false, err
} else {
// Update the start args with those from the parent stage
maps.Copy(ib.Args, parentArgs)
}
pullPolicy := s.executor.pullPolicy
s.executor.stagesLock.Lock()
Expand Down Expand Up @@ -1369,7 +1375,7 @@ func (s *StageExecutor) Execute(ctx context.Context, base string) (imgID string,
// 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, from, s.stages[:s.index]); isStage && err != nil {
if isStage, _, err := s.executor.waitForStage(ctx, from, s.stages[:s.index]); isStage && err != nil {
return "", nil, false, err
}
if otherStage, ok := s.executor.stages[from]; ok && otherStage.index < s.index {
Expand Down Expand Up @@ -1742,6 +1748,8 @@ func (s *StageExecutor) Execute(ctx context.Context, base string) (imgID string,
}
}

s.executor.freezeArgs(s.name, ib.Args)

return imgID, ref, onlyBaseImage, nil
}

Expand Down

0 comments on commit d2f617d

Please sign in to comment.