Browse Source

Refactor out image id stuff to fix broken dockerhub updates

Paul Bellamy 3 years ago
parent
commit
7eada3a602
No account linked to committer's email address

+ 8
- 4
automator/automator.go View File

@@ -130,8 +130,12 @@ func (a *Automator) handleAutomatedInstanceJob(logger log.Logger, j *jobs.Job) (
130 130
 	images := instance.ImageMap{}
131 131
 	for _, service := range services {
132 132
 		for _, container := range service.ContainersOrNil() {
133
-			repo := flux.ParseImageID(container.Image).Repository()
134
-			images[repo] = nil
133
+			id, err := flux.ParseImageID(container.Image)
134
+			if err != nil {
135
+				// Container is running an invalid image? what?
136
+				continue
137
+			}
138
+			images[id.Repository()] = nil
135 139
 		}
136 140
 	}
137 141
 	for repo := range images {
@@ -169,14 +173,14 @@ func (a *Automator) handleAutomatedInstanceJob(logger log.Logger, j *jobs.Job) (
169 173
 			Key: strings.Join([]string{
170 174
 				jobs.ReleaseJob,
171 175
 				string(params.InstanceID),
172
-				string(imageID),
176
+				imageID.String(),
173 177
 				"automated",
174 178
 			}, "|"),
175 179
 			Method:   jobs.ReleaseJob,
176 180
 			Priority: jobs.PriorityBackground,
177 181
 			Params: jobs.ReleaseJobParams{
178 182
 				ServiceSpecs: serviceSpecs,
179
-				ImageSpec:    flux.ImageSpec(imageID),
183
+				ImageSpec:    flux.ImageSpec(imageID.String()),
180 184
 				Kind:         flux.ReleaseKindExecute,
181 185
 			},
182 186
 		})

+ 23
- 0
image.go View File

@@ -0,0 +1,23 @@
1
+package flux
2
+
3
+import (
4
+	"time"
5
+)
6
+
7
+// Image can't really be a primitive string only, because we need to also
8
+// record information about it's creation time. (maybe more in the future)
9
+type Image struct {
10
+	ImageID
11
+	CreatedAt *time.Time `json:",omitempty"`
12
+}
13
+
14
+func ParseImage(s string, createdAt *time.Time) (Image, error) {
15
+	id, err := ParseImageID(s)
16
+	if err != nil {
17
+		return Image{}, err
18
+	}
19
+	return Image{
20
+		ImageID:   id,
21
+		CreatedAt: createdAt,
22
+	}, nil
23
+}

+ 103
- 0
image_id.go View File

@@ -0,0 +1,103 @@
1
+package flux
2
+
3
+import (
4
+	"fmt"
5
+	"strings"
6
+
7
+	"github.com/pkg/errors"
8
+)
9
+
10
+const (
11
+	dockerHubHost    = "index.docker.io"
12
+	dockerHubLibrary = "library"
13
+)
14
+
15
+var (
16
+	ErrInvalidImageID   = errors.New("invalid image ID")
17
+	ErrBlankImageID     = errors.Wrap(ErrInvalidImageID, "blank image name")
18
+	ErrMalformedImageID = errors.Wrap(ErrInvalidImageID, `expected image name as either <image>:<tag> or just <image>`)
19
+)
20
+
21
+// ImageID is a fully qualified name that refers to a particular Image.
22
+// It is in the format: host[:port]/Namespace/Image[:tag]
23
+// Here, we refer to the "name" == Namespace/Image
24
+type ImageID struct {
25
+	Host, Namespace, Image, Tag string
26
+}
27
+
28
+func ParseImageID(s string) (ImageID, error) {
29
+	if s == "" {
30
+		return ImageID{}, ErrBlankImageID
31
+	}
32
+	var img ImageID
33
+	parts := strings.Split(s, ":")
34
+	switch len(parts) {
35
+	case 0:
36
+		return ImageID{}, ErrMalformedImageID
37
+	case 1:
38
+		img.Tag = "latest"
39
+	case 2:
40
+		img.Tag = parts[1]
41
+		s = parts[0]
42
+	default:
43
+		return ImageID{}, ErrMalformedImageID
44
+	}
45
+	if s == "" {
46
+		return ImageID{}, ErrBlankImageID
47
+	}
48
+	parts = strings.Split(s, "/")
49
+	switch len(parts) {
50
+	case 1:
51
+		img.Host = dockerHubHost
52
+		img.Namespace = dockerHubLibrary
53
+		img.Image = parts[0]
54
+	case 2:
55
+		img.Host = dockerHubHost
56
+		img.Namespace = parts[0]
57
+		img.Image = parts[1]
58
+	case 3:
59
+		img.Host = parts[0]
60
+		img.Namespace = parts[1]
61
+		img.Image = parts[2]
62
+	default:
63
+		return ImageID{}, ErrMalformedImageID
64
+	}
65
+	return img, nil
66
+}
67
+
68
+// Fully qualified name
69
+func (i ImageID) String() string {
70
+	if i.Image == "" {
71
+		return "" // Doesn't make sense to return anything if it doesn't even have an image
72
+	}
73
+	var ta string
74
+	if i.Tag != "" {
75
+		ta = fmt.Sprintf(":%s", i.Tag)
76
+	}
77
+	return fmt.Sprintf("%s%s", i.Repository(), ta)
78
+}
79
+
80
+// Repository returns the short version of an image's repository (trimming if dockerhub)
81
+func (i ImageID) Repository() string {
82
+	r := i.HostNamespaceImage()
83
+	r = strings.TrimPrefix(r, dockerHubHost+"/")
84
+	r = strings.TrimPrefix(r, dockerHubLibrary+"/")
85
+	return r
86
+}
87
+
88
+// HostNamespaceImage includes all parts of the image, even if it is from dockerhub.
89
+func (i ImageID) HostNamespaceImage() string {
90
+	return fmt.Sprintf("%s/%s/%s", i.Host, i.Namespace, i.Image)
91
+}
92
+
93
+func (i ImageID) NamespaceImage() string {
94
+	return fmt.Sprintf("%s/%s", i.Namespace, i.Image)
95
+}
96
+
97
+func (i ImageID) FullID() string {
98
+	return fmt.Sprintf("%s/%s/%s:%s", i.Host, i.Namespace, i.Image, i.Tag)
99
+}
100
+
101
+func (i ImageID) Components() (host, repo, tag string) {
102
+	return i.Host, fmt.Sprintf("%s/%s", i.Namespace, i.Image), i.Tag
103
+}

registry/image_test.go → image_id_test.go View File

@@ -1,22 +1,23 @@
1
-package registry
1
+package flux
2 2
 
3 3
 import (
4 4
 	"fmt"
5 5
 	"testing"
6 6
 )
7 7
 
8
-func TestImage_ParseImage(t *testing.T) {
8
+func TestImageID_ParseImageID(t *testing.T) {
9 9
 	for _, x := range []struct {
10 10
 		test     string
11 11
 		expected string
12 12
 	}{
13
-		{"alpine", "index.docker.io/library/alpine:latest"},
14
-		{"library/alpine", "index.docker.io/library/alpine:latest"},
13
+		{"alpine", "alpine:latest"},
14
+		{"library/alpine", "alpine:latest"},
15
+		{"alpine:mytag", "alpine:mytag"},
15 16
 		{"quay.io/library/alpine", "quay.io/library/alpine:latest"},
16 17
 		{"quay.io/library/alpine:latest", "quay.io/library/alpine:latest"},
17 18
 		{"quay.io/library/alpine:mytag", "quay.io/library/alpine:mytag"},
18 19
 	} {
19
-		i, err := ParseImage(x.test, nil)
20
+		i, err := ParseImageID(x.test)
20 21
 		if err != nil {
21 22
 			t.Fatalf("Failed parsing %q, expected %q", x.test, x.expected)
22 23
 		}
@@ -26,29 +27,30 @@ func TestImage_ParseImage(t *testing.T) {
26 27
 	}
27 28
 }
28 29
 
29
-func TestImage_ParseImageErrorCases(t *testing.T) {
30
+func TestImageID_ParseImageIDErrorCases(t *testing.T) {
30 31
 	for _, x := range []struct {
31 32
 		test string
32 33
 	}{
33 34
 		{""},
35
+		{":tag"},
34 36
 		{"alpine::"},
35 37
 		{"alpine:invalid:"},
36 38
 		{"/too/many/slashes/"},
37 39
 	} {
38
-		_, err := ParseImage(x.test, nil)
40
+		_, err := ParseImageID(x.test)
39 41
 		if err == nil {
40 42
 			t.Fatalf("Expected parse failure for %q", x.test)
41 43
 		}
42 44
 	}
43 45
 }
44 46
 
45
-func TestImage_TestComponents(t *testing.T) {
47
+func TestImageID_TestComponents(t *testing.T) {
46 48
 	host := "quay.io"
47 49
 	namespace := "namespace"
48 50
 	image := "myrepo"
49 51
 	tag := "mytag"
50 52
 	fqn := fmt.Sprintf("%v/%v/%v:%v", host, namespace, image, tag)
51
-	i, err := ParseImage(fqn, nil)
53
+	i, err := ParseImageID(fqn)
52 54
 	if err != nil {
53 55
 		t.Fatal(err)
54 56
 	}

+ 20
- 6
instance/instance.go View File

@@ -101,8 +101,12 @@ func (h *Instance) CollectAvailableImages(services []platform.Service) (ImageMap
101 101
 	images := ImageMap{}
102 102
 	for _, service := range services {
103 103
 		for _, container := range service.ContainersOrNil() {
104
-			repo := flux.ParseImageID(container.Image).Repository()
105
-			images[repo] = nil
104
+			id, err := flux.ParseImageID(container.Image)
105
+			if err != nil {
106
+				// container is running an invalid image id? what?
107
+				return nil, err
108
+			}
109
+			images[id.Repository()] = nil
106 110
 		}
107 111
 	}
108 112
 	for repo := range images {
@@ -116,8 +120,13 @@ func (h *Instance) CollectAvailableImages(services []platform.Service) (ImageMap
116 120
 		}
117 121
 		res := make([]flux.ImageDescription, len(imageRepo))
118 122
 		for i, im := range imageRepo {
123
+			id, err := flux.ParseImageID(im.String())
124
+			if err != nil {
125
+				// registry returned an invalid image id
126
+				return nil, err
127
+			}
119 128
 			res[i] = flux.ImageDescription{
120
-				ID:        flux.ParseImageID(im.String()),
129
+				ID:        id,
121 130
 				CreatedAt: im.CreatedAt,
122 131
 			}
123 132
 		}
@@ -138,8 +147,13 @@ func (h *Instance) GetRepository(repo string) (res []flux.ImageDescription, err
138 147
 	}
139 148
 	res = make([]flux.ImageDescription, len(images))
140 149
 	for i, im := range images {
150
+		id, err := flux.ParseImageID(im.String())
151
+		if err != nil {
152
+			// registry returned an invalid image id
153
+			return nil, err
154
+		}
141 155
 		res[i] = flux.ImageDescription{
142
-			ID:        flux.ParseImageID(im.String()),
156
+			ID:        id,
143 157
 			CreatedAt: im.CreatedAt,
144 158
 		}
145 159
 	}
@@ -165,9 +179,9 @@ func (h *Instance) ExactImages(images []flux.ImageID) (ImageMap, error) {
165 179
 
166 180
 // Checks whether the given image exists in the repository.
167 181
 // Return true if exist, false otherwise
168
-func (h *Instance) imageExists(image flux.ImageID) (bool, error) {
182
+func (h *Instance) imageExists(imageID flux.ImageID) (bool, error) {
169 183
 	// Use this method to parse the image, because it is safe. I.e. it will error and inform the user if it is malformed.
170
-	img, err := registry.ParseImage(string(image), nil)
184
+	img, err := flux.ParseImage(imageID.String(), nil)
171 185
 	if err != nil {
172 186
 		return false, err
173 187
 	}

+ 6
- 4
instance/instance_test.go View File

@@ -8,8 +8,8 @@ import (
8 8
 
9 9
 var (
10 10
 	exampleImage   = "index.docker.io/owner/repo:tag"
11
-	parsedImage, _ = registry.ParseImage(exampleImage, nil)
12
-	testRegistry   = registry.NewMockRegistry([]registry.Image{
11
+	parsedImage, _ = flux.ParseImage(exampleImage, nil)
12
+	testRegistry   = registry.NewMockRegistry([]flux.Image{
13 13
 		parsedImage,
14 14
 	}, nil)
15 15
 )
@@ -27,7 +27,8 @@ func TestInstance_ImageExists(t *testing.T) {
27 27
 }
28 28
 
29 29
 func testImageExists(t *testing.T, i Instance, image string, expected bool) {
30
-	b, err := i.imageExists(flux.ParseImageID(image))
30
+	id, _ := flux.ParseImageID(image)
31
+	b, err := i.imageExists(id)
31 32
 	if err != nil {
32 33
 		t.Fatalf("%v: error when requesting image %q", err.Error(), image)
33 34
 	}
@@ -40,7 +41,8 @@ func TestInstance_ErrWhenBlank(t *testing.T) {
40 41
 	i := Instance{
41 42
 		registry: testRegistry,
42 43
 	}
43
-	_, err := i.imageExists(flux.ParseImageID(""))
44
+	id, _ := flux.ParseImageID("")
45
+	_, err := i.imageExists(id)
44 46
 	if err == nil {
45 47
 		t.Fatal("Was expecting error")
46 48
 	}

+ 10
- 9
platform/kubernetes/update.go View File

@@ -18,9 +18,9 @@ import (
18 18
 //
19 19
 // This function has many additional requirements that are likely in flux. Read
20 20
 // the source to learn about them.
21
-func UpdatePodController(def []byte, newImageName string, trace io.Writer) ([]byte, error) {
21
+func UpdatePodController(def []byte, newImageID flux.ImageID, trace io.Writer) ([]byte, error) {
22 22
 	var buf bytes.Buffer
23
-	err := tryUpdate(string(def), newImageName, trace, &buf)
23
+	err := tryUpdate(string(def), newImageID, trace, &buf)
24 24
 	return buf.Bytes(), err
25 25
 }
26 26
 
@@ -67,9 +67,7 @@ func UpdatePodController(def []byte, newImageName string, trace io.Writer) ([]by
67 67
 //         ports:
68 68
 //         - containerPort: 80
69 69
 // ```
70
-func tryUpdate(def, newImageStr string, trace io.Writer, out io.Writer) error {
71
-	newImage := flux.ParseImageID(newImageStr)
72
-
70
+func tryUpdate(def string, newImage flux.ImageID, trace io.Writer, out io.Writer) error {
73 71
 	nameRE := multilineRE(
74 72
 		`metadata:\s*`,
75 73
 		`(?:  .*\n)*  name:\s*"?([\w-]+)"?\s*`,
@@ -84,17 +82,20 @@ func tryUpdate(def, newImageStr string, trace io.Writer, out io.Writer) error {
84 82
 	imageRE := multilineRE(
85 83
 		`      containers:.*`,
86 84
 		`(?:      .*\n)*(?:  ){3,4}- name:\s*"?([\w-]+)"?(?:\s.*)?`,
87
-		`(?:  ){4,5}image:\s*"?(`+newImage.Repository()+`:[\w][\w.-]{0,127})"?(\s.*)?`,
85
+		`(?:  ){4,5}image:\s*"?(`+newImage.Repository()+`(:[\w][\w.-]{0,127})?)"?(\s.*)?`,
88 86
 	)
89 87
 	// tag part of regexp from
90 88
 	// https://github.com/docker/distribution/blob/master/reference/regexp.go#L36
91 89
 
92 90
 	matches = imageRE.FindStringSubmatch(def)
93 91
 	if matches == nil || len(matches) < 3 {
94
-		return fmt.Errorf("Could not find image name")
92
+		return fmt.Errorf("Could not find image name: %s", newImage.Repository())
95 93
 	}
96 94
 	containerName := matches[1]
97
-	oldImage := flux.ParseImageID(matches[2])
95
+	oldImage, err := flux.ParseImageID(matches[2])
96
+	if err != nil {
97
+		return err
98
+	}
98 99
 	fmt.Fprintf(trace, "Found container %q using image %v in fragment:\n\n%s\n\n", containerName, oldImage, matches[0])
99 100
 
100 101
 	if oldImage.Repository() != newImage.Repository() {
@@ -145,7 +146,7 @@ func tryUpdate(def, newImageStr string, trace io.Writer, out io.Writer) error {
145 146
 		`((?:  ){3,4}- name:\s*`+containerName+`)`,
146 147
 		`((?:  ){4,5}image:\s*) .*`,
147 148
 	)
148
-	replaceImage := fmt.Sprintf("$1\n$2 %s$3", string(newImage))
149
+	replaceImage := fmt.Sprintf("$1\n$2 %s$3", newImage.String())
149 150
 	withNewImage := replaceImageRE.ReplaceAllString(withNewLabels, replaceImage)
150 151
 
151 152
 	fmt.Fprint(out, withNewImage)

+ 49
- 2
platform/kubernetes/update_test.go View File

@@ -5,11 +5,17 @@ import (
5 5
 	"fmt"
6 6
 	"os"
7 7
 	"testing"
8
+
9
+	"github.com/weaveworks/flux"
8 10
 )
9 11
 
10 12
 func testUpdate(t *testing.T, name, caseIn, updatedImage, caseOut string) {
13
+	id, err := flux.ParseImageID(updatedImage)
14
+	if err != nil {
15
+		t.Fatal(err)
16
+	}
11 17
 	var trace, out bytes.Buffer
12
-	if err := tryUpdate(caseIn, updatedImage, &trace, &out); err != nil {
18
+	if err := tryUpdate(caseIn, id, &trace, &out); err != nil {
13 19
 		fmt.Fprintln(os.Stderr, "Failed:", name)
14 20
 		fmt.Fprintf(os.Stderr, "--- TRACE ---\n"+trace.String()+"\n---\n")
15 21
 		t.Fatal(err)
@@ -17,7 +23,7 @@ func testUpdate(t *testing.T, name, caseIn, updatedImage, caseOut string) {
17 23
 	if string(out.Bytes()) != caseOut {
18 24
 		fmt.Fprintln(os.Stderr, "Failed:", name)
19 25
 		fmt.Fprintf(os.Stderr, "--- TRACE ---\n"+trace.String()+"\n---\n")
20
-		t.Fatalf("Did not get expected result, instead got\n\n%s", string(out.Bytes()))
26
+		t.Fatalf("Did not get expected result:\n\n%s\n\nInstead got:\n\n%s", caseOut, string(out.Bytes()))
21 27
 	}
22 28
 }
23 29
 
@@ -28,6 +34,7 @@ func TestUpdates(t *testing.T) {
28 34
 		{"old version like number", case2out, case2reverseImage, case2},
29 35
 		{"name label out of order", case3, case3image, case3out},
30 36
 		{"version (tag) with dots", case4, case4image, case4out},
37
+		{"minimal dockerhub image name", case5, case5image, case5out},
31 38
 	} {
32 39
 		testUpdate(t, c[0], c[1], c[2], c[3])
33 40
 	}
@@ -321,3 +328,43 @@ spec:
321 328
               - all
322 329
           readOnlyRootFilesystem: true
323 330
 `
331
+
332
+const case5 = `---
333
+apiVersion: extensions/v1beta1
334
+kind: Deployment
335
+metadata:
336
+  name: nginx
337
+spec:
338
+  replicas: 1
339
+  template:
340
+    metadata:
341
+      labels:
342
+        name: nginx
343
+    spec:
344
+      containers:
345
+      - name: nginx
346
+        image: nginx
347
+        ports:
348
+        - containerPort: 80
349
+`
350
+
351
+const case5image = "nginx:1.10-alpine"
352
+
353
+const case5out = `---
354
+apiVersion: extensions/v1beta1
355
+kind: Deployment
356
+metadata:
357
+  name: nginx
358
+spec:
359
+  replicas: 1
360
+  template:
361
+    metadata:
362
+      labels:
363
+        name: nginx
364
+    spec:
365
+      containers:
366
+      - name: nginx
367
+        image: nginx:1.10-alpine
368
+        ports:
369
+        - containerPort: 80
370
+`

+ 0
- 94
registry/image.go View File

@@ -1,94 +0,0 @@
1
-package registry
2
-
3
-import (
4
-	"fmt"
5
-	"strings"
6
-	"time"
7
-)
8
-
9
-const (
10
-	dockerHubHost    = "index.docker.io"
11
-	dockerHubLibrary = "library"
12
-)
13
-
14
-// An Image is a fully qualified name that refers to a particular Image.
15
-// It is in the format: host[:port]/Namespace/Image[:tag]
16
-// Here, we refer to the "name" == Namespace/Image
17
-
18
-// Image can't really be a primitive string only, because we need to also
19
-// record information about it's creation time. (maybe more in the future)
20
-type Image struct {
21
-	Host, Namespace, Image, Tag string
22
-	CreatedAt                   *time.Time `json:",omitempty"`
23
-}
24
-
25
-func ParseImage(s string, createdAt *time.Time) (img Image, err error) {
26
-	img = Image{
27
-		CreatedAt: createdAt,
28
-	}
29
-	if s == "" {
30
-		err = fmt.Errorf(`expected Image name as either <Image>:<tag> or just <Image>`)
31
-		return
32
-	}
33
-	parts := strings.Split(s, ":")
34
-	switch len(parts) {
35
-	case 0:
36
-		err = fmt.Errorf(`expected Image name as either <Image>:<tag> or just <Image>`)
37
-		return
38
-	case 1:
39
-		img.Tag = "latest"
40
-		break
41
-	case 2:
42
-		img.Tag = parts[1]
43
-		s = parts[0]
44
-	default:
45
-		err = fmt.Errorf(`expected Image name as either <Image>:<tag> or just <Image>`)
46
-		return
47
-	}
48
-	parts = strings.Split(s, "/")
49
-	switch len(parts) {
50
-	case 1:
51
-		img.Host = dockerHubHost
52
-		img.Namespace = dockerHubLibrary
53
-		img.Image = parts[0]
54
-	case 2:
55
-		img.Host = dockerHubHost
56
-		img.Namespace = parts[0]
57
-		img.Image = parts[1]
58
-	case 3:
59
-		img.Host = parts[0]
60
-		img.Namespace = parts[1]
61
-		img.Image = parts[2]
62
-	default:
63
-		err = fmt.Errorf(`expected Image name as either <Image>:<tag> or just <Image>`)
64
-		return
65
-	}
66
-	return
67
-}
68
-
69
-// Fully qualified name
70
-func (i Image) String() string {
71
-	if i.Image == "" {
72
-		return "" // Doesn't make sense to return anything if it doesn't even have an image
73
-	}
74
-	var ho, na, ta string
75
-	if i.Host != "" {
76
-		ho = fmt.Sprintf("%s/", i.Host)
77
-	}
78
-	if i.Namespace != "" {
79
-		na = fmt.Sprintf("%s/", i.Namespace)
80
-	}
81
-	if i.Tag != "" {
82
-		ta = fmt.Sprintf(":%s", i.Tag)
83
-	}
84
-
85
-	return fmt.Sprintf("%s%s%s%s", ho, na, i.Image, ta)
86
-}
87
-
88
-func (i Image) HostNamespaceImage() string {
89
-	return fmt.Sprintf("%s/%s/%s", i.Host, i.Namespace, i.Image)
90
-}
91
-
92
-func (i Image) NamespaceImage() string {
93
-	return fmt.Sprintf("%s/%s", i.Namespace, i.Image)
94
-}

+ 10
- 9
registry/mock.go View File

@@ -3,6 +3,7 @@ package registry
3 3
 import (
4 4
 	"github.com/docker/distribution/manifest/schema1"
5 5
 	"github.com/pkg/errors"
6
+
6 7
 	"github.com/weaveworks/flux"
7 8
 )
8 9
 
@@ -12,12 +13,12 @@ type mockClientAdapter struct {
12 13
 }
13 14
 
14 15
 type mockRemote struct {
15
-	img  Image
16
+	img  flux.Image
16 17
 	tags []string
17 18
 	err  error
18 19
 }
19 20
 
20
-func NewMockRemote(img Image, tags []string, err error) Remote {
21
+func NewMockRemote(img flux.Image, tags []string, err error) Remote {
21 22
 	return &mockRemote{
22 23
 		img:  img,
23 24
 		tags: tags,
@@ -29,9 +30,9 @@ func (r *mockRemote) Tags(repository Repository) ([]string, error) {
29 30
 	return r.tags, r.err
30 31
 }
31 32
 
32
-func (r *mockRemote) Manifest(repository Repository, tag string) (Image, error) {
33
+func (r *mockRemote) Manifest(repository Repository, tag string) (flux.Image, error) {
33 34
 	if tag == "error" {
34
-		return Image{}, errors.New("Mock is set to error when tag == error")
35
+		return flux.Image{}, errors.New("Mock is set to error when tag == error")
35 36
 	}
36 37
 	return r.img, r.err
37 38
 }
@@ -76,22 +77,22 @@ func (m *mockRemoteFactory) CreateFor(repository string) (Remote, error) {
76 77
 }
77 78
 
78 79
 type mockRegistry struct {
79
-	imgs []Image
80
+	imgs []flux.Image
80 81
 	err  error
81 82
 }
82 83
 
83
-func NewMockRegistry(images []Image, err error) Registry {
84
+func NewMockRegistry(images []flux.Image, err error) Registry {
84 85
 	return &mockRegistry{
85 86
 		imgs: images,
86 87
 		err:  err,
87 88
 	}
88 89
 }
89 90
 
90
-func (m *mockRegistry) GetRepository(repository Repository) ([]Image, error) {
91
+func (m *mockRegistry) GetRepository(repository Repository) ([]flux.Image, error) {
91 92
 	return m.imgs, m.err
92 93
 }
93 94
 
94
-func (m *mockRegistry) GetImage(repository Repository, tag string) (Image, error) {
95
+func (m *mockRegistry) GetImage(repository Repository, tag string) (flux.Image, error) {
95 96
 	if len(m.imgs) > 0 {
96 97
 		for _, i := range m.imgs {
97 98
 			if i.String() == repository.ToImage(tag).String() {
@@ -99,5 +100,5 @@ func (m *mockRegistry) GetImage(repository Repository, tag string) (Image, error
99 100
 			}
100 101
 		}
101 102
 	}
102
-	return Image{}, errors.New("not found")
103
+	return flux.Image{}, errors.New("not found")
103 104
 }

+ 5
- 4
registry/monitoring.go View File

@@ -2,10 +2,11 @@
2 2
 package registry
3 3
 
4 4
 import (
5
+	"fmt"
5 6
 	"strconv"
6 7
 	"time"
7 8
 
8
-	"fmt"
9
+	"github.com/weaveworks/flux"
9 10
 	fluxmetrics "github.com/weaveworks/flux/metrics"
10 11
 )
11 12
 
@@ -23,7 +24,7 @@ func NewInstrumentedRegistry(next Registry, metrics Metrics) InstrumentedRegistr
23 24
 	}
24 25
 }
25 26
 
26
-func (m *instrumentedRegistry) GetRepository(repository Repository) (res []Image, err error) {
27
+func (m *instrumentedRegistry) GetRepository(repository Repository) (res []flux.Image, err error) {
27 28
 	start := time.Now()
28 29
 	res, err = m.next.GetRepository(repository)
29 30
 	m.metrics.FetchDuration.With(
@@ -33,7 +34,7 @@ func (m *instrumentedRegistry) GetRepository(repository Repository) (res []Image
33 34
 	return
34 35
 }
35 36
 
36
-func (m *instrumentedRegistry) GetImage(repository Repository, tag string) (res Image, err error) {
37
+func (m *instrumentedRegistry) GetImage(repository Repository, tag string) (res flux.Image, err error) {
37 38
 	start := time.Now()
38 39
 	res, err = m.next.GetImage(repository, tag)
39 40
 	m.metrics.FetchDuration.With(
@@ -57,7 +58,7 @@ func NewInstrumentedRemote(next Remote, metrics Metrics) Remote {
57 58
 	}
58 59
 }
59 60
 
60
-func (m *instrumentedRemote) Manifest(repository Repository, tag string) (res Image, err error) {
61
+func (m *instrumentedRemote) Manifest(repository Repository, tag string) (res flux.Image, err error) {
61 62
 	start := time.Now()
62 63
 	res, err = m.next.Manifest(repository, tag)
63 64
 	m.metrics.RequestDuration.With(

+ 12
- 9
registry/registry.go View File

@@ -2,9 +2,12 @@
2 2
 package registry
3 3
 
4 4
 import (
5
-	"github.com/go-kit/kit/log"
6 5
 	"sort"
7 6
 	"time"
7
+
8
+	"github.com/go-kit/kit/log"
9
+
10
+	"github.com/weaveworks/flux"
8 11
 )
9 12
 
10 13
 const (
@@ -14,8 +17,8 @@ const (
14 17
 
15 18
 // The Registry interface is a domain specific API to access container registries.
16 19
 type Registry interface {
17
-	GetRepository(repository Repository) ([]Image, error)
18
-	GetImage(repository Repository, tag string) (Image, error)
20
+	GetRepository(repository Repository) ([]flux.Image, error)
21
+	GetImage(repository Repository, tag string) (flux.Image, error)
19 22
 }
20 23
 
21 24
 type registry struct {
@@ -41,7 +44,7 @@ func NewRegistry(c RemoteClientFactory, l log.Logger, m Metrics) Registry {
41 44
 //   foo/helloworld         -> index.docker.io/foo/helloworld
42 45
 //   quay.io/foo/helloworld -> quay.io/foo/helloworld
43 46
 //
44
-func (reg *registry) GetRepository(img Repository) (_ []Image, err error) {
47
+func (reg *registry) GetRepository(img Repository) (_ []flux.Image, err error) {
45 48
 	rem, err := reg.newRemote(img)
46 49
 	if err != nil {
47 50
 		return
@@ -62,7 +65,7 @@ func (reg *registry) GetRepository(img Repository) (_ []Image, err error) {
62 65
 }
63 66
 
64 67
 // Get a single Image from the registry if it exists
65
-func (reg *registry) GetImage(img Repository, tag string) (_ Image, err error) {
68
+func (reg *registry) GetImage(img Repository, tag string) (_ flux.Image, err error) {
66 69
 	rem, err := reg.newRemote(img)
67 70
 	if err != nil {
68 71
 		return
@@ -79,12 +82,12 @@ func (reg *registry) newRemote(img Repository) (rem Remote, err error) {
79 82
 	return
80 83
 }
81 84
 
82
-func (reg *registry) tagsToRepository(remote Remote, repository Repository, tags []string) ([]Image, error) {
85
+func (reg *registry) tagsToRepository(remote Remote, repository Repository, tags []string) ([]flux.Image, error) {
83 86
 	// one way or another, we'll be finishing all requests
84 87
 	defer remote.Cancel()
85 88
 
86 89
 	type result struct {
87
-		image Image
90
+		image flux.Image
88 91
 		err   error
89 92
 	}
90 93
 
@@ -107,7 +110,7 @@ func (reg *registry) tagsToRepository(remote Remote, repository Repository, tags
107 110
 	}
108 111
 	close(toFetch)
109 112
 
110
-	images := make([]Image, cap(fetched))
113
+	images := make([]flux.Image, cap(fetched))
111 114
 	for i := 0; i < cap(fetched); i++ {
112 115
 		res := <-fetched
113 116
 		if res.err != nil {
@@ -122,7 +125,7 @@ func (reg *registry) tagsToRepository(remote Remote, repository Repository, tags
122 125
 
123 126
 // -----
124 127
 
125
-type byCreatedDesc []Image
128
+type byCreatedDesc []flux.Image
126 129
 
127 130
 func (is byCreatedDesc) Len() int      { return len(is) }
128 131
 func (is byCreatedDesc) Swap(i, j int) { is[i], is[j] = is[j], is[i] }

+ 11
- 8
registry/registry_test.go View File

@@ -1,12 +1,15 @@
1 1
 package registry
2 2
 
3 3
 import (
4
-	"github.com/go-kit/kit/log"
5
-	"github.com/pkg/errors"
6 4
 	"sort"
7 5
 	"strconv"
8 6
 	"testing"
9 7
 	"time"
8
+
9
+	"github.com/go-kit/kit/log"
10
+	"github.com/pkg/errors"
11
+
12
+	"github.com/weaveworks/flux"
10 13
 )
11 14
 
12 15
 var (
@@ -92,12 +95,12 @@ func TestRegistry_GetRepositoryManifestError(t *testing.T) {
92 95
 func TestRegistry_OrderByCreationDate(t *testing.T) {
93 96
 	time0 := testTime.Add(time.Second)
94 97
 	time2 := testTime.Add(-time.Second)
95
-	imA, _ := ParseImage("my/Image:3", &testTime)
96
-	imB, _ := ParseImage("my/Image:1", &time0)
97
-	imC, _ := ParseImage("my/Image:4", &time2)
98
-	imD, _ := ParseImage("my/Image:0", nil)       // test nil
99
-	imE, _ := ParseImage("my/Image:2", &testTime) // test equal
100
-	imgs := []Image{imA, imB, imC, imD, imE}
98
+	imA, _ := flux.ParseImage("my/Image:3", &testTime)
99
+	imB, _ := flux.ParseImage("my/Image:1", &time0)
100
+	imC, _ := flux.ParseImage("my/Image:4", &time2)
101
+	imD, _ := flux.ParseImage("my/Image:0", nil)       // test nil
102
+	imE, _ := flux.ParseImage("my/Image:2", &testTime) // test equal
103
+	imgs := []flux.Image{imA, imB, imC, imD, imE}
101 104
 	sort.Sort(byCreatedDesc(imgs))
102 105
 	for i, im := range imgs {
103 106
 		if strconv.Itoa(i) != im.Tag {

+ 7
- 4
registry/remote.go View File

@@ -4,13 +4,16 @@ import (
4 4
 	"context"
5 5
 	"encoding/json"
6 6
 	"fmt"
7
-	"github.com/docker/distribution/manifest/schema1"
8 7
 	"time"
8
+
9
+	"github.com/docker/distribution/manifest/schema1"
10
+
11
+	"github.com/weaveworks/flux"
9 12
 )
10 13
 
11 14
 type Remote interface {
12 15
 	Tags(repository Repository) ([]string, error)
13
-	Manifest(repository Repository, tag string) (Image, error)
16
+	Manifest(repository Repository, tag string) (flux.Image, error)
14 17
 	Cancel()
15 18
 }
16 19
 
@@ -30,8 +33,8 @@ func (rc *remote) Tags(repository Repository) (_ []string, err error) {
30 33
 	return rc.client.Tags(repository.NamespaceImage())
31 34
 }
32 35
 
33
-func (rc *remote) Manifest(repository Repository, tag string) (img Image, err error) {
34
-	img, err = ParseImage(fmt.Sprintf("%s:%s", repository.String(), tag), nil)
36
+func (rc *remote) Manifest(repository Repository, tag string) (img flux.Image, err error) {
37
+	img, err = flux.ParseImage(fmt.Sprintf("%s:%s", repository.String(), tag), nil)
35 38
 	if err != nil {
36 39
 		return
37 40
 	}

+ 8
- 6
registry/remote_factory_test.go View File

@@ -3,10 +3,12 @@ package registry
3 3
 import (
4 4
 	"encoding/base64"
5 5
 	"fmt"
6
-	"github.com/go-kit/kit/log"
7
-	"github.com/weaveworks/flux"
8 6
 	"testing"
9 7
 	"time"
8
+
9
+	"github.com/go-kit/kit/log"
10
+
11
+	"github.com/weaveworks/flux"
10 12
 )
11 13
 
12 14
 // Note: This actually goes off to docker hub to find the Image.
@@ -14,7 +16,7 @@ import (
14 16
 func TestRemoteFactory_CreateForDockerHub(t *testing.T) {
15 17
 	// No credentials required for public Image
16 18
 	fact := NewRemoteClientFactory(Credentials{}, log.NewNopLogger(), nil, time.Second)
17
-	img, err := ParseImage("alpine:latest", nil)
19
+	img, err := flux.ParseImage("alpine:latest", nil)
18 20
 	testRepository = RepositoryFromImage(img)
19 21
 	if err != nil {
20 22
 		t.Fatal(err)
@@ -28,14 +30,14 @@ func TestRemoteFactory_CreateForDockerHub(t *testing.T) {
28 30
 		t.Fatal(err)
29 31
 	}
30 32
 	expected := "index.docker.io/library/alpine:latest"
31
-	if res.String() != expected {
32
-		t.Fatal("Expected %q. Got %q", expected, res.String())
33
+	if res.FullID() != expected {
34
+		t.Fatal("Expected %q. Got %q", expected, res.FullID())
33 35
 	}
34 36
 }
35 37
 
36 38
 func TestRemoteFactory_InvalidHost(t *testing.T) {
37 39
 	fact := NewRemoteClientFactory(Credentials{}, log.NewNopLogger(), nil, time.Second)
38
-	img, err := ParseImage("invalid.host/library/alpine:latest", nil)
40
+	img, err := flux.ParseImage("invalid.host/library/alpine:latest", nil)
39 41
 	if err != nil {
40 42
 		t.Fatal(err)
41 43
 	}

+ 8
- 5
registry/remote_test.go View File

@@ -1,10 +1,13 @@
1 1
 package registry
2 2
 
3 3
 import (
4
-	"github.com/docker/distribution/manifest/schema1"
5
-	"github.com/pkg/errors"
6 4
 	"testing"
7 5
 	"time"
6
+
7
+	"github.com/docker/distribution/manifest/schema1"
8
+	"github.com/pkg/errors"
9
+
10
+	"github.com/weaveworks/flux"
8 11
 )
9 12
 
10 13
 const testTagStr = "tag"
@@ -12,7 +15,7 @@ const testImageStr = "index.docker.io/test/Image:" + testTagStr
12 15
 const constTime = "2017-01-13T16:22:58.009923189Z"
13 16
 
14 17
 var (
15
-	img, _         = ParseImage(testImageStr, nil)
18
+	img, _         = flux.ParseImage(testImageStr, nil)
16 19
 	testRepository = RepositoryFromImage(img)
17 20
 
18 21
 	man = schema1.SignedManifest{
@@ -39,8 +42,8 @@ func TestRemoteClient_ParseManifest(t *testing.T) {
39 42
 	if err != nil {
40 43
 		t.Fatal(err.Error())
41 44
 	}
42
-	if string(desc.String()) != testImageStr {
43
-		t.Fatalf("Expecting %q but got %q", testImageStr, string(desc.String()))
45
+	if string(desc.FullID()) != testImageStr {
46
+		t.Fatalf("Expecting %q but got %q", testImageStr, string(desc.FullID()))
44 47
 	}
45 48
 	if desc.CreatedAt.Format(time.RFC3339Nano) != constTime {
46 49
 		t.Fatalf("Expecting %q but got %q", constTime, desc.CreatedAt.Format(time.RFC3339Nano))

+ 8
- 4
registry/repository.go View File

@@ -1,17 +1,21 @@
1 1
 package registry
2 2
 
3
+import (
4
+	"github.com/weaveworks/flux"
5
+)
6
+
3 7
 type Repository struct {
4
-	img Image // Internally we use an image to store data
8
+	img flux.Image // Internally we use an image to store data
5 9
 }
6 10
 
7
-func RepositoryFromImage(img Image) Repository {
11
+func RepositoryFromImage(img flux.Image) Repository {
8 12
 	return Repository{
9 13
 		img: img,
10 14
 	}
11 15
 }
12 16
 
13 17
 func ParseRepository(imgStr string) (Repository, error) {
14
-	i, err := ParseImage(imgStr, nil)
18
+	i, err := flux.ParseImage(imgStr, nil)
15 19
 	if err != nil {
16 20
 		return Repository{}, err
17 21
 	}
@@ -32,7 +36,7 @@ func (r Repository) String() string {
32 36
 	return r.img.HostNamespaceImage()
33 37
 }
34 38
 
35
-func (r Repository) ToImage(tag string) Image {
39
+func (r Repository) ToImage(tag string) flux.Image {
36 40
 	newImage := r.img
37 41
 	newImage.Tag = tag
38 42
 	return newImage

+ 3
- 4
release/image_selector.go View File

@@ -20,9 +20,8 @@ func ImageSelectorForSpec(spec flux.ImageSpec) ImageSelector {
20 20
 	case flux.ImageSpecNone:
21 21
 		return LatestConfig
22 22
 	default:
23
-		return ExactlyTheseImages([]flux.ImageID{
24
-			flux.ParseImageID(string(spec)),
25
-		})
23
+		id, _ := flux.ParseImageID(string(spec))
24
+		return ExactlyTheseImages([]flux.ImageID{id})
26 25
 	}
27 26
 }
28 27
 
@@ -58,7 +57,7 @@ var (
58 57
 func ExactlyTheseImages(images []flux.ImageID) ImageSelector {
59 58
 	var imageText []string
60 59
 	for _, image := range images {
61
-		imageText = append(imageText, string(image))
60
+		imageText = append(imageText, image.String())
62 61
 	}
63 62
 	return funcImageSelector{
64 63
 		text: strings.Join(imageText, ", "),

+ 6
- 2
release/releaser.go View File

@@ -272,7 +272,11 @@ func CalculateUpdates(services []platform.Service, images instance.ImageMap, pri
272 272
 			continue
273 273
 		}
274 274
 		for _, container := range containers {
275
-			currentImageID := flux.ParseImageID(container.Image)
275
+			currentImageID, err := flux.ParseImageID(container.Image)
276
+			if err != nil {
277
+				// container is running an invalid image id? what?
278
+				continue
279
+			}
276 280
 			latestImage := images.LatestImage(currentImageID.Repository())
277 281
 			if latestImage == nil {
278 282
 				continue
@@ -406,7 +410,7 @@ func (r *Releaser) releaseActionUpdatePodController(service flux.ServiceID, upda
406 410
 				//
407 411
 				// Note 2: we keep overwriting the same def, to handle multiple
408 412
 				// images in a single file.
409
-				def, err = kubernetes.UpdatePodController(def, string(update.Target), ioutil.Discard)
413
+				def, err = kubernetes.UpdatePodController(def, update.Target, ioutil.Discard)
410 414
 				if err != nil {
411 415
 					return "", errors.Wrapf(err, "updating pod controller for %s", update.Target)
412 416
 				}

+ 3
- 2
server/server.go View File

@@ -146,10 +146,11 @@ func (s *Server) ListServices(inst flux.InstanceID, namespace string) (res []flu
146 146
 func containers2containers(cs []platform.Container) []flux.Container {
147 147
 	res := make([]flux.Container, len(cs))
148 148
 	for i, c := range cs {
149
+		id, _ := flux.ParseImageID(c.Image)
149 150
 		res[i] = flux.Container{
150 151
 			Name: c.Name,
151 152
 			Current: flux.ImageDescription{
152
-				ID: flux.ParseImageID(c.Image),
153
+				ID: id,
153 154
 			},
154 155
 		}
155 156
 	}
@@ -198,7 +199,7 @@ func (s *Server) ListImages(inst flux.InstanceID, spec flux.ServiceSpec) (res []
198 199
 
199 200
 func containersWithAvailable(service platform.Service, images instance.ImageMap) (res []flux.Container) {
200 201
 	for _, c := range service.ContainersOrNil() {
201
-		id := flux.ParseImageID(c.Image)
202
+		id, _ := flux.ParseImageID(c.Image)
202 203
 		repo := id.Repository()
203 204
 		available := images[repo]
204 205
 		res = append(res, flux.Container{

+ 6
- 51
service.go View File

@@ -21,7 +21,6 @@ const (
21 21
 
22 22
 var (
23 23
 	ErrInvalidServiceID   = errors.New("invalid service ID")
24
-	ErrInvalidImageID     = errors.New("invalid image ID")
25 24
 	ErrInvalidReleaseKind = errors.New("invalid release kind")
26 25
 )
27 26
 
@@ -147,49 +146,6 @@ func (ids ServiceIDs) Intersection(others ServiceIDSet) ServiceIDSet {
147 146
 	return set.Intersection(others)
148 147
 }
149 148
 
150
-type ImageID string // "quay.io/weaveworks/helloworld:v1"
151
-
152
-func ParseImageID(s string) ImageID {
153
-	return ImageID(s) // technically all strings are valid
154
-}
155
-
156
-func MakeImageID(registry, name, tag string) ImageID {
157
-	result := name
158
-	if registry != "" {
159
-		result = registry + "/" + name
160
-	}
161
-	if tag != "" {
162
-		result = result + ":" + tag
163
-	}
164
-	return ImageID(result)
165
-}
166
-
167
-func (id ImageID) Components() (registry, name, tag string) {
168
-	s := string(id)
169
-	toks := strings.SplitN(s, "/", 3)
170
-	if len(toks) == 3 {
171
-		registry = toks[0]
172
-		s = fmt.Sprintf("%s/%s", toks[1], toks[2])
173
-	}
174
-	toks = strings.SplitN(s, ":", 2)
175
-	if len(toks) == 2 {
176
-		tag = toks[1]
177
-	}
178
-	name = toks[0]
179
-	return registry, name, tag
180
-}
181
-
182
-func (id ImageID) Repository() string {
183
-	registry, name, _ := id.Components()
184
-	if registry != "" && name != "" {
185
-		return registry + "/" + name
186
-	}
187
-	if name != "" {
188
-		return name
189
-	}
190
-	return ""
191
-}
192
-
193 149
 type ServiceSpec string // ServiceID or "<all>"
194 150
 
195 151
 func ParseServiceSpec(s string) (ServiceSpec, error) {
@@ -216,15 +172,14 @@ func ParseImageSpec(s string) (ImageSpec, error) {
216 172
 	if s == string(ImageSpecLatest) || s == string(ImageSpecNone) {
217 173
 		return ImageSpec(s), nil
218 174
 	}
219
-	id := ParseImageID(s)
220
-	_, name, tag := id.Components()
221
-	if name == "" {
222
-		return "", errors.Wrap(ErrInvalidImageID, "blank image name")
223
-	}
224
-	if tag == "" {
175
+
176
+	parts := strings.Split(s, ":")
177
+	if len(parts) != 2 || parts[1] == "" {
225 178
 		return "", errors.Wrap(ErrInvalidImageID, "blank tag (if you want latest, explicitly state the tag :latest)")
226 179
 	}
227
-	return ImageSpec(id), nil
180
+
181
+	id, err := ParseImageID(s)
182
+	return ImageSpec(id.String()), err
228 183
 }
229 184
 
230 185
 type ImageStatus struct {

Loading…
Cancel
Save