Preserve the original inhibitor detection valid for now #781

pirat89 · 2022-06-21T14:30:34Z

As we substituted Tags and Flags with Groups recently, it has
impact on original detection of inhibitor inside leapp-repository
which is not compatible with the new reporting implementation.
As it could happen that new framework and old leapp-repository could
be installed on the system, the consequence in such a case is that
inhibitors will not be caught and user running:

  # leapp upgrade --reboot

could hit various serious issues. This change make sure we keep the
original functionality working, until we drop flags & tags
completely, providing enough time to everyone to adapt to new
changes.

github-actions · 2022-06-21T14:30:53Z

Thank you for contributing to the Leapp project!

Please note that every PR needs to comply with the Leapp Guidelines and must pass all tests in order to be mergable.
If you want to request a review or rebuild a package in copr, you can use following commands as a comment:

review please to notify leapp developers of review request
/packit copr-build to submit a public copr build using packit

To launch regression testing public members of oamg organization can leave the following comment:

/rerun to schedule basic regression tests using this pr build and leapp-repository*master* as artifacts
/rerun 42 to schedule basic regression tests using this pr build and leapp-repository*PR42* as artifacts
/rerun-all to schedule all tests (including sst) using this pr build and leapp-repository*master* as artifacts
/rerun-all 42 to schedule all tests (including sst) using this pr build and leapp-repository*PR42* as artifacts

Please open ticket in case you experience technical problem with the CI. (RH internal only)

Note: In case there are problems with tests not being triggered automatically on new PR/commit or pending for a long time, please consider rerunning the CI by commenting leapp-ci build (might require several comments). If the problem persists, contact leapp-infra.

centos-ci · 2022-06-21T14:31:02Z

Can one of the admins verify this patch?

MichalHe

I've just reviewed the changes, thus, I cannot approve it until I test it. However, looks good to me with a minor nitpicks.

leapp/reporting/__init__.py

leapp/utils/report.py

MichalHe · 2022-06-23T09:57:00Z

Tested the patch, works as expected. Builds used during the testing:

leapp - current master
leapp-PR781 - this PR
leapp-repository-old - leapp repository without modified inhibition detection
leapp-repository-new - leapp repository with modified inhibition detection

Correctly inhibits?	`leapp`	`leapp-PR781`
`leapp-repository-old`	🔴	✔️
`leapp-repository-new`	✔️	✔️

pirat89 · 2022-06-24T11:00:06Z

/packit build

fernflower · 2022-07-12T11:29:54Z

Please fix linter error.

************* Module leapp.reporting.__init__
leapp/reporting/__init__.py:330:0: C0301: Line too long (157/120) (line-too-long)

pirat89 · 2022-07-12T15:53:11Z

@fernflower fixed & squashed

fernflower · 2022-07-12T19:08:14Z

/rerun

github-actions · 2022-07-12T19:14:53Z

Copr build succeeded: https://copr.fedorainfracloud.org/coprs/build/4630650

github-actions · 2022-07-12T19:34:36Z

Testing Farm request for RHEL-8.6-rhui/4630618;4630650 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

github-actions · 2022-07-12T19:41:00Z

Testing Farm request for RHEL-7.9-rhui/4630618;4630650 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

github-actions · 2022-07-12T19:52:13Z

Testing Farm request for RHEL-8.6.0-Nightly/4630618;4630650 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

github-actions · 2022-07-12T20:11:47Z

Testing Farm request for RHEL-7.9-ZStream/4630618;4630650 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

fernflower · 2022-07-25T11:34:22Z

/rerun

github-actions · 2022-07-25T11:47:17Z

Copr build succeeded: https://copr.fedorainfracloud.org/coprs/build/4684364

github-actions · 2022-07-25T11:54:44Z

Testing Farm request for RHEL-8.6-rhui/4656862;4684364 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

github-actions · 2022-07-25T12:13:52Z

Testing Farm request for RHEL-7.9-rhui/4656862;4684364 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

github-actions · 2022-07-25T12:23:05Z

Testing Farm request for RHEL-8.6.0-Nightly/4656862;4684364 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

leapp/utils/report.py

github-actions · 2022-07-25T12:45:59Z

Testing Farm request for RHEL-7.9-ZStream/4656862;4684364 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

fernflower

msg.pop('flags') should be changed to msg.pop('flags', None)

fernflower · 2022-07-26T15:01:32Z

@pirat89 Tried adding a commit into your branch and got permission denied. Could you please allow it?
Or maybe just fix the issue I pointed out?

diff --git a/leapp/utils/report.py b/leapp/utils/report.py
index 6248101..71a2f29 100644
--- a/leapp/utils/report.py
+++ b/leapp/utils/report.py
@@ -147,5 +147,7 @@ def generate_report_file(messages_to_report, context, path, report_schema='1.1.0
                 # be negatively affected until we drop flags and tags completely.
                 messages_to_report = list(messages_to_report)
                 for msg in messages_to_report:
-                    msg.pop('flags')
+                    # NOTE(ivasilev) As flags field is created only if there is an inhibitor a default value to pop
+                    # has to be specified not to end up with a KeyError
+                    msg.pop('flags', None)
             json.dump({'entries': messages_to_report, 'leapp_run_id': context}, f, indent=2)

pirat89 · 2022-07-26T15:23:22Z

@fernflower I cannot find out why you do not have the permission to commit into my fork, as regarding the github settings you should be able to do it. Let's experiment with that in future. I will just update the commit now to unblock it.

As we substituted Tags and Flags with Groups recently, it has impact on original detection of inhibitor inside leapp-repository which is not compatible with the new reporting implementation. As it could happen that new framework and old leapp-repository could be installed on the system, the consequence in such a case is that inhibitors will not be caught and user running: # leapp upgrade --reboot could hit various serious issues. This change make sure we keep the original functionality working, until we drop flags & tags completely, providing enough time to everyone to adapt to new changes. Co-authored-by: Michal Hečko <michal.sk.com@gmail.com> Co-authored-by: Ina Vasilevskaya <ivasilev@redhat.com>

addressed

fernflower · 2022-07-26T17:03:41Z

/rerun

github-actions · 2022-07-26T17:14:09Z

Copr build succeeded: https://copr.fedorainfracloud.org/coprs/build/4686481

github-actions · 2022-07-26T17:22:38Z

Testing Farm request for RHEL-8.6-rhui/4685924;4686481 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

github-actions · 2022-07-26T17:40:24Z

Testing Farm request for RHEL-7.9-rhui/4685924;4686481 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

github-actions · 2022-07-26T17:51:14Z

Testing Farm request for RHEL-8.6.0-Nightly/4685924;4686481 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

github-actions · 2022-07-26T19:24:48Z

Testing Farm request for RHEL-7.9-ZStream/4685924;4686481 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

fernflower

Now the tests are fine.

## Packaging - bumped leapp-framework to 3.1 (oamg#677) ## Framework ### Fixes - Fixed a problem where passing environment variables to an executed child process modified the environment variables of the parent process (oamg#784) - Ignore invalid FQDNs (oamg#790) ### Enhancements - Deprecate `reporting.(Tags|Flags)`, replaced by `reporting.Groups` (oamg#677, oamg#781, oamg#788) - Introduce `is_inhibitor` function (oamg#677) - Introduce a `Blob` model field (oamg#789) - Introduce new report JSON schema v1.2.0 (default: 1.1.0) (oamg#677) ## Leapp (tool) ### Fixes - Handle missing CLI commands gracefully (oamg#785) - Requires to be executed by root only (oamg#775)

## Packaging - bumped leapp-framework to 3.1 (oamg#677) ## Framework ### Fixes - Fixed a problem where passing environment variables to an executed child process modified the environment variables of the parent process (oamg#784) - Ignore invalid FQDNs (oamg#790) ### Enhancements - Deprecate `reporting.(Tags|Flags)`, replaced by `reporting.Groups` (oamg#677, oamg#781, oamg#788) - Introduce `is_inhibitor` function (oamg#677) - Introduce a `Blob` model field (oamg#789) - Introduce new report JSON schema v1.2.0 (default: 1.1.0) (oamg#677) ## Leapp (tool) ### Fixes - Handle missing CLI commands gracefully (oamg#785) - Requires to be executed by root only (oamg#775) Signed-off-by: Petr Stodulka <pstodulk@redhat.com>

## Packaging - bumped leapp-framework to 3.1 (#677) ## Framework ### Fixes - Fixed a problem where passing environment variables to an executed child process modified the environment variables of the parent process (#784) - Ignore invalid FQDNs (#790) ### Enhancements - Deprecate `reporting.(Tags|Flags)`, replaced by `reporting.Groups` (#677, #781, #788) - Introduce `is_inhibitor` function (#677) - Introduce a `Blob` model field (#789) - Introduce new report JSON schema v1.2.0 (default: 1.1.0) (#677) ## Leapp (tool) ### Fixes - Handle missing CLI commands gracefully (#785) - Requires to be executed by root only (#775) Signed-off-by: Petr Stodulka <pstodulk@redhat.com> Signed-off-by: Petr Stodulka <pstodulk@redhat.com>

MichalHe reviewed Jun 21, 2022

View reviewed changes

leapp/reporting/__init__.py Outdated Show resolved Hide resolved

leapp/utils/report.py Show resolved Hide resolved

leapp/utils/report.py Show resolved Hide resolved

pirat89 force-pushed the sanitize-inhibitors branch from bdee2f0 to 5c398e3 Compare July 12, 2022 15:52

pirat89 marked this pull request as ready for review July 13, 2022 10:11

fernflower reviewed Jul 25, 2022

View reviewed changes

leapp/utils/report.py Outdated Show resolved Hide resolved

fernflower previously requested changes Jul 25, 2022

View reviewed changes

pirat89 force-pushed the sanitize-inhibitors branch from 5c398e3 to e256a79 Compare July 26, 2022 15:26

fernflower approved these changes Jul 27, 2022

View reviewed changes

fernflower merged commit 62680b9 into oamg:master Jul 27, 2022

pirat89 mentioned this pull request Aug 23, 2022

Release 0.15.0 #795

Merged

pirat89 added the changelog-checked The merger/reviewer checked the changelog draft document and updated it when relevant label Feb 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Preserve the original inhibitor detection valid for now #781

Preserve the original inhibitor detection valid for now #781

pirat89 commented Jun 21, 2022 •

edited

Loading

github-actions bot commented Jun 21, 2022

centos-ci commented Jun 21, 2022

MichalHe left a comment

MichalHe commented Jun 23, 2022 •

edited

Loading

pirat89 commented Jun 24, 2022

fernflower commented Jul 12, 2022

pirat89 commented Jul 12, 2022

fernflower commented Jul 12, 2022

github-actions bot commented Jul 12, 2022

github-actions bot commented Jul 12, 2022

github-actions bot commented Jul 12, 2022

github-actions bot commented Jul 12, 2022

github-actions bot commented Jul 12, 2022

fernflower commented Jul 25, 2022

github-actions bot commented Jul 25, 2022

github-actions bot commented Jul 25, 2022

github-actions bot commented Jul 25, 2022

github-actions bot commented Jul 25, 2022

github-actions bot commented Jul 25, 2022

fernflower left a comment

fernflower commented Jul 26, 2022 •

edited

Loading

pirat89 commented Jul 26, 2022 •

edited

Loading

fernflower commented Jul 26, 2022

github-actions bot commented Jul 26, 2022

github-actions bot commented Jul 26, 2022

github-actions bot commented Jul 26, 2022

github-actions bot commented Jul 26, 2022

github-actions bot commented Jul 26, 2022

fernflower left a comment

Preserve the original inhibitor detection valid for now #781

Preserve the original inhibitor detection valid for now #781

Conversation

pirat89 commented Jun 21, 2022 • edited Loading

github-actions bot commented Jun 21, 2022

Thank you for contributing to the Leapp project!

centos-ci commented Jun 21, 2022

MichalHe left a comment

Choose a reason for hiding this comment

MichalHe commented Jun 23, 2022 • edited Loading

pirat89 commented Jun 24, 2022

fernflower commented Jul 12, 2022

pirat89 commented Jul 12, 2022

fernflower commented Jul 12, 2022

github-actions bot commented Jul 12, 2022

github-actions bot commented Jul 12, 2022

github-actions bot commented Jul 12, 2022

github-actions bot commented Jul 12, 2022

github-actions bot commented Jul 12, 2022

fernflower commented Jul 25, 2022

github-actions bot commented Jul 25, 2022

github-actions bot commented Jul 25, 2022

github-actions bot commented Jul 25, 2022

github-actions bot commented Jul 25, 2022

github-actions bot commented Jul 25, 2022

fernflower left a comment

Choose a reason for hiding this comment

fernflower commented Jul 26, 2022 • edited Loading

pirat89 commented Jul 26, 2022 • edited Loading

fernflower commented Jul 26, 2022

github-actions bot commented Jul 26, 2022

github-actions bot commented Jul 26, 2022

github-actions bot commented Jul 26, 2022

github-actions bot commented Jul 26, 2022

github-actions bot commented Jul 26, 2022

fernflower left a comment

Choose a reason for hiding this comment

pirat89 commented Jun 21, 2022 •

edited

Loading

MichalHe commented Jun 23, 2022 •

edited

Loading

fernflower commented Jul 26, 2022 •

edited

Loading

pirat89 commented Jul 26, 2022 •

edited

Loading