Repository navigation
fix(ci): read the upstream commit of Konflux images for main E2E coverage - #5729
Conversation
…rage
The main OCP Helm nightly deploys quay.io/rhdh/rhdh-hub-rhel10:next, a
Konflux build whose org.opencontainers.image.revision label names the
GitLab midstream commit. GitHub does not know that commit, so the on-main
check failed and every project skipped its Codecov upload:
Could not confirm that main contains image revision 30c68de...
The downstream build writes the upstream commit into the app's
build-metadata.json ("Upstream": ".../tree/main @ <short sha>"). When the
label is not on main, testing::_coverage_main_commit now greps that entry
from the bundle in the pod running the image (oc exec), expands the short
sha from the checkout or the GitHub API, and checks it is on main. The
community image keeps using its label, which names the upstream commit.
The pod listing now carries pod and container names so the exec targets
the container that runs the image. The tests' fake oc runs the real grep
against a fake bundle, so the pattern itself is covered.
RHIDP-13242
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Gustavo Lira e Silva <guga.java@gmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5729 +/- ##
==========================================
- Coverage 75.70% 69.00% -6.70%
==========================================
Files 65 58 -7
Lines 708 555 -153
Branches 100 83 -17
==========================================
- Hits 536 383 -153
Misses 171 171
Partials 1 1
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
Without it, the upload line names a sha with nothing saying it came from the bundle rather than the image label, which makes a wrong attribution hard to trace back. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Gustavo Lira e Silva <guga.java@gmail.com>
Review fixes for the build-metadata fallback: - The per-pod jsonpath put every container on one line and awk read it in groups of three. A container waiting for its image has an empty imageID, awk collapsed the gap, and the groups shifted: the RHDH container was dropped or a container name was inspected as a digest. The listing is now `oc get pods -o json | jq ... @tsv`, one line per container, and the test fake emits real pod JSON so the filter itself is exercised. - The default test pod lists a sidecar before the RHDH container, so a read of the wrong container fails the exec-target assertion. - The skip warning and the fallback info line say the label is "not confirmed on main": a GitHub outage also lands there, not only a commit outside main. - The Upstream grep tolerates a space after the colon. - _build_metadata_upstream echoes the rev-parse result explicitly, and its Args list one argument per line like the rest of lib/. - Doc: one skip condition instead of two overlapping ones. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Gustavo Lira e Silva <guga.java@gmail.com>
|
The container image build workflow finished with status: |
gashcrumb
left a comment
gashcrumb
left a comment
There was a problem hiding this comment.
Thanks. I checked the premise before the diff: skopeo inspect on quay.io/rhdh/rhdh-hub-rhel10:next shows org.opencontainers.image.revision and vcs-ref both naming the GitLab midstream sha. The image carries no upstream label, so reading the app's build metadata is the right call rather than a shortcut. The jq … @tsv listing and the extra log line both read well.
I'm requesting a change on one point. A pod that carries the image with an empty imageID still suppresses the upload for the whole project (first inline), and the test that claims to cover that case never reaches it (second inline). The other inlines are suggestions: the metadata scrape being tied to a string outside this repo, how the failures will read in a nightly log, and two doc inaccuracies.
Three small things as well. The new docstring omits the # RHIDP-13242: tag that its three neighbours carry. _build_metadata_upstream reads $1 $2 $3 inline where every sibling names its arguments first.
Address review on the Konflux upstream-commit fallback: - Drop containers with an empty imageID from the pod listing, so a replica still pulling the image no longer counts as a second digest and suppresses the upload. - Exec only into a running container of a pod that is not being deleted. - Match `redhat-developer/rhdh/tree/main ?@ ?<sha>` anywhere in the bundle instead of the exact "Upstream" entry, and pick the sha with `sort -u | head -1` so the result does not depend on readdir order. - Log why the fallback found nothing: a failed `oc exec` (exit status, stderr at debug level), metadata naming no main commit, GitHub not returning the short sha, or GitHub answering with non-JSON. - Name `_build_metadata_upstream` arguments and tag it RHIDP-13242. - Docs: the metadata lives in the minified bundle, the image is the one the job deploys, and the short sha is expanded through the checkout. Tests now put the empty imageID on the RHDH container and cover each log message, metadata shape and pod state; each fails when its guard is mutated. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
gashcrumb
left a comment
gashcrumb
left a comment
There was a problem hiding this comment.
Verified against 6fa7cc7. select(.imageID != "") keeps a pulling replica out of the listing, and the replacement test puts the empty imageID on a replica that is still pulling and asserts the upload proceeds. That is the case the old test never reached. The deletionTimestamp and state.running columns also close the hole where oc exec could land on a terminating pod.
The scrape no longer depends on the "Upstream" key or the exact spacing, and tree/release-2.1 is now a negative case, which is what makes the wider pattern safe. Failure logging separates an unreadable bundle, a grep miss, a failed GitHub call, and a non-JSON response, which is what the nightly log needed. Docs read correctly now too.
One thing is still worth doing outside this diff: confirm pods/exec on the test clusters for the job's service account. That path only runs in the nightly, so nothing here exercises the permission.
1bc3f63
into
redhat-developer:main
* fix(ci): tell a missing build-metadata entry from a failed read testing::_build_metadata_upstream treated exit 1 with empty stderr as "grep matched nothing". That only holds for a local grep: oc exec writes "command terminated with exit code 1" to stderr whenever the remote command exits non-zero. On a real cluster, a bundle without the metadata entry, or one that names another repository or branch, was logged as "Could not read the build metadata (exit 1)", which points at permissions or the path. The "names no redhat-developer/rhdh main commit" branch never ran. The upload was skipped in both cases. Only the reason in the log was wrong. grep now runs under sh -c in the container, which folds its no-match exit 1 into 0. Any non-zero exit from oc exec is then a failed read. The fake oc in the unit tests now reports a failed remote command on stderr the way oc does. With that change, the three "names another repository/branch/a sha too short" cases fail against the old code. The "no space after the colon" case replaced '": "' in a fixture that has no such text, so it repeated the base case. It now adds a space after the colon, and every metadata-shape case asserts that its variant differs from the base fixture. Follow-up to #5729. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(ci): note that the build-metadata read needs a shell in the image Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Description
Follow-up to #5702. The first main OCP Helm nightly after it merged (run 2114775302033903616) collected E2E coverage but skipped the Codecov upload in all four projects:
That nightly deploys
quay.io/rhdh/rhdh-hub-rhel10:next, a Konflux build. Itsorg.opencontainers.image.revisionlabel names the GitLab midstream commit, which does not exist on GitHub, so the on-main check fails. The upstream commit is still in the image: the downstream build writes it into the app's build metadata, which is minified into a bundle chunk:Changes
testing::_coverage_main_commitstill tries the revision label first (that is correct for the community image). When the label is not on main, it grepsredhat-developer/rhdh/tree/main ?@ ?<sha>from/opt/app-root/src/packages/app/distin a running RHDH container (oc exec), takingsort -u | head -1of the matches. It then expands the short sha from the checkout or the GitHub commits API and checks that main contains it.oc get pods -o json | jq … @tsv, one line per container that runs the image. Containers with an emptyimageID(still pulling) are dropped, so they do not count as a second digest. The exec targets a running container in a pod that is not being deleted, never a sidecar.oc execfailed (exit status, stderr at debug level), the metadata names no main commit, GitHub did not return the short sha, or GitHub answered with non-JSON.oc execneedspods/execin the test namespace, a new permission for this job.The fallback does not use the checkout's HEAD:
nextcan lag main, so HEAD may not be the commit that was tested.Testing
yarn test:unitine2e-tests: 287 passed, 1 skipped.yarn shellcheck,yarn fmt:checkand the.ciPrettier config are clean;yarn lintreports only the existingkube-client/index.tserror that main also has.@, a suffix, another key), chunks naming two commits, a refusedoc exec, a moved bundle, three GitHub failures, terminating and waiting replicas. A replica still pulling the image (emptyimageIDon the RHDH container) no longer blocks the upload. The fakeoc get podsemits real pod JSON, so the jq filter runs; the fakeoc execruns the realgrepagainst a fake bundle.imageIDfilter, the terminating and running checks, the old strict regex,sort -u, the exec-failure branch, and the two GitHub messages.grep -rhoEin the realquay.io/rhdh/rhdh-hub-rhel10:next(GNU grep 3.11): it extracts the upstream sha from the bundle and matches the metadata mutations listed above.Jira: RHIDP-13242
🤖 Generated with Claude Code