OTA-1348: Fix dkrv2_openshift_secondary_metadata_scraper openshift_secondary_metadata_extraction test
#978
Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
DavidHurta
force-pushed
the
fix-dkrv2_openshift_secondary_metadata_scraper-openshift_secondary_metadata_extraction-test
branch
from
fad69ed to
2e6b58a
Compare
|
/test cargo-test |
1 similar comment
|
/test cargo-test |
|
/test all |
DavidHurta
force-pushed
the
fix-dkrv2_openshift_secondary_metadata_scraper-openshift_secondary_metadata_extraction-test
branch
from
369fc98 to
49a6e92
Compare
DavidHurta
changed the title
openshift_secondary_metadata_extraction failing testdkrv2_openshift_secondary_metadata_scraper openshift_secondary_metadata_extraction test
openshift-ci-robot
added
the
jira/valid-reference
|
@DavidHurta: This pull request references OTA-1348 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set. In 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. |
|
/test all |
DavidHurta
force-pushed
the
fix-dkrv2_openshift_secondary_metadata_scraper-openshift_secondary_metadata_extraction-test
branch
from
49a6e92 to
383c6e7
Compare
|
/test all |
There is a missing forward slash when appending image repository to a registry. This is resulting in the following test to fail: ``` cincinnati: plugins::internal::graph_builder:: dkrv2_openshift_secondary_metadata_scraper::plugin::network_tests:: openshift_secondary_metadata_extraction ``` The test fails with an error message (note the missing forward slash in the URL): ``` test failure Error: http transport error: error sending request for url (https://registry.ci.openshift.orgcincinnati-ci-public/v2/): error trying to connect: dns error: failed to lookup address information: Name or service not known ``` The new change is identical to that of the same logic in the cincinnati/src/plugins/internal/graph_builder/release_scrape_dockerv2/plugin.rs file. [1] https://github.com/openshift/cincinnati/blob/4398c21d56c19f6f6cef2655fef38a1b822a5aa6/cincinnati/src/plugins/internal/graph_builder/release_scrape_dockerv2/plugin.rs#L121
There is a lot of CI clusters. The app.ci [1] cluster is the required one. Clarify this in the header comment. [1] https://console-openshift-console.apps.ci.l2s4.p1.openshiftapps.com/
… in test Dockerfile The previously used quay.io/app-sre/busybox base container image is not easily accessible and requires authorization. Only the `tar` command is needed from the base image. Instead, use an accessible base container image registry.access.redhat.com/ubi9/ubi, which contains the necessary tooling, authorization is not needed, and is from a reputable source. The base image is quite large for its purpose; however, the image is only used as a builder image and its content are not part of the final image.
… for images Otherwise, the manifests of the images are created with the format `application/vnd.oci.image.manifest.v1+json`. This format is not supported by the dkregistry-rs library [1] and thus also by the DkrV2OpenshiftSecondaryMetadataScraperSettings plugin which uses the dkregistry library. The DkrV2OpenshiftSecondaryMetadataScraperSettings plugin creates a registry client [2] that has concrete accepted type of content [3]. The oci container image format is not one of them. The openshift_secondary_metadata_extraction test was failing on 404 HTTP responses when it was fetching a container manifest file. Even though the image did exist, the client filled the `Accept` Header with docker formats. The registry did not have such a manifest, it had only the `oci` format manifest. Thus, it returned the 404 code. Following is a representation using curl of this issue: ``` $ curl -v -H "Authorization: Bearer $BEARER" \ -H "Accept: application/vnd.docker.distribution.manifest.v2+json" \ -X GET \ "https://registry.ci.openshift.org/v2/cincinnati-ci-public/cincinnati-graph-data/manifests/6420f7fbf3724e1e5e329ae8d1e2985973f60c14" ... OMITTED OUTPUT > GET /v2/cincinnati-ci-public/cincinnati-graph-data/manifests/6420f7fbf3724e1e5e329ae8d1e2985973f60c14 HTTP/2 > Host: registry.ci.openshift.org > User-Agent: curl/8.6.0 > Authorization: Bearer > Accept: application/vnd.docker.distribution.manifest.v2+json > * TLSv1.3 (IN), TLS handshake, Newsession Ticket (4): * TLSv1.3 (IN), TLS handshake, Newsession Ticket (4): * old SSL session ID is stale, removing < HTTP/2 404 < content-type: application/json < docker-distribution-api-version: registry/2.0 < x-registry-supports-signatures: 1 < content-length: 122 < date: Tue, 05 Nov 2024 10:49:47 GMT < set-cookie: <cookie>; path=/; HttpOnly; Secure; SameSite=None < {"errors":[{"code":"MANIFEST_UNKNOWN","message":"OCI manifest found, but accept header does not support OCI manifests"}]} * Connection #0 to host registry.ci.openshift.org left intact ``` [1] https://github.com/camallo/dkregistry-rs/blob/3e242ee9e39646da6ff4a886e080085cc1810d37/src/mediatypes.rs#L10-L39 [2] https://github.com/openshift/cincinnati/blob/04ede1e16e14e9acf6b56515db49a014b82c1fd7/cincinnati/src/plugins/internal/graph_builder/dkrv2_openshift_secondary_metadata_scraper/plugin.rs#L276-L283 [3] https://github.com/openshift/cincinnati/blob/04ede1e16e14e9acf6b56515db49a014b82c1fd7/cincinnati/src/plugins/internal/graph_builder/release_scrape_dockerv2/registry/mod.rs#L213-L217
… newly updated test container image A new [1] test image was pushed. It is necessary to update its local signature file. Done using the gpg utility. [1] registry.ci.openshift.org/cincinnati-ci-public/cincinnati-graph-data:6420f7fbf3724e1e5e329ae8d1e2985973f60c14
DavidHurta
force-pushed
the
fix-dkrv2_openshift_secondary_metadata_scraper-openshift_secondary_metadata_extraction-test
branch
from
383c6e7 to
8f1f2a4
Compare
...nati/src/plugins/internal/graph_builder/dkrv2_openshift_secondary_metadata_scraper/plugin.rs
Outdated
Show resolved
Hide resolved
| @@ -15,7 +15,7 @@ for target in "${@}"; do | |||
| tag="${tag_raw/_/-}" | |||
|
|
|||
| full_tag="${REPO_BASE}-${repo}:${tag}" | |||
| buildah bud -t "${full_tag}" "${target}" | |||
| buildah bud -t "${full_tag}" --format docker "${target}" | |||
There was a problem hiding this comment.
This graph-builder/tests/images/build-n-push-buildah.sh script is run in two places.
- cincinnati/src/plugins/internal/graph_builder/dkrv2_openshift_secondary_metadata_scraper/test_fixtures/build.sh
- dist/prepare_ci_images.sh
This change is needed for the former script as described in the change's commit. However, I have not run the latter script with the new changes. I would like to run it after all tests are fixed to verify the change and to more easily notice any tests start breaking.
Just checked, it seems running the script won't be necessary probably; the existing images produced by the latter script already have the docker format.
$ podman manifest inspect registry.ci.openshift.org/cincinnati-ci-public/cincinnati-test-emptyfirsttag-public-manual:0.0.0 | jq .mediaType
WARN[0000] The manifest type application/vnd.docker.distribution.manifest.v2+json is not a manifest list but a single image.
"application/vnd.docker.distribution.manifest.v2+json"
|
@DavidHurta: This pull request references OTA-1348 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set. In 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. |
|
/uncc LalatenduMohanty |
openshift-ci
bot
requested review from
petr-muller
and removed request for
LalatenduMohanty
openshift-ci
bot
added
the
approved
|
/test all |
|
/test customrust-e2e |
2 similar comments
|
/test customrust-e2e |
|
/test customrust-e2e |
| "(^|/)LICENSE$", | ||
| "(^|/)channels/.+\\.ya+ml$", | ||
| "(^|/)blocked-edges/.+\\.ya+ml$", | ||
| "(^|/)raw/metadata.json$", | ||
| "(^|/)version$", |
There was a problem hiding this comment.
i'm particularly concerned about this change.
we've similar code fragment in the other secondary parser https://github.com/openshift/cincinnati/blob/master/cincinnati/src/plugins/internal/graph_builder/github_openshift_secondary_metadata_scraper/plugin.rs#L13-L19
does this change affect the outcome of unit/e2es etc ?
There was a problem hiding this comment.
When debugging the unit tests I have written down my findings in the 005098f commit:
...
Currently, the regex patterns inside the
dkrv2_openshift_secondary_metadata_scraperplugin are matched
against the archive files (e.g.version); however, regex
patterns inside thegithub_openshift_secondary_metadata_scraper
plugin are matched against the files prefixed with the archive
directory (e.g.archive/version). This is most likely dependent
on the underlying decoder library or the specific archive.
...
I did not have much luck in understanding exactly why. I have concluded that the chosen decoder or the archive likely influences the functionality.
Let me run the CI on the PR changes excluding this one to check e2e 👍 Without this change, the unit tests locally did not pass for me as nothing was getting matched by the original DEFAULT_OUTPUT_ALLOWLIST regex patterns.
There was a problem hiding this comment.
The e2e job passes with and without this change. Check PR test history. Although, I do not know what the e2e job is testing.
The cargo test job starts failing with panicked at 'no files were extracted':
thread 'plugins::internal::graph_builder::dkrv2_openshift_secondary_metadata_scraper::plugin::network_tests::openshift_secondary_metadata_extraction' panicked at 'no files were extracted', cincinnati/src/plugins/internal/graph_builder/dkrv2_openshift_secondary_metadata_scraper/plugin.rs:723:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
This means the current regex patterns on master were not able to match anything, and thus nothing was extracted.
There was a problem hiding this comment.
The c466179 in the full PR test history reverted the commits regarding the modifications to the DEFAULT_OUTPUT_ALLOWLIST variable.
I am dropping the reverts.
|
/hold |
|
/test customrust-e2e |
DavidHurta
force-pushed
the
fix-dkrv2_openshift_secondary_metadata_scraper-openshift_secondary_metadata_extraction-test
branch
from
c466179 to
005098f
Compare
|
/unhold |
openshift-ci
bot
removed
the
do-not-merge/hold
|
/test e2e |
haha, do you want to address this in this PR and see the tests pass, or do you want to make a followup? |
|
Thanks, Petr, for noticing. I have already created #997 💪 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: DavidHurta, petr-muller The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/override ci/prow/cargo-test |
|
@petr-muller: Overrode contexts on behalf of petr-muller: ci/prow/cargo-test, ci/prow/customrust-cargo-test In 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 kubernetes-sigs/prow repository. |
|
/hold |
openshift-ci
bot
added
the
do-not-merge/hold
|
I am still waiting to resolve the #978 (comment) with @PratikMahajan. |
|
/test cargo-test customrust-cargo-test |
|
With #997 merged we should see beautiful full green runs now |
|
@DavidHurta: all tests passed! Full PR test history. Your PR dashboard. 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 kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
|
|
/joke |
|
@petr-muller: What do vegetarian zombies eat? Grrrrrainnnnnssss. In 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 kubernetes-sigs/prow repository. |
Fixes the
cincinnati: plugins::internal::graph_builder::dkrv2_openshift_secondary_metadata_scraper::plugin::network_tests::openshift_secondary_metadata_extractiontest that is currently failing.