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

[RHELC-1164] Update RHSM custom facts during analysis #1043

Merged
merged 5 commits into from
Mar 7, 2024

Conversation

r0x0d
Copy link
Member

@r0x0d r0x0d commented Jan 23, 2024

Enable the RHSM custom facts update during analysis mode.

The update will only happen if the system was registered before, otherwise, we won't do nothing

Jira Issues: RHELC-1164

Checklist

  • PR has been tested manually in a VM (either author or reviewer)
  • Jira issue has been made public if possible
  • [RHELC-] is part of the PR title
  • GitHub label has been added to help with Release notes
  • PR title explains the change from the user's point of view
  • Code and tests are documented properly
  • The commits are squashed to as few commits as possible (without losing data)
  • When merged: Jira issue has been updated to Release Pending if relevant

Copy link

codecov bot commented Jan 23, 2024

Codecov Report

Attention: Patch coverage is 75.00000% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 95.28%. Comparing base (ca1a4a2) to head (7e8a982).
Report is 3 commits behind head on main.

Files Patch % Lines
convert2rhel/main.py 75.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1043      +/-   ##
==========================================
- Coverage   95.30%   95.28%   -0.02%     
==========================================
  Files          51       51              
  Lines        4645     4649       +4     
  Branches      822      824       +2     
==========================================
+ Hits         4427     4430       +3     
  Misses        139      139              
- Partials       79       80       +1     
Flag Coverage Δ
centos-linux-7 90.40% <75.00%> (-0.02%) ⬇️
centos-linux-8 91.38% <75.00%> (-0.02%) ⬇️
centos-linux-9 91.43% <75.00%> (-0.02%) ⬇️

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.

convert2rhel/main.py Outdated Show resolved Hide resolved
convert2rhel/main.py Outdated Show resolved Hide resolved
Copy link
Contributor

@pr-watson pr-watson left a comment

Choose a reason for hiding this comment

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

A few suggestions to improve the comments, otherwise looks good

Copy link
Member

@Venefilyn Venefilyn left a comment

Choose a reason for hiding this comment

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

LGTM!

@r0x0d r0x0d added the tests/tier0 PR ready to run the essential test suit. Equivalent to `/packit test --labels tier0`. label Jan 30, 2024
@has-bot
Copy link
Member

has-bot commented Jan 30, 2024

/packit test --labels tier0


@oamg/conversions-qe please review results and provide ack.

@r0x0d r0x0d force-pushed the analysis-facts-collection branch from ecf3514 to 51567ea Compare January 30, 2024 18:00
@r0x0d r0x0d force-pushed the analysis-facts-collection branch from 51567ea to d59cadf Compare February 12, 2024 13:28
@r0x0d
Copy link
Member Author

r0x0d commented Feb 12, 2024

/packit test --labels tier0

convert2rhel/main.py Outdated Show resolved Hide resolved
convert2rhel/main.py Outdated Show resolved Hide resolved
@r0x0d r0x0d force-pushed the analysis-facts-collection branch from a5c85de to 4f4f2e1 Compare February 15, 2024 18:20
@r0x0d
Copy link
Member Author

r0x0d commented Feb 16, 2024

/packit test --labels tier0

@r0x0d r0x0d added the kind/feature New feature or request label Feb 16, 2024
@r0x0d
Copy link
Member Author

r0x0d commented Feb 19, 2024

/packit test --labels tier0

@r0x0d
Copy link
Member Author

r0x0d commented Feb 20, 2024

/packit test --labels tier0

@r0x0d r0x0d force-pushed the analysis-facts-collection branch 2 times, most recently from a01b849 to ca9ad24 Compare February 21, 2024 14:43
@r0x0d
Copy link
Member Author

r0x0d commented Feb 21, 2024

/packit test --labels tier0

1 similar comment
@r0x0d
Copy link
Member Author

r0x0d commented Feb 21, 2024

/packit test --labels tier0

@r0x0d r0x0d force-pushed the analysis-facts-collection branch from ca9ad24 to 86b8df6 Compare March 5, 2024 13:15
r0x0d and others added 4 commits March 5, 2024 10:15
Enable the RHSM custom facts update during analysis mode.

The update will only happen if the system was registered before, otherwise, we won't do nothing

Signed-off-by: Rodolfo Olivieri <rolivier@redhat.com>
Signed-off-by: Rodolfo Olivieri <rolivier@redhat.com>
Signed-off-by: Rodolfo Olivieri <rolivier@redhat.com>
Co-authored-by: Adam Hosek <hosek.adam@outlook.com>
@r0x0d r0x0d force-pushed the analysis-facts-collection branch from 86b8df6 to 00f3bbd Compare March 5, 2024 13:15
@r0x0d
Copy link
Member Author

r0x0d commented Mar 6, 2024

/packit test --labels tier0

@r0x0d r0x0d force-pushed the analysis-facts-collection branch from 00f3bbd to ed58914 Compare March 6, 2024 12:38
@r0x0d
Copy link
Member Author

r0x0d commented Mar 6, 2024

/packit test --labels tier0

1 similar comment
@r0x0d
Copy link
Member Author

r0x0d commented Mar 6, 2024

/packit test --labels tier0

@r0x0d r0x0d force-pushed the analysis-facts-collection branch from ed58914 to 7e8a982 Compare March 6, 2024 18:35
@r0x0d
Copy link
Member Author

r0x0d commented Mar 6, 2024

/packit test --labels tier0

Copy link
Member

@danmyway danmyway left a comment

Choose a reason for hiding this comment

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

Looks good @r0x0d, thanks!
Just a couple of tweaks I would not block the merge with.

Comment on lines +8 to +12
summary+: |
RHSM facts update after analysis
description+: |
This test verifies that the RHSM facts are uploaded when running
convert2rhel with analysis option.
Copy link
Member

Choose a reason for hiding this comment

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

This is just duplicating the summary and description from the root of the path in the file.
Could be consolidated, I wouldn't block the merge on this though.

pytest -svv -m test_rhsm_facts_called_in_analysis
adjust+:
- enabled: false
when: distro == oracle-7, oracle-8
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
when: distro == oracle-7, oracle-8
when: distro == oracle

The filtering calls the startswith() method, I believe, so no need to include both.
Again not a blocker, will fix in a different PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

Alright, thanks!

@danmyway danmyway merged commit b74b8da into oamg:main Mar 7, 2024
45 of 57 checks passed
@r0x0d r0x0d deleted the analysis-facts-collection branch March 7, 2024 13:40
jochapma pushed a commit to jochapma/convert2rhel that referenced this pull request Mar 11, 2024
* Update RHSM custom facts during analysis

Enable the RHSM custom facts update during analysis mode.

The update will only happen if the system was registered before, otherwise, we won't do nothing

Signed-off-by: Rodolfo Olivieri <rolivier@redhat.com>

* Fix failling main test

Signed-off-by: Rodolfo Olivieri <rolivier@redhat.com>

* Add integration tests

Signed-off-by: Rodolfo Olivieri <rolivier@redhat.com>

* Apply suggestions from code review

Co-authored-by: Adam Hosek <hosek.adam@outlook.com>

* Update integration tests

---------

Signed-off-by: Rodolfo Olivieri <rolivier@redhat.com>
Co-authored-by: Adam Hosek <hosek.adam@outlook.com>
@hosekadam hosekadam mentioned this pull request May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New feature or request tests/tier0 PR ready to run the essential test suit. Equivalent to `/packit test --labels tier0`.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants