-
Notifications
You must be signed in to change notification settings - Fork 60
[WIP] OCPBUGS-56269: Remove registry cache placeholder on scrape failure #1025
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
base: master
Are you sure you want to change the base?
[WIP] OCPBUGS-56269: Remove registry cache placeholder on scrape failure #1025
Conversation
|
@PratikMahajan: This pull request references Jira Issue OCPBUGS-56269, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: PratikMahajan The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
d2dddf1 to
b806084
Compare
Current production Cincinnati occasionally confuses arch-specific shards of multi-arch images as single-arch releases Possibly triggered by a race or other buggy handling of Quay 502s while scraping releases. fix: check if the metadata is multi for layered and registry and ignore those single arch shards Co-authored-by: wking <wking@tremily.us>
This reverts commit 9713747.
replace the placeholders with `None` so no more 0.0.0 and remove the placeholder from the cache of we fail to get any release metadata Co-authored-by: wking <wking@tremily.us>
b806084 to
ed9c72f
Compare
|
@PratikMahajan: This pull request references Jira Issue OCPBUGS-56269, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
| Err(e) => { | ||
| debug!( | ||
| return Err(format_err!( | ||
| "[{}] Could not assemble metadata from layer ({}): {}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey look, there's some CI coverage in cargo-test:
cincinnati: plugins::internal::graph_builder::release_scrape_dockerv2::plugin::network_tests::scrape_public_with_first_empty_tag_must_succeed expand_less 1s
{Error: failed to fetch all release metadata from registry.ci.openshift.org/cincinnati-ci-public/cincinnati-test-emptyfirsttag-public-manual test failure with exit code 101 Error: failed to fetch all release metadata from registry.ci.openshift.org/cincinnati-ci-public/cincinnati-test-emptyfirsttag-public-manual
Caused by:
[0.0.0] Could not assemble metadata from layer (sha256:4ca545ee6d5db5c1170386eeb39b2ffe3bd46e5d4a73a9acbebc805f19607eb3): 'release-manifests/release-metadata' not found}
cincinnati: plugins::internal::graph_builder::release_scrape_dockerv2::plugin::network_tests::scrape_public_multiarch_manual_succeeds expand_less 1s
{Error: failed to fetch all release metadata from registry.ci.openshift.org/cincinnati-ci-public/cincinnati-test-public-multiarch-manual test failure with exit code 101 Error: failed to fetch all release metadata from registry.ci.openshift.org/cincinnati-ci-public/cincinnati-test-public-multiarch-manual
Caused by:
[0.0.3-amd64] Could not assemble metadata from layer (sha256:4ca545ee6d5db5c1170386eeb39b2ffe3bd46e5d4a73a9acbebc805f19607eb3): 'release-manifests/release-metadata' not found}
I'm not clear yet on whether that's turning up fixtures/tests we should adjust, or pointing out issues in our new code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, seems like that's "we didn't find find release-manifests/release-metadata in the top layer", and we need to adjust our error handling here to allow it to continue further in the for layer_digest in layer_digests loop, in case we can find the metadata file in a deeper layer. I'm not sure how to express that in Rust 😅 I think we might need to adjust assemble_metadata to return an Option, so it can say "I was successfully able to process this blob, but I didn't find metadata_filename inside it"?
|
@PratikMahajan: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
Issues go stale after 90d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle stale |
|
Important Review skippedAuto reviews are limited based on label configuration. 🚫 Review skipped — only excluded labels are configured. (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
|
Stale issues rot after 30d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle rotten |
replace the placeholders with
Noneso no more 0.0.0
and
remove the placeholder from the cache of we fail to get any release
metadata