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>
(cherry picked from commit b53ea19c4989e5d2a5b90033de6fd55b423302a0)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Tonis Tiigi 4 years ago
parent
commit
da1a672102
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 {
 type puller struct {
 	is               *Source
 	is               *Source
 	resolveLocalOnce sync.Once
 	resolveLocalOnce sync.Once
+	g                flightcontrol.Group
 	src              *source.ImageIdentifier
 	src              *source.ImageIdentifier
 	desc             ocispec.Descriptor
 	desc             ocispec.Descriptor
 	ref              string
 	ref              string
@@ -253,9 +254,7 @@ func (p *puller) resolveLocal() {
 }
 }
 
 
 func (p *puller) resolve(ctx context.Context, g session.Group) error {
 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())
 		resolveProgressDone := oneOffProgress(ctx, "resolve "+p.src.Reference.String())
 		defer func() {
 		defer func() {
 			resolveProgressDone(err)
 			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
 		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()
 	k := cacheKeyFromConfig(p.config).String()
 	if k == "" {
 	if k == "" {
 		dgst, err := p.mainManifestKey(p.platform)
 		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) {
 func (p *puller) Snapshot(ctx context.Context, g session.Group) (cache.ImmutableRef, error) {
 	p.resolveLocal()
 	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 {
 	if p.config != nil {
@@ -801,6 +806,7 @@ func cacheKeyFromConfig(dt []byte) digest.Digest {
 	var img ocispec.Image
 	var img ocispec.Image
 	err := json.Unmarshal(dt, &img)
 	err := json.Unmarshal(dt, &img)
 	if err != nil {
 	if err != nil {
+		logrus.WithError(err).Errorf("failed to unmarshal image config for cache key %v", err)
 		return digest.FromBytes(dt)
 		return digest.FromBytes(dt)
 	}
 	}
 	if img.RootFS.Type != "layers" || len(img.RootFS.DiffIDs) == 0 {
 	if img.RootFS.Type != "layers" || len(img.RootFS.DiffIDs) == 0 {