Browse Source

Be more tolertant towards missing image manifests

Before this change, we only accepted image repository metadata if a manifest was
present for all image tags (which is to be expected without inconsistencies).

However, enforcing fully consistent repository is a bit too strict, since:
1. Popular repositories may actually miss tag manifests (e.g. `bitnami/redis`),
   which has been broken for months.
2. Users may not be able to enforce such consistency.
3. The offending image tags can be ignored if filtered out by container
   image annotation patterns.

This change allows for the repository cache warmer to accept partial repository
information. Consistency is only enforced after tags are filtered by the
container image pattern annotations.

In particular, this change adds:
* A new storage format in the repository cache, splitting the information into
  tags and image metadata.
* More tolerance towards tags missing metadata when listing images (e.g. `fluxctl list-images`).
* Tolerance towards tags missing metadata during auto-releases, provided that
  those tags are not matched by container image pattern annotations.

It's worth noting that we can do better for `semver` patterns, since they don't
depend on image metadata for sorting (just on the tags themselves). `semver`
patterns could tolerate tags without metadata, even after filtering. This
however, is not provided by this change.
Alfonso Acosta 3 months ago
parent
commit
fd5f744b0c

+ 4
- 4
api/v6/container.go View File

@@ -27,8 +27,8 @@ type Container struct {
27 27
 }
28 28
 
29 29
 // NewContainer creates a Container given a list of images and the current image
30
-func NewContainer(name string, images update.ImageInfos, currentImage image.Info, tagPattern policy.Pattern, fields []string) (Container, error) {
31
-	sorted := images.Sort(tagPattern)
30
+func NewContainer(name string, images []image.Info, currentImage image.Info, tagPattern policy.Pattern, fields []string) (Container, error) {
31
+	sorted := update.SortImages(images, tagPattern)
32 32
 
33 33
 	// All images
34 34
 	imagesCount := len(sorted)
@@ -44,8 +44,8 @@ func NewContainer(name string, images update.ImageInfos, currentImage image.Info
44 44
 	}
45 45
 	newImagesCount := len(newImages)
46 46
 
47
-	// Filtered images
48
-	filteredImages := sorted.Filter(tagPattern)
47
+	// Filtered images (which respects sorting)
48
+	filteredImages := update.SortedImageInfos(update.FilterImages(sorted, tagPattern))
49 49
 	filteredImagesCount := len(filteredImages)
50 50
 	var newFilteredImages update.SortedImageInfos
51 51
 	for _, img := range filteredImages {

+ 4
- 5
api/v6/container_test.go View File

@@ -8,7 +8,6 @@ import (
8 8
 
9 9
 	"github.com/weaveworks/flux/image"
10 10
 	"github.com/weaveworks/flux/policy"
11
-	"github.com/weaveworks/flux/update"
12 11
 )
13 12
 
14 13
 func TestNewContainer(t *testing.T) {
@@ -21,7 +20,7 @@ func TestNewContainer(t *testing.T) {
21 20
 
22 21
 	type args struct {
23 22
 		name         string
24
-		images       update.ImageInfos
23
+		images       []image.Info
25 24
 		currentImage image.Info
26 25
 		tagPattern   policy.Pattern
27 26
 		fields       []string
@@ -36,7 +35,7 @@ func TestNewContainer(t *testing.T) {
36 35
 			name: "Simple",
37 36
 			args: args{
38 37
 				name:         "container1",
39
-				images:       update.ImageInfos{testImage},
38
+				images:       []image.Info{testImage},
40 39
 				currentImage: testImage,
41 40
 				tagPattern:   policy.PatternAll,
42 41
 			},
@@ -56,7 +55,7 @@ func TestNewContainer(t *testing.T) {
56 55
 			name: "Semver filtering and sorting",
57 56
 			args: args{
58 57
 				name:         "container-semver",
59
-				images:       update.ImageInfos{currentSemver, newSemver, oldSemver, testImage},
58
+				images:       []image.Info{currentSemver, newSemver, oldSemver, testImage},
60 59
 				currentImage: currentSemver,
61 60
 				tagPattern:   policy.NewPattern("semver:*"),
62 61
 			},
@@ -114,7 +113,7 @@ func TestFilterContainerFields(t *testing.T) {
114 113
 			wantErr: false,
115 114
 		},
116 115
 		{
117
-			name: "Filter",
116
+			name: "FilterImages",
118 117
 			args: args{
119 118
 				container: testContainer,
120 119
 				fields:    []string{"Name", "Available", "NewAvailableImagesCount", "NewFilteredImagesCount"},

+ 13
- 2
daemon/daemon.go View File

@@ -652,8 +652,19 @@ func getWorkloadContainers(workload cluster.Workload, imageRepos update.ImageRep
652 652
 		}
653 653
 		tagPattern := policy.GetTagPattern(policies, c.Name)
654 654
 
655
-		images := imageRepos.GetRepoImages(imageRepo)
656
-		currentImage := images.FindWithRef(c.Image)
655
+		repoMetadata := imageRepos.GetRepositoryMetadata(imageRepo)
656
+		var images []image.Info
657
+		// Build images, tolerating tags with missing metadata
658
+		for _, tag := range repoMetadata.Tags {
659
+			info, ok := repoMetadata.Images[tag]
660
+			if !ok {
661
+				info = image.Info{
662
+					ID: image.Ref{Tag: tag},
663
+				}
664
+			}
665
+			images = append(images, info)
666
+		}
667
+		currentImage := repoMetadata.FindImageWithRef(c.Image)
657 668
 
658 669
 		container, err := v6.NewContainer(c.Name, images, currentImage, tagPattern, fields)
659 670
 		if err != nil {

+ 4
- 4
daemon/daemon_test.go View File

@@ -571,7 +571,6 @@ func TestDaemon_JobStatusWithNoCache(t *testing.T) {
571 571
 
572 572
 func TestDaemon_Automated(t *testing.T) {
573 573
 	d, start, clean, k8s, _, _ := mockDaemon(t)
574
-	start()
575 574
 	defer clean()
576 575
 	w := newWait(t)
577 576
 
@@ -589,6 +588,7 @@ func TestDaemon_Automated(t *testing.T) {
589 588
 	k8s.SomeWorkloadsFunc = func([]flux.ResourceID) ([]cluster.Workload, error) {
590 589
 		return []cluster.Workload{workload}, nil
591 590
 	}
591
+	start()
592 592
 
593 593
 	// updates from helloworld:master-xxx to helloworld:2
594 594
 	w.ForImageTag(t, d, wl, container, "2")
@@ -596,12 +596,11 @@ func TestDaemon_Automated(t *testing.T) {
596 596
 
597 597
 func TestDaemon_Automated_semver(t *testing.T) {
598 598
 	d, start, clean, k8s, _, _ := mockDaemon(t)
599
-	start()
600 599
 	defer clean()
601 600
 	w := newWait(t)
602 601
 
603 602
 	resid := flux.MustParseResourceID("default:deployment/semver")
604
-	service := cluster.Workload{
603
+	workload := cluster.Workload{
605 604
 		ID: resid,
606 605
 		Containers: cluster.ContainersOrExcuse{
607 606
 			Containers: []resource.Container{
@@ -613,8 +612,9 @@ func TestDaemon_Automated_semver(t *testing.T) {
613 612
 		},
614 613
 	}
615 614
 	k8s.SomeWorkloadsFunc = func([]flux.ResourceID) ([]cluster.Workload, error) {
616
-		return []cluster.Workload{service}, nil
615
+		return []cluster.Workload{workload}, nil
617 616
 	}
617
+	start()
618 618
 
619 619
 	// helloworld:3 is older than helloworld:2 but semver orders by version
620 620
 	w.ForImageTag(t, d, resid.String(), container, "3")

+ 8
- 5
daemon/images.go View File

@@ -90,16 +90,19 @@ func calculateChanges(logger log.Logger, candidateWorkloads resources, workloads
90 90
 			pattern := policy.GetTagPattern(p, container.Name)
91 91
 			repo := currentImageID.Name
92 92
 			logger := log.With(logger, "workload", workload.ID, "container", container.Name, "repo", repo, "pattern", pattern, "current", currentImageID)
93
+			repoMetadata := imageRepos.GetRepositoryMetadata(repo)
94
+			images, err := update.FilterAndSortRepositoryMetadata(repoMetadata, pattern)
95
+			if err != nil {
96
+				logger.Log("warning", fmt.Sprintf("inconsistent repository metadata: %s", err), "action", "skip container")
97
+				continue containers
98
+			}
93 99
 
94
-			images := imageRepos.GetRepoImages(repo)
95
-			filteredImages := images.FilterAndSort(pattern)
96
-
97
-			if latest, ok := filteredImages.Latest(); ok && latest.ID != currentImageID {
100
+			if latest, ok := images.Latest(); ok && latest.ID != currentImageID {
98 101
 				if latest.ID.Tag == "" {
99 102
 					logger.Log("warning", "untagged image in available images", "action", "skip container")
100 103
 					continue containers
101 104
 				}
102
-				current := images.FindWithRef(currentImageID)
105
+				current := repoMetadata.FindImageWithRef(currentImageID)
103 106
 				if current.CreatedAt.IsZero() || latest.CreatedAt.IsZero() {
104 107
 					logger.Log("warning", "image with zero created timestamp", "current", fmt.Sprintf("%s (%s)", current.ID, current.CreatedAt), "latest", fmt.Sprintf("%s (%s)", latest.ID, latest.CreatedAt), "action", "skip container")
105 108
 					continue containers

+ 37
- 0
image/image.go View File

@@ -281,6 +281,43 @@ func (im *Info) UnmarshalJSON(b []byte) error {
281 281
 	return err
282 282
 }
283 283
 
284
+// RepositoryMetadata contains the image metadata information found in an
285
+// image repository.
286
+//
287
+// `Images` is indexed by `Tags`. Note that `Images` may be partial/incomplete,
288
+// (i.e. entries from `Tags` may not have a corresponding key in `Images`),
289
+// this indicates that the tag manifest was missing or corrupted in the
290
+// repository.
291
+type RepositoryMetadata struct {
292
+	Tags   []string        // all the tags found in the repository
293
+	Images map[string]Info // indexed by `Tags`, but may not include keys for all entries in `Tags`
294
+}
295
+
296
+// FindImageWithRef returns image.Info given an image ref. If the image cannot be
297
+// found, it returns the image.Info with the ID provided.
298
+func (rm RepositoryMetadata) FindImageWithRef(ref Ref) Info {
299
+	for _, img := range rm.Images {
300
+		if img.ID == ref {
301
+			return img
302
+		}
303
+	}
304
+	return Info{ID: ref}
305
+}
306
+
307
+// GetImageTagInfo gets the information of all image tags.
308
+// If there are tags missing information, an error is returned
309
+func (rm RepositoryMetadata) GetImageTagInfo() ([]Info, error) {
310
+	result := make([]Info, len(rm.Tags), len(rm.Tags))
311
+	for i, tag := range rm.Tags {
312
+		info, ok := rm.Images[tag]
313
+		if !ok {
314
+			return nil, fmt.Errorf("missing metadata for image tag %q", tag)
315
+		}
316
+		result[i] = info
317
+	}
318
+	return result, nil
319
+}
320
+
284 321
 func decodeTime(s string, t *time.Time) error {
285 322
 	if s == "" {
286 323
 		*t = time.Time{}

+ 2
- 6
registry/cache/cache.go View File

@@ -39,16 +39,12 @@ func NewManifestKey(image image.CanonicalRef) Keyer {
39 39
 
40 40
 func (k *manifestKey) Key() string {
41 41
 	return strings.Join([]string{
42
-		"registryhistoryv3", // Just to version in case we need to change format later.
42
+		"registryhistoryv3", // Bump the version number if the cache format changes
43 43
 		k.fullRepositoryPath,
44 44
 		k.reference,
45 45
 	}, "|")
46 46
 }
47 47
 
48
-type tagKey struct {
49
-	fullRepositoryPath string
50
-}
51
-
52 48
 type repoKey struct {
53 49
 	fullRepositoryPath string
54 50
 }
@@ -59,7 +55,7 @@ func NewRepositoryKey(repo image.CanonicalName) Keyer {
59 55
 
60 56
 func (k *repoKey) Key() string {
61 57
 	return strings.Join([]string{
62
-		"registryrepov3",
58
+		"registryrepov4", // Bump the version number if the cache format changes
63 59
 		k.fullRepositoryPath,
64 60
 	}, "|")
65 61
 }

+ 13
- 16
registry/cache/memcached/integration_test.go View File

@@ -10,6 +10,7 @@ import (
10 10
 	"time"
11 11
 
12 12
 	"github.com/go-kit/kit/log"
13
+	"github.com/stretchr/testify/assert"
13 14
 
14 15
 	"github.com/weaveworks/flux/image"
15 16
 	"github.com/weaveworks/flux/registry"
@@ -69,27 +70,23 @@ Loop:
69 70
 		case <-timeout.C:
70 71
 			t.Fatal("Cache timeout")
71 72
 		case <-tick.C:
72
-			_, err := r.GetRepositoryImages(id.Name)
73
+			_, err := r.GetImageRespositoryMetadata(id.Name)
73 74
 			if err == nil {
74 75
 				break Loop
75 76
 			}
76 77
 		}
77 78
 	}
78 79
 
79
-	img, err := r.GetRepositoryImages(id.Name)
80
-	if err != nil {
81
-		t.Fatal(err)
82
-	}
83
-	if len(img) == 0 {
84
-		t.Fatal("Length of returned images should be > 0")
85
-	}
86
-	// None of the images should have an empty ID or a creation time of zero
87
-	for _, i := range img {
88
-		if i.ID.String() == "" || i.ID.Tag == "" {
89
-			t.Fatalf("Image should not have empty name or tag. Got: %q", i.ID.String())
90
-		}
91
-		if i.CreatedAt.IsZero() {
92
-			t.Fatalf("Created time should not be zero for image %q", i.ID.String())
93
-		}
80
+	repoMetadata, err := r.GetImageRespositoryMetadata(id.Name)
81
+	assert.NoError(t, err)
82
+	assert.True(t, len(repoMetadata.Images) > 0, "Length of returned images should be > 0")
83
+	assert.Equal(t, len(repoMetadata.Images), len(repoMetadata.Tags), "the length of tags and images should match")
84
+
85
+	for _, tag := range repoMetadata.Tags {
86
+		i, ok := repoMetadata.Images[tag]
87
+		assert.True(t, ok, "tag doesn't have image information %s", tag)
88
+		// None of the images should have an empty ID or a creation time of zero
89
+		assert.True(t, i.ID.String() != "" && i.ID.Tag != "", "Image should not have empty name or tag. Got: %q", i.ID.String())
90
+		assert.NotZero(t, i.CreatedAt, "Created time should not be zero for image %q", i.ID.String())
94 91
 	}
95 92
 }

+ 11
- 17
registry/cache/registry.go View File

@@ -31,35 +31,29 @@ type Cache struct {
31 31
 	Reader Reader
32 32
 }
33 33
 
34
-// GetRepositoryImages returns the list of image manifests in an image
34
+// GetImageRespositoryMetadata returns the metadata from an image
35 35
 // repository (e.g,. at "quay.io/weaveworks/flux")
36
-func (c *Cache) GetRepositoryImages(id image.Name) ([]image.Info, error) {
36
+func (c *Cache) GetImageRespositoryMetadata(id image.Name) (image.RepositoryMetadata, error) {
37 37
 	repoKey := NewRepositoryKey(id.CanonicalName())
38 38
 	bytes, _, err := c.Reader.GetKey(repoKey)
39 39
 	if err != nil {
40
-		return nil, err
40
+		return image.RepositoryMetadata{}, err
41 41
 	}
42 42
 	var repo ImageRepository
43 43
 	if err = json.Unmarshal(bytes, &repo); err != nil {
44
-		return nil, err
44
+		return image.RepositoryMetadata{}, err
45 45
 	}
46 46
 
47 47
 	// We only care about the error if we've never successfully
48 48
 	// updated the result.
49 49
 	if repo.LastUpdate.IsZero() {
50 50
 		if repo.LastError != "" {
51
-			return nil, errors.New(repo.LastError)
51
+			return image.RepositoryMetadata{}, errors.New(repo.LastError)
52 52
 		}
53
-		return nil, ErrNotCached
53
+		return image.RepositoryMetadata{}, ErrNotCached
54 54
 	}
55 55
 
56
-	images := make([]image.Info, len(repo.Images))
57
-	var i int
58
-	for _, im := range repo.Images {
59
-		images[i] = im
60
-		i++
61
-	}
62
-	return images, nil
56
+	return repo.RepositoryMetadata, nil
63 57
 }
64 58
 
65 59
 // GetImage gets the manifest of a specific image ref, from its
@@ -85,11 +79,11 @@ func (c *Cache) GetImage(id image.Ref) (image.Info, error) {
85 79
 // ImageRepository holds the last good information on an image
86 80
 // repository.
87 81
 //
88
-// Whenever we successfully fetch a full set of image info,
89
-// `LastUpdate` and `Images` shall each be assigned a value, and
82
+// Whenever we successfully fetch a set (partial or full) of image metadata,
83
+// `LastUpdate`, `Tags` and `Images` shall each be assigned a value, and
90 84
 // `LastError` will be cleared.
91 85
 //
92
-// If we cannot for any reason obtain a full set of image info,
86
+// If we cannot for any reason obtain the set of image metadata,
93 87
 // `LastError` shall be assigned a value, and the other fields left
94 88
 // alone.
95 89
 //
@@ -99,7 +93,7 @@ func (c *Cache) GetImage(id image.Ref) (image.Info, error) {
99 93
 // value (show the images, but also indicate there's a problem, for
100 94
 // example).
101 95
 type ImageRepository struct {
96
+	image.RepositoryMetadata
102 97
 	LastError  string
103 98
 	LastUpdate time.Time
104
-	Images     map[string]image.Info
105 99
 }

+ 20
- 7
registry/cache/warming.go View File

@@ -3,6 +3,7 @@ package cache
3 3
 import (
4 4
 	"context"
5 5
 	"encoding/json"
6
+	"fmt"
6 7
 	"net"
7 8
 	"strings"
8 9
 	"sync"
@@ -263,6 +264,7 @@ func (w *Warmer) warm(ctx context.Context, now time.Time, logger log.Logger, id
263 264
 
264 265
 	var fetchMx sync.Mutex // also guards access to newImages
265 266
 	var successCount int
267
+	var manifestUnknownCount int
266 268
 
267 269
 	if len(toUpdate) > 0 {
268 270
 		logger.Log("info", "refreshing image", "image", id, "tag_count", len(tags), "to_update", len(toUpdate), "of_which_refresh", refresh, "of_which_missing", missing)
@@ -303,13 +305,21 @@ func (w *Warmer) warm(ctx context.Context, now time.Time, logger log.Logger, id
303 305
 						return
304 306
 					}
305 307
 
306
-					// abort the image tags fetching if we've been rate limited
307
-					if strings.Contains(err.Error(), "429") {
308
+					switch {
309
+					case strings.Contains(err.Error(), "429"):
310
+						// abort the image tags fetching if we've been rate limited
308 311
 						once.Do(func() {
309 312
 							errorLogger.Log("warn", "aborting image tag fetching due to rate limiting, will try again later")
310 313
 							cancel()
311 314
 						})
312
-					} else {
315
+					case strings.Contains(err.Error(), "manifest unknown"):
316
+						// Registry is corrupted, keep going, this manifest may not be relevant for automatic updates
317
+						fetchMx.Lock()
318
+						manifestUnknownCount++
319
+						fetchMx.Unlock()
320
+						errorLogger.Log("warn", fmt.Sprintf("manifest for tag %s missing in registry %s", imageID.Tag, imageID.Name),
321
+							"impact", "flux will fail to auto-release workloads with matching images, ask the respository administrator to fix the inconsistency")
322
+					default:
313 323
 						errorLogger.Log("err", err, "ref", imageID)
314 324
 					}
315 325
 					return
@@ -364,12 +374,15 @@ func (w *Warmer) warm(ctx context.Context, now time.Time, logger log.Logger, id
364 374
 		logger.Log("updated", id.String(), "successful", successCount, "attempted", len(toUpdate))
365 375
 	}
366 376
 
367
-	// We managed to fetch new metadata for everything we were missing
368
-	// (if anything). Ratchet the result forward.
369
-	if successCount == len(toUpdate) {
377
+	// We managed to fetch new metadata for everything we needed.
378
+	// Ratchet the result forward.
379
+	if successCount+manifestUnknownCount == len(toUpdate) {
370 380
 		repo = ImageRepository{
371 381
 			LastUpdate: time.Now(),
372
-			Images:     newImages,
382
+			RepositoryMetadata: image.RepositoryMetadata{
383
+				Images: newImages,
384
+				Tags:   tags,
385
+			},
373 386
 		}
374 387
 		// If we got through all that without bumping into `HTTP 429
375 388
 		// Too Many Requests` (or other problems), we can potentially

+ 44
- 3
registry/cache/warming_test.go View File

@@ -6,6 +6,7 @@ import (
6 6
 	"testing"
7 7
 	"time"
8 8
 
9
+	"github.com/docker/distribution/registry/api/errcode"
9 10
 	"github.com/go-kit/kit/log"
10 11
 	"github.com/stretchr/testify/assert"
11 12
 
@@ -73,12 +74,52 @@ func TestWarmThenQuery(t *testing.T) {
73 74
 	warmer.warm(context.TODO(), now, logger, repo, registry.NoCredentials())
74 75
 
75 76
 	registry := &Cache{Reader: cache}
76
-	repoInfo, err := registry.GetRepositoryImages(ref.Name)
77
+	repoInfo, err := registry.GetImageRespositoryMetadata(ref.Name)
77 78
 	assert.NoError(t, err)
78 79
 
79 80
 	// Otherwise, we should get what we put in ...
80
-	assert.Len(t, repoInfo, 1)
81
-	assert.Equal(t, ref.String(), repoInfo[0].ID.String())
81
+	assert.Len(t, repoInfo.Tags, 1)
82
+	assert.Equal(t, ref.String(), repoInfo.Images[repoInfo.Tags[0]].ID.String())
83
+}
84
+
85
+func TestWarmManifestUnknown(t *testing.T) {
86
+	tagWithMissingMetadata := "4.0.8-r3"
87
+	client := &mock.Client{
88
+		TagsFn: func() ([]string, error) {
89
+			println("asked for tags")
90
+			return []string{tagWithMissingMetadata}, nil
91
+		},
92
+		ManifestFn: func(tag string) (registry.ImageEntry, error) {
93
+			println("asked for manifest", tag)
94
+			err := errcode.Errors{
95
+				errcode.Error{Code: 1012,
96
+					Message: "manifest unknown",
97
+					Detail: map[string]interface{}{
98
+						"Name":     "bitnami/redis",
99
+						"Revision": "sha256:9c7f4a0958280a55a4337d74c22260bc338c26a0a2de493a8ad69dd73fd5c290",
100
+					},
101
+				},
102
+			}
103
+			return registry.ImageEntry{}, err
104
+		},
105
+	}
106
+	factory := &mock.ClientFactory{Client: client}
107
+	cache := &mem{}
108
+	warmer := &Warmer{clientFactory: factory, cache: cache, burst: 10}
109
+
110
+	logger := log.NewNopLogger()
111
+
112
+	now := time.Now()
113
+	redisRef, _ := image.ParseRef("bitnami/redis:5.0.2")
114
+	repo := redisRef.Name
115
+	warmer.warm(context.TODO(), now, logger, repo, registry.NoCredentials())
116
+
117
+	registry := &Cache{Reader: cache}
118
+	repoInfo, err := registry.GetImageRespositoryMetadata(repo)
119
+	assert.NoError(t, err)
120
+
121
+	assert.Len(t, repoInfo.Tags, 1)
122
+	assert.Equal(t, tagWithMissingMetadata, repoInfo.Tags[0])
82 123
 }
83 124
 
84 125
 func TestRefreshDeadline(t *testing.T) {

+ 8
- 4
registry/mock/mock.go View File

@@ -44,15 +44,19 @@ type Registry struct {
44 44
 	Err    error
45 45
 }
46 46
 
47
-func (m *Registry) GetRepositoryImages(id image.Name) ([]image.Info, error) {
48
-	var imgs []image.Info
47
+func (m *Registry) GetImageRespositoryMetadata(id image.Name) (image.RepositoryMetadata, error) {
48
+	result := image.RepositoryMetadata{
49
+		Images: map[string]image.Info{},
50
+	}
49 51
 	for _, i := range m.Images {
50 52
 		// include only if it's the same repository in the same place
51 53
 		if i.ID.Image == id.Image {
52
-			imgs = append(imgs, i)
54
+			tag := i.ID.Tag
55
+			result.Tags = append(result.Tags, tag)
56
+			result.Images[tag] = i
53 57
 		}
54 58
 	}
55
-	return imgs, m.Err
59
+	return result, m.Err
56 60
 }
57 61
 
58 62
 func (m *Registry) GetImage(id image.Ref) (image.Info, error) {

+ 2
- 2
registry/monitoring.go View File

@@ -46,9 +46,9 @@ func NewInstrumentedRegistry(next Registry) Registry {
46 46
 	}
47 47
 }
48 48
 
49
-func (m *instrumentedRegistry) GetRepositoryImages(id image.Name) (res []image.Info, err error) {
49
+func (m *instrumentedRegistry) GetImageRespositoryMetadata(id image.Name) (res image.RepositoryMetadata, err error) {
50 50
 	start := time.Now()
51
-	res, err = m.next.GetRepositoryImages(id)
51
+	res, err = m.next.GetImageRespositoryMetadata(id)
52 52
 	registryDuration.With(
53 53
 		fluxmetrics.LabelSuccess, strconv.FormatBool(err == nil),
54 54
 	).Observe(time.Since(start).Seconds())

+ 1
- 1
registry/registry.go View File

@@ -12,7 +12,7 @@ var (
12 12
 
13 13
 // Registry is a store of image metadata.
14 14
 type Registry interface {
15
-	GetRepositoryImages(image.Name) ([]image.Info, error)
15
+	GetImageRespositoryMetadata(image.Name) (image.RepositoryMetadata, error)
16 16
 	GetImage(image.Ref) (image.Info, error)
17 17
 }
18 18
 

+ 1
- 1
remote/rpc/compat.go View File

@@ -177,7 +177,7 @@ func listImagesWithOptions(ctx context.Context, client listImagesWithoutOptionsC
177 177
 			}
178 178
 			tagPattern := policy.GetTagPattern(p, container.Name)
179 179
 			// Create a new container using the same function used in v10
180
-			newContainer, err := v6.NewContainer(container.Name, update.ImageInfos(container.Available), container.Current, tagPattern, opts.OverrideContainerFields)
180
+			newContainer, err := v6.NewContainer(container.Name, container.Available, container.Current, tagPattern, opts.OverrideContainerFields)
181 181
 			if err != nil {
182 182
 				return statuses, err
183 183
 			}

+ 70
- 61
update/images.go View File

@@ -14,83 +14,74 @@ import (
14 14
 	"github.com/weaveworks/flux/resource"
15 15
 )
16 16
 
17
-type imageReposMap map[image.CanonicalName]ImageInfos
17
+type imageReposMap map[image.CanonicalName]image.RepositoryMetadata
18 18
 
19
-// ImageRepos contains a map of image repositories to their images
19
+// ImageRepos contains a map of image repositories to their metadata
20 20
 type ImageRepos struct {
21 21
 	imageRepos imageReposMap
22 22
 }
23 23
 
24
-// GetRepoImages returns image.Info entries for all the images in the
24
+// GetRepositoryMetadata returns the metadata for all the images in the
25 25
 // named image repository.
26
-func (r ImageRepos) GetRepoImages(repo image.Name) ImageInfos {
27
-	if canon, ok := r.imageRepos[repo.CanonicalName()]; ok {
28
-		infos := make([]image.Info, len(canon))
29
-		for i := range canon {
30
-			infos[i] = canon[i]
31
-			infos[i].ID = repo.ToRef(infos[i].ID.Tag)
26
+func (r ImageRepos) GetRepositoryMetadata(repo image.Name) image.RepositoryMetadata {
27
+	if metadata, ok := r.imageRepos[repo.CanonicalName()]; ok {
28
+		// copy tags
29
+		tagsCopy := make([]string, len(metadata.Tags))
30
+		copy(tagsCopy, metadata.Tags)
31
+		// copy images
32
+		imagesCopy := make(map[string]image.Info, len(metadata.Images))
33
+		for tag, info := range metadata.Images {
34
+			// The registry (cache) stores metadata with canonical image
35
+			// names (e.g., `index.docker.io/library/alpine`). We rewrite the
36
+			// names based on how we were queried (repo), which could
37
+			// be non-canonical representation (e.g. `alpine`).
38
+			info.ID = repo.ToRef(info.ID.Tag)
39
+			imagesCopy[tag] = info
32 40
 		}
33
-		return infos
41
+		return image.RepositoryMetadata{tagsCopy, imagesCopy}
34 42
 	}
35
-	return nil
43
+	return image.RepositoryMetadata{}
36 44
 }
37 45
 
38
-// ImageInfos is a list of image.Info which can be filtered.
39
-type ImageInfos []image.Info
40
-
41 46
 // SortedImageInfos is a list of sorted image.Info
42 47
 type SortedImageInfos []image.Info
43 48
 
44
-// Filter returns only the images that match the pattern, in a new list.
45
-func (ii ImageInfos) Filter(pattern policy.Pattern) ImageInfos {
46
-	return filterImages(ii, pattern)
47
-}
48
-
49
-// Sort orders the images according to the pattern order in a new list.
50
-func (ii ImageInfos) Sort(pattern policy.Pattern) SortedImageInfos {
51
-	return sortImages(ii, pattern)
49
+// FilterImages returns only the images that match the pattern, in a new list.
50
+func FilterImages(images []image.Info, pattern policy.Pattern) []image.Info {
51
+	return filterImages(images, pattern)
52 52
 }
53 53
 
54
-// FilterAndSort is an optimized helper function to compose filtering and sorting.
55
-func (ii ImageInfos) FilterAndSort(pattern policy.Pattern) SortedImageInfos {
56
-	filtered := ii.Filter(pattern)
57
-	// Do not call sortImages() here which will clone the list that we already
58
-	// cloned in ImageInfos.Filter()
59
-	image.Sort(filtered, pattern.Newer)
60
-	return SortedImageInfos(filtered)
54
+// SortImages orders the images according to the pattern order in a new list.
55
+func SortImages(images []image.Info, pattern policy.Pattern) SortedImageInfos {
56
+	return sortImages(images, pattern)
61 57
 }
62 58
 
63
-// FindWithRef returns image.Info given an image ref. If the image cannot be
64
-// found, it returns the image.Info with the ID provided.
65
-func (ii ImageInfos) FindWithRef(ref image.Ref) image.Info {
66
-	for _, img := range ii {
67
-		if img.ID == ref {
68
-			return img
69
-		}
59
+// FilterAndSortRepositoryMetadata obtains all the image information from the metadata
60
+// after filtering and sorting. Filtering happens in the metadata directly to minimize
61
+// problems with tag inconsistencies (i.e. tags without matching image information)
62
+func FilterAndSortRepositoryMetadata(rm image.RepositoryMetadata, pattern policy.Pattern) (SortedImageInfos, error) {
63
+	// Do the filtering
64
+	filteredMetadata := image.RepositoryMetadata{
65
+		Tags:   filterTags(rm.Tags, pattern),
66
+		Images: rm.Images,
67
+	}
68
+	filteredImages, err := filteredMetadata.GetImageTagInfo()
69
+	if err != nil {
70
+		return nil, err
70 71
 	}
71
-	return image.Info{ID: ref}
72
+	return SortImages(filteredImages, pattern), nil
72 73
 }
73 74
 
74 75
 // Latest returns the latest image from SortedImageInfos. If no such image exists,
75 76
 // returns a zero value and `false`, and the caller can decide whether
76 77
 // that's an error or not.
77
-func (is SortedImageInfos) Latest() (image.Info, bool) {
78
-	if len(is) > 0 {
79
-		return is[0], true
78
+func (sii SortedImageInfos) Latest() (image.Info, bool) {
79
+	if len(sii) > 0 {
80
+		return sii[0], true
80 81
 	}
81 82
 	return image.Info{}, false
82 83
 }
83 84
 
84
-// Filter returns only the images that match the pattern, in a new list.
85
-func (is SortedImageInfos) Filter(pattern policy.Pattern) SortedImageInfos {
86
-	return SortedImageInfos(filterImages(is, pattern))
87
-}
88
-
89
-// Sort orders the images according to the pattern order in a new list.
90
-func (is SortedImageInfos) Sort(pattern policy.Pattern) SortedImageInfos {
91
-	return sortImages(is, pattern)
92
-}
93
-
94 85
 func sortImages(images []image.Info, pattern policy.Pattern) SortedImageInfos {
95 86
 	var sorted SortedImageInfos
96 87
 	for _, i := range images {
@@ -100,16 +91,29 @@ func sortImages(images []image.Info, pattern policy.Pattern) SortedImageInfos {
100 91
 	return sorted
101 92
 }
102 93
 
103
-// filterImages keeps the sort order pristine.
104
-func filterImages(images []image.Info, pattern policy.Pattern) ImageInfos {
105
-	var filtered ImageInfos
94
+func matchWithLatest(pattern policy.Pattern, tag string) bool {
95
+	// Ignore latest if and only if it's not what the user wants.
96
+	if pattern != policy.PatternLatest && strings.EqualFold(tag, "latest") {
97
+		return false
98
+	}
99
+	return pattern.Matches(tag)
100
+}
101
+
102
+func filterTags(tags []string, pattern policy.Pattern) []string {
103
+	var filtered []string
104
+	for _, tag := range tags {
105
+		if matchWithLatest(pattern, tag) {
106
+			filtered = append(filtered, tag)
107
+		}
108
+	}
109
+	return filtered
110
+}
111
+
112
+func filterImages(images []image.Info, pattern policy.Pattern) []image.Info {
113
+	var filtered []image.Info
106 114
 	for _, i := range images {
107 115
 		tag := i.ID.Tag
108
-		// Ignore latest if and only if it's not what the user wants.
109
-		if pattern != policy.PatternLatest && strings.EqualFold(tag, "latest") {
110
-			continue
111
-		}
112
-		if pattern.Matches(tag) {
116
+		if matchWithLatest(pattern, tag) {
113 117
 			filtered = append(filtered, i)
114 118
 		}
115 119
 	}
@@ -144,11 +148,11 @@ func FetchImageRepos(reg registry.Registry, cs containers, logger log.Logger) (I
144 148
 	imageRepos := imageReposMap{}
145 149
 	for i := 0; i < cs.Len(); i++ {
146 150
 		for _, container := range cs.Containers(i) {
147
-			imageRepos[container.Image.CanonicalName()] = nil
151
+			imageRepos[container.Image.CanonicalName()] = image.RepositoryMetadata{}
148 152
 		}
149 153
 	}
150 154
 	for repo := range imageRepos {
151
-		images, err := reg.GetRepositoryImages(repo.Name)
155
+		images, err := reg.GetImageRespositoryMetadata(repo.Name)
152 156
 		if err != nil {
153 157
 			// Not an error if missing. Use empty images.
154 158
 			if !fluxerr.IsMissing(err) {
@@ -173,7 +177,12 @@ func exactImageRepos(reg registry.Registry, images []image.Ref) (ImageRepos, err
173 177
 		if !exist {
174 178
 			return ImageRepos{}, errors.Wrap(image.ErrInvalidImageID, fmt.Sprintf("image %q does not exist", id))
175 179
 		}
176
-		m[id.CanonicalName()] = []image.Info{{ID: id}}
180
+		m[id.CanonicalName()] = image.RepositoryMetadata{
181
+			Tags: []string{id.Tag},
182
+			Images: map[string]image.Info{
183
+				id.Tag: {ID: id},
184
+			},
185
+		}
177 186
 	}
178 187
 	return ImageRepos{m}, nil
179 188
 }

+ 56
- 18
update/images_test.go View File

@@ -8,6 +8,7 @@ import (
8 8
 
9 9
 	"github.com/weaveworks/flux/image"
10 10
 	"github.com/weaveworks/flux/policy"
11
+	"github.com/weaveworks/flux/registry/mock"
11 12
 )
12 13
 
13 14
 var (
@@ -18,32 +19,46 @@ var (
18 19
 	}
19 20
 )
20 21
 
22
+func buildImageRepos(t *testing.T) ImageRepos {
23
+	registry := mock.Registry{Images: infos}
24
+	repoMetadata, err := registry.GetImageRespositoryMetadata(name.Name)
25
+	assert.NoError(t, err)
26
+	return ImageRepos{
27
+		imageRepos: imageReposMap{name.Name.CanonicalName(): repoMetadata},
28
+	}
29
+}
30
+
31
+func getFilteredAndSortedImagesFromRepos(t *testing.T, imageName string, repos ImageRepos) SortedImageInfos {
32
+	metadata := repos.GetRepositoryMetadata(mustParseName(imageName))
33
+	images, err := FilterAndSortRepositoryMetadata(metadata, policy.PatternAll)
34
+	assert.NoError(t, err)
35
+	return images
36
+}
37
+
21 38
 // TestDecanon checks that we return appropriate image names when
22 39
 // asked for images. The registry (cache) stores things with canonical
23 40
 // names (e.g., `index.docker.io/library/alpine`), but we ask
24 41
 // questions in terms of everyday names (e.g., `alpine`).
25 42
 func TestDecanon(t *testing.T) {
26
-	m := ImageRepos{imageReposMap{
27
-		name: infos,
28
-	}}
43
+	imageRepos := buildImageRepos(t)
29 44
 
30
-	filteredImages := m.GetRepoImages(mustParseName("weaveworks/helloworld")).FilterAndSort(policy.PatternAll)
31
-	latest, ok := filteredImages.Latest()
45
+	images := getFilteredAndSortedImagesFromRepos(t, "weaveworks/helloworld", imageRepos)
46
+	latest, ok := images.Latest()
32 47
 	if !ok {
33 48
 		t.Error("did not find latest image")
34 49
 	} else if latest.ID.Name != mustParseName("weaveworks/helloworld") {
35 50
 		t.Error("name did not match what was asked")
36 51
 	}
37 52
 
38
-	filteredImages = m.GetRepoImages(mustParseName("index.docker.io/weaveworks/helloworld")).FilterAndSort(policy.PatternAll)
39
-	latest, ok = filteredImages.Latest()
53
+	images = getFilteredAndSortedImagesFromRepos(t, "index.docker.io/weaveworks/helloworld", imageRepos)
54
+	latest, ok = images.Latest()
40 55
 	if !ok {
41 56
 		t.Error("did not find latest image")
42 57
 	} else if latest.ID.Name != mustParseName("index.docker.io/weaveworks/helloworld") {
43 58
 		t.Error("name did not match what was asked")
44 59
 	}
45 60
 
46
-	avail := m.GetRepoImages(mustParseName("weaveworks/helloworld"))
61
+	avail := getFilteredAndSortedImagesFromRepos(t, "weaveworks/helloworld", imageRepos)
47 62
 	if len(avail) != len(infos) {
48 63
 		t.Errorf("expected %d available images, got %d", len(infos), len(avail))
49 64
 	}
@@ -54,6 +69,25 @@ func TestDecanon(t *testing.T) {
54 69
 	}
55 70
 }
56 71
 
72
+func TestMetadataInConsistencyTolerance(t *testing.T) {
73
+	imageRepos := buildImageRepos(t)
74
+	metadata := imageRepos.GetRepositoryMetadata(mustParseName("weaveworks/helloworld"))
75
+	images, err := FilterAndSortRepositoryMetadata(metadata, policy.NewPattern("semver:*"))
76
+	assert.NoError(t, err)
77
+
78
+	// Let's make the metadata inconsistent by adding a non-semver tag
79
+	metadata.Tags = append(metadata.Tags, "latest")
80
+	// Filtering and sorting should still work
81
+	images2, err := FilterAndSortRepositoryMetadata(metadata, policy.NewPattern("semver:*"))
82
+	assert.NoError(t, err)
83
+	assert.Equal(t, images, images2)
84
+
85
+	// However, an inconsistency in a semver tag should make filtering and sorting fail
86
+	metadata.Tags = append(metadata.Tags, "v9")
87
+	_, err = FilterAndSortRepositoryMetadata(metadata, policy.NewPattern("semver:*"))
88
+	assert.Error(t, err)
89
+}
90
+
57 91
 func TestImageInfos_Filter_latest(t *testing.T) {
58 92
 	latest := image.Info{
59 93
 		ID: image.Ref{Name: image.Name{Image: "flux"}, Tag: "latest"},
@@ -61,11 +95,11 @@ func TestImageInfos_Filter_latest(t *testing.T) {
61 95
 	other := image.Info{
62 96
 		ID: image.Ref{Name: image.Name{Image: "moon"}, Tag: "v0"},
63 97
 	}
64
-	ii := ImageInfos{latest, other}
65
-	assert.Equal(t, SortedImageInfos{latest}, ii.FilterAndSort(policy.PatternLatest))
66
-	assert.Equal(t, SortedImageInfos{latest}, ii.FilterAndSort(policy.NewPattern("latest")))
67
-	assert.Equal(t, SortedImageInfos{other}, ii.FilterAndSort(policy.PatternAll))
68
-	assert.Equal(t, SortedImageInfos{other}, ii.FilterAndSort(policy.NewPattern("*")))
98
+	images := []image.Info{latest, other}
99
+	assert.Equal(t, []image.Info{latest}, filterImages(images, policy.PatternLatest))
100
+	assert.Equal(t, []image.Info{latest}, filterImages(images, policy.NewPattern("latest")))
101
+	assert.Equal(t, []image.Info{other}, filterImages(images, policy.PatternAll))
102
+	assert.Equal(t, []image.Info{other}, filterImages(images, policy.NewPattern("*")))
69 103
 }
70 104
 
71 105
 func TestImageInfos_Filter_semver(t *testing.T) {
@@ -73,14 +107,18 @@ func TestImageInfos_Filter_semver(t *testing.T) {
73 107
 	semver0 := image.Info{ID: image.Ref{Name: image.Name{Image: "moon"}, Tag: "v0.0.1"}}
74 108
 	semver1 := image.Info{ID: image.Ref{Name: image.Name{Image: "earth"}, Tag: "1.0.0"}}
75 109
 
76
-	ii := ImageInfos{latest, semver0, semver1}
77
-	assert.Equal(t, SortedImageInfos{semver1, semver0}, ii.FilterAndSort(policy.NewPattern("semver:*")))
78
-	assert.Equal(t, SortedImageInfos{semver1}, ii.FilterAndSort(policy.NewPattern("semver:~1")))
110
+	filterAndSort := func(images []image.Info, pattern policy.Pattern) SortedImageInfos {
111
+		filtered := FilterImages(images, pattern)
112
+		return SortImages(filtered, pattern)
113
+	}
114
+	images := []image.Info{latest, semver0, semver1}
115
+	assert.Equal(t, SortedImageInfos{semver1, semver0}, filterAndSort(images, policy.NewPattern("semver:*")))
116
+	assert.Equal(t, SortedImageInfos{semver1}, filterAndSort(images, policy.NewPattern("semver:~1")))
79 117
 }
80 118
 
81 119
 func TestAvail(t *testing.T) {
82
-	m := ImageRepos{imageReposMap{name: infos}}
83
-	avail := m.GetRepoImages(mustParseName("weaveworks/goodbyeworld"))
120
+	imageRepos := buildImageRepos(t)
121
+	avail := getFilteredAndSortedImagesFromRepos(t, "weaveworks/goodbyeworld", imageRepos)
84 122
 	if len(avail) > 0 {
85 123
 		t.Errorf("did not expect available images, but got %#v", avail)
86 124
 	}

+ 8
- 2
update/release_image.go View File

@@ -234,8 +234,14 @@ func (s ReleaseImageSpec) calculateImageUpdates(rc ReleaseContext, candidates []
234 234
 				}
235 235
 			}
236 236
 
237
-			filteredImages := imageRepos.GetRepoImages(currentImageID.Name).FilterAndSort(tagPattern)
238
-			latestImage, ok := filteredImages.Latest()
237
+			metadata := imageRepos.GetRepositoryMetadata(currentImageID.Name)
238
+			sortedImages, err := FilterAndSortRepositoryMetadata(metadata, tagPattern)
239
+			if err != nil {
240
+				// missing image repository metadata
241
+				ignoredOrSkipped = ReleaseStatusUnknown
242
+				continue
243
+			}
244
+			latestImage, ok := sortedImages.Latest()
239 245
 			if !ok {
240 246
 				if currentImageID.CanonicalName() != singleRepo {
241 247
 					ignoredOrSkipped = ReleaseStatusIgnored

Loading…
Cancel
Save