Browse code

Chnage LookupRemoteImage to return error This commit is patch for following comment // TODO: This method should return the errors instead of masking them and returning false

Signed-off-by: Daehyeok Mun <daehyeok@gmail.com>
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>

Daehyeok Mun authored on 2014/11/16 22:25:10
Showing 3 changed files
... ...
@@ -115,17 +115,16 @@ func (s *TagStore) pushRepository(r *registry.Session, out io.Writer, localName,
115 115
 	}
116 116
 	for _, ep := range repoData.Endpoints {
117 117
 		out.Write(sf.FormatStatus("", "Pushing repository %s (%d tags)", localName, nTag))
118
-
119 118
 		for _, imgId := range imgList {
120
-			if r.LookupRemoteImage(imgId, ep, repoData.Tokens) {
121
-				out.Write(sf.FormatStatus("", "Image %s already pushed, skipping", utils.TruncateID(imgId)))
122
-			} else {
119
+			if err := r.LookupRemoteImage(imgId, ep, repoData.Tokens); err != nil {
120
+				log.Errorf("Error in LookupRemoteImage: %s", err)
123 121
 				if _, err := s.pushImage(r, out, remoteName, imgId, ep, repoData.Tokens, sf); err != nil {
124 122
 					// FIXME: Continue on error?
125 123
 					return err
126 124
 				}
125
+			} else {
126
+				out.Write(sf.FormatStatus("", "Image %s already pushed, skipping", utils.TruncateID(imgId)))
127 127
 			}
128
-
129 128
 			for _, tag := range tagsByImage[imgId] {
130 129
 				out.Write(sf.FormatStatus("", "Pushing tag for rev [%s] on {%s}", utils.TruncateID(imgId), ep+"repositories/"+remoteName+"/tags/"+tag))
131 130
 
... ...
@@ -58,10 +58,11 @@ func TestGetRemoteHistory(t *testing.T) {
58 58
 
59 59
 func TestLookupRemoteImage(t *testing.T) {
60 60
 	r := spawnTestRegistrySession(t)
61
-	found := r.LookupRemoteImage(imageID, makeURL("/v1/"), token)
62
-	assertEqual(t, found, true, "Expected remote lookup to succeed")
63
-	found = r.LookupRemoteImage("abcdef", makeURL("/v1/"), token)
64
-	assertEqual(t, found, false, "Expected remote lookup to fail")
61
+	err := r.LookupRemoteImage(imageID, makeURL("/v1/"), token)
62
+	assertEqual(t, err, nil, "Expected error of remote lookup to nil")
63
+	if err := r.LookupRemoteImage("abcdef", makeURL("/v1/"), token); err == nil {
64
+		t.Fatal("Expected error of remote lookup to not nil")
65
+	}
65 66
 }
66 67
 
67 68
 func TestGetRemoteImageJSON(t *testing.T) {
... ...
@@ -102,22 +102,21 @@ func (r *Session) GetRemoteHistory(imgID, registry string, token []string) ([]st
102 102
 }
103 103
 
104 104
 // Check if an image exists in the Registry
105
-// TODO: This method should return the errors instead of masking them and returning false
106
-func (r *Session) LookupRemoteImage(imgID, registry string, token []string) bool {
107
-
105
+func (r *Session) LookupRemoteImage(imgID, registry string, token []string) error {
108 106
 	req, err := r.reqFactory.NewRequest("GET", registry+"images/"+imgID+"/json", nil)
109 107
 	if err != nil {
110
-		log.Errorf("Error in LookupRemoteImage %s", err)
111
-		return false
108
+		return err
112 109
 	}
113 110
 	setTokenAuth(req, token)
114 111
 	res, _, err := r.doRequest(req)
115 112
 	if err != nil {
116
-		log.Errorf("Error in LookupRemoteImage %s", err)
117
-		return false
113
+		return err
118 114
 	}
119 115
 	res.Body.Close()
120
-	return res.StatusCode == 200
116
+	if res.StatusCode != 200 {
117
+		return utils.NewHTTPRequestError(fmt.Sprintf("HTTP code %d", res.StatusCode), res)
118
+	}
119
+	return nil
121 120
 }
122 121
 
123 122
 // Retrieve an image from the Registry.