review comments

Author: soltysh

pkg/cmd/cli/cmd/importimage.go

@@ -265,7 +265,7 @@ func (o *ImportImageOptions) createImageImport() (*imageapi.ImageStream, *imagea
 
 	// the stream already exists
 	if len(stream.Spec.DockerImageRepository) == 0 && len(stream.Spec.Tags) == 0 {
-		return nil, nil, fmt.Errorf("image stream has not defined anything to import")
+		return nil, nil, fmt.Errorf("image stream does not have valid docker images to be imported")
 	}
 
 	if o.All {
@@ -285,45 +285,6 @@ func (o *ImportImageOptions) createImageImport() (*imageapi.ImageStream, *imagea
 	return stream, isi, nil
 }
 
-func (o *ImportImageOptions) newImageStream() (*imageapi.ImageStream, *imageapi.ImageStreamImport) {
-	from := o.From
-	tag := o.Tag
-	if len(from) == 0 {
-		from = o.Target
-	}
-	var stream *imageapi.ImageStream
-	// create new ImageStream
-	if o.All {
-		stream = &imageapi.ImageStream{
-			ObjectMeta: kapi.ObjectMeta{Name: o.Name},
-			Spec:       imageapi.ImageStreamSpec{DockerImageRepository: from},
-		}
-	} else {
-		stream = &imageapi.ImageStream{
-			ObjectMeta: kapi.ObjectMeta{Name: o.Name},
-			Spec: imageapi.ImageStreamSpec{
-				Tags: map[string]imageapi.TagReference{
-					tag: {
-						From: &kapi.ObjectReference{
-							Kind: "DockerImage",
-							Name: from,
-						},
-					},
-				},
-			},
-		}
-	}
-	// and accompanying ImageStreamImport
-	var isi *imageapi.ImageStreamImport
-	if o.All {
-		isi = o.newImageStreamImportAll(stream, from)
-	} else {
-		isi = o.newImageStreamImportTags(stream, map[string]string{tag: from})
-	}
-
-	return stream, isi
-}
-
 func (o *ImportImageOptions) importAll(stream *imageapi.ImageStream) (*imageapi.ImageStreamImport, error) {
 	from := o.From
 	// update ImageStream appropriately
@@ -338,7 +299,7 @@ func (o *ImportImageOptions) importAll(stream *imageapi.ImageStream) (*imageapi.
 				}
 			}
 			if len(tags) == 0 {
-				return nil, fmt.Errorf("image stream does not have importable tags (pointing to DockerImage)")
+				return nil, fmt.Errorf("image stream does not have tags pointing to external docker images")
 			}
 			return o.newImageStreamImportTags(stream, tags), nil
 		}
@@ -414,6 +375,45 @@ func (o *ImportImageOptions) importTag(stream *imageapi.ImageStream) (*imageapi.
 	return o.newImageStreamImportTags(stream, map[string]string{tag: from}), nil
 }
 
+func (o *ImportImageOptions) newImageStream() (*imageapi.ImageStream, *imageapi.ImageStreamImport) {
+	from := o.From
+	tag := o.Tag
+	if len(from) == 0 {
+		from = o.Target
+	}
+	var stream *imageapi.ImageStream
+	// create new ImageStream
+	if o.All {
+		stream = &imageapi.ImageStream{
+			ObjectMeta: kapi.ObjectMeta{Name: o.Name},
+			Spec:       imageapi.ImageStreamSpec{DockerImageRepository: from},
+		}
+	} else {
+		stream = &imageapi.ImageStream{
+			ObjectMeta: kapi.ObjectMeta{Name: o.Name},
+			Spec: imageapi.ImageStreamSpec{
+				Tags: map[string]imageapi.TagReference{
+					tag: {
+						From: &kapi.ObjectReference{
+							Kind: "DockerImage",
+							Name: from,
+						},
+					},
+				},
+			},
+		}
+	}
+	// and accompanying ImageStreamImport
+	var isi *imageapi.ImageStreamImport
+	if o.All {
+		isi = o.newImageStreamImportAll(stream, from)
+	} else {
+		isi = o.newImageStreamImportTags(stream, map[string]string{tag: from})
+	}
+
+	return stream, isi
+}
+
 func (o *ImportImageOptions) newImageStreamImport(stream *imageapi.ImageStream) (*imageapi.ImageStreamImport, bool) {
 	isi := &imageapi.ImageStreamImport{
 		ObjectMeta: kapi.ObjectMeta{

pkg/cmd/cli/cmd/importimage_test.go

@@ -182,7 +182,7 @@ func TestCreateImageImport(t *testing.T) {
 		"import all from .spec.tags no DockerImage tags": {
 			name: "testis",
 			all:  true,
-			err:  "does not have importable tags",
+			err:  "does not have tags pointing to external docker images",
 			stream: &imageapi.ImageStream{
 				ObjectMeta: kapi.ObjectMeta{Name: "testis", Namespace: "other"},
 				Spec: imageapi.ImageStreamSpec{
@@ -194,7 +194,7 @@ func TestCreateImageImport(t *testing.T) {
 		},
 		"empty image stream": {
 			name: "testis",
-			err:  "image stream has not defined anything to import",
+			err:  "does not have valid docker images",
 			stream: &imageapi.ImageStream{
 				ObjectMeta: kapi.ObjectMeta{Name: "testis", Namespace: "other"},
 			},