Skip to content

Fix Release bumping for RHEL in maintenance phase#673

Open
nforro wants to merge 1 commit into
packit:mainfrom
nforro:maintenance
Open

Fix Release bumping for RHEL in maintenance phase#673
nforro wants to merge 1 commit into
packit:mainfrom
nforro:maintenance

Conversation

@nforro

@nforro nforro commented Jul 9, 2026

Copy link
Copy Markdown
Member

RHEL version in maintenance phase (currently RHEL 8.10) is being fixed via CentOS Stream, but the fixes still target an internal Z-Stream branch, so Release bumping must follow Z-Stream logic.

@qodo-for-packit

Copy link
Copy Markdown

PR Summary by Qodo

Fix release bumping for maintenance-phase RHEL CentOS Stream branches

🐞 Bug fix 🧪 Tests 🕐 40+ Minutes

Grey Divider

AI Description

• Treat CentOS Stream maintenance branches (e.g. c8s) as Z-stream for release bumping.
• Add branch classifier using RHEL stream config to detect maintenance majors.
• Extend unit tests to cover maintenance-branch detection and spec Release updates.
Diagram

graph TD
A["UpdateReleaseTool._run"] --> B["_get_higher_stream_branch()"] -->|"found"| F["_set_zstream_release()"]
A["UpdateReleaseTool._run"] --> B -->|"none"| C["is_maintenance_cs_branch()"] --> D["load_rhel_config()"] --> E["get_maintenance_majors()"]
C -->|"true"| F
C -->|"false"| G["_bump_or_reset_release()"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Fold maintenance-CS logic into _get_higher_stream_branch()
  • ➕ Keeps all 'treat as Z-stream' routing logic in one place
  • ➕ Avoids additional async call site logic in UpdateReleaseTool._run
  • ➖ Mixes branch-parsing concerns with config-backed lifecycle state (maintenance)
  • ➖ May reduce clarity by overloading a function named around 'higher stream' detection
2. Load/cache RHEL config once per tool run (or per process)
  • ➕ Avoids repeated async config loads when the tool is invoked frequently
  • ➕ Makes classification deterministic for the duration of a run
  • ➖ Requires lifecycle management (cache invalidation) if config can change
  • ➖ Adds complexity if current behavior expects fresh config per call
3. Introduce a single branch classification API (enum: Y/Z/maintenance-CS/...)
  • ➕ Makes routing decisions explicit and easier to extend for future branch types
  • ➕ Reduces scattered branch-type heuristics across tools
  • ➖ Bigger refactor than needed for this targeted fix
  • ➖ More code churn for a maintenance-branch edge case

Recommendation: The PR’s approach is a good minimal fix: it reuses existing Z-stream bumping behavior and adds a focused classifier backed by the authoritative RHEL stream config, with tests covering both the classifier and the tool routing. If this logic expands further, consider introducing a central branch classification API or adding lightweight caching for load_rhel_config().

Files changed (4) +133 / -4

Bug fix (2) +21 / -4
version_utils.pyIntroduce is_maintenance_cs_branch() helper using RHEL stream config +12/-0

Introduce is_maintenance_cs_branch() helper using RHEL stream config

• Adds an async helper that parses a branch name, filters to CentOS Stream majors, loads RHEL stream config, and determines whether the major is in maintenance (present in current_z_streams but not current_y_streams). This enables other components to treat maintenance CS branches with Z-stream semantics.

ymir/common/version_utils.py

specfile.pyRoute maintenance CS branches through Z-stream release bumping +9/-4

Route maintenance CS branches through Z-stream release bumping

• Extends UpdateReleaseTool behavior so that if no higher-stream branch is found, a maintenance CentOS Stream branch (e.g. c8s) is treated like Z-stream and uses _set_zstream_release(). Updates the tool description to reflect this expanded Z-stream routing condition.

ymir/tools/unprivileged/specfile.py

Tests (2) +112 / -0
test_version_utils.pyAdd tests for maintenance CentOS Stream branch detection +33/-0

Add tests for maintenance CentOS Stream branch detection

• Adds flexmock-based mocking for config loading and introduces a new async parametrized test for is_maintenance_cs_branch(). The test validates that only c8s (given the provided config) is treated as a maintenance-phase CentOS Stream branch.

ymir/common/tests/unit/test_version_utils.py

test_specfile.pyAdd UpdateReleaseTool tests for maintenance CS Z-stream bumping path +79/-0

Add UpdateReleaseTool tests for maintenance CS Z-stream bumping path

• Adds a dedicated test covering c8s behavior to ensure the tool applies Z-stream release bumping semantics for maintenance CS branches across spec variants and rebase scenarios. Also ensures existing tests mock is_maintenance_cs_branch() explicitly to avoid unintended behavior changes.

ymir/tools/unprivileged/tests/unit/test_specfile.py

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces support for CentOS Stream branches of RHEL versions in the maintenance phase (such as c8s) to utilize Z-stream release bumping logic. It adds the helper function is_maintenance_cs_branch with corresponding unit tests and integrates it into the UpdateReleaseTool. Feedback on the changes highlights an efficiency issue where setting higher_stream_branch to the same value as dist_git_branch causes redundant, concurrent I/O calls to get_latest_candidate_build inside _set_zstream_release. An optimization is suggested to prevent these duplicate calls when the branches are identical.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread ymir/tools/unprivileged/specfile.py Outdated
@qodo-for-packit

qodo-for-packit Bot commented Jul 9, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 7 rules

Grey Divider


Action required

1. Main branch misdetected ✓ Resolved 🐞 Bug ≡ Correctness
Description
is_maintenance_cs_branch() treats any branch parsed as (major, None) as a CentOS Stream branch, so
an internal main branch like rhel-8-main would be classified as a maintenance CS branch when major 8
is in maintenance. This can route UpdateReleaseTool into Z-stream release logic for the wrong
branch/tag set, yielding incorrect Release values or failing when expected Koji tags don’t exist.
Code

ymir/common/version_utils.py[R128-137]

+async def is_maintenance_cs_branch(branch: str) -> bool:
+    """Check if branch is a CentOS Stream branch for a RHEL version in maintenance phase."""
+    parsed = parse_branch_name(branch)
+    if not parsed or parsed[1] is not None:
+        return False
+    major, _ = parsed
+    from ymir.common.config import load_rhel_config
+
+    config = await load_rhel_config()
+    return major in get_maintenance_majors(config)
Relevance

⭐⭐⭐ High

Team recently refined branch parsing to avoid misapplying Z-stream logic to non-zstream/main
branches (PR #599).

PR-#599

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
parse_branch_name returns (major, None) for both CentOS Stream branches (c9s) and RHEL main
branches (rhel-10-main), and is_maintenance_cs_branch only checks for minor is None before
consulting maintenance majors, so it can’t distinguish these branch families. The project already
defines is_cs_branch as a strict ^c\d+s$ match, and UpdateReleaseTool uses
is_maintenance_cs_branch to decide whether to run Z-stream release bump logic.

ymir/common/version_utils.py[59-84]
ymir/common/version_utils.py[128-137]
ymir/common/base_utils.py[163-168]
ymir/tools/unprivileged/specfile.py[383-405]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`is_maintenance_cs_branch()` currently cannot distinguish `c8s` from `rhel-8-main` because both parse to `(major, None)`, so it may return `True` for RHEL main branches of a maintenance major.

### Issue Context
The codebase already has a strict CentOS Stream branch detector (`is_cs_branch` matching `^c\d+s$`). The maintenance-phase check should only ever apply to CS branches.

### Fix Focus Areas
- ymir/common/version_utils.py[128-137]
- ymir/common/base_utils.py[163-168]

### Suggested change
- In `is_maintenance_cs_branch`, first check `is_cs_branch(branch)` (import from `ymir.common.base_utils`). If it’s not a CS branch, return `False` without calling `parse_branch_name()`/`load_rhel_config()`.
- Then extract the major from the CS branch (either via `parse_branch_name` or a small CS regex) and check it against `get_maintenance_majors(config)`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Config errors bypass ToolError ✓ Resolved 🐞 Bug ☼ Reliability
Description
UpdateReleaseTool._run() calls is_maintenance_cs_branch() before entering its try/except, so
FileNotFoundError/ValueError from load_rhel_config() will escape without being wrapped as ToolError.
This changes the tool’s failure contract and can crash callers that expect ToolError from tool
execution.
Code

ymir/tools/unprivileged/specfile.py[R389-393]

        spec_path = get_absolute_path(tool_input.spec, self)
+        higher_stream_branch = self._get_higher_stream_branch(tool_input.dist_git_branch)
+        if not higher_stream_branch and await is_maintenance_cs_branch(tool_input.dist_git_branch):
+            higher_stream_branch = tool_input.dist_git_branch
        try:
-            if not (higher_stream_branch := self._get_higher_stream_branch(tool_input.dist_git_branch)):
-                await self._bump_or_reset_release(spec_path, tool_input.rebase)
-            else:
Relevance

⭐⭐⭐ High

Repo prefers consistent ToolError-wrapping for underlying exceptions; similar hardening accepted in
privileged tools (PR #596).

PR-#596

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
is_maintenance_cs_branch() awaits load_rhel_config(), which can raise FileNotFoundError or
ValueError. Because _run() performs the await before entering its try: block, those exceptions
won’t be caught and wrapped into a ToolError.

ymir/tools/unprivileged/specfile.py[383-406]
ymir/common/version_utils.py[128-137]
ymir/common/config.py[15-31]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`await is_maintenance_cs_branch(...)` is executed before the `try:` block in `UpdateReleaseTool._run()`. If config loading fails, the exception bypasses the existing `except Exception as e: raise ToolError(...)` wrapper.

### Issue Context
`load_rhel_config()` raises on missing/malformed `rhel-config.json`, and `is_maintenance_cs_branch()` awaits it.

### Fix Focus Areas
- ymir/tools/unprivileged/specfile.py[389-406]
- ymir/common/version_utils.py[128-137]
- ymir/common/config.py[15-31]

### Suggested change
- Expand the `try:` block to include computing `higher_stream_branch` *and* the `await is_maintenance_cs_branch(...)` call, or add a small local `try/except` around the maintenance check that re-raises as `ToolError` with context (branch name, missing config, etc.).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

3. Heavy version_utils import ✓ Resolved 🐞 Bug ➹ Performance ⭐ New
Description
ymir.common.version_utils now imports is_cs_branch from base_utils at module import time, which
transitively imports redis and other heavy modules even for callers that only need lightweight
version parsing helpers. This expands version_utils' dependency surface and increases import-time
overhead across the codebase.
Code

ymir/common/version_utils.py[R11-12]

+from ymir.common.base_utils import is_cs_branch
+
Relevance

⭐⭐⭐ High

PR #428 explicitly refactored to avoid heavy transitive imports; team likely agrees to avoid
base_utils import in version_utils.

PR-#428

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The PR adds a module-level import from base_utils in version_utils; base_utils imports redis at
module scope, so importing version_utils now transitively imports redis even if only parse_* helpers
are used.

ymir/common/version_utils.py[8-14]
ymir/common/base_utils.py[1-18]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`ymir/common/version_utils.py` added a top-level import of `is_cs_branch` from `ymir.common.base_utils`. `base_utils` imports `redis.asyncio` and other heavier modules at import time, so importing `version_utils` now always pulls in those dependencies.

### Issue Context
`version_utils` appears intended to stay lightweight (regex parsing / comparisons). Only `is_maintenance_cs_branch()` needs the CentOS Stream branch check, so the heavy import can be avoided for most users.

### Fix Focus Areas
- ymir/common/version_utils.py[8-15]
- ymir/common/base_utils.py[1-18]

### Suggested fix
Move the `is_cs_branch` import into `is_maintenance_cs_branch()` (local import), or inline a small `CS_BRANCH_PATTERN` check inside `version_utils` to avoid importing `base_utils` at module load.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Previous review results

Review updated until commit ab1db38

Results up to commit 646fd32


🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)


Action required
1. Main branch misdetected ✓ Resolved 🐞 Bug ≡ Correctness
Description
is_maintenance_cs_branch() treats any branch parsed as (major, None) as a CentOS Stream branch, so
an internal main branch like rhel-8-main would be classified as a maintenance CS branch when major 8
is in maintenance. This can route UpdateReleaseTool into Z-stream release logic for the wrong
branch/tag set, yielding incorrect Release values or failing when expected Koji tags don’t exist.
Code

ymir/common/version_utils.py[R128-137]

+async def is_maintenance_cs_branch(branch: str) -> bool:
+    """Check if branch is a CentOS Stream branch for a RHEL version in maintenance phase."""
+    parsed = parse_branch_name(branch)
+    if not parsed or parsed[1] is not None:
+        return False
+    major, _ = parsed
+    from ymir.common.config import load_rhel_config
+
+    config = await load_rhel_config()
+    return major in get_maintenance_majors(config)
Relevance

⭐⭐⭐ High

Team recently refined branch parsing to avoid misapplying Z-stream logic to non-zstream/main
branches (PR #599).

PR-#599

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
parse_branch_name returns (major, None) for both CentOS Stream branches (c9s) and RHEL main
branches (rhel-10-main), and is_maintenance_cs_branch only checks for minor is None before
consulting maintenance majors, so it can’t distinguish these branch families. The project already
defines is_cs_branch as a strict ^c\d+s$ match, and UpdateReleaseTool uses
is_maintenance_cs_branch to decide whether to run Z-stream release bump logic.

ymir/common/version_utils.py[59-84]
ymir/common/version_utils.py[128-137]
ymir/common/base_utils.py[163-168]
ymir/tools/unprivileged/specfile.py[383-405]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`is_maintenance_cs_branch()` currently cannot distinguish `c8s` from `rhel-8-main` because both parse to `(major, None)`, so it may return `True` for RHEL main branches of a maintenance major.

### Issue Context
The codebase already has a strict CentOS Stream branch detector (`is_cs_branch` matching `^c\d+s$`). The maintenance-phase check should only ever apply to CS branches.

### Fix Focus Areas
- ymir/common/version_utils.py[128-137]
- ymir/common/base_utils.py[163-168]

### Suggested change
- In `is_maintenance_cs_branch`, first check `is_cs_branch(branch)` (import from `ymir.common.base_utils`). If it’s not a CS branch, return `False` without calling `parse_branch_name()`/`load_rhel_config()`.
- Then extract the major from the CS branch (either via `parse_branch_name` or a small CS regex) and check it against `get_maintenance_majors(config)`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended
2. Config errors bypass ToolError ✓ Resolved 🐞 Bug ☼ Reliability
Description
UpdateReleaseTool._run() calls is_maintenance_cs_branch() before entering its try/except, so
FileNotFoundError/ValueError from load_rhel_config() will escape without being wrapped as ToolError.
This changes the tool’s failure contract and can crash callers that expect ToolError from tool
execution.
Code

ymir/tools/unprivileged/specfile.py[R389-393]

        spec_path = get_absolute_path(tool_input.spec, self)
+        higher_stream_branch = self._get_higher_stream_branch(tool_input.dist_git_branch)
+        if not higher_stream_branch and await is_maintenance_cs_branch(tool_input.dist_git_branch):
+            higher_stream_branch = tool_input.dist_git_branch
        try:
-            if not (higher_stream_branch := self._get_higher_stream_branch(tool_input.dist_git_branch)):
-                await self._bump_or_reset_release(spec_path, tool_input.rebase)
-            else:
Relevance

⭐⭐⭐ High

Repo prefers consistent ToolError-wrapping for underlying exceptions; similar hardening accepted in
privileged tools (PR #596).

PR-#596

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
is_maintenance_cs_branch() awaits load_rhel_config(), which can raise FileNotFoundError or
ValueError. Because _run() performs the await before entering its try: block, those exceptions
won’t be caught and wrapped into a ToolError.

ymir/tools/unprivileged/specfile.py[383-406]
ymir/common/version_utils.py[128-137]
ymir/common/config.py[15-31]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`await is_maintenance_cs_branch(...)` is executed before the `try:` block in `UpdateReleaseTool._run()`. If config loading fails, the exception bypasses the existing `except Exception as e: raise ToolError(...)` wrapper.

### Issue Context
`load_rhel_config()` raises on missing/malformed `rhel-config.json`, and `is_maintenance_cs_branch()` awaits it.

### Fix Focus Areas
- ymir/tools/unprivileged/specfile.py[389-406]
- ymir/common/version_utils.py[128-137]
- ymir/common/config.py[15-31]

### Suggested change
- Expand the `try:` block to include computing `higher_stream_branch` *and* the `await is_maintenance_cs_branch(...)` call, or add a small local `try/except` around the maintenance check that re-raises as `ToolError` with context (branch name, missing config, etc.).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Qodo Logo

Comment thread ymir/common/version_utils.py Outdated
Comment thread ymir/tools/unprivileged/specfile.py
@nforro

nforro commented Jul 10, 2026

Copy link
Copy Markdown
Member Author

/gemini review

@nforro

nforro commented Jul 10, 2026

Copy link
Copy Markdown
Member Author

/agentic_review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds support for CentOS Stream branches of RHEL versions in the maintenance phase (such as c8s) within the release updating tool. It introduces the is_maintenance_cs_branch utility and updates UpdateReleaseTool to handle these branches similarly to Z-Stream branches by making the higher_stream_branch parameter optional. The feedback points out a potential static analysis warning in _set_zstream_release where latest_higher_stream_build is referenced outside the conditional block where it is defined, and suggests nesting the comparison logic to resolve this.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread ymir/tools/unprivileged/specfile.py Outdated
Comment thread ymir/common/version_utils.py Outdated
@qodo-for-packit

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 7ee5923

@nforro

nforro commented Jul 10, 2026

Copy link
Copy Markdown
Member Author

/gemini review

@nforro

nforro commented Jul 10, 2026

Copy link
Copy Markdown
Member Author

/agentic_review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds support for CentOS Stream branches of RHEL versions in the maintenance phase (such as c8s) within the release update tool. It introduces the is_maintenance_cs_branch utility, updates UpdateReleaseTool to handle these branches similarly to Z-stream branches by making the higher stream branch optional, and adds corresponding unit tests. The review feedback suggests adding a defensive check when unpacking the result of parse_branch_name to prevent a potential TypeError if it returns None.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread ymir/common/version_utils.py Outdated
@nforro

nforro commented Jul 10, 2026

Copy link
Copy Markdown
Member Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds support for CentOS Stream branches in the maintenance phase (e.g., c8s) to receive Z-stream release bumping by introducing is_maintenance_cs_branch and updating UpdateReleaseTool. The reviewer suggests that instead of querying candidate builds directly using the CentOS Stream branch name, the tool should resolve them to their corresponding internal Z-stream branch (e.g., rhel-8.10.0) where the actual builds reside. To achieve this, the reviewer recommends introducing a new helper get_maintenance_zstream_branch and updating the tool implementation and unit tests accordingly.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread ymir/common/version_utils.py Outdated
Comment thread ymir/tools/unprivileged/specfile.py Outdated
Comment thread ymir/tools/unprivileged/specfile.py Outdated
Comment thread ymir/tools/unprivileged/tests/unit/test_specfile.py Outdated
Comment thread ymir/tools/unprivileged/tests/unit/test_specfile.py Outdated
Comment thread ymir/tools/unprivileged/tests/unit/test_specfile.py Outdated
TomasTomecek
TomasTomecek previously approved these changes Jul 14, 2026
@nforro

nforro commented Jul 21, 2026

Copy link
Copy Markdown
Member Author

Gemini was right, even if for maintenance phase RHEL we don't need the base release at all in the majority of cases, there could be some when it is required to ensure a proper upgrade path, and deriving it from a CentOS Stream build is wrong, as that would correspond to the last Y-Stream build and ignore any follow-up Z-Stream builds.

@nforro
nforro force-pushed the maintenance branch 2 times, most recently from 7866a49 to e3ab036 Compare July 21, 2026 05:45
RHEL version in maintenance phase (currently RHEL 8.10) is being fixed
via CentOS Stream, but the fixes still target an internal Z-Stream
branch, so Release bumping must follow Z-Stream logic.

Signed-off-by: Nikola Forró <nforro@redhat.com>
Assisted-by: Claude Opus 4.6 via Claude Code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants