Hide Forgot
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).
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.
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