Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DO NOT MERGE: Patch: Handle bad manifests with more logging and retries #2

Draft
wants to merge 3 commits into
base: guy/EEESUPPORT-11240/do-not-merge-tweak-layout-for-bazel-gazelle-module
Choose a base branch
from

Conversation

guyboltonking
Copy link
Owner

@guyboltonking guyboltonking commented Aug 22, 2024

This is a patch in support of DataDog/rules_oci#62; it should be reviewed in tandem.

@guyboltonking guyboltonking changed the base branch from main to guy/EEESUPPORT-11240/do-not-merge-tweak-layout-for-bazel-gazelle-module August 22, 2024 14:17
@guyboltonking guyboltonking changed the title Handle bad manifests with more logging and retries DO NOT MERGE: Patch: Handle bad manifests with more logging and retries Aug 22, 2024
}

if err := validateMediaType(ctx, p, desc.MediaType); err != nil {
return nil, fmt.Errorf("children: invalid desc %s: %w", desc.Digest, err)
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the error we've been seeing, with a JSON error in err, which suggests that the json.Unmarshal call in validateMediaType is where we're failing. That in turn suggests we need to retry the ReadBlob and the validateMediaType, which is why we extract readValidatedManifestBlob and wrap it in readValidatedManifestBlobWithRetry.


retriesRemaining -= 1

time.Sleep(time.Duration(readValidatedManifestBlobDelaySeconds) * time.Second)
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be fine, since time.Sleep is meant to interact safely with goroutines (which is what this is called within).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant