Bug 1309536
| Summary: | preupg exits with zero even on module errors | ||
|---|---|---|---|
| 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: | medium | Docs Contact: | |
| Priority: | unspecified | ||
| Version: | 6.8 | CC: | fkluknav, mbocek, ovasik, phracek, pstodulk, tcerna, ttomecek |
| Target Milestone: | rc | Keywords: | Extras, Reopened |
| Target Release: | --- | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Whiteboard: | |||
| Fixed In Version: | preupgrade-assistant-2.2.0-1.el6 | Doc Type: | No Doc Update |
| Doc Text: |
undefined
|
Story Points: | --- |
| Clone Of: | Environment: | ||
| Last Closed: | 2017-03-21 12:08:52 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, 1393496 | ||
Good point. I would like to explain it a little bit more.
First of all. Preupgrade-assistant does not depend on preupgrade-assistant-contents and If modules fail or even finish with error, preupgrade-assistant does not care about it.
And it should remain as it is. Even OpenSCAP returns 0 in case that a module finishes with ERROR status
~~~
From `man oscap`
EXIT STATUS
Normally, the exit status is 0 when operation finished successfully and 1 otherwise. In
cases when oscap performs evaluation of the system it may return 2 indicating success of
the operation but incompliance of the assessed system.
~~~
I have discuss it with OpenSCAP folks and it should return 0 also in case that some content fails.
BUT, preupgrade-assistant has to return none zero return value if there is a traceback or problem inside preupgrade-assistant itself.
In case, module uses a wrong parameters or even module fails with another reason then it is not a problem on preupgrade-assistant side.
I don't agree. Of course it's p-a's problem. A module fails and reports error. Since p-a is the framework, it's p-a's responsibility to bring this into attention; nobody else can do that. By returning 0, preupg basically silencing (to certain tools that would read ES) the problem that the module reported. From usability POV, it's irrelevant whether p-a uses a monolithic or modular architecture, save for the actual implementation (oscap or whatnot). User asks for assessment and does not get it. That's not [[ $? == 0 ]]. Here is an upstream issue for it. https://github.com/phracek/preupgrade-assistant/issues/94 This is finally fixed by upstream commits: https://github.com/phracek/preupgrade-assistant/commit/2144504489a17fef35b960ceaed7da09f59b5b56 https://github.com/phracek/preupgrade-assistant/commit/9c125a82fd236c4894ad101863b4944980c0448a For the record: Exit status table suggested in comment 0 is obsolete; current scheme can be seen on mailing list: http://post-office.corp.redhat.com/archives/rhel-inplaceupgrades/2016-August/msg00028.html The scheme will still need clarification, however automated test is almost ready (se External Tracker field of this ticket) and will be updated afterwards. In preupgrade-assistant-2.2.0-1.el6, preupg exits with non-zero code on module errors. See the below table that show what exit codes are returned based on module results.
MODULE FUNC CALLS || PREUPG BEHAVIOR
------.-------------||--------.-------------.
exit_x | log_x_risk || result | exit status |
=======|============||========|=============|
fail | (none) || fail | 2 |
fail | slight || n_ins | 0 |
fail | medium || n_ins | 0 |
fail | high || n_act | 1 |
fail | extreme || fail | 2 |
-------|------------||--------|-------------|
fixed | * || fixed | 0 |
-------|------------||--------|-------------|
info | * || info | 0 |
-------|------------||--------|-------------|
n_app | * || napp | 0 |
-------|------------||--------|-------------|
pass | * || pass | 0 |
-------|------------||--------|-------------|
error | * || error | 2 |
-------|------------||--------|-------------|
exit_x or || | |
log_x_risk || error | 2 |
not called || | |
-------|------------||--------|-------------|
/CoreOS/preupgrade-assistant/api/mseverity covers this, and it passes, so OK from me. /CoreOS/preupgrade-assistant/api/mseverity covers this, and it passes, so OK from me. 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-2017-0819.html |
Description of problem ====================== Even if there were multiple errors during assessment, preupg will exit with status of zero, indicating success to surrounding code. This makes it harder to catch problems early. Version-Release number of selected component ============================================ preupgrade-assistant-2.1.4-9.el6.noarch How reproducible ================ Always Steps to Reproduce ================== 1. Prepare a module that exits with error result (eg. open a module script and add `exit_error` call) 2. Run `preupg; echo es=$?` 3. Observe exit status (`es=?`) Actual results ============== 0 Expected results ================ 1 or more Additional info =============== I'm not sure if exit status semantics have ever been specified anywhere; if not, my suggestion would be: * 0 if all went OK (none or non-blocking risks), * 1 on blocking risks, * 2 on usage error (eg. bad params), * 3 on module errors (above mentioned case), * 4 on PA errors, * 5 on unknown errors (if possible).