1. origin
  2. Commit 1b8c4af9616ab4451be4a9e1489d825caf96e113
Browse code

Issue 7555 - fixed importimage which was picking wrong docker pull spec for images that failed previous import.

Maciej Szulik authored on 2016/02/26 21:26:45
Showing 2 changed files
pkg/cmd/cli/cmd/importimage.go
History View file @ 1b8c4af
... ... 
@@ -45,7 +45,7 @@ func NewCmdImportImage(fullName string, f *clientcmd.Factory, out io.Writer) *co
45 45 
 			kcmdutil.CheckErr(opts.Run())
46 46 
 		},
47 47 
 	}
48 
-	cmd.Flags().StringVar(&opts.From, "from", "", "A Docker image repository or tag to import images from")
48 
+	cmd.Flags().StringVar(&opts.From, "from", "", "A Docker image repository to import images from")
49 49 
 	cmd.Flags().BoolVar(&opts.Confirm, "confirm", false, "If true, allow the image stream import location to be set or changed")
50 50 
 	cmd.Flags().BoolVar(&opts.All, "all", false, "If true, import all tags from the provided source on creation or if --from is specified")
51 51 
 	cmd.Flags().BoolVar(&opts.Insecure, "insecure", false, "If true, allow importing from registries that have invalid HTTPS certificates or are hosted via HTTP")
... ... 
@@ -288,8 +288,13 @@ func (o *ImportImageOptions) createImageImport() (*imageapi.ImageStream, *imagea
288 288
289 289 
 		if o.All {
290 290 
 			// importing a whole repository
291 
+			// TODO soltysh: write tests to cover all the possible usecases!!!
291 292 
 			if len(from) == 0 {
292 
-				from = o.Target
293 
+				if len(stream.Spec.DockerImageRepository) == 0 {
294 
+					// FIXME soltysh:
295 
+					return nil, nil, fmt.Errorf("flag --all is applicable only to images with spec.dockerImageRepository defined")
296 
+				}
297 
+				from = stream.Spec.DockerImageRepository
293 298 
 			}
294 299 
 			if from != stream.Spec.DockerImageRepository {
295 300 
 				if !o.Confirm {
... ... 
@@ -337,7 +342,7 @@ func (o *ImportImageOptions) createImageImport() (*imageapi.ImageStream, *imagea
337 337 
 			} else {
338 338 
 				// create a new tag
339 339 
 				if len(from) == 0 {
340 
-					from = o.Target
340 
+					from = stream.Spec.DockerImageRepository
341 341 
 				}
342 342 
 				existing = &imageapi.TagReference{
343 343 
 					From: &kapi.ObjectReference{
pkg/cmd/cli/cmd/importimage_test.go
History View file @ 1b8c4af
344 344 
new file mode 100644
... ... 
@@ -0,0 +1,163 @@
0 
+package cmd
1 
+
2 
+import (
3 
+	"strings"
4 
+	"testing"
5 
+
6 
+	"github.com/spf13/cobra"
7 
+
8 
+	kapi "k8s.io/kubernetes/pkg/api"
9 
+
10 
+	"github.com/openshift/origin/pkg/client/testclient"
11 
+	imageapi "github.com/openshift/origin/pkg/image/api"
12 
+)
13 
+
14 
+func TestCreateImageImport(t *testing.T) {
15 
+	testCases := []struct {
16 
+		name   string
17 
+		stream *imageapi.ImageStream
18 
+		all    bool
19 
+		err    string
20 
+		from   []kapi.ObjectReference
21 
+	}{
22 
+		{
23 
+			// 0: checking import's from when only .spec.dockerImageRepository is set, no status
24 
+			name: "testis",
25 
+			stream: &imageapi.ImageStream{
26 
+				ObjectMeta: kapi.ObjectMeta{
27 
+					Name:      "testis",
28 
+					Namespace: "other",
29 
+				},
30 
+				Spec: imageapi.ImageStreamSpec{
31 
+					DockerImageRepository: "repo.com/somens/someimage",
32 
+					Tags: make(map[string]imageapi.TagReference),
33 
+				},
34 
+			},
35 
+			from: []kapi.ObjectReference{
36 
+				{
37 
+					Kind: "DockerImage",
38 
+					Name: "repo.com/somens/someimage",
39 
+				},
40 
+			},
41 
+		},
42 
+		{
43 
+			// 1: checking import's from when only .spec.dockerImageRepository is set, no status (with all flag set)
44 
+			name: "testis",
45 
+			stream: &imageapi.ImageStream{
46 
+				ObjectMeta: kapi.ObjectMeta{
47 
+					Name:      "testis",
48 
+					Namespace: "other",
49 
+				},
50 
+				Spec: imageapi.ImageStreamSpec{
51 
+					DockerImageRepository: "repo.com/somens/someimage",
52 
+					Tags: make(map[string]imageapi.TagReference),
53 
+				},
54 
+			},
55 
+			all: true,
56 
+			from: []kapi.ObjectReference{
57 
+				{
58 
+					Kind: "DockerImage",
59 
+					Name: "repo.com/somens/someimage",
60 
+				},
61 
+			},
62 
+		},
63 
+		{
64 
+			// 2: with --all flag only .spec.dockerImageRepository is handled
65 
+			name: "testis",
66 
+			stream: &imageapi.ImageStream{
67 
+				ObjectMeta: kapi.ObjectMeta{
68 
+					Name:      "testis",
69 
+					Namespace: "other",
70 
+				},
71 
+				Spec: imageapi.ImageStreamSpec{
72 
+					Tags: map[string]imageapi.TagReference{
73 
+						"latest": {
74 
+							From: &kapi.ObjectReference{Kind: "DockerImage", Name: "repo.com/somens/someimage:latest"},
75 
+						},
76 
+					},
77 
+				},
78 
+			},
79 
+			all: true,
80 
+			err: "all is applicable only to images with spec.dockerImageRepository",
81 
+		},
82 
+		{
83 
+			// 3: empty image stream
84 
+			name: "testis",
85 
+			stream: &imageapi.ImageStream{
86 
+				ObjectMeta: kapi.ObjectMeta{
87 
+					Name:      "testis",
88 
+					Namespace: "other",
89 
+				},
90 
+			},
91 
+			err: "image stream has not defined anything to import",
92 
+		},
93 
+		{
94 
+			// 4: correct import of latest tag with tags specified in .spec.Tags
95 
+			name: "testis:latest",
96 
+			stream: &imageapi.ImageStream{
97 
+				ObjectMeta: kapi.ObjectMeta{
98 
+					Name:      "testis",
99 
+					Namespace: "other",
100 
+				},
101 
+				Spec: imageapi.ImageStreamSpec{
102 
+					Tags: map[string]imageapi.TagReference{
103 
+						"latest": {
104 
+							From: &kapi.ObjectReference{Kind: "DockerImage", Name: "repo.com/somens/someimage:latest"},
105 
+						},
106 
+					},
107 
+				},
108 
+			},
109 
+			from: []kapi.ObjectReference{
110 
+				{
111 
+					Kind: "DockerImage",
112 
+					Name: "repo.com/somens/someimage:latest",
113 
+				},
114 
+			},
115 
+		},
116 
+	}
117 
+
118 
+	for idx, test := range testCases {
119 
+		fake := testclient.NewSimpleFake(test.stream)
120 
+		o := ImportImageOptions{
121 
+			Target:    test.stream.Name,
122 
+			All:       test.all,
123 
+			Namespace: test.stream.Namespace,
124 
+			isClient:  fake.ImageStreams(test.stream.Namespace),
125 
+		}
126 
+		// we need to run Validate, because it sets appropriate Name and Tag
127 
+		if err := o.Validate(&cobra.Command{}); err != nil {
128 
+			t.Errorf("(%d) unexpected error: %v", idx, err)
129 
+		}
130 
+
131 
+		_, isi, err := o.createImageImport()
132 
+		// check errors
133 
+		if len(test.err) > 0 {
134 
+			if err == nil || !strings.Contains(err.Error(), test.err) {
135 
+				t.Errorf("(%d) unexpected error: expected %s, got %v", idx, test.err, err)
136 
+			}
137 
+			if isi != nil {
138 
+				t.Errorf("(%d) unexpected import spec: expected nil, got %#v", idx, isi)
139 
+			}
140 
+			continue
141 
+		}
142 
+		if len(test.err) == 0 && err != nil {
143 
+			t.Errorf("(%d) unexpected error: %v", idx, err)
144 
+			continue
145 
+		}
146 
+		// check values
147 
+		if test.all {
148 
+			if !kapi.Semantic.DeepEqual(isi.Spec.Repository.From, test.from[0]) {
149 
+				t.Errorf("(%d) unexpected import spec, expected %#v, got %#v", idx, test.from[0], isi.Spec.Repository.From)
150 
+			}
151 
+		} else {
152 
+			if len(isi.Spec.Images) != len(test.from) {
153 
+				t.Errorf("(%d) unexpected number of images, expected %d, got %d", idx, len(test.from), len(isi.Spec.Images))
154 
+			}
155 
+			for i := 0; i < len(test.from); i++ {
156 
+				if !kapi.Semantic.DeepEqual(isi.Spec.Images[i].From, test.from[i]) {
157 
+					t.Errorf("(%d) unexpected import spec[%d], expected %#v, got %#v", idx, i, test.from[i], isi.Spec.Images[i].From)
158 
+				}
159 
+			}
160 
+		}
161 
+	}
162 
+}