From 57e9ba44d3d0ce6706ecff21bbb0393ea619f68c Mon Sep 17 00:00:00 2001 From: Gregor Riepl Date: Fri, 22 Nov 2024 20:28:00 +0100 Subject: [PATCH] Inherit arguments between stages Signed-off-by: Gregor Riepl --- imagebuildah/executor.go | 21 +++++++++++++++------ imagebuildah/stage_executor.go | 16 ++++++++++++---- 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/imagebuildah/executor.go b/imagebuildah/executor.go index e3ee9fc4fa0..5012ee50bd5 100644 --- a/imagebuildah/executor.go +++ b/imagebuildah/executor.go @@ -164,6 +164,7 @@ type Executor struct { compatVolumes types.OptionalBool compatScratchConfig types.OptionalBool noPivotRoot bool + argResult map[string]map[string]string } type imageTypeAndHistoryAndDiffIDs struct { @@ -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 @@ -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 { @@ -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() diff --git a/imagebuildah/stage_executor.go b/imagebuildah/stage_executor.go index 3b1784e750c..19ec65cc7ea 100644 --- a/imagebuildah/stage_executor.go +++ b/imagebuildah/stage_executor.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "io" + "maps" "os" "path" "path/filepath" @@ -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 { @@ -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 @@ -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.Insert(ib.Args, maps.All(parentArgs)) } pullPolicy := s.executor.pullPolicy s.executor.stagesLock.Lock() @@ -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 { @@ -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 }