Browse Source

builder: fix pull synchronization regression

Config resolution was synchronized based on a wrong key as ref
variable is initialized only after in the same function. Using
the right key isn't fully correct either as the synchronized method
changes properties of the puller instance and can't be just skipped.
Added better error handling for the same case as well.

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Tonis Tiigi 4 years ago
parent
commit
b53ea19c49
1 changed files with 11 additions and 5 deletions
  1. 11 5
      builder/builder-next/adapters/containerimage/pull.go

+ 11 - 5
builder/builder-next/adapters/containerimage/pull.go

@@ -183,6 +183,7 @@ func (is *Source) Resolve(ctx context.Context, id source.Identifier, sm *session
 type puller struct {
 	is               *Source
 	resolveLocalOnce sync.Once
+	g                flightcontrol.Group
 	src              *source.ImageIdentifier
 	desc             ocispec.Descriptor
 	ref              string
@@ -253,9 +254,7 @@ func (p *puller) resolveLocal() {
 }
 
 func (p *puller) resolve(ctx context.Context, g session.Group) error {
-	// key is used to synchronize resolutions that can happen in parallel when doing multi-stage.
-	key := "resolve::" + p.ref + "::" + platforms.Format(p.platform)
-	_, err := p.is.g.Do(ctx, key, func(ctx context.Context) (_ interface{}, err error) {
+	_, err := p.g.Do(ctx, "", func(ctx context.Context) (_ interface{}, err error) {
 		resolveProgressDone := oneOffProgress(ctx, "resolve "+p.src.Reference.String())
 		defer func() {
 			resolveProgressDone(err)
@@ -329,6 +328,10 @@ func (p *puller) CacheKey(ctx context.Context, g session.Group, index int) (stri
 		return dgst.String(), nil, false, nil
 	}
 
+	if len(p.config) == 0 {
+		return "", nil, false, errors.Errorf("invalid empty config file resolved for %s", p.src.Reference.String())
+	}
+
 	k := cacheKeyFromConfig(p.config).String()
 	if k == "" {
 		dgst, err := p.mainManifestKey(p.platform)
@@ -360,8 +363,10 @@ func (p *puller) getRef(ctx context.Context, diffIDs []layer.DiffID, opts ...cac
 
 func (p *puller) Snapshot(ctx context.Context, g session.Group) (cache.ImmutableRef, error) {
 	p.resolveLocal()
-	if err := p.resolve(ctx, g); err != nil {
-		return nil, err
+	if len(p.config) == 0 {
+		if err := p.resolve(ctx, g); err != nil {
+			return nil, err
+		}
 	}
 
 	if p.config != nil {
@@ -801,6 +806,7 @@ func cacheKeyFromConfig(dt []byte) digest.Digest {
 	var img ocispec.Image
 	err := json.Unmarshal(dt, &img)
 	if err != nil {
+		logrus.WithError(err).Errorf("failed to unmarshal image config for cache key %v", err)
 		return digest.FromBytes(dt)
 	}
 	if img.RootFS.Type != "layers" || len(img.RootFS.DiffIDs) == 0 {