Bug 1309536 - preupg exits with zero even on module errors
preupg exits with zero even on module errors
Status: CLOSED ERRATA
Product: Red Hat Enterprise Linux 6
Classification: Red Hat
Component: preupgrade-assistant (Show other bugs)
6.8
Unspecified Unspecified
unspecified Severity medium
: rc
: ---
Assigned To: Petr Hracek
Alois Mahdal
: Extras, Reopened
Depends On:
Blocks: 1335121 1393496
  Show dependency treegraph
 
Reported: 2016-02-17 23:20 EST by Alois Mahdal
Modified: 2017-03-21 08:08 EDT (History)
7 users (show)

See Also:
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 08:08:52 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Alois Mahdal 2016-02-17 23:20:29 EST
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).
Comment 2 Petr Hracek 2016-02-18 04:02:00 EST
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.
Comment 3 Alois Mahdal 2016-04-08 13:36:12 EDT
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 ]].
Comment 4 Petr Hracek 2016-04-20 08:45:31 EDT
Here is an upstream issue for it.
https://github.com/phracek/preupgrade-assistant/issues/94
Comment 8 Alois Mahdal 2016-08-31 20:58:27 EDT
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.
Comment 12 Michal Bocek 2016-12-14 07:45:03 EST
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      ||        |             |
-------|------------||--------|-------------|
Comment 14 Alois Mahdal 2017-01-31 11:44:28 EST
/CoreOS/preupgrade-assistant/api/mseverity covers this, and it passes, so OK from me.
Comment 15 Alois Mahdal 2017-01-31 11:44:29 EST
/CoreOS/preupgrade-assistant/api/mseverity covers this, and it passes, so OK from me.
Comment 17 errata-xmlrpc 2017-03-21 08:08:52 EDT
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

Note You need to log in before you can comment on or make changes to this bug.