Bug 1331629
| Summary: | `INPLACERISK: NONE: foo` does not really make any sense | ||
|---|---|---|---|
| Product: | Red Hat Enterprise Linux 6 | Reporter: | Alois Mahdal <amahdal> |
| Component: | preupgrade-assistant | Assignee: | Petr Hracek <phracek> |
| Status: | CLOSED ERRATA | QA Contact: | Alois Mahdal <amahdal> |
| Severity: | unspecified | Docs Contact: | |
| Priority: | unspecified | ||
| Version: | 6.8 | CC: | fkluknav, jmazanek, phracek, pstodulk, tcerna, ttomecek |
| Target Milestone: | rc | Keywords: | Extras |
| Target Release: | --- | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Whiteboard: | |||
| Fixed In Version: | preupgrade-assistant-2.1.8-4.el6 | Doc Type: | Bug Fix |
| Doc Text: |
If there is no risk detected, the Preupgrade Assistant no longer writes logs and returns an info message instead.
|
Story Points: | --- |
| Clone Of: | Environment: | ||
| Last Closed: | 2016-11-04 08:57:05 UTC | Type: | Bug |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
| Bug Depends On: | |||
| Bug Blocks: | 1335121 | ||
I partially agree. Originally, this risk level was supposed to indicate what was checked - and basically saying "Ok, this seems to be safe". However, this was based on assumption we will have some separate report of risks. Nowadays, just info message should be enough and much less confusing. Therefore ack for devel side. Moving to Petr, however, it would require checking all scripts for existance of "none" risk usage. (In reply to Ondrej Vasik from comment #3) > Moving to Petr, however, it would require checking all scripts for existance > of "none" risk usage. JFTR; bug 1356057 is the one to track removal of NONE usage from modules. Not removed; still present in
* common.sh (Bash API)
* tests (created "manually" using echo),
* more places:
@fullmoon|work:~/vcs.ipu/preupgrade-assistant((2.1.8))$ grep _none_ -r
common.sh:303:log_none_risk() {
preuputils/xml_utils.py:179: check_func = {'log_': ['log_none_risk', 'log_slight_risk',
@fullmoon|work:~/vcs.ipu/preupgrade-assistant((2.1.8))$ grep NONE -r
tests/FOOBAR6_7/dummy/needs_inspection/dummy_failed.sh:5:echo "INPLACERISK: NONE: This is None Risk"
tests/FOOBAR6_7/dummy/needs_inspection/solution.txt:3:INPLACERISK: NONE: None inplace risk
tests/test_inplace_risks.py:87: temp_file = self._copy_xccdf_file(b'INPLACERISK: NONE: Test None Inplace risk')
common.sh:307: log_risk "NONE" "$1"
preup/cli.py:103: "0 ... NONE, SLIGHT risks were detected." + "\n" * 20 +
preup/xccdf.py:28: 'NONE:': 0,
preup/report_parser.py:40: of NONE, SLIGHT or MEDIUM risk
preup/report_parser.py:222: NEEDS_INSPECTION in case that risks are NONE or SLIGHT
man/preupg.1:58:Return values are 0 for NONE, SLIGHT risks or 1 for MEDIUM, HIGH risks or 2 for EXTREME risk.
preup_ui/xmlrpc_backend/views.py:53: <meta name="robots" content="NONE,NOARCHIVE" />
@fullmoon|work:~/vcs.ipu/preupgrade-assistant((2.1.8))$
(not all places are necessarily be relevant but you get the point)
preupgrade-assistant-2.1.9-1.el6: this time it's wiped out properly. :) (Also from modules, as tracked by bug 1356057) (Well, except it's still mentioned in one note in the manpage, but since the manpage is so obsolete and incomplete it's ready for rewrite, that should not block us.) Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. For information on the advisory, and where to find the updated files, follow the link below. If the solution does not work for you, open a new bug report. https://rhn.redhat.com/errata/RHBA-2016-2616.html |
Description of problem ====================== log_none_risk() does not make sense. If you want to log a risk, you log a risk because there is one, of certain "risk level". But there's no such thing as "no risk level" as long as there is a risk. Also, the effect of using such function is confusing in the report: INPLACERISK: NONE: Blah blah... Is this a risk or not? Let's stop messing with people's heads. Including our own: if we agree "it should log risk" does this count?