Log unexpected responses

This accomplishes a few long-standing TODO items, but also helps users
in showing exact registry error messages

Signed-off-by: Ilya Dmitrichenko <errordeveloper@gmail.com>

remotes/docker/auth/fetch.go

@@ -19,15 +19,13 @@ package auth
 import (
 	"context"
 	"encoding/json"
-	"fmt"
-	"io"
-	"io/ioutil"
 	"net/http"
 	"net/url"
 	"strings"
 	"time"
 
 	"github.com/containerd/containerd/log"
+	remoteserrors "github.com/containerd/containerd/remotes/errors"
 	"github.com/pkg/errors"
 	"golang.org/x/net/context/ctxhttp"
 )
@@ -38,25 +36,6 @@ var (
 	ErrNoToken = errors.New("authorization server did not include a token in the response")
 )
 
-// ErrUnexpectedStatus is returned if a token request returned with unexpected HTTP status
-type ErrUnexpectedStatus struct {
-	Status     string
-	StatusCode int
-	Body       []byte
-}
-
-func (e ErrUnexpectedStatus) Error() string {
-	return fmt.Sprintf("unexpected status: %s", e.Status)
-}
-
-func newUnexpectedStatusErr(resp *http.Response) error {
-	var b []byte
-	if resp.Body != nil {
-		b, _ = ioutil.ReadAll(io.LimitReader(resp.Body, 64000)) // 64KB
-	}
-	return ErrUnexpectedStatus{Status: resp.Status, StatusCode: resp.StatusCode, Body: b}
-}
-
 // GenerateTokenOptions generates options for fetching a token based on a challenge
 func GenerateTokenOptions(ctx context.Context, host, username, secret string, c Challenge) (TokenOptions, error) {
 	realm, ok := c.Parameters["realm"]
@@ -140,7 +119,7 @@ func FetchTokenWithOAuth(ctx context.Context, client *http.Client, headers http.
 	defer resp.Body.Close()
 
 	if resp.StatusCode < 200 || resp.StatusCode >= 400 {
-		return nil, errors.WithStack(newUnexpectedStatusErr(resp))
+		return nil, errors.WithStack(remoteserrors.NewUnexpectedStatusErr(resp))
 	}
 
 	decoder := json.NewDecoder(resp.Body)
@@ -202,7 +181,7 @@ func FetchToken(ctx context.Context, client *http.Client, headers http.Header, t
 	defer resp.Body.Close()
 
 	if resp.StatusCode < 200 || resp.StatusCode >= 400 {
-		return nil, errors.WithStack(newUnexpectedStatusErr(resp))
+		return nil, errors.WithStack(remoteserrors.NewUnexpectedStatusErr(resp))
 	}
 
 	decoder := json.NewDecoder(resp.Body)

remotes/docker/authorizer.go

@@ -27,6 +27,7 @@ import (
 	"github.com/containerd/containerd/errdefs"
 	"github.com/containerd/containerd/log"
 	"github.com/containerd/containerd/remotes/docker/auth"
+	remoteerrors "github.com/containerd/containerd/remotes/errors"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
@@ -278,7 +279,7 @@ func (ah *authHandler) doBearerAuth(ctx context.Context) (token string, err erro
 		// TODO: Allow setting client_id
 		resp, err := auth.FetchTokenWithOAuth(ctx, ah.client, ah.header, "containerd-client", to)
 		if err != nil {
-			var errStatus auth.ErrUnexpectedStatus
+			var errStatus remoteerrors.ErrUnexpectedStatus
 			if errors.As(err, &errStatus) {
 				// Registries without support for POST may return 404 for POST /v2/token.
 				// As of September 2017, GCR is known to return 404.

remotes/docker/pusher.go

@@ -30,6 +30,7 @@ import (
 	"github.com/containerd/containerd/images"
 	"github.com/containerd/containerd/log"
 	"github.com/containerd/containerd/remotes"
+	remoteserrors "github.com/containerd/containerd/remotes/errors"
 	digest "github.com/opencontainers/go-digest"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
@@ -112,8 +113,9 @@ func (p dockerPusher) Push(ctx context.Context, desc ocispec.Descriptor) (conten
 				return nil, errors.Wrapf(errdefs.ErrAlreadyExists, "content %v on remote", desc.Digest)
 			}
 		} else if resp.StatusCode != http.StatusNotFound {
-			// TODO: log error
-			return nil, errors.Errorf("unexpected response: %s", resp.Status)
+			err := remoteserrors.NewUnexpectedStatusErr(resp)
+			log.G(ctx).WithField("resp", resp).WithField("body", string(err.(remoteserrors.ErrUnexpectedStatus).Body)).Debug("unexpected response")
+			return nil, err
 		}
 	}
 
@@ -166,8 +168,9 @@ func (p dockerPusher) Push(ctx context.Context, desc ocispec.Descriptor) (conten
 			})
 			return nil, errors.Wrapf(errdefs.ErrAlreadyExists, "content %v on remote", desc.Digest)
 		default:
-			// TODO: log error
-			return nil, errors.Errorf("unexpected response: %s", resp.Status)
+			err := remoteserrors.NewUnexpectedStatusErr(resp)
+			log.G(ctx).WithField("resp", resp).WithField("body", string(err.(remoteserrors.ErrUnexpectedStatus).Body)).Debug("unexpected response")
+			return nil, err
 		}
 
 		var (
@@ -244,8 +247,9 @@ func (p dockerPusher) Push(ctx context.Context, desc ocispec.Descriptor) (conten
 		switch resp.StatusCode {
 		case http.StatusOK, http.StatusCreated, http.StatusNoContent:
 		default:
-			// TODO: log error
-			pr.CloseWithError(errors.Errorf("unexpected response: %s", resp.Status))
+			err := remoteserrors.NewUnexpectedStatusErr(resp)
+			log.G(ctx).WithField("resp", resp).WithField("body", string(err.(remoteserrors.ErrUnexpectedStatus).Body)).Debug("unexpected response")
+			pr.CloseWithError(err)
 		}
 		respC <- resp
 	}()