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-1174] Update remediation key to match leapp report metadata #949

Merged
merged 4 commits into from
Jan 22, 2024

Conversation

r0x0d
Copy link
Member

@r0x0d r0x0d commented Oct 5, 2023

The remediation key was singular in our report, and leapp one is in plural. We are matching the key in our report to remove this workaround from the pre-conversion script that is executed with the rhc-worker-script.

Jira Issues: RHELC-1174

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

@r0x0d r0x0d changed the title [RHELC-1174Update remediation key to match leapp report metadata [RHELC-1174] Update remediation key to match leapp report metadata Oct 5, 2023
@r0x0d r0x0d requested a review from abadger October 5, 2023 17:36
@r0x0d r0x0d added tests/tier0 PR ready to run the essential test suit. Equivalent to `/packit test --labels tier0`. tests/sanity PR ready to run the sanity test suit. Equivalent to `/packit test --labels sanity`. and removed tests/tier0 PR ready to run the essential test suit. Equivalent to `/packit test --labels tier0`. labels Oct 5, 2023
@has-bot
Copy link
Member

has-bot commented Oct 5, 2023

/packit test --labels tier0


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

@r0x0d r0x0d added the skip/changelog If it should be excluded from changelog or Release notes. Such as infra, reverted PRs, etc. label Oct 5, 2023
@has-bot
Copy link
Member

has-bot commented Oct 5, 2023

/packit test --labels sanity


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

@codecov
Copy link

codecov bot commented Oct 5, 2023

Codecov Report

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

Comparison is base (c24bdb6) 94.32% compared to head (0fa9277) 94.32%.
Report is 2 commits behind head on main.

❗ Current head 0fa9277 differs from pull request most recent head fbc2bf4. Consider uploading reports for the commit fbc2bf4 to get more accurate results

Files Patch % Lines
convert2rhel/exceptions.py 66.66% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #949   +/-   ##
=======================================
  Coverage   94.32%   94.32%           
=======================================
  Files          47       47           
  Lines        4549     4549           
  Branches      810      810           
=======================================
  Hits         4291     4291           
  Misses        182      182           
  Partials       76       76           
Flag Coverage Δ
centos-linux-7 89.50% <93.75%> (ø)
centos-linux-8 90.50% <93.75%> (ø)
centos-linux-9 90.56% <93.75%> (ø)

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.

@r0x0d
Copy link
Member Author

r0x0d commented Oct 9, 2023

/packit test --labels sanity

r0x0d added a commit to r0x0d/convert2rhel-insights-tasks that referenced this pull request Oct 9, 2023
With the convert2rhel PR oamg/convert2rhel#949
to update their report to use the word "remediations" instead of
"remediation", we had to update our script to match the same change, but
in case that there is some legacy convert2rhel running that is still in
the 1.4/1.5 release, we need to take care of the "remediation" keyword.

Signed-off-by: Rodolfo Olivieri <rolivier@redhat.com>
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.

Changes look good to me

andywaltlova pushed a commit to r0x0d/convert2rhel-insights-tasks that referenced this pull request Oct 16, 2023
With the convert2rhel PR oamg/convert2rhel#949
to update their report to use the word "remediations" instead of
"remediation", we had to update our script to match the same change, but
in case that there is some legacy convert2rhel running that is still in
the 1.4/1.5 release, we need to take care of the "remediation" keyword.

Signed-off-by: Rodolfo Olivieri <rolivier@redhat.com>
r0x0d added a commit to oamg/convert2rhel-insights-tasks that referenced this pull request Oct 19, 2023
With the convert2rhel PR oamg/convert2rhel#949
to update their report to use the word "remediations" instead of
"remediation", we had to update our script to match the same change, but
in case that there is some legacy convert2rhel running that is still in
the 1.4/1.5 release, we need to take care of the "remediation" keyword.

Signed-off-by: Rodolfo Olivieri <rolivier@redhat.com>
@Venefilyn
Copy link
Member

Needs a quick rebase, doesn't hurt to check if there are any leftover remediation= fields

@r0x0d
Copy link
Member Author

r0x0d commented Oct 31, 2023

Needs a quick rebase, doesn't hurt to check if there are any leftover remediation= fields

gotcha. doing it right now

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.

Needs another rebase due to PRs I merged today

@r0x0d
Copy link
Member Author

r0x0d commented Nov 27, 2023

Needs another rebase due to PRs I merged today

Will do it today.

@r0x0d
Copy link
Member Author

r0x0d commented Nov 29, 2023

Needs another rebase due to PRs I merged today

Will do it today.

Done the rebase, @SpyTec.

@Venefilyn
Copy link
Member

/packit test --labels sanity

1 similar comment
@Venefilyn
Copy link
Member

/packit test --labels sanity

@Venefilyn
Copy link
Member

Some leftovers

        [2024-01-05T09:13:29-0500] TASK - [Pre-conversion analysis report] ***********************************
        [2024-01-05T09:13:29-0500] DEBUG - /var/run/lock/convert2rhel.pid PID 1609 unlocked.
        Traceback (most recent call last):
          File "/usr/lib/python3.6/site-packages/convert2rhel/main.py", line 129, in main_locked
            disable_colors=logger_module.should_disable_color_output(),
          File "/usr/lib/python3.6/site-packages/convert2rhel/actions/report.py", line 219, in summary
            combined_results_and_message = get_combined_results_and_message(results)
          File "/usr/lib/python3.6/site-packages/convert2rhel/actions/report.py", line 135, in get_combined_results_and_message
            "remediations": action_value["result"]["remediation"],
        KeyError: 'remediation'
        
        During handling of the above exception, another exception occurred:
        
        Traceback (most recent call last):
          File "/usr/lib/python3.6/site-packages/convert2rhel/main.py", line 166, in main_locked
            return _handle_main_exceptions(process_phase, pre_conversion_results)
          File "/usr/lib/python3.6/site-packages/convert2rhel/main.py", line 195, in _handle_main_exceptions
            disable_colors=logger_module.should_disable_color_output(),
          File "/usr/lib/python3.6/site-packages/convert2rhel/actions/report.py", line 219, in summary
            combined_results_and_message = get_combined_results_and_message(results)
          File "/usr/lib/python3.6/site-packages/convert2rhel/actions/report.py", line 135, in get_combined_results_and_message
            "remediations": action_value["result"]["remediation"],
        KeyError: 'remediation'
        
        During handling of the above exception, another exception occurred:
        
        Traceback (most recent call last):
          File "/usr/bin/convert2rhel", line 11, in <module>
            load_entry_point('convert2rhel==1.6.1', 'console_scripts', 'convert2rhel')()
          File "/usr/lib/python3.6/site-packages/convert2rhel/initialize.py", line 82, in run
            return main.main()
          File "/usr/lib/python3.6/site-packages/convert2rhel/main.py", line 87, in main
            return main_locked()
          File "/usr/lib/python3.6/site-packages/convert2rhel/main.py", line 173, in main_locked
            actions.report.summary_as_txt(pre_conversion_results)
          File "/usr/lib/python3.6/site-packages/convert2rhel/actions/report.py", line 304, in summary_as_txt
            combined_results_and_message = get_combined_results_and_message(results)
          File "/usr/lib/python3.6/site-packages/convert2rhel/actions/report.py", line 135, in get_combined_results_and_message
            "remediations": action_value["result"]["remediation"],
        KeyError: 'remediation'
        FAILED

bocekm
bocekm previously requested changes Jan 11, 2024
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.

We need a new schema version. The latest one (https://github.com/oamg/convert2rhel/blob/main/schemas/assessment-schema-1.1.json) has remediation a it needs to have remediations.

@r0x0d
Copy link
Member Author

r0x0d commented Jan 17, 2024

Will work on this as soon as possible!

The remediation key was singular in our report, and leapp one is in
plural. We are matching the key in our report to remove this workaround
from the pre-conversion script that is executed with the
rhc-worker-script.

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

Update remediation key in actions and tests

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

Update more remediation keys

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

Update remaining remediation keys

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

Fix keys in leftover files after rebase

Signed-off-by: Rodolfo Olivieri <rolivier@redhat.com>
Signed-off-by: Rodolfo Olivieri <rolivier@redhat.com>
@r0x0d
Copy link
Member Author

r0x0d commented Jan 17, 2024

/packit test --labels sanity

1 similar comment
@r0x0d
Copy link
Member Author

r0x0d commented Jan 17, 2024

/packit test --labels sanity

@pr-watson
Copy link
Contributor

@r0x0d changes look good, just made one suggestion

Co-authored-by: Preston Watson <prwatson@redhat.com>
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.

Schema diff LGTM, just one change needed

92c92
<                 "remediation": {
---
>                 "remediations": {
105c105
<             "required": ["title", "description", "diagnosis", "id", "remediation", "variables"]
---
>             "required": ["title", "description", "diagnosis", "id", "remediations", "variables"]

schemas/assessment-schema-1.2.json Outdated Show resolved Hide resolved
Co-authored-by: E Gustavsson <eric@spytec.se>
@Venefilyn
Copy link
Member

/packit test --labels sanity

@Venefilyn
Copy link
Member

Merging once sanity tests are green enough -- won't be for 7 or Alma 8 atm due to infra and Alma changes

@Venefilyn Venefilyn added kind/feature New feature or request and removed skip/changelog If it should be excluded from changelog or Release notes. Such as infra, reverted PRs, etc. labels Jan 22, 2024
@Venefilyn Venefilyn merged commit e67ab3c into oamg:main Jan 22, 2024
25 of 61 checks passed
@Venefilyn Venefilyn mentioned this pull request Feb 22, 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/sanity PR ready to run the sanity test suit. Equivalent to `/packit test --labels sanity`.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants