Rhel under major 7 yum adapation - #2458
Open
pindo696 wants to merge 2 commits into
Open
Conversation
Reviewer's GuideThis PR adjusts how RHEL releasever is handled when building VMaaS requests, dropping numeric releasevers for RHEL major versions <= 7 and adding optional logging around the releasever evaluation via Unleash feature flags, plus wiring Unleash initialization into the listener startup and adding unit coverage for the new behavior. Sequence diagram for releasever adaptation during inventory processingsequenceDiagram
participant InventoryProcessor
participant adapt_releasever
participant UNLEASH
participant LOGGER
participant VMaaS
InventoryProcessor->>InventoryProcessor:_parse_system_data(msg)
InventoryProcessor->>InventoryProcessor:get rhsm_ver and system_profile.releasever
InventoryProcessor->>adapt_releasever:adapt_releasever(rhsm_ver_or_sp_releasever)
adapt_releasever-->>InventoryProcessor:releasever_or_None
alt UNLEASH.is_enabled(RELEASEVER_LOG_FEATURE)
InventoryProcessor->>UNLEASH:is_enabled(RELEASEVER_LOG_FEATURE)
UNLEASH-->>InventoryProcessor:is_enabled_result
InventoryProcessor->>LOGGER:info(rhsm_ver, sp_releasever, releasever)
end
InventoryProcessor->>VMaaS:_format_repo_list(yum_repos, basearch, releasever)
Sequence diagram for listener startup with Unleash initializationsequenceDiagram
participant ListenerMain
participant initialize_unleash
participant UNLEASH
ListenerMain->>ListenerMain:main()
ListenerMain->>ListenerMain:init_logging()
ListenerMain->>initialize_unleash:initialize_unleash()
initialize_unleash->>UNLEASH:configure_client()
UNLEASH-->>initialize_unleash:client_ready
ListenerMain->>ListenerMain:a_ensure_minimal_schema_version()
ListenerMain->>ListenerMain:run()
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The
adapt_releaseverdocstring and inline comments could be clarified to explicitly describe the exact conditions under which the releasever is set toNone(numeric with a dot and major <= 7) to avoid confusion for future maintainers. - Consider making
adapt_releaseverslightly more defensive about unexpectedreleaseverformats (e.g., usingsplit('.')and validating the major component) so it is easier to reason about and less sensitive to odd input values.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `adapt_releasever` docstring and inline comments could be clarified to explicitly describe the exact conditions under which the releasever is set to `None` (numeric with a dot and major <= 7) to avoid confusion for future maintainers.
- Consider making `adapt_releasever` slightly more defensive about unexpected `releasever` formats (e.g., using `split('.')` and validating the major component) so it is easier to reason about and less sensitive to odd input values.
## Individual Comments
### Comment 1
<location path="listener/common.py" line_range="71-78" />
<code_context>
+def adapt_releasever(releasever: str | None) -> str | None:
+ """
+ Drop RHEL <= 7 numeric releasevers.
+ This supresses user ``subscription-manager release --set=...``
+ setup, which is intended in this case
+ to match xServer yum repos for x.y releasever.
+ This can be removed once the RHEL 7 dies.
+ """
+ if not releasever:
</code_context>
<issue_to_address>
**nitpick (typo):** Minor wording/typo issues in the new docstring.
Please fix the spelling of `supresses` → `suppresses`, and rephrase "once the RHEL 7 dies" to something like "once RHEL 7 reaches end of life" for clearer documentation.
```suggestion
def adapt_releasever(releasever: str | None) -> str | None:
"""
Drop RHEL <= 7 numeric releasevers.
This suppresses user ``subscription-manager release --set=...``
setup, which is intended in this case
to match xServer yum repos for x.y releasever.
This can be removed once RHEL 7 reaches end of life.
"""
```
</issue_to_address>
### Comment 2
<location path="listener/inventory_processor.py" line_range="156-162" />
<code_context>
basearch = system_profile.get("basearch") or system_profile.get("arch")
rhsm_ver = system_profile.get("rhsm", {}).get("version")
- releasever = rhsm_ver or system_profile.get("releasever")
+ releasever = adapt_releasever(rhsm_ver or system_profile.get("releasever"))
+ if UNLEASH.is_enabled(RELEASEVER_LOG_FEATURE):
+ LOGGER.info(
+ "releasever sources: rhsm_ver=%s, sp_releasever=%s, result_releasever=%s",
+ rhsm_ver,
+ system_profile.get("releasever"),
+ releasever,
+ )
</code_context>
<issue_to_address>
**suggestion (performance):** Consider how often the info-level releasever logging will fire and whether that aligns with expectations.
This log line will run on every inventory message when the feature flag is enabled, using `LOGGER.info`. In high-traffic environments that could generate substantial log volume and affect noise or storage. If this is mainly for diagnostics, consider `debug` level or sampling/throttling; if `info` is intentional, documenting that in the feature flag or rollout notes would help set expectations.
Suggested implementation:
```python
releasever = adapt_releasever(rhsm_ver or system_profile.get("releasever"))
if UNLEASH.is_enabled(RELEASEVER_LOG_FEATURE):
LOGGER.debug(
"releasever sources: rhsm_ver=%s, sp_releasever=%s, result_releasever=%s",
rhsm_ver,
system_profile.get("releasever"),
```
1. Update the documentation/comments for `RELEASEVER_LOG_FEATURE` (wherever it is defined) to note that the logging is at `debug` level and intended for diagnostics in high-traffic environments.
2. If there are rollout notes or operational docs describing this feature flag, adjust them to match the new log level and expected usage.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Ommit releasever in vmaas_json for rhel major versions <= 7 when matching repos so rhel repolist is closer to yum. RHINENG-26908
Added an unleash debug log so we can better see what is being passed to releasever so we can possibly make the decission process of releasever more accurate for rhel <= 7 as I am not able to access inventory.
pindo696
force-pushed
the
7.9-and-7Server-yum-adapation
branch
from
August 12, 2026 08:32
45c0c1f to
df1624a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reproduced on a RHEL 7.9 VM: yum repolist looked the same for 7.9 and after
subscription-manager release --set=7Server(and checking redhat.repo). Vulnerability report did not. With rhsm_version=7.9 we got different number of CVEs reported. Same host, different releasever only.That’s because vmaas filters repos by releasever. So 7.9 and 7Server hit different repo sets - which is intentional as strict matching and caused by setting
subscription-manager release --set=7.9which is a user preference setup.RHEL 7 has the dual 7.9 and 7Server naming that looks like that yum treats as equivalent. This hotfix drops the releasever for RHEL versions under major 7 which has less narrow match. Returning None (omitting releasever) produces the same results as the old 7Server hardcode RedHatInsights/vmaas-lib#140.
We don’t see this on RHEL 8+ since releasevers are numeric and already match what is being stored (RHEL 8+ uses {major}.{minor}.)
In the second commit I also added logging so we can better track the evaluation and see what is being passed to releasever so we can possibly make the decision process more accurate.
I was unable to reproduce on RHEL 6 as it was unable to register the machine with entitlement server.
TLDR: Drop releasever from vmaas_json request for RHEL major versions under 7.
RHINENG-26908
Secure Coding Checklist
Summary by Sourcery
Adjust release version handling for RHEL systems to avoid overly strict repo filtering on RHEL 7 and add feature-flagged logging for release version evaluation.
New Features:
Enhancements:
Tests: