Record unusable image manifests, rather than fail

registry/cache/cache.go

@@ -47,17 +47,6 @@ type tagKey struct {
 	fullRepositoryPath string
 }
 
-func NewTagKey(id image.CanonicalName) Keyer {
-	return &tagKey{id.String()}
-}
-
-func (k *tagKey) Key() string {
-	return strings.Join([]string{
-		"registrytagsv3", // Just to version in case we need to change format later.
-		k.fullRepositoryPath,
-	}, "|")
-}
-
 type repoKey struct {
 	fullRepositoryPath string
 }

registry/cache/registry.go

@@ -9,6 +9,7 @@ import (
 
 	fluxerr "github.com/weaveworks/flux/errors"
 	"github.com/weaveworks/flux/image"
+	"github.com/weaveworks/flux/registry"
 )
 
 var (
@@ -72,12 +73,15 @@ func (c *Cache) GetImage(id image.Ref) (image.Info, error) {
 	if err != nil {
 		return image.Info{}, err
 	}
-	var img image.Info
+	var img registry.ImageEntry
 	err = json.Unmarshal(val, &img)
 	if err != nil {
 		return image.Info{}, err
 	}
-	return img, nil
+	if img.ExcludedReason != "" {
+		return image.Info{}, errors.New(img.ExcludedReason)
+	}
+	return img.Info, nil
 }
 
 // ImageRepository holds the last good information on an image

registry/cache/warming.go

@@ -77,7 +77,7 @@ func (w *Warmer) Loop(logger log.Logger, stop <-chan struct{}, wg *sync.WaitGrou
 	// If there are none, images used in the cluster are refreshed;
 	// but no more often than once every `askForNewImagesInterval`,
 	// since there is no effective back-pressure on cache refreshes
-	// and it would spin freely otherwise).
+	// and it would spin freely otherwise.
 	for {
 		select {
 		case <-stop:
@@ -230,6 +230,9 @@ func (w *Warmer) warm(ctx context.Context, logger log.Logger, id image.Name, cre
 					errorLogger.Log("err", errors.Wrap(err, "requesting manifests"))
 					return
 				}
+				if img.ExcludedReason != "" {
+					errorLogger.Log("excluded", img.ExcludedReason)
+				}
 
 				key := NewManifestKey(img.ID.CanonicalRef())
 				// Write back to memcached
@@ -245,7 +248,9 @@ func (w *Warmer) warm(ctx context.Context, logger log.Logger, id image.Name, cre
 				}
 				successMx.Lock()
 				successCount++
-				newImages[imageID.Tag] = img
+				if img.ExcludedReason == "" {
+					newImages[imageID.Tag] = img.Info
+				}
 				successMx.Unlock()
 			}(imID)
 		}

registry/cache/warming_test.go

@@ -55,13 +55,15 @@ func TestWarm(t *testing.T) {
 		TagsFn: func() ([]string, error) {
 			return []string{"tag"}, nil
 		},
-		ManifestFn: func(tag string) (image.Info, error) {
+		ManifestFn: func(tag string) (registry.ImageEntry, error) {
 			if tag != "tag" {
 				t.Errorf("remote client was asked for %q instead of %q", tag, "tag")
 			}
-			return image.Info{
-				ID:        ref,
-				CreatedAt: time.Now(),
+			return registry.ImageEntry{
+				Info: image.Info{
+					ID:        ref,
+					CreatedAt: time.Now(),
+				},
 			}, nil
 		},
 	}

registry/client.go

@@ -18,13 +18,18 @@ import (
 	"github.com/weaveworks/flux/image"
 )
 
+type ImageEntry struct {
+	image.Info     `json:",inline"`
+	ExcludedReason string `json:",omitempty"`
+}
+
 // Client is a remote registry client for a particular image
 // repository (e.g., for quay.io/weaveworks/flux). It is an interface
 // so we can wrap it in instrumentation, write fake implementations,
 // and so on.
 type Client interface {
 	Tags(context.Context) ([]string, error)
-	Manifest(ctx context.Context, ref string) (image.Info, error)
+	Manifest(ctx context.Context, ref string) (ImageEntry, error)
 }
 
 // ClientFactory supplies Client implementations for a given repo,
@@ -64,22 +69,22 @@ func (a *Remote) Tags(ctx context.Context) ([]string, error) {
 
 // Manifest fetches the metadata for an image reference; currently
 // assumed to be in the same repo as that provided to `NewRemote(...)`
-func (a *Remote) Manifest(ctx context.Context, ref string) (image.Info, error) {
+func (a *Remote) Manifest(ctx context.Context, ref string) (ImageEntry, error) {
 	repository, err := client.NewRepository(named{a.repo}, a.base, a.transport)
 	if err != nil {
-		return image.Info{}, err
+		return ImageEntry{}, err
 	}
 	manifests, err := repository.Manifests(ctx)
 	if err != nil {
-		return image.Info{}, err
+		return ImageEntry{}, err
 	}
 	var manifestDigest digest.Digest
 	digestOpt := client.ReturnContentDigest(&manifestDigest)
 	manifest, fetchErr := manifests.Get(ctx, digest.Digest(ref), digestOpt, distribution.WithTagOption{ref})
 
 interpret:
 	if fetchErr != nil {
-		return image.Info{}, fetchErr
+		return ImageEntry{}, fetchErr
 	}
 
 	info := image.Info{ID: a.repo.ToRef(ref), Digest: manifestDigest.String()}
@@ -98,7 +103,7 @@ interpret:
 		}
 
 		if err = json.Unmarshal([]byte(man.History[0].V1Compatibility), &v1); err != nil {
-			return image.Info{}, err
+			return ImageEntry{}, err
 		}
 		// This is not the ImageID that Docker uses, but assumed to
 		// identify the image as it's the topmost layer.
@@ -108,7 +113,7 @@ interpret:
 		var man schema2.Manifest = deserialised.Manifest
 		configBytes, err := repository.Blobs(ctx).Get(ctx, man.Config.Digest)
 		if err != nil {
-			return image.Info{}, err
+			return ImageEntry{}, err
 		}
 
 		var config struct {
@@ -117,7 +122,7 @@ interpret:
 			OS      string    `json:"os"`
 		}
 		if err = json.Unmarshal(configBytes, &config); err != nil {
-			return image.Info{}, err
+			return ImageEntry{}, err
 		}
 		// This _is_ what Docker uses as its Image ID.
 		info.ImageID = man.Config.Digest.String()
@@ -131,10 +136,10 @@ interpret:
 				goto interpret
 			}
 		}
-		return image.Info{}, errors.New("no suitable manifest (linux amd64) in manifestlist")
+		return ImageEntry{ExcludedReason: "no suitable manifest (linux amd64) in manifestlist"}, nil
 	default:
 		t := reflect.TypeOf(manifest)
-		return image.Info{}, errors.New("unknown manifest type: " + t.String())
+		return ImageEntry{}, errors.New("unknown manifest type: " + t.String())
 	}
-	return info, nil
+	return ImageEntry{Info: info}, nil
 }

registry/mock/mock.go

@@ -10,11 +10,11 @@ import (
 )
 
 type Client struct {
-	ManifestFn func(ref string) (image.Info, error)
+	ManifestFn func(ref string) (registry.ImageEntry, error)
 	TagsFn     func() ([]string, error)
 }
 
-func (m *Client) Manifest(ctx context.Context, tag string) (image.Info, error) {
+func (m *Client) Manifest(ctx context.Context, tag string) (registry.ImageEntry, error) {
 	return m.ManifestFn(tag)
 }
 

registry/monitoring.go

@@ -74,7 +74,7 @@ func NewInstrumentedClient(next Client) Client {
 	}
 }
 
-func (m *instrumentedClient) Manifest(ctx context.Context, ref string) (res image.Info, err error) {
+func (m *instrumentedClient) Manifest(ctx context.Context, ref string) (res ImageEntry, err error) {
 	start := time.Now()
 	res, err = m.next.Manifest(ctx, ref)
 	remoteDuration.With(