Omit Node Info rendering for 4.18 and earlier#787
Conversation
Profiling the changelog handler for a two-release diff
(4.22.0-rc.4 → 4.22.0-rc.5, two machine-OS streams) showed the
following oc invocations in the NodeImageSectionMarkdown path:
Step Duration
oc adm release info -o json <to> ~4.2s ListMachineOSStreams
oc adm release info --image-for \
rhel-coreos <from> ~4.1s ImageReferenceForComponent
oc adm release info --image-for \
rhel-coreos-10 <from> ~4.0s ImageReferenceForComponent
oc adm release info --rpmdb-diff ... ~18s RpmDiffForStream (×2)
The two --image-for calls together added ~8 seconds of wall time per
changelog request. Each invoked a separate oc process for data that is
already present in the release JSON: references.spec.tags[*].from.name
contains the full pullspec for every component in the release, and that
JSON is already cached in groupcache under the "releaseinfo" key.
Changes:
- Extend releaseInfoImageRefs with from.name so imageRefFromReleaseJSON
can extract pullspecs from the cached release JSON.
- ExecReleaseInfo.ImageReferenceForComponent now calls ReleaseInfo() and
parses the JSON instead of shelling out to oc --image-for.
- CachingReleaseInfo "imagefor" groupcache getter fetches "releaseinfo"
via cache.Get and parses locally — no extra oc invocation.
- CachingReleaseInfo "machineosstreams" getter likewise reads from the
"releaseinfo" cache entry instead of calling info.ListMachineOSStreams
(which bypassed groupcache and ran oc independently), eliminating a
duplicate oc adm release info -o json call per request.
rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
ImageReferenceForComponent now resolves from groupcache (JSON parse, no oc subprocess, no /tmp/rpmdb/ access) after the --image-for elimination. Move it outside the RpmdbOCSlots semaphore in both NodeImageSectionMarkdown and the API handler so the slot is only held during RpmListForStream and RpmDiffForStream — the calls that actually spawn oc subprocesses and write to /tmp/rpmdb/. On a cold cache with two machine-OS streams, profiling showed: ImageReferenceForComponent × 2: ~8s (now outside semaphore) RpmListForStream × 2: ~8s (inside semaphore) RpmDiffForStream × 2: ~18s (inside semaphore) This reduces per-request semaphore hold time from ~34s to ~26s, allowing more concurrent changelog requests to proceed before the 16-slot limit causes 429 responses. On a freshly restarted controller where hundreds of release pages hit the semaphore simultaneously, this frees ~8 seconds of slot time per request for other callers. rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
rpmdb collection on 4.18 and earlier requires pulling the entire rhel-coreos image. 4.19+ stores the rpmdb separately, making it significantly cheaper. Add ReleaseTagHasCheapRpmdb() which returns false for 4.x tags where x <= 18, and use it to skip Node Info in: - getChangeLog() / renderChangeLog() (HTML page and compare page) - apiReleaseInfo() (API endpoint) Unparseable tags (nightlies, CI builds) are not gated. rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Skipping CI for Draft Pull Request. |
openshift-ci
Bot
added
the
needs-rebase
|
PR needs rebase. 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. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sdodson The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
1 participant
rpmdb collection on 4.18 and earlier requires pulling the entire
rhel-coreosimage. 4.19+ stores the rpmdb separately, making it significantly cheaper.Changes
Adds
ReleaseTagHasCheapRpmdb()topkg/release-controller/semver.go, which returnsfalsefor4.xtags wherex <= 18. Unparseable tags (nightlies, CI builds) are not gated.Uses the new function to skip Node Info in three places:
getChangeLog()/renderChangeLog()— HTML page and compare pageapiReleaseInfo()— API endpoint (/api/v1/releasestream/{release}/release/{tag})For gated releases the API response will have a null
nodeImageStreamsfield, which is already a valid state. The#node-image-infoanchor links thatocmay emit in the changelog body for older releases are left as dead anchors for now.