Skip to content

Commit

Permalink
chunked: define error for partial pulls not available
Browse files Browse the repository at this point in the history
define a new error type so that the caller can determine whether it is
safe to ignore the error and retrieve the resource fully.

Closes: #2115

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
  • Loading branch information
giuseppe committed Oct 1, 2024
1 parent 4bf3f07 commit f095cb5
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 18 deletions.
19 changes: 19 additions & 0 deletions pkg/chunked/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,22 @@ type ErrBadRequest struct { //nolint: errname
func (e ErrBadRequest) Error() string {
return "bad request"
}

// ErrFallbackToOrdinaryLayerDownload is a custom error type that
// suggests to the caller that a fallback mechanism can be used
// instead of a hard failure.
type ErrFallbackToOrdinaryLayerDownload struct {
Err error
}

func (c ErrFallbackToOrdinaryLayerDownload) Error() string {
return c.Err.Error()
}

func (c ErrFallbackToOrdinaryLayerDownload) Unwrap() error {
return c.Err
}

func newErrFallbackToOrdinaryLayerDownload(err error) error {
return ErrFallbackToOrdinaryLayerDownload{Err: err}
}
68 changes: 50 additions & 18 deletions pkg/chunked/storage_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,10 @@ type chunkedDiffer struct {
useFsVerity graphdriver.DifferFsVerity
fsVerityDigests map[string]string
fsVerityMutex sync.Mutex

// canFallback is set to true if the pull operation can be retried with a different
// method.
canFallback bool
}

var xattrsToIgnore = map[string]interface{}{
Expand Down Expand Up @@ -143,11 +147,13 @@ func (c *chunkedDiffer) convertTarToZstdChunked(destDirectory string, payload *o
}

// GetDiffer returns a differ than can be used with ApplyDiffWithDiffer.
// If it returns an error that implements IsErrFallbackToOrdinaryLayerDownload, the caller can
// retry the operation with a different method.
func GetDiffer(ctx context.Context, store storage.Store, blobDigest digest.Digest, blobSize int64, annotations map[string]string, iss ImageSourceSeekable) (graphdriver.Differ, error) {
pullOptions := store.PullOptions()

if !parseBooleanPullOption(pullOptions, "enable_partial_images", true) {
return nil, errors.New("enable_partial_images not configured")
return nil, newErrFallbackToOrdinaryLayerDownload(errors.New("partial images are disabled"))
}

zstdChunkedTOCDigestString, hasZstdChunkedTOC := annotations[internal.ManifestChecksumKey]
Expand All @@ -157,29 +163,50 @@ func GetDiffer(ctx context.Context, store storage.Store, blobDigest digest.Diges
return nil, errors.New("both zstd:chunked and eStargz TOC found")
}

convertImages := parseBooleanPullOption(pullOptions, "convert_images", false)

if !hasZstdChunkedTOC && !hasEstargzTOC && !convertImages {
return nil, newErrFallbackToOrdinaryLayerDownload(errors.New("no TOC found and convert_images is not configured"))
}

// The fallback mechanism must be avoided when convert_images is enabled.
canUseFallback := !convertImages

var err error
var differ graphdriver.Differ
// At this point one of hasZstdChunkedTOC, hasEstargzTOC or convertImages is true.
if hasZstdChunkedTOC {
zstdChunkedTOCDigest, err := digest.Parse(zstdChunkedTOCDigestString)
if err != nil {
return nil, fmt.Errorf("parsing zstd:chunked TOC digest %q: %w", zstdChunkedTOCDigestString, err)
zstdChunkedTOCDigest, err2 := digest.Parse(zstdChunkedTOCDigestString)
if err2 != nil {
return nil, err2
}
return makeZstdChunkedDiffer(store, blobSize, zstdChunkedTOCDigest, annotations, iss, pullOptions)
}
if hasEstargzTOC {
estargzTOCDigest, err := digest.Parse(estargzTOCDigestString)
if err != nil {
return nil, fmt.Errorf("parsing estargz TOC digest %q: %w", estargzTOCDigestString, err)
differ, err = makeZstdChunkedDiffer(store, blobSize, zstdChunkedTOCDigest, annotations, iss, pullOptions, canUseFallback)
if err == nil {
logrus.Debugf("Created zstd:chunked differ for blob %q", blobDigest)
return differ, err
}
} else if hasEstargzTOC {
estargzTOCDigest, err2 := digest.Parse(estargzTOCDigestString)
if err2 != nil {
return nil, err
}
differ, err = makeEstargzChunkedDiffer(store, blobSize, estargzTOCDigest, iss, pullOptions, canUseFallback)
if err == nil {
logrus.Debugf("Created eStargz differ for blob %q", blobDigest)
return differ, err
}
return makeEstargzChunkedDiffer(store, blobSize, estargzTOCDigest, iss, pullOptions)
}
// If convert_images is enabled, always attempt to convert it instead of returning an error or falling back to a different method.
if convertImages {
logrus.Debugf("Created differ to convert blob %q", blobDigest)
return makeConvertFromRawDiffer(store, blobDigest, blobSize, iss, pullOptions)
}

return makeConvertFromRawDiffer(store, blobDigest, blobSize, iss, pullOptions)
logrus.Debugf("Could not create differ for blob %q: %v", blobDigest, err)
return nil, err
}

func makeConvertFromRawDiffer(store storage.Store, blobDigest digest.Digest, blobSize int64, iss ImageSourceSeekable, pullOptions map[string]string) (*chunkedDiffer, error) {
if !parseBooleanPullOption(pullOptions, "convert_images", false) {
return nil, errors.New("convert_images not configured")
}

layersCache, err := getLayersCache(store)
if err != nil {
return nil, err
Expand All @@ -197,7 +224,7 @@ func makeConvertFromRawDiffer(store storage.Store, blobDigest digest.Digest, blo
}, nil
}

func makeZstdChunkedDiffer(store storage.Store, blobSize int64, tocDigest digest.Digest, annotations map[string]string, iss ImageSourceSeekable, pullOptions map[string]string) (*chunkedDiffer, error) {
func makeZstdChunkedDiffer(store storage.Store, blobSize int64, tocDigest digest.Digest, annotations map[string]string, iss ImageSourceSeekable, pullOptions map[string]string, canFallback bool) (*chunkedDiffer, error) {
manifest, toc, tarSplit, tocOffset, err := readZstdChunkedManifest(iss, tocDigest, annotations)
if err != nil {
return nil, fmt.Errorf("read zstd:chunked manifest: %w", err)
Expand All @@ -220,10 +247,11 @@ func makeZstdChunkedDiffer(store storage.Store, blobSize int64, tocDigest digest
stream: iss,
tarSplit: tarSplit,
tocOffset: tocOffset,
canFallback: canFallback,
}, nil
}

func makeEstargzChunkedDiffer(store storage.Store, blobSize int64, tocDigest digest.Digest, iss ImageSourceSeekable, pullOptions map[string]string) (*chunkedDiffer, error) {
func makeEstargzChunkedDiffer(store storage.Store, blobSize int64, tocDigest digest.Digest, iss ImageSourceSeekable, pullOptions map[string]string, canFallback bool) (*chunkedDiffer, error) {
manifest, tocOffset, err := readEstargzChunkedManifest(iss, blobSize, tocDigest)
if err != nil {
return nil, fmt.Errorf("read zstd:chunked manifest: %w", err)
Expand All @@ -244,6 +272,7 @@ func makeEstargzChunkedDiffer(store storage.Store, blobSize int64, tocDigest dig
pullOptions: pullOptions,
stream: iss,
tocOffset: tocOffset,
canFallback: canFallback,
}, nil
}

Expand Down Expand Up @@ -949,6 +978,9 @@ func (c *chunkedDiffer) retrieveMissingFiles(stream ImageSourceSeekable, dirfd i
if _, ok := err.(ErrBadRequest); ok {
// If the server cannot handle at least 64 chunks in a single request, just give up.
if len(chunksToRequest) < 64 {
if c.canFallback {
return newErrFallbackToOrdinaryLayerDownload(err)
}
return err
}

Expand Down

0 comments on commit f095cb5

Please sign in to comment.