So I've spent a little bit of time digging into the logic of the resolver/fetcher in the docker remote.

I've identified the locations in the resolver and fetcher that would require updating to improve the mirror fallback functionality.

Resolver:

Lines 285 to 290 in 3a31ce2

    
           if err != nil { 
        
           	if errors.Cause(err) == ErrInvalidAuthorization { 
        
           		err = errors.Wrapf(err, "pull access denied, repository does not exist or may require authorization") 
        
           	} 
        
           	return "", ocispec.Descriptor{}, err 
        
           }

Fetcher:

containerd/remotes/docker/fetcher.go

Lines 102 to 108 in 3a31ce2

    
           if err != nil { 
        
           	if errdefs.IsNotFound(err) { 
        
           		continue // try another host 
        
           	} 
        
           	return nil, err 
        
           }

containerd/remotes/docker/fetcher.go

Lines 119 to 125 in 3a31ce2

    
           if err != nil { 
        
           	if errdefs.IsNotFound(err) { 
        
           		continue // try another host 
        
           	} 
        
           	return nil, err 
        
           }

One approach we could consider is blindly trying the next host if an error is returned:

for _, host := range r.hosts {
	...
	resp, err := req.doWithRetries(ctx, nil)
	if err != nil {
		// log error
		continue // blindly try next host
	}
	...
}

This approach works, but I'm not sure on whether this is desired.

Another option, which I think is more elegant is to check the error and only continue on specific errors:

for _, host := range r.hosts {
	...
	resp, err := req.doWithRetries(ctx, nil)
	if err != nil {
		if continueOnError(err) {
			continue // try next host
		}
		return "", ocispec.Descriptor{}, err
	}
	...
}

continueOnError might be similar to the function in moby - https://github.com/moby/moby/blob/3152f9436292115c97b4d8bb18c66cf97876ee75/distribution/errors.go#L115-L144

One concern I had that some of the errors I've faced are quite nested and also don't implement causer, e.g.

failed to do request: Get http://127.0.0.1:1234/v2/library/ubuntu/manifests/sha256:3590458403b522985068fa21888da3e351e5c72833936757c33baf9555b09e1e: dial tcp 127.0.0.1:1234: connect: connection refused

So determining the root cause of the error might be tricky. errors.Cause isn't able to unwrap the error down to the root.

Thoughts @dmcgowan?

	if err != nil {
	if errors.Cause(err) == ErrInvalidAuthorization {
	err = errors.Wrapf(err, "pull access denied, repository does not exist or may require authorization")
	}
	return "", ocispec.Descriptor{}, err
	}

	if err != nil {
	if errdefs.IsNotFound(err) {
	continue // try another host
	}

	return nil, err
	}

More robust mirror fallback #3850

Description

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions