Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support more RHEL product key in subscription_manage_status #4291

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rahulXs
Copy link
Contributor

@rahulXs rahulXs commented Nov 27, 2024

All Pull Requests:

Check all that apply:

  • Have you followed the guidelines in our Contributing document, including the instructions about commit messages?
  • No Sensitive Data in this change?
  • Is this PR to correct an issue?
  • Is this PR an enhancement?

Complete Description of Additions/Changes:

Product Key Parsing: Added support for parsing certain product key lines that start with "Red Hat Enterprise Linux for Virtual Datacenters", handling them as keys and capturing the following description lines as their values.

Here are couple of examples:

Test data sample 1:

# subscription-manager status
+-------------------------------------------+
 System Status Details
+-------------------------------------------+
Overall Status: Insufficient

Red Hat Enterprise Linux for Virtual Datacenters, Premium:
- Guest has not been reported on any host and is using a temporary unmapped guest subscription. For more information, please see https://access.redhat.com/solutions/XXXX

Test data sample 2:

+-------------------------------------------+
   System Status Details
+-------------------------------------------+
Overall Status: Insufficient
Red Hat Enterprise Linux for Virtual Datacenters, Standard:
- Guest has not been reported on any host and is using a temporary unmapped guest subscription. For more information, please see https://access.redhat.com/solutions/XXXX
System Purpose Status: Matched

Test Updates: The test files have been updated to ensure that the new product key parsing is properly handled, ensuring correctness across different types of content.

Pytest report:

 insights-core   parser_enhance_subsmngrstatus  pytest -v insights/tests/parsers/test_subscription_manager.py
================================================= test session starts ==================================================
platform darwin -- Python 3.11.4, pytest-7.4.3, pluggy-1.3.0 -- /Users/rahushar/virtualenvs/gss-rules/bin/python3.11
cachedir: .pytest_cache
rootdir: /Users/rahushar/Projects/insights-core
configfile: setup.cfg
plugins: cov-4.1.0
collected 7 items

insights/tests/parsers/test_subscription_manager.py::test_subman_facts PASSED                                    [ 14%]
insights/tests/parsers/test_subscription_manager.py::test_subman_id PASSED                                       [ 28%]
insights/tests/parsers/test_subscription_manager.py::test_subman_status PASSED                                   [ 42%]
insights/tests/parsers/test_subscription_manager.py::test_subman_facts_ng PASSED                                 [ 57%]
insights/tests/parsers/test_subscription_manager.py::test_subman_id_ng PASSED                                    [ 71%]
insights/tests/parsers/test_subscription_manager.py::test_subman_status_ng PASSED                                [ 85%]
insights/tests/parsers/test_subscription_manager.py::test_doc_examples PASSED                                    [100%]

================================================== 7 passed in 0.04s ===================================================

@codecov-commenter
Copy link

codecov-commenter commented Nov 27, 2024

Codecov Report

Attention: Patch coverage is 87.50000% with 1 line in your changes missing coverage. Please review.

Project coverage is 77.20%. Comparing base (c9f5231) to head (26d5acf).

Files with missing lines Patch % Lines
insights/parsers/subscription_manager.py 87.50% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4291   +/-   ##
=======================================
  Coverage   77.20%   77.20%           
=======================================
  Files         764      764           
  Lines       41544    41552    +8     
  Branches     8780     8783    +3     
=======================================
+ Hits        32073    32082    +9     
+ Misses       8413     8412    -1     
  Partials     1058     1058           
Flag Coverage Δ
unittests 77.19% <87.50%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Lorquas
Copy link
Member

Lorquas commented Nov 27, 2024

Can one of the admins verify this patch?

Copy link
Contributor

@xiangce xiangce left a comment

Choose a reason for hiding this comment

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

Hi @rahulXs, thanks for the enhancement.

CMIIW, however, I think the Standard or Premium in the following line

Red Hat Enterprise Linux for Virtual Datacenters, Standard:

Looks like a more useful value. And if you wrap the Red Hat Enterprise Linux for Virtual Datacenters, as one key of the dict together with Standard or Premium, it results in different keys for different hosts, which makes a bit difficult for users to get them (they have to iterate the dict and check the words in each key).

And the following line looks not useful information.

- Guest has not been reported on any host and is using a temporary unmapped guest subscription. For more information, please see https://access.redhat.com/solutions/XXXX

I suggest you to take the Red Hat Enterprise Linux for Virtual Datacenters and Standard or Premium as a key:value pair for this case. (and discard the line that starts with - Guest, unless it's useful for rules)

@xiangce xiangce changed the title feat: add support for parsing RHEL product key in subscription_manage… feat: support more RHEL product key in subscription_manage_status Nov 28, 2024
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.

4 participants