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-1397] Always save conversion facts file #1102

Merged
merged 5 commits into from
Feb 21, 2024

Conversation

jochapma
Copy link
Contributor

@jochapma jochapma commented Feb 19, 2024

Always create the conversion facts file.

This change removes the CONVERT2RHEL_DISABLE_TELEMETRY environment variable.

Jira Issues: RHELC-1397

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

@jochapma jochapma self-assigned this Feb 19, 2024
@Venefilyn Venefilyn changed the title [RHELC-1397] Always save conversion facts. [RHELC-1397] Always save conversion facts file Feb 19, 2024
Copy link

codecov bot commented Feb 19, 2024

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (9b08cc6) 94.89% compared to head (00bedb5) 94.84%.
Report is 3 commits behind head on main.

❗ Current head 00bedb5 differs from pull request most recent head d2db893. Consider uploading reports for the commit d2db893 to get more accurate results

Files Patch % Lines
convert2rhel/breadcrumbs.py 33.33% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1102      +/-   ##
==========================================
- Coverage   94.89%   94.84%   -0.06%     
==========================================
  Files          49       49              
  Lines        4549     4541       -8     
  Branches      809      806       -3     
==========================================
- Hits         4317     4307      -10     
- Misses        156      158       +2     
  Partials       76       76              
Flag Coverage Δ
centos-linux-7 90.02% <33.33%> (-0.07%) ⬇️
centos-linux-8 91.02% <33.33%> (-0.07%) ⬇️
centos-linux-9 91.07% <33.33%> (-0.06%) ⬇️

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.

@jochapma jochapma added the kind/feature New feature or request label Feb 19, 2024
@jochapma jochapma added the tests/tier0 PR ready to run the essential test suit. Equivalent to `/packit test --labels tier0`. label Feb 19, 2024
@has-bot
Copy link
Member

has-bot commented Feb 19, 2024

/packit test --labels tier0


Comment generated by an automation.

@jochapma jochapma marked this pull request as ready for review February 19, 2024 16:08
@jochapma jochapma requested review from danmyway and r0x0d February 19, 2024 16:08
Copy link
Member

@r0x0d r0x0d left a comment

Choose a reason for hiding this comment

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

Haven't tested it locally, but it looks good!

@r0x0d
Copy link
Member

r0x0d commented Feb 20, 2024

/packit test --labels tier0

1 similar comment
@pr-watson
Copy link
Contributor

/packit test --labels tier0

Copy link
Member

@bocekm bocekm left a comment

Choose a reason for hiding this comment

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

@sideangleside mentioned that what we do is not telemetry.

convert2rhel/main.py Outdated Show resolved Hide resolved
convert2rhel/main.py Outdated Show resolved Hide resolved
convert2rhel/breadcrumbs.py Outdated Show resolved Hide resolved
convert2rhel/breadcrumbs.py Outdated Show resolved Hide resolved
@jochapma jochapma requested a review from bocekm February 21, 2024 13:50
@Venefilyn
Copy link
Member

/packit test --labels tier0

@jochapma jochapma merged commit 7abe24d into oamg:main Feb 21, 2024
12 of 55 checks passed
@jochapma jochapma deleted the bugs/rhelc-1397 branch February 21, 2024 15:15
@Venefilyn Venefilyn mentioned this pull request Feb 22, 2024
jochapma added a commit to jochapma/convert2rhel that referenced this pull request Mar 11, 2024
* Always save conversion facts.

* Remove unit tests related to disabling telemetry.

* Remove disabled telemetry checks from integration tests.

* Apply suggestions from code review

Co-authored-by: Michal Bocek <[email protected]>

---------

Co-authored-by: Michal Bocek <[email protected]>
loggerinst.info("Skipping, telemetry disabled.")
"""Print information about data collection and ask for acknowledgement."""
loggerinst.info(
"The convert2rhel utility generates a /etc/rhsm/facts/convert2rhel.fact file that contains the below data"
Copy link
Member

Choose a reason for hiding this comment

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

The path is wrong. Fixed in #1183.

bocekm added a commit to bocekm/automated-satellite that referenced this pull request Feb 7, 2025
- The --no-rpm-va option has no effect with `convert2rhel analysis` anymore
  (rpm -Va runs always and a warning is printed when the option is used saying
  that it has no effect)
-- Related: oamg/convert2rhel#875
- The CONVERT2RHEL_DISABLE_TELEMETRY env var was removed
-- Related: oamg/convert2rhel#1102
- The CONVERT2RHEL_UNSUPPORTED_INCOMPLETE_ROLLBACK was removed in favor
  of CONVERT2RHEL_INCOMPLETE_ROLLBACK
-- Related: oamg/convert2rhel#1147
- The CONVERT2RHEL_LATEST_VERSION env var has never been a valid env var
  recognized by convert2rhel. It is an ID of one of convert2rhel
  Actions. A similar env var to this is CONVERT2RHEL_UNSUPPORTED_VERSION
  which is in recent versions (2.x) removed in favor of
  CONVERT2RHEL_ALLOW_OLDER_VERSION.
bocekm added a commit to bocekm/automated-satellite that referenced this pull request Feb 7, 2025
- The --no-rpm-va option has no effect with `convert2rhel analysis` anymore
  (rpm -Va runs always and a warning is printed when the option is used saying
  that it has no effect)
-- Related: oamg/convert2rhel#875
- The CONVERT2RHEL_DISABLE_TELEMETRY env var was removed
-- Related: oamg/convert2rhel#1102
- The CONVERT2RHEL_UNSUPPORTED_INCOMPLETE_ROLLBACK was removed in favor
  of CONVERT2RHEL_INCOMPLETE_ROLLBACK
-- Related: oamg/convert2rhel#1147
- The CONVERT2RHEL_LATEST_VERSION env var has never been a valid env var
  recognized by convert2rhel. It is an ID of one of convert2rhel
  Actions. A similar env var to this is CONVERT2RHEL_UNSUPPORTED_VERSION
  which is in recent versions (2.x) removed in favor of
  CONVERT2RHEL_ALLOW_OLDER_VERSION.
bocekm added a commit to bocekm/automated-satellite that referenced this pull request Feb 7, 2025
- The --no-rpm-va option has no effect with `convert2rhel analysis` anymore
  (rpm -Va runs always and a warning is printed when the option is used saying
  that it has no effect)
-- Related: oamg/convert2rhel#875
- The CONVERT2RHEL_DISABLE_TELEMETRY env var was removed
-- Related: oamg/convert2rhel#1102
- The CONVERT2RHEL_UNSUPPORTED_INCOMPLETE_ROLLBACK was removed in favor
  of CONVERT2RHEL_INCOMPLETE_ROLLBACK
-- Related: oamg/convert2rhel#1147
- The CONVERT2RHEL_LATEST_VERSION env var has never been a valid env var
  recognized by convert2rhel. It is an ID of one of convert2rhel
  Actions. A similar env var to this is CONVERT2RHEL_UNSUPPORTED_VERSION
  which is in recent versions (2.x) removed in favor of
  CONVERT2RHEL_ALLOW_OLDER_VERSION.
bocekm added a commit to bocekm/automated-satellite that referenced this pull request Feb 7, 2025
- The --no-rpm-va option has no effect with `convert2rhel analysis` anymore
  (rpm -Va runs always and a warning is printed when the option is used saying
  that it has no effect)
-- Related: oamg/convert2rhel#875
- The CONVERT2RHEL_DISABLE_TELEMETRY env var was removed
-- Related: oamg/convert2rhel#1102
- The CONVERT2RHEL_UNSUPPORTED_INCOMPLETE_ROLLBACK was removed in favor
  of CONVERT2RHEL_INCOMPLETE_ROLLBACK
-- Related: oamg/convert2rhel#1147
- The CONVERT2RHEL_LATEST_VERSION env var has never been a valid env var
  recognized by convert2rhel. It is an ID of one of convert2rhel
  Actions. A similar env var to this is CONVERT2RHEL_UNSUPPORTED_VERSION
  which is in recent versions (2.x) removed in favor of
  CONVERT2RHEL_ALLOW_OLDER_VERSION.
bocekm added a commit to bocekm/automated-satellite that referenced this pull request Feb 7, 2025
- The --no-rpm-va option has no effect with `convert2rhel analysis` anymore
  (rpm -Va runs always and a warning is printed when the option is used saying
  that it has no effect)
-- Related: oamg/convert2rhel#875
- The CONVERT2RHEL_DISABLE_TELEMETRY env var was removed
-- Related: oamg/convert2rhel#1102
- The CONVERT2RHEL_UNSUPPORTED_INCOMPLETE_ROLLBACK was removed in favor
  of CONVERT2RHEL_INCOMPLETE_ROLLBACK
-- Related: oamg/convert2rhel#1147
- The CONVERT2RHEL_LATEST_VERSION env var has never been a valid env var
  recognized by convert2rhel. It is an ID of one of convert2rhel
  Actions. A similar env var to this is CONVERT2RHEL_UNSUPPORTED_VERSION
  which is in recent versions (2.x) removed in favor of
  CONVERT2RHEL_ALLOW_OLDER_VERSION.

Also, the convert_reboot_requested should not be needed when it comes to
just analyzing the system (convert2rhel analyze). The --restart convert2rhel
option is heeded just when it comes to the actual conversion.

And, when the workshop executes and older convert2rhel version (on
purpose) then the pre-conversion analysis ends up with two inhibitors:
- Outdated convert2rhel version detected
- Outdated packages detected
This commit adds the related environment variables to override the
inhibitors.
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