Browse Source

Migrate interfaces to just use flux.ImageID, remove tag parameter

Phil Winder 2 years ago
parent
commit
99d201473c

+ 8
- 0
image.go View File

@@ -119,6 +119,14 @@ func (i ImageID) Components() (host, repo, tag string) {
119 119
 	return i.Host, fmt.Sprintf("%s/%s", i.Namespace, i.Image), i.Tag
120 120
 }
121 121
 
122
+// WithNewTag makes a new copy of an ImageID with a new tag
123
+func (i ImageID) WithNewTag(t string) ImageID {
124
+	var img ImageID
125
+	img = i
126
+	img.Tag = t
127
+	return img
128
+}
129
+
122 130
 // Image can't really be a primitive string only, because we need to also
123 131
 // record information about its creation time. (maybe more in the future)
124 132
 type Image struct {

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

@@ -177,8 +177,8 @@ type manifestKey struct {
177 177
 	username, fullRepositoryPath, reference string
178 178
 }
179 179
 
180
-func NewManifestKey(username string, id flux.ImageID, reference string) (Key, error) {
181
-	return &manifestKey{username, id.HostNamespaceImage(), reference}, nil
180
+func NewManifestKey(username string, id flux.ImageID) (Key, error) {
181
+	return &manifestKey{username, id.HostNamespaceImage(), id.Tag}, nil
182 182
 }
183 183
 
184 184
 func (k *manifestKey) String() string {

+ 5
- 6
registry/client.go View File

@@ -14,7 +14,7 @@ import (
14 14
 // It might be a chache, it might be a real registry.
15 15
 type Client interface {
16 16
 	Tags(id flux.ImageID) ([]string, error)
17
-	Manifest(id flux.ImageID, tag string) (flux.Image, error)
17
+	Manifest(id flux.ImageID) (flux.Image, error)
18 18
 	Cancel()
19 19
 }
20 20
 
@@ -34,8 +34,8 @@ func (a *Remote) Tags(id flux.ImageID) ([]string, error) {
34 34
 
35 35
 // We need to do some adapting here to convert from the return values
36 36
 // from dockerregistry to our domain types.
37
-func (a *Remote) Manifest(id flux.ImageID, tag string) (flux.Image, error) {
38
-	history, err := a.Registry.Manifest(id.NamespaceImage(), tag)
37
+func (a *Remote) Manifest(id flux.ImageID) (flux.Image, error) {
38
+	history, err := a.Registry.Manifest(id.NamespaceImage(), id.Tag)
39 39
 	if err != nil || history == nil {
40 40
 		return flux.Image{}, errors.Wrap(err, "getting remote manifest")
41 41
 	}
@@ -51,7 +51,6 @@ func (a *Remote) Manifest(id flux.ImageID, tag string) (flux.Image, error) {
51 51
 	var topmost v1image
52 52
 	var img flux.Image
53 53
 	img.ID = id
54
-	img.ID.Tag = tag
55 54
 	if len(history) > 0 {
56 55
 		if err = json.Unmarshal([]byte(history[0].V1Compatibility), &topmost); err == nil {
57 56
 			if !topmost.Created.IsZero() {
@@ -91,9 +90,9 @@ func NewCache(creds Credentials, cr cache.Reader, expiry time.Duration, logger l
91 90
 	}
92 91
 }
93 92
 
94
-func (c *Cache) Manifest(id flux.ImageID, tag string) (flux.Image, error) {
93
+func (c *Cache) Manifest(id flux.ImageID) (flux.Image, error) {
95 94
 	creds := c.creds.credsFor(id.Host)
96
-	key, err := cache.NewManifestKey(creds.username, id, tag)
95
+	key, err := cache.NewManifestKey(creds.username, id)
97 96
 	if err != nil {
98 97
 		return flux.Image{}, err
99 98
 	}

+ 3
- 3
registry/integration_test.go View File

@@ -57,7 +57,7 @@ func TestWarming_WarmerWriteCacheRead(t *testing.T) {
57 57
 			return []flux.ImageID{id}
58 58
 		},
59 59
 		logger.With("component", "queue"),
60
-		time.Second,
60
+		100*time.Millisecond,
61 61
 	)
62 62
 
63 63
 	w := Warmer{
@@ -89,9 +89,9 @@ Loop:
89 89
 	for {
90 90
 		select {
91 91
 		case <-timeout.C:
92
-			return
92
+			t.Fatal("Cache timeout")
93 93
 		case <-tick.C:
94
-			_, err := r.GetImage(id, "latest")
94
+			_, err := r.GetImage(id)
95 95
 			if err == nil {
96 96
 				break Loop
97 97
 			}

+ 4
- 5
registry/mock.go View File

@@ -17,7 +17,7 @@ type mockRemote struct {
17 17
 	err  error
18 18
 }
19 19
 
20
-type ManifestFunc func(id flux.ImageID, tag string) (flux.Image, error)
20
+type ManifestFunc func(id flux.ImageID) (flux.Image, error)
21 21
 type TagsFunc func(id flux.ImageID) ([]string, error)
22 22
 type mockDockerClient struct {
23 23
 	manifest ManifestFunc
@@ -31,8 +31,8 @@ func NewMockClient(manifest ManifestFunc, tags TagsFunc) Client {
31 31
 	}
32 32
 }
33 33
 
34
-func (m *mockDockerClient) Manifest(id flux.ImageID, tag string) (flux.Image, error) {
35
-	return m.manifest(id, tag)
34
+func (m *mockDockerClient) Manifest(id flux.ImageID) (flux.Image, error) {
35
+	return m.manifest(id)
36 36
 }
37 37
 
38 38
 func (m *mockDockerClient) Tags(id flux.ImageID) ([]string, error) {
@@ -82,8 +82,7 @@ func (m *mockRegistry) GetRepository(id flux.ImageID) ([]flux.Image, error) {
82 82
 	return imgs, m.err
83 83
 }
84 84
 
85
-func (m *mockRegistry) GetImage(id flux.ImageID, tag string) (flux.Image, error) {
86
-	id.Tag = tag
85
+func (m *mockRegistry) GetImage(id flux.ImageID) (flux.Image, error) {
87 86
 	if len(m.imgs) > 0 {
88 87
 		for _, i := range m.imgs {
89 88
 			if i.ID.String() == id.String() {

+ 4
- 4
registry/monitoring.go View File

@@ -54,9 +54,9 @@ func (m *instrumentedRegistry) GetRepository(id flux.ImageID) (res []flux.Image,
54 54
 	return
55 55
 }
56 56
 
57
-func (m *instrumentedRegistry) GetImage(id flux.ImageID, tag string) (res flux.Image, err error) {
57
+func (m *instrumentedRegistry) GetImage(id flux.ImageID) (res flux.Image, err error) {
58 58
 	start := time.Now()
59
-	res, err = m.next.GetImage(id, tag)
59
+	res, err = m.next.GetImage(id)
60 60
 	fetchDuration.With(
61 61
 		fluxmetrics.LabelSuccess, strconv.FormatBool(err == nil),
62 62
 	).Observe(time.Since(start).Seconds())
@@ -75,9 +75,9 @@ func NewInstrumentedClient(next Client) Client {
75 75
 	}
76 76
 }
77 77
 
78
-func (m *instrumentedClient) Manifest(id flux.ImageID, tag string) (res flux.Image, err error) {
78
+func (m *instrumentedClient) Manifest(id flux.ImageID) (res flux.Image, err error) {
79 79
 	start := time.Now()
80
-	res, err = m.next.Manifest(id, tag)
80
+	res, err = m.next.Manifest(id)
81 81
 	requestDuration.With(
82 82
 		LabelRequestKind, RequestKindMetadata,
83 83
 		fluxmetrics.LabelSuccess, strconv.FormatBool(err == nil),

+ 4
- 4
registry/registry.go View File

@@ -26,7 +26,7 @@ const (
26 26
 // The Registry interface is a domain specific API to access container registries.
27 27
 type Registry interface {
28 28
 	GetRepository(id flux.ImageID) ([]flux.Image, error)
29
-	GetImage(id flux.ImageID, tag string) (flux.Image, error)
29
+	GetImage(id flux.ImageID) (flux.Image, error)
30 30
 }
31 31
 
32 32
 type registry struct {
@@ -74,12 +74,12 @@ func (reg *registry) GetRepository(id flux.ImageID) ([]flux.Image, error) {
74 74
 }
75 75
 
76 76
 // Get a single Image from the registry if it exists
77
-func (reg *registry) GetImage(id flux.ImageID, tag string) (_ flux.Image, err error) {
77
+func (reg *registry) GetImage(id flux.ImageID) (_ flux.Image, err error) {
78 78
 	rem, err := reg.newClient(id)
79 79
 	if err != nil {
80 80
 		return
81 81
 	}
82
-	return rem.Manifest(id, tag)
82
+	return rem.Manifest(id)
83 83
 }
84 84
 
85 85
 func (reg *registry) newClient(id flux.ImageID) (Client, error) {
@@ -106,7 +106,7 @@ func (reg *registry) tagsToRepository(client Client, id flux.ImageID, tags []str
106 106
 	for i := 0; i < maxConcurrency; i++ {
107 107
 		go func() {
108 108
 			for tag := range toFetch {
109
-				image, err := client.Manifest(id, tag)
109
+				image, err := client.Manifest(id.WithNewTag(tag)) // Copy the imageID to avoid races
110 110
 				if err != nil {
111 111
 					reg.Logger.Log("registry-metadata-err", err)
112 112
 				}

+ 5
- 4
registry/registry_test.go View File

@@ -31,7 +31,7 @@ var (
31 31
 var (
32 32
 	testTags = []string{testTagStr, "anotherTag"}
33 33
 	mClient  = NewMockClient(
34
-		func(repository flux.ImageID, tag string) (flux.Image, error) {
34
+		func(repository flux.ImageID) (flux.Image, error) {
35 35
 			img, _ := flux.ParseImage(testImageStr, time.Time{})
36 36
 			return img, nil
37 37
 		},
@@ -66,7 +66,7 @@ func TestRegistry_GetRepositoryFactoryError(t *testing.T) {
66 66
 
67 67
 func TestRegistry_GetRepositoryManifestError(t *testing.T) {
68 68
 	errClient := NewMockClient(
69
-		func(repository flux.ImageID, tag string) (flux.Image, error) {
69
+		func(repository flux.ImageID) (flux.Image, error) {
70 70
 			return flux.Image{}, errors.New("")
71 71
 		},
72 72
 		func(repository flux.ImageID) ([]string, error) {
@@ -110,7 +110,8 @@ func TestRemoteFactory_RawClient(t *testing.T) {
110 110
 	if err != nil {
111 111
 		t.Fatal(err)
112 112
 	}
113
-	newImg, err := client.Manifest(id, tags[0])
113
+	id.Tag = tags[0]
114
+	newImg, err := client.Manifest(id)
114 115
 	if err != nil {
115 116
 		t.Fatal(err)
116 117
 	}
@@ -133,7 +134,7 @@ func TestRemoteFactory_InvalidHost(t *testing.T) {
133 134
 	if err != nil {
134 135
 		return
135 136
 	}
136
-	_, err = client.Manifest(invalidId, invalidId.Tag)
137
+	_, err = client.Manifest(invalidId)
137 138
 	if err == nil {
138 139
 		t.Fatal("Expected error due to invalid host but got none.")
139 140
 	}

+ 3
- 2
registry/warming.go View File

@@ -85,7 +85,8 @@ func (w *Warmer) warm(id flux.ImageID) {
85 85
 	for _, tag := range tags {
86 86
 		// See if we have the manifest already cached
87 87
 		// We don't want to re-download a manifest again.
88
-		key, err := cache.NewManifestKey(username, id, tag)
88
+		i := id.WithNewTag(tag)
89
+		key, err := cache.NewManifestKey(username, i)
89 90
 		if err != nil {
90 91
 			w.Logger.Log("err", errors.Wrap(err, "creating key for memcache"))
91 92
 			return
@@ -96,7 +97,7 @@ func (w *Warmer) warm(id flux.ImageID) {
96 97
 		}
97 98
 
98 99
 		// Get the image from the remote
99
-		img, err := client.Manifest(id, tag)
100
+		img, err := client.Manifest(i)
100 101
 		if err != nil {
101 102
 			w.Logger.Log("err", err.Error())
102 103
 			continue

+ 1
- 1
update/images.go View File

@@ -91,7 +91,7 @@ func exactImages(reg registry.Registry, images []flux.ImageID) (ImageMap, error)
91 91
 // Checks whether the given image exists in the repository.
92 92
 // Return true if exist, false otherwise
93 93
 func imageExists(reg registry.Registry, imageID flux.ImageID) (bool, error) {
94
-	_, err := reg.GetImage(imageID, imageID.Tag)
94
+	_, err := reg.GetImage(imageID)
95 95
 	if err != nil {
96 96
 		return false, nil
97 97
 	}

Loading…
Cancel
Save