Skip to content

Commit

Permalink
Merge pull request #557 from vrothberg/parallel-push
Browse files Browse the repository at this point in the history
Parallel push
  • Loading branch information
mtrmac authored Jan 17, 2019
2 parents 23de304 + 12eacf6 commit e42fdf3
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 45 deletions.
29 changes: 14 additions & 15 deletions docker/docker_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ type dockerClient struct {
password string
signatureBase signatureStorageBase
scope authScope
extraScope *authScope // If non-nil, a temporary extra token scope (necessary for mounting from another repo)
// The following members are detected registry properties:
// They are set after a successful detectProperties(), and never change afterwards.
scheme string // Empty value also used to indicate detectProperties() has not yet succeeded.
Expand Down Expand Up @@ -282,7 +281,7 @@ func CheckAuth(ctx context.Context, sys *types.SystemContext, username, password
client.username = username
client.password = password

resp, err := client.makeRequest(ctx, "GET", "/v2/", nil, nil, v2Auth)
resp, err := client.makeRequest(ctx, "GET", "/v2/", nil, nil, v2Auth, nil)
if err != nil {
return err
}
Expand Down Expand Up @@ -363,7 +362,7 @@ func SearchRegistry(ctx context.Context, sys *types.SystemContext, registry, ima
u.RawQuery = q.Encode()

logrus.Debugf("trying to talk to v1 search endpoint\n")
resp, err := client.makeRequest(ctx, "GET", u.String(), nil, nil, noAuth)
resp, err := client.makeRequest(ctx, "GET", u.String(), nil, nil, noAuth, nil)
if err != nil {
logrus.Debugf("error getting search results from v1 endpoint %q: %v", registry, err)
} else {
Expand All @@ -380,7 +379,7 @@ func SearchRegistry(ctx context.Context, sys *types.SystemContext, registry, ima
}

logrus.Debugf("trying to talk to v2 search endpoint\n")
resp, err := client.makeRequest(ctx, "GET", "/v2/_catalog", nil, nil, v2Auth)
resp, err := client.makeRequest(ctx, "GET", "/v2/_catalog", nil, nil, v2Auth, nil)
if err != nil {
logrus.Debugf("error getting search results from v2 endpoint %q: %v", registry, err)
} else {
Expand Down Expand Up @@ -409,20 +408,20 @@ func SearchRegistry(ctx context.Context, sys *types.SystemContext, registry, ima

// makeRequest creates and executes a http.Request with the specified parameters, adding authentication and TLS options for the Docker client.
// The host name and schema is taken from the client or autodetected, and the path is relative to it, i.e. the path usually starts with /v2/.
func (c *dockerClient) makeRequest(ctx context.Context, method, path string, headers map[string][]string, stream io.Reader, auth sendAuth) (*http.Response, error) {
func (c *dockerClient) makeRequest(ctx context.Context, method, path string, headers map[string][]string, stream io.Reader, auth sendAuth, extraScope *authScope) (*http.Response, error) {
if err := c.detectProperties(ctx); err != nil {
return nil, err
}

url := fmt.Sprintf("%s://%s%s", c.scheme, c.registry, path)
return c.makeRequestToResolvedURL(ctx, method, url, headers, stream, -1, auth)
return c.makeRequestToResolvedURL(ctx, method, url, headers, stream, -1, auth, extraScope)
}

// makeRequestToResolvedURL creates and executes a http.Request with the specified parameters, adding authentication and TLS options for the Docker client.
// streamLen, if not -1, specifies the length of the data expected on stream.
// makeRequest should generally be preferred.
// TODO(runcom): too many arguments here, use a struct
func (c *dockerClient) makeRequestToResolvedURL(ctx context.Context, method, url string, headers map[string][]string, stream io.Reader, streamLen int64, auth sendAuth) (*http.Response, error) {
func (c *dockerClient) makeRequestToResolvedURL(ctx context.Context, method, url string, headers map[string][]string, stream io.Reader, streamLen int64, auth sendAuth, extraScope *authScope) (*http.Response, error) {
req, err := http.NewRequest(method, url, stream)
if err != nil {
return nil, err
Expand All @@ -441,7 +440,7 @@ func (c *dockerClient) makeRequestToResolvedURL(ctx context.Context, method, url
req.Header.Add("User-Agent", c.sys.DockerRegistryUserAgent)
}
if auth == v2Auth {
if err := c.setupRequestAuth(req); err != nil {
if err := c.setupRequestAuth(req, extraScope); err != nil {
return nil, err
}
}
Expand All @@ -460,7 +459,7 @@ func (c *dockerClient) makeRequestToResolvedURL(ctx context.Context, method, url
// 2) gcr.io is sending 401 without a WWW-Authenticate header in the real request
//
// debugging: https://github.com/containers/image/pull/211#issuecomment-273426236 and follows up
func (c *dockerClient) setupRequestAuth(req *http.Request) error {
func (c *dockerClient) setupRequestAuth(req *http.Request, extraScope *authScope) error {
if len(c.challenges) == 0 {
return nil
}
Expand All @@ -474,10 +473,10 @@ func (c *dockerClient) setupRequestAuth(req *http.Request) error {
case "bearer":
cacheKey := ""
scopes := []authScope{c.scope}
if c.extraScope != nil {
if extraScope != nil {
// Using ':' as a separator here is unambiguous because getBearerToken below uses the same separator when formatting a remote request (and because repository names can't contain colons).
cacheKey = fmt.Sprintf("%s:%s", c.extraScope.remoteName, c.extraScope.actions)
scopes = append(scopes, *c.extraScope)
cacheKey = fmt.Sprintf("%s:%s", extraScope.remoteName, extraScope.actions)
scopes = append(scopes, *extraScope)
}
var token bearerToken
t, inCache := c.tokenCache.Load(cacheKey)
Expand Down Expand Up @@ -564,7 +563,7 @@ func (c *dockerClient) detectPropertiesHelper(ctx context.Context) error {

ping := func(scheme string) error {
url := fmt.Sprintf(resolvedPingV2URL, scheme, c.registry)
resp, err := c.makeRequestToResolvedURL(ctx, "GET", url, nil, nil, -1, noAuth)
resp, err := c.makeRequestToResolvedURL(ctx, "GET", url, nil, nil, -1, noAuth, nil)
if err != nil {
logrus.Debugf("Ping %s err %s (%#v)", url, err.Error(), err)
return err
Expand All @@ -591,7 +590,7 @@ func (c *dockerClient) detectPropertiesHelper(ctx context.Context) error {
// best effort to understand if we're talking to a V1 registry
pingV1 := func(scheme string) bool {
url := fmt.Sprintf(resolvedPingV1URL, scheme, c.registry)
resp, err := c.makeRequestToResolvedURL(ctx, "GET", url, nil, nil, -1, noAuth)
resp, err := c.makeRequestToResolvedURL(ctx, "GET", url, nil, nil, -1, noAuth, nil)
if err != nil {
logrus.Debugf("Ping %s err %s (%#v)", url, err.Error(), err)
return false
Expand Down Expand Up @@ -625,7 +624,7 @@ func (c *dockerClient) detectProperties(ctx context.Context) error {
// using the original data structures.
func (c *dockerClient) getExtensionsSignatures(ctx context.Context, ref dockerReference, manifestDigest digest.Digest) (*extensionSignatureList, error) {
path := fmt.Sprintf(extensionsSignaturePath, reference.Path(ref.ref), manifestDigest)
res, err := c.makeRequest(ctx, "GET", path, nil, nil, v2Auth)
res, err := c.makeRequest(ctx, "GET", path, nil, nil, v2Auth, nil)
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion docker/docker_image.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func GetRepositoryTags(ctx context.Context, sys *types.SystemContext, ref types.
tags := make([]string, 0)

for {
res, err := client.makeRequest(ctx, "GET", path, nil, nil, v2Auth)
res, err := client.makeRequest(ctx, "GET", path, nil, nil, v2Auth, nil)
if err != nil {
return nil, err
}
Expand Down
42 changes: 18 additions & 24 deletions docker/docker_image_dest.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func (c *sizeCounter) Write(p []byte) (n int, err error) {

// HasThreadSafePutBlob indicates whether PutBlob can be executed concurrently.
func (d *dockerImageDestination) HasThreadSafePutBlob() bool {
return false
return true
}

// PutBlob writes contents of stream and returns data representing the result (with all data filled in).
Expand All @@ -140,7 +140,7 @@ func (d *dockerImageDestination) PutBlob(ctx context.Context, stream io.Reader,
// FIXME? Chunked upload, progress reporting, etc.
uploadPath := fmt.Sprintf(blobUploadPath, reference.Path(d.ref.ref))
logrus.Debugf("Uploading %s", uploadPath)
res, err := d.c.makeRequest(ctx, "POST", uploadPath, nil, nil, v2Auth)
res, err := d.c.makeRequest(ctx, "POST", uploadPath, nil, nil, v2Auth, nil)
if err != nil {
return types.BlobInfo{}, err
}
Expand All @@ -157,7 +157,7 @@ func (d *dockerImageDestination) PutBlob(ctx context.Context, stream io.Reader,
digester := digest.Canonical.Digester()
sizeCounter := &sizeCounter{}
tee := io.TeeReader(stream, io.MultiWriter(digester.Hash(), sizeCounter))
res, err = d.c.makeRequestToResolvedURL(ctx, "PATCH", uploadLocation.String(), map[string][]string{"Content-Type": {"application/octet-stream"}}, tee, inputInfo.Size, v2Auth)
res, err = d.c.makeRequestToResolvedURL(ctx, "PATCH", uploadLocation.String(), map[string][]string{"Content-Type": {"application/octet-stream"}}, tee, inputInfo.Size, v2Auth, nil)
if err != nil {
logrus.Debugf("Error uploading layer chunked, response %#v", res)
return types.BlobInfo{}, err
Expand All @@ -176,7 +176,7 @@ func (d *dockerImageDestination) PutBlob(ctx context.Context, stream io.Reader,
// TODO: check inputInfo.Digest == computedDigest https://github.com/containers/image/pull/70#discussion_r77646717
locationQuery.Set("digest", computedDigest.String())
uploadLocation.RawQuery = locationQuery.Encode()
res, err = d.c.makeRequestToResolvedURL(ctx, "PUT", uploadLocation.String(), map[string][]string{"Content-Type": {"application/octet-stream"}}, nil, -1, v2Auth)
res, err = d.c.makeRequestToResolvedURL(ctx, "PUT", uploadLocation.String(), map[string][]string{"Content-Type": {"application/octet-stream"}}, nil, -1, v2Auth, nil)
if err != nil {
return types.BlobInfo{}, err
}
Expand All @@ -194,10 +194,10 @@ func (d *dockerImageDestination) PutBlob(ctx context.Context, stream io.Reader,
// blobExists returns true iff repo contains a blob with digest, and if so, also its size.
// If the destination does not contain the blob, or it is unknown, blobExists ordinarily returns (false, -1, nil);
// it returns a non-nil error only on an unexpected failure.
func (d *dockerImageDestination) blobExists(ctx context.Context, repo reference.Named, digest digest.Digest) (bool, int64, error) {
func (d *dockerImageDestination) blobExists(ctx context.Context, repo reference.Named, digest digest.Digest, extraScope *authScope) (bool, int64, error) {
checkPath := fmt.Sprintf(blobsPath, reference.Path(repo), digest.String())
logrus.Debugf("Checking %s", checkPath)
res, err := d.c.makeRequest(ctx, "HEAD", checkPath, nil, nil, v2Auth)
res, err := d.c.makeRequest(ctx, "HEAD", checkPath, nil, nil, v2Auth, extraScope)
if err != nil {
return false, -1, err
}
Expand All @@ -218,7 +218,7 @@ func (d *dockerImageDestination) blobExists(ctx context.Context, repo reference.
}

// mountBlob tries to mount blob srcDigest from srcRepo to the current destination.
func (d *dockerImageDestination) mountBlob(ctx context.Context, srcRepo reference.Named, srcDigest digest.Digest) error {
func (d *dockerImageDestination) mountBlob(ctx context.Context, srcRepo reference.Named, srcDigest digest.Digest, extraScope *authScope) error {
u := url.URL{
Path: fmt.Sprintf(blobUploadPath, reference.Path(d.ref.ref)),
RawQuery: url.Values{
Expand All @@ -228,7 +228,7 @@ func (d *dockerImageDestination) mountBlob(ctx context.Context, srcRepo referenc
}
mountPath := u.String()
logrus.Debugf("Trying to mount %s", mountPath)
res, err := d.c.makeRequest(ctx, "POST", mountPath, nil, nil, v2Auth)
res, err := d.c.makeRequest(ctx, "POST", mountPath, nil, nil, v2Auth, extraScope)
if err != nil {
return err
}
Expand All @@ -246,7 +246,7 @@ func (d *dockerImageDestination) mountBlob(ctx context.Context, srcRepo referenc
return errors.Wrap(err, "Error determining upload URL after a mount attempt")
}
logrus.Debugf("... started an upload instead of mounting, trying to cancel at %s", uploadLocation.String())
res2, err := d.c.makeRequestToResolvedURL(ctx, "DELETE", uploadLocation.String(), nil, nil, -1, v2Auth)
res2, err := d.c.makeRequestToResolvedURL(ctx, "DELETE", uploadLocation.String(), nil, nil, -1, v2Auth, extraScope)
if err != nil {
logrus.Debugf("Error trying to cancel an inadvertent upload: %s", err)
} else {
Expand Down Expand Up @@ -276,7 +276,7 @@ func (d *dockerImageDestination) TryReusingBlob(ctx context.Context, info types.
}

// First, check whether the blob happens to already exist at the destination.
exists, size, err := d.blobExists(ctx, d.ref.ref, info.Digest)
exists, size, err := d.blobExists(ctx, d.ref.ref, info.Digest, nil)
if err != nil {
return false, types.BlobInfo{}, err
}
Expand All @@ -286,15 +286,6 @@ func (d *dockerImageDestination) TryReusingBlob(ctx context.Context, info types.
}

// Then try reusing blobs from other locations.

// Checking candidateRepo, and mounting from it, requires an expanded token scope.
// We still want to reuse the ping information and other aspects of the client, so rather than make a fresh copy, there is this a bit ugly extraScope hack.
if d.c.extraScope != nil {
return false, types.BlobInfo{}, errors.New("Internal error: dockerClient.extraScope was set before TryReusingBlob")
}
defer func() {
d.c.extraScope = nil
}()
for _, candidate := range cache.CandidateLocations(d.ref.Transport(), bicTransportScope(d.ref), info.Digest, canSubstitute) {
candidateRepo, err := parseBICLocationReference(candidate.Location)
if err != nil {
Expand All @@ -314,7 +305,10 @@ func (d *dockerImageDestination) TryReusingBlob(ctx context.Context, info types.
}

// Whatever happens here, don't abort the entire operation. It's likely we just don't have permissions, and if it is a critical network error, we will find out soon enough anyway.
d.c.extraScope = &authScope{

// Checking candidateRepo, and mounting from it, requires an
// expanded token scope.
extraScope := &authScope{
remoteName: reference.Path(candidateRepo),
actions: "pull",
}
Expand All @@ -325,7 +319,7 @@ func (d *dockerImageDestination) TryReusingBlob(ctx context.Context, info types.
// Even worse, docker/distribution does not actually reasonably implement canceling uploads
// (it would require a "delete" action in the token, and Quay does not give that to anyone, so we can't ask);
// so, be a nice client and don't create unnecesary upload sessions on the server.
exists, size, err := d.blobExists(ctx, candidateRepo, candidate.Digest)
exists, size, err := d.blobExists(ctx, candidateRepo, candidate.Digest, extraScope)
if err != nil {
logrus.Debugf("... Failed: %v", err)
continue
Expand All @@ -335,7 +329,7 @@ func (d *dockerImageDestination) TryReusingBlob(ctx context.Context, info types.
continue // logrus.Debug() already happened in blobExists
}
if candidateRepo.Name() != d.ref.ref.Name() {
if err := d.mountBlob(ctx, candidateRepo, candidate.Digest); err != nil {
if err := d.mountBlob(ctx, candidateRepo, candidate.Digest, extraScope); err != nil {
logrus.Debugf("... Mount failed: %v", err)
continue
}
Expand Down Expand Up @@ -369,7 +363,7 @@ func (d *dockerImageDestination) PutManifest(ctx context.Context, m []byte) erro
if mimeType != "" {
headers["Content-Type"] = []string{mimeType}
}
res, err := d.c.makeRequest(ctx, "PUT", path, headers, bytes.NewReader(m), v2Auth)
res, err := d.c.makeRequest(ctx, "PUT", path, headers, bytes.NewReader(m), v2Auth, nil)
if err != nil {
return err
}
Expand Down Expand Up @@ -574,7 +568,7 @@ sigExists:
}

path := fmt.Sprintf(extensionsSignaturePath, reference.Path(d.ref.ref), d.manifestDigest.String())
res, err := d.c.makeRequest(ctx, "PUT", path, nil, bytes.NewReader(body), v2Auth)
res, err := d.c.makeRequest(ctx, "PUT", path, nil, bytes.NewReader(body), v2Auth, nil)
if err != nil {
return err
}
Expand Down
10 changes: 5 additions & 5 deletions docker/docker_image_src.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func (s *dockerImageSource) fetchManifest(ctx context.Context, tagOrDigest strin
path := fmt.Sprintf(manifestPath, reference.Path(s.ref.ref), tagOrDigest)
headers := make(map[string][]string)
headers["Accept"] = manifest.DefaultRequestedManifestMIMETypes
res, err := s.c.makeRequest(ctx, "GET", path, headers, nil, v2Auth)
res, err := s.c.makeRequest(ctx, "GET", path, headers, nil, v2Auth, nil)
if err != nil {
return nil, "", err
}
Expand Down Expand Up @@ -137,7 +137,7 @@ func (s *dockerImageSource) getExternalBlob(ctx context.Context, urls []string)
err error
)
for _, url := range urls {
resp, err = s.c.makeRequestToResolvedURL(ctx, "GET", url, nil, nil, -1, noAuth)
resp, err = s.c.makeRequestToResolvedURL(ctx, "GET", url, nil, nil, -1, noAuth, nil)
if err == nil {
if resp.StatusCode != http.StatusOK {
err = errors.Errorf("error fetching external blob from %q: %d (%s)", url, resp.StatusCode, http.StatusText(resp.StatusCode))
Expand Down Expand Up @@ -176,7 +176,7 @@ func (s *dockerImageSource) GetBlob(ctx context.Context, info types.BlobInfo, ca

path := fmt.Sprintf(blobsPath, reference.Path(s.ref.ref), info.Digest.String())
logrus.Debugf("Downloading %s", path)
res, err := s.c.makeRequest(ctx, "GET", path, nil, nil, v2Auth)
res, err := s.c.makeRequest(ctx, "GET", path, nil, nil, v2Auth, nil)
if err != nil {
return nil, 0, err
}
Expand Down Expand Up @@ -340,7 +340,7 @@ func deleteImage(ctx context.Context, sys *types.SystemContext, ref dockerRefere
return err
}
getPath := fmt.Sprintf(manifestPath, reference.Path(ref.ref), refTail)
get, err := c.makeRequest(ctx, "GET", getPath, headers, nil, v2Auth)
get, err := c.makeRequest(ctx, "GET", getPath, headers, nil, v2Auth, nil)
if err != nil {
return err
}
Expand All @@ -362,7 +362,7 @@ func deleteImage(ctx context.Context, sys *types.SystemContext, ref dockerRefere

// When retrieving the digest from a registry >= 2.3 use the following header:
// "Accept": "application/vnd.docker.distribution.manifest.v2+json"
delete, err := c.makeRequest(ctx, "DELETE", deletePath, headers, nil, v2Auth)
delete, err := c.makeRequest(ctx, "DELETE", deletePath, headers, nil, v2Auth, nil)
if err != nil {
return err
}
Expand Down

0 comments on commit e42fdf3

Please sign in to comment.