From d6330f220b78dce95c0b94c8bfcc7ab0332142e2 Mon Sep 17 00:00:00 2001 From: Daehyeok Mun Date: Sun, 16 Nov 2014 22:25:10 +0900 Subject: [PATCH] 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 Signed-off-by: Michael Crosby Upstream-commit: 8123c1e9fef0eb0d6b4e89dce4089276b751906c Component: engine --- components/engine/graph/push.go | 9 ++++----- components/engine/registry/registry_test.go | 9 +++++---- components/engine/registry/session.go | 15 +++++++-------- 3 files changed, 16 insertions(+), 17 deletions(-) diff --git a/components/engine/graph/push.go b/components/engine/graph/push.go index 29fc4a066d..77db243817 100644 --- a/components/engine/graph/push.go +++ b/components/engine/graph/push.go @@ -115,17 +115,16 @@ func (s *TagStore) pushRepository(r *registry.Session, out io.Writer, localName, } for _, ep := range repoData.Endpoints { out.Write(sf.FormatStatus("", "Pushing repository %s (%d tags)", localName, nTag)) - for _, imgId := range imgList { - if r.LookupRemoteImage(imgId, ep, repoData.Tokens) { - out.Write(sf.FormatStatus("", "Image %s already pushed, skipping", utils.TruncateID(imgId))) - } else { + if err := r.LookupRemoteImage(imgId, ep, repoData.Tokens); err != nil { + log.Errorf("Error in LookupRemoteImage: %s", err) if _, err := s.pushImage(r, out, remoteName, imgId, ep, repoData.Tokens, sf); err != nil { // FIXME: Continue on error? return err } + } else { + out.Write(sf.FormatStatus("", "Image %s already pushed, skipping", utils.TruncateID(imgId))) } - for _, tag := range tagsByImage[imgId] { out.Write(sf.FormatStatus("", "Pushing tag for rev [%s] on {%s}", utils.TruncateID(imgId), ep+"repositories/"+remoteName+"/tags/"+tag)) diff --git a/components/engine/registry/registry_test.go b/components/engine/registry/registry_test.go index d24a5f5751..5fd80da100 100644 --- a/components/engine/registry/registry_test.go +++ b/components/engine/registry/registry_test.go @@ -58,10 +58,11 @@ func TestGetRemoteHistory(t *testing.T) { func TestLookupRemoteImage(t *testing.T) { r := spawnTestRegistrySession(t) - found := r.LookupRemoteImage(imageID, makeURL("/v1/"), token) - assertEqual(t, found, true, "Expected remote lookup to succeed") - found = r.LookupRemoteImage("abcdef", makeURL("/v1/"), token) - assertEqual(t, found, false, "Expected remote lookup to fail") + err := r.LookupRemoteImage(imageID, makeURL("/v1/"), token) + assertEqual(t, err, nil, "Expected error of remote lookup to nil") + if err := r.LookupRemoteImage("abcdef", makeURL("/v1/"), token); err == nil { + t.Fatal("Expected error of remote lookup to not nil") + } } func TestGetRemoteImageJSON(t *testing.T) { diff --git a/components/engine/registry/session.go b/components/engine/registry/session.go index 4b2f55225f..28cf18fbe3 100644 --- a/components/engine/registry/session.go +++ b/components/engine/registry/session.go @@ -102,22 +102,21 @@ func (r *Session) GetRemoteHistory(imgID, registry string, token []string) ([]st } // Check if an image exists in the Registry -// TODO: This method should return the errors instead of masking them and returning false -func (r *Session) LookupRemoteImage(imgID, registry string, token []string) bool { - +func (r *Session) LookupRemoteImage(imgID, registry string, token []string) error { req, err := r.reqFactory.NewRequest("GET", registry+"images/"+imgID+"/json", nil) if err != nil { - log.Errorf("Error in LookupRemoteImage %s", err) - return false + return err } setTokenAuth(req, token) res, _, err := r.doRequest(req) if err != nil { - log.Errorf("Error in LookupRemoteImage %s", err) - return false + return err } res.Body.Close() - return res.StatusCode == 200 + if res.StatusCode != 200 { + return utils.NewHTTPRequestError(fmt.Sprintf("HTTP code %d", res.StatusCode), res) + } + return nil } // Retrieve an image from the Registry.