Browse Source

Refactor imagesMap functions to make clear we are handling maps of image repos

Aaron Kirkbride 1 year ago
parent
commit
dff25169bd

+ 5
- 5
daemon/daemon.go View File

@@ -143,17 +143,17 @@ func (d *Daemon) ListImages(ctx context.Context, spec update.ResourceSpec) ([]v6
143 143
 		services, err = d.Cluster.SomeControllers([]flux.ResourceID{id})
144 144
 	}
145 145
 
146
-	images, err := update.CollectAvailableImages(d.Registry, clusterContainers(services), d.Logger)
146
+	imageRepos, err := update.FetchImageRepos(d.Registry, clusterContainers(services), d.Logger)
147 147
 	if err != nil {
148 148
 		return nil, errors.Wrap(err, "getting images for services")
149 149
 	}
150 150
 
151 151
 	var res []v6.ImageStatus
152 152
 	for _, service := range services {
153
-		containers := containersWithAvailable(service, images)
153
+		serviceContainers := getServiceContainers(service, imageRepos)
154 154
 		res = append(res, v6.ImageStatus{
155 155
 			ID:         service.ID,
156
-			Containers: containers,
156
+			Containers: serviceContainers,
157 157
 		})
158 158
 	}
159 159
 
@@ -544,9 +544,9 @@ func containers2containers(cs []resource.Container) []v6.Container {
544 544
 	return res
545 545
 }
546 546
 
547
-func containersWithAvailable(service cluster.Controller, images update.ImageMap) (res []v6.Container) {
547
+func getServiceContainers(service cluster.Controller, imageRepos update.ImageRepos) (res []v6.Container) {
548 548
 	for _, c := range service.ContainersOrNil() {
549
-		available := images.Available(c.Image.Name)
549
+		available := imageRepos.Available(c.Image.Name)
550 550
 		availableErr := ""
551 551
 		if available == nil {
552 552
 			availableErr = registry.ErrNoImageData.Error()

+ 2
- 2
daemon/images.go View File

@@ -34,7 +34,7 @@ func (d *Daemon) pollForNewImages(logger log.Logger) {
34 34
 		return
35 35
 	}
36 36
 	// Check the latest available image(s) for each service
37
-	imageMap, err := update.CollectAvailableImages(d.Registry, clusterContainers(services), logger)
37
+	imageRepos, err := update.FetchImageRepos(d.Registry, clusterContainers(services), logger)
38 38
 	if err != nil {
39 39
 		logger.Log("error", errors.Wrap(err, "fetching image updates"))
40 40
 		return
@@ -55,7 +55,7 @@ func (d *Daemon) pollForNewImages(logger log.Logger) {
55 55
 			repo := currentImageID.Name
56 56
 			logger.Log("repo", repo, "pattern", pattern)
57 57
 
58
-			if latest, ok := imageMap.LatestImage(repo, pattern); ok && latest.ID != currentImageID {
58
+			if latest, ok := imageRepos.LatestImage(repo, pattern); ok && latest.ID != currentImageID {
59 59
 				if latest.ID.Tag == "" {
60 60
 					logger.Log("msg", "untagged image in available images", "action", "skip", "available", repo)
61 61
 					continue

+ 2
- 2
registry/cache/memcached/integration_test.go View File

@@ -66,14 +66,14 @@ Loop:
66 66
 		case <-timeout.C:
67 67
 			t.Fatal("Cache timeout")
68 68
 		case <-tick.C:
69
-			_, err := r.GetRepository(id.Name)
69
+			_, err := r.GetSortedRepositoryImages(id.Name)
70 70
 			if err == nil {
71 71
 				break Loop
72 72
 			}
73 73
 		}
74 74
 	}
75 75
 
76
-	img, err := r.GetRepository(id.Name)
76
+	img, err := r.GetSortedRepositoryImages(id.Name)
77 77
 	if err != nil {
78 78
 		t.Fatal(err)
79 79
 	}

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

@@ -31,9 +31,9 @@ type Cache struct {
31 31
 	Reader Reader
32 32
 }
33 33
 
34
-// GetRepository returns the list of image manifests in an image
34
+// GetSortedRepositoryImages returns the list of image manifests in an image
35 35
 // repository (e.g,. at "quay.io/weaveworks/flux")
36
-func (c *Cache) GetRepository(id image.Name) ([]image.Info, error) {
36
+func (c *Cache) GetSortedRepositoryImages(id image.Name) ([]image.Info, error) {
37 37
 	repoKey := NewRepositoryKey(id.CanonicalName())
38 38
 	bytes, _, err := c.Reader.GetKey(repoKey)
39 39
 	if err != nil {

+ 1
- 1
registry/cache/warming_test.go View File

@@ -71,7 +71,7 @@ func TestWarm(t *testing.T) {
71 71
 	warmer.warm(context.TODO(), logger, repo, registry.NoCredentials())
72 72
 
73 73
 	registry := &Cache{Reader: c}
74
-	repoInfo, err := registry.GetRepository(ref.Name)
74
+	repoInfo, err := registry.GetSortedRepositoryImages(ref.Name)
75 75
 	if err != nil {
76 76
 		t.Error(err)
77 77
 	}

+ 1
- 1
registry/mock/mock.go View File

@@ -40,7 +40,7 @@ type Registry struct {
40 40
 	Err    error
41 41
 }
42 42
 
43
-func (m *Registry) GetRepository(id image.Name) ([]image.Info, error) {
43
+func (m *Registry) GetSortedRepositoryImages(id image.Name) ([]image.Info, error) {
44 44
 	var imgs []image.Info
45 45
 	for _, i := range m.Images {
46 46
 		// include only if it's the same repository in the same place

+ 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) GetRepository(id image.Name) (res []image.Info, err error) {
49
+func (m *instrumentedRegistry) GetSortedRepositoryImages(id image.Name) (res []image.Info, err error) {
50 50
 	start := time.Now()
51
-	res, err = m.next.GetRepository(id)
51
+	res, err = m.next.GetSortedRepositoryImages(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
-	GetRepository(image.Name) ([]image.Info, error)
15
+	GetSortedRepositoryImages(image.Name) ([]image.Info, error)
16 16
 	GetImage(image.Ref) (image.Info, error)
17 17
 }
18 18
 

+ 27
- 26
update/images.go View File

@@ -14,10 +14,11 @@ import (
14 14
 	"github.com/weaveworks/flux/resource"
15 15
 )
16 16
 
17
-type infoMap map[image.CanonicalName][]image.Info
17
+type imageReposMap map[image.CanonicalName][]image.Info
18 18
 
19
-type ImageMap struct {
20
-	images infoMap
19
+// ImageRepos contains a map of image repositories to their images
20
+type ImageRepos struct {
21
+	imageRepos imageReposMap
21 22
 }
22 23
 
23 24
 // LatestImage returns the latest releasable image for a repository
@@ -26,8 +27,8 @@ type ImageMap struct {
26 27
 // in descending order of latestness.) If no such image exists,
27 28
 // returns a zero value and `false`, and the caller can decide whether
28 29
 // that's an error or not.
29
-func (m ImageMap) LatestImage(repo image.Name, tagGlob string) (image.Info, bool) {
30
-	for _, available := range m.images[repo.CanonicalName()] {
30
+func (r ImageRepos) LatestImage(repo image.Name, tagGlob string) (image.Info, bool) {
31
+	for _, available := range r.imageRepos[repo.CanonicalName()] {
31 32
 		tag := available.ID.Tag
32 33
 		// Ignore latest if and only if it's not what the user wants.
33 34
 		if !strings.EqualFold(tagGlob, "latest") && strings.EqualFold(tag, "latest") {
@@ -45,8 +46,8 @@ func (m ImageMap) LatestImage(repo image.Name, tagGlob string) (image.Info, bool
45 46
 
46 47
 // Available returns image.Info entries for all the images in the
47 48
 // named image repository.
48
-func (m ImageMap) Available(repo image.Name) []image.Info {
49
-	if canon, ok := m.images[repo.CanonicalName()]; ok {
49
+func (r ImageRepos) Available(repo image.Name) []image.Info {
50
+	if canon, ok := r.imageRepos[repo.CanonicalName()]; ok {
50 51
 		infos := make([]image.Info, len(canon))
51 52
 		for i := range canon {
52 53
 			infos[i] = canon[i]
@@ -73,50 +74,50 @@ func (cs controllerContainers) Containers(i int) []resource.Container {
73 74
 	return cs[i].Controller.ContainersOrNil()
74 75
 }
75 76
 
76
-// collectUpdateImages is a convenient shim to
77
-// `CollectAvailableImages`.
78
-func collectUpdateImages(registry registry.Registry, updateable []*ControllerUpdate, logger log.Logger) (ImageMap, error) {
79
-	return CollectAvailableImages(registry, controllerContainers(updateable), logger)
77
+// fetchUpdatableImageRepos is a convenient shim to
78
+// `FetchImageRepos`.
79
+func fetchUpdatableImageRepos(registry registry.Registry, updateable []*ControllerUpdate, logger log.Logger) (ImageRepos, error) {
80
+	return FetchImageRepos(registry, controllerContainers(updateable), logger)
80 81
 }
81 82
 
82
-// CollectAvailableImages finds all the known image metadata for
83
+// FetchImageRepos finds all the known image metadata for
83 84
 // containers in the controllers given.
84
-func CollectAvailableImages(reg registry.Registry, cs containers, logger log.Logger) (ImageMap, error) {
85
-	images := infoMap{}
85
+func FetchImageRepos(reg registry.Registry, cs containers, logger log.Logger) (ImageRepos, error) {
86
+	imageRepos := imageReposMap{}
86 87
 	for i := 0; i < cs.Len(); i++ {
87 88
 		for _, container := range cs.Containers(i) {
88
-			images[container.Image.CanonicalName()] = nil
89
+			imageRepos[container.Image.CanonicalName()] = nil
89 90
 		}
90 91
 	}
91
-	for name := range images {
92
-		imageRepo, err := reg.GetRepository(name.Name)
92
+	for repo := range imageRepos {
93
+		sortedRepoImages, err := reg.GetSortedRepositoryImages(repo.Name)
93 94
 		if err != nil {
94 95
 			// Not an error if missing. Use empty images.
95 96
 			if !fluxerr.IsMissing(err) {
96
-				logger.Log("err", errors.Wrapf(err, "fetching image metadata for %s", name))
97
+				logger.Log("err", errors.Wrapf(err, "fetching image metadata for %s", repo))
97 98
 				continue
98 99
 			}
99 100
 		}
100
-		images[name] = imageRepo
101
+		imageRepos[repo] = sortedRepoImages
101 102
 	}
102
-	return ImageMap{images}, nil
103
+	return ImageRepos{imageRepos}, nil
103 104
 }
104 105
 
105
-// Create a map of images. It will check that each image exists.
106
-func exactImages(reg registry.Registry, images []image.Ref) (ImageMap, error) {
107
-	m := infoMap{}
106
+// Create a map of image repos to images. It will check that each image exists.
107
+func exactImageRepos(reg registry.Registry, images []image.Ref) (ImageRepos, error) {
108
+	m := imageReposMap{}
108 109
 	for _, id := range images {
109 110
 		// We must check that the exact images requested actually exist. Otherwise we risk pushing invalid images to git.
110 111
 		exist, err := imageExists(reg, id)
111 112
 		if err != nil {
112
-			return ImageMap{}, errors.Wrap(image.ErrInvalidImageID, err.Error())
113
+			return ImageRepos{}, errors.Wrap(image.ErrInvalidImageID, err.Error())
113 114
 		}
114 115
 		if !exist {
115
-			return ImageMap{}, errors.Wrap(image.ErrInvalidImageID, fmt.Sprintf("image %q does not exist", id))
116
+			return ImageRepos{}, errors.Wrap(image.ErrInvalidImageID, fmt.Sprintf("image %q does not exist", id))
116 117
 		}
117 118
 		m[id.CanonicalName()] = []image.Info{{ID: id}}
118 119
 	}
119
-	return ImageMap{m}, nil
120
+	return ImageRepos{m}, nil
120 121
 }
121 122
 
122 123
 // Checks whether the given image exists in the repository.

+ 4
- 4
update/images_test.go View File

@@ -20,18 +20,18 @@ var (
20 20
 // names (e.g., `index.docker.io/library/alpine`), but we ask
21 21
 // questions in terms of everyday names (e.g., `alpine`).
22 22
 func TestDecanon(t *testing.T) {
23
-	m := ImageMap{infoMap{
23
+	m := ImageRepos{imageReposMap{
24 24
 		name: infos,
25 25
 	}}
26 26
 
27
-	latest, ok := m.LatestImage(mustParseName("weaveworks/helloworld"), "*")
27
+	latest, ok := m.LatestFilteredImage(mustParseName("weaveworks/helloworld"), "*")
28 28
 	if !ok {
29 29
 		t.Error("did not find latest image")
30 30
 	} else if latest.ID.Name != mustParseName("weaveworks/helloworld") {
31 31
 		t.Error("name did not match what was asked")
32 32
 	}
33 33
 
34
-	latest, ok = m.LatestImage(mustParseName("index.docker.io/weaveworks/helloworld"), "*")
34
+	latest, ok = m.LatestFilteredImage(mustParseName("index.docker.io/weaveworks/helloworld"), "*")
35 35
 	if !ok {
36 36
 		t.Error("did not find latest image")
37 37
 	} else if latest.ID.Name != mustParseName("index.docker.io/weaveworks/helloworld") {
@@ -50,7 +50,7 @@ func TestDecanon(t *testing.T) {
50 50
 }
51 51
 
52 52
 func TestAvail(t *testing.T) {
53
-	m := ImageMap{infoMap{name: infos}}
53
+	m := ImageRepos{imageReposMap{name: infos}}
54 54
 	avail := m.Available(mustParseName("weaveworks/goodbyeworld"))
55 55
 	if len(avail) > 0 {
56 56
 		t.Errorf("did not expect available images, but got %#v", avail)

+ 5
- 5
update/release.go View File

@@ -188,20 +188,20 @@ func (s ReleaseSpec) markSkipped(results Result) {
188 188
 // if not, it indicates there's likely some problem with the running
189 189
 // system vs the definitions given in the repo.)
190 190
 func (s ReleaseSpec) calculateImageUpdates(rc ReleaseContext, candidates []*ControllerUpdate, results Result, logger log.Logger) ([]*ControllerUpdate, error) {
191
-	// Compile an `ImageMap` of all relevant images
192
-	var images ImageMap
191
+	// Compile an `ImageRepos` of all relevant images
192
+	var imageRepos ImageRepos
193 193
 	var singleRepo image.CanonicalName
194 194
 	var err error
195 195
 
196 196
 	switch s.ImageSpec {
197 197
 	case ImageSpecLatest:
198
-		images, err = collectUpdateImages(rc.Registry(), candidates, logger)
198
+		imageRepos, err = fetchUpdatableImageRepos(rc.Registry(), candidates, logger)
199 199
 	default:
200 200
 		var ref image.Ref
201 201
 		ref, err = s.ImageSpec.AsRef()
202 202
 		if err == nil {
203 203
 			singleRepo = ref.CanonicalName()
204
-			images, err = exactImages(rc.Registry(), []image.Ref{ref})
204
+			imageRepos, err = exactImageRepos(rc.Registry(), []image.Ref{ref})
205 205
 		}
206 206
 	}
207 207
 
@@ -231,7 +231,7 @@ func (s ReleaseSpec) calculateImageUpdates(rc ReleaseContext, candidates []*Cont
231 231
 		for _, container := range containers {
232 232
 			currentImageID := container.Image
233 233
 
234
-			latestImage, ok := images.LatestImage(currentImageID.Name, "*")
234
+			latestImage, ok := imageRepos.LatestImage(currentImageID.Name, "*")
235 235
 			if !ok {
236 236
 				if currentImageID.CanonicalName() != singleRepo {
237 237
 					ignoredOrSkipped = ReleaseStatusIgnored

Loading…
Cancel
Save