lockfile: align RPM versions across architectures to highest common EVR - #3219
lockfile: align RPM versions across architectures to highest common EVR#3219redhat-chai-bot wants to merge 1 commit into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
|
Warning Review limit reached
Next review available in: 10 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository: openshift-eng/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
WalkthroughLockfile generation now aligns mismatched RPM versions across architectures. It selects the highest EVR available for all applicable architectures, replaces entries from loaded repositories, preserves pinned entries, and validates repository lookup behavior with comprehensive tests. ChangesCross-architecture RPM alignment
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant LockfileGeneration
participant Alignment
participant RepositoryLookup
participant LoadedRepositories
LockfileGeneration->>Alignment: align resolved RPM entries
Alignment->>RepositoryLookup: locate target EVR
RepositoryLookup->>LoadedRepositories: search RPM metadata
LoadedRepositories-->>RepositoryLookup: return matching RPM
RepositoryLookup-->>Alignment: return replacement metadata
Alignment-->>LockfileGeneration: return aligned entries
🚥 Pre-merge checks | ✅ 10 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (10 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
/hold for tests |
|
@rayfordj: Overrode contexts on behalf of rayfordj: ci/prow/art-pre-commit-check 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 kubernetes-sigs/prow repository. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
doozer/doozerlib/lockfile.py (1)
851-879: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winConsider validating the target on all affected arches before replacing any.
The loop replaces each arch independently. If the target EVR is missing from one arch's loaded repos, that arch keeps its higher version while the other arches are downgraded. The result is still mismatched, and the downgrades were then unnecessary.
test_alignment_warns_when_target_version_not_foundcovers the two-arch case only, where no replacement happens.Collect the candidate replacements per package first. Apply them only when every arch above the target resolves the target EVR. Otherwise skip that package and log one warning.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@doozer/doozerlib/lockfile.py` around lines 851 - 879, Update the replacement logic around replacement_rpms and _find_rpm_by_evr_in_repos to collect candidates per package across all affected arches before mutating replacement_rpms. Apply replacements only when every arch above the target resolves the target EVR; if any arch is missing it, skip that package entirely and emit one warning, preserving all existing versions. Keep already-aligned arches excluded from validation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@doozer/doozerlib/lockfile.py`:
- Around line 790-808: Update the docstring for the RPM alignment method to
describe selecting the minimum of the per-architecture latest EVRs, matching the
implementation and inline comment near the target-version calculation. Remove
the claim that this value is the highest EVR available on all architectures,
while preserving the documented handling of pinned entries and returned
structure.
- Around line 890-899: Update the rebuild loop in _fetch_rpms_info_per_arch’s
alignment logic to deduplicate each arch list by (rpm.name, rpm.evr), matching
the existing invariant used when fetching RPM metadata. Ensure replacing the
latest entry does not append a duplicate when a pinned entry already has the
target EVR, while preserving distinct packages and sorting the resulting list.
---
Nitpick comments:
In `@doozer/doozerlib/lockfile.py`:
- Around line 851-879: Update the replacement logic around replacement_rpms and
_find_rpm_by_evr_in_repos to collect candidates per package across all affected
arches before mutating replacement_rpms. Apply replacements only when every arch
above the target resolves the target EVR; if any arch is missing it, skip that
package entirely and emit one warning, preserving all existing versions. Keep
already-aligned arches excluded from validation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift-eng/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e64aa445-6c32-49c1-a993-001131dbf79a
📒 Files selected for processing (2)
doozer/doozerlib/lockfile.pydoozer/tests/test_lockfile_alignment.py
|
/override ci/prow/art-pre-commit-check |
|
@rayfordj: Overrode contexts on behalf of rayfordj: ci/prow/art-pre-commit-check 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 kubernetes-sigs/prow repository. |
87a01d0 to
32ac21b
Compare
|
/override ci/prow/art-pre-commit-check |
|
@rayfordj: Overrode contexts on behalf of rayfordj: ci/prow/art-pre-commit-check 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 kubernetes-sigs/prow repository. |
When independent per-arch RPM resolution picks different "latest" versions, Conforma rpm_packages.unique_version violations occur at release time because multi-arch images end up with different RPM versions per architecture. Add _align_cross_arch_versions() to RPMLockfileGenerator that runs after validation and before the lockfile is written. For each package appearing on 2+ arches with mismatched latest EVRs, it downgrades the higher arches to the highest version available on ALL arches by scanning loaded repos for the target version. NVR-pinned entries are preserved unchanged. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
32ac21b to
8f1e569
Compare
|
/override ci/prow/art-pre-commit-check |
|
@rayfordj: Overrode contexts on behalf of rayfordj: ci/prow/art-pre-commit-check 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 kubernetes-sigs/prow repository. |
|
@redhat-chai-bot: The following test 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. |
|
/override ci/prow/art-pre-commit-check |
|
@rayfordj: Overrode contexts on behalf of rayfordj: ci/prow/art-pre-commit-check 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 kubernetes-sigs/prow repository. |
|
/close |
|
@rayfordj: Closed this PR. 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 kubernetes-sigs/prow repository. |
Summary
Add cross-architecture RPM version alignment to the lockfile generator so that all architectures use the same RPM version in the lockfile. This prevents Conforma
rpm_packages.unique_versionviolations during Konflux release pipelines.Problem
The current
generate_lockfile()flow resolves RPMs independently per architecture via_fetch_rpms_info_per_arch(). Each arch picks its own highest available EVR from the configured repos. When an RPM update has propagated to one arch's repos but not another's, the lockfile ends up with different versions per arch. This causes theverify-conformarelease task to reject the build withrpm_packages.unique_versionviolations._validate_cross_arch_version_sets()(added in PR #2593 for ART-14488) detects these mismatches but only logs a WARNING — there is no reconciliation step.This gap caused MTA 8.2 build failures (build/layered-products #12088) where all Konflux builds succeeded but the release pipeline rejected them due to cross-arch RPM version mismatches.
Changes
doozer/doozerlib/lockfile.py:New method
_align_cross_arch_versions(rpms_info_by_arch, repo_names)onRPMLockfileGenerator:New helper
_find_rpm_by_evr_in_repos(pkg_name, target_evr, arch, repo_names)that searches loaded repos in canonical order for a specific package+EVR+arch combinationIntegration: single-line call in
generate_lockfile()after the existing_validate_cross_arch_version_sets()warning and before lockfile YAML is writtendoozer/tests/test_lockfile_alignment.py:Testing
All 76 tests pass (56 existing + 20 new).
Related Issues
perfRPMAI-generated. Review for accuracy.
@rayfordj requested in Slack thread
Summary by CodeRabbit