Bug 994250 - rhqctl always returns exit code 0
rhqctl always returns exit code 0
Status: CLOSED CURRENTRELEASE
Product: RHQ Project
Classification: Other
Component: Launch Scripts (Show other bugs)
4.9
Unspecified Unspecified
unspecified Severity unspecified (vote)
: ---
: RHQ 4.11
Assigned To: Michael Burman
Mike Foley
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2013-08-06 17:28 EDT by Michael Burman
Modified: 2014-07-21 06:13 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2014-07-21 06:13:37 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)
Assign return values to each rhqctl command (67.53 KB, patch)
2013-08-06 17:36 EDT, Michael Burman
no flags Details | Diff
Earlier patch rebased to 4.10 master. (73.11 KB, patch)
2013-12-19 18:36 EST, Michael Burman
no flags Details | Diff
Earlier patch rebased to 4.10 master. (74.37 KB, patch)
2013-12-28 17:01 EST, Michael Burman
no flags Details | Diff
patch to be applied on top of the earlier one (12.29 KB, patch)
2014-01-02 10:52 EST, John Mazzitelli
no flags Details | Diff
patch to be applied on top of the earlier one (part deux) (11.92 KB, patch)
2014-01-02 10:58 EST, John Mazzitelli
no flags Details | Diff
additional patch to be applied (14.93 KB, patch)
2014-01-03 17:24 EST, John Mazzitelli
no flags Details | Diff
Uses wrapper to catch the return value instead of the ExecuteException (45.54 KB, patch)
2014-02-21 17:34 EST, Michael Burman
no flags Details | Diff

  None (edit)
Description Michael Burman 2013-08-06 17:28:33 EDT
Description of problem: rhqctl always returns the exit code 0, even if something fails / isn't running.


Version-Release number of selected component (if applicable): newest 4.9.0-SNAPSHOT


How reproducible: Always.


Steps to Reproduce:
1.
2.
3.

Actual results:


Expected results: exit codes based on the LSB coding:

0 = OK
1 = Operation failed
2 = Invalid argument
3 = Status command - not running
4 = Status command - unknown
5 = Not installed


Additional info:
Comment 1 Michael Burman 2013-08-06 17:36:44 EDT
Created attachment 783521 [details]
Assign return values to each rhqctl command

Assigns return values for each rhqctl command to indicate the end result. For example to rhqctl status:

michael@grace-mint ~/projects/rhq/dev-container/rhq-server/bin $ ./rhqctl status
00:16:50,906 INFO  [org.jboss.modules] JBoss Modules version 1.2.0.CR1
RHQ Storage Node               (no pid file) IS NOT running
RHQ Server                     (no pid file) IS NOT running
JBossAS Java VM child process  (no pid file) IS NOT running
RHQ Agent                      (no pid file) IS NOT running
michael@grace-mint ~/projects/rhq/dev-container/rhq-server/bin $ echo $?
3
michael@grace-mint ~/projects/rhq/dev-container/rhq-server/bin $

Also fixes rhq-server.sh to return the correct status code of 3 (instead of 0) if either JVM or SERVER is down:

michael@grace-mint ~/projects/rhq/dev-container/rhq-server/bin $ ./rhq-server.sh status
RHQ Server                     (no pid file) IS NOT running
JBossAS Java VM child process  (no pid file) IS NOT running
michael@grace-mint ~/projects/rhq/dev-container/rhq-server/bin $ echo $?
3
michael@grace-mint ~/projects/rhq/dev-container/rhq-server/bin $
Comment 2 Heiko W. Rupp 2013-12-07 16:59:14 EST
We should revisit this.
Comment 3 John Mazzitelli 2013-12-11 11:59:55 EST
Michael, we're looking at trying to apply this patch so it gets into RHQ 4.10 release, but the patch no longer applies to master (the rhqctl stuff has changed in the past few months since this patch was created). Would you be able to attach a new patch that can apply over current master?
Comment 4 Michael Burman 2013-12-19 18:36:13 EST
Created attachment 839282 [details]
Earlier patch rebased to 4.10 master.

Here's the rebased version.
Comment 5 Michael Burman 2013-12-19 18:39:01 EST
Ah, that patch is missing the changes to the rhq-server.sh. Too late today to do that, but I'll continue..
Comment 6 Michael Burman 2013-12-28 17:01:02 EST
Created attachment 842792 [details]
Earlier patch rebased to 4.10 master.

Fixed the rhq-server.sh as well (status & clean commands) in this patch.
Comment 7 John Mazzitelli 2014-01-02 10:52:45 EST
Created attachment 844599 [details]
patch to be applied on top of the earlier one

this is a patch that should be applied on top of the one already attached. This cleans up some things that I saw during the peer review of the merge.
Comment 8 John Mazzitelli 2014-01-02 10:58:24 EST
Created attachment 844601 [details]
patch to be applied on top of the earlier one (part deux)

opps, bad patch - re-attaching a good one. this is to be applied on top of the main patch.
Comment 9 John Mazzitelli 2014-01-02 11:15:04 EST
I merged the main patch (which was two commits) then commited a third to correct some issues. All is done and this is ready for ON_QA. Here's the git commits that went into master:

e626a35f95f2ad0c360438fdf87e7308b85f668a
72625626a668034023dcfaa39f283e9f28ea5749
9128c69cfe78f1de182c46bbfeebaadb18001327
Comment 10 John Mazzitelli 2014-01-03 14:32:20 EST
bad things happened when this patch went in - see bug #1048201

three different people are seeing problems - if we can't fix, we'll have to back out the three commits from master
Comment 11 John Mazzitelli 2014-01-03 14:54:28 EST
(In reply to John Mazzitelli from comment #10)
> bad things happened when this patch went in - see bug #1048201
> 
> three different people are seeing problems - if we can't fix, we'll have to
> back out the three commits from master

Here's something that is bad:

The new code in AbstractInstall is:

            log.info("The server installer is running");
            return executeHandler.getExitValue();
        } catch (IOException e) {
            log.error("An error occurred while starting the server installer: " + e.getMessage());
            return RHQControl.EXIT_CODE_NOT_INSTALLED;
        }

the old code was:

            log.info("The server installer is running");
        } catch (Exception e) {
            log.error("An error occurred while starting the server installer: " + e.getMessage());
        }

Notice the call to getExitValue now - well, this throws an exception because apparently the installer didn't finish yet. The exception is:

    java.lang.IllegalStateException: The process has not exited yet therefore no result is available ...

Now notice the catch clause is no longer Exception - its only IOException. So this exception isn't caught, bubbles up, and causes problems.
Comment 12 John Mazzitelli 2014-01-03 17:24:22 EST
Created attachment 845114 [details]
additional patch to be applied

this is an additional patch that needs to be applied. This fixes some problems with the install and status commands.
Comment 13 John Mazzitelli 2014-01-03 17:27:42 EST
the latest patch has been pushed to master. This is the fourth commit applied:

6b5624ee8eba0fbe9876a442b374610c996b0100

The server can be installed again. And I also fixed some issues with the status command that also was broken.

Reassigning this to Michael for him to reevaluate this fix. Make sure its really doing what he thinks its doing and make sure everything works.
Comment 14 Heiko W. Rupp 2014-02-18 08:08:34 EST
Where do we stand here? Michael, could you please have a look?
Comment 15 Michael Burman 2014-02-21 17:34:20 EST
Created attachment 866289 [details]
Uses wrapper to catch the return value instead of the ExecuteException

Newer patch uses a wrapper class (ExecuteAssist) to catch the ExecuteException and return correct values instead. It also simplifies the code by removing the need to each time create new DefaultExecutor.

michael@grace-mint ~/projects/rhq/dev-container/rhq-server/bin $ ./rhqctl status
00:30:12,739 INFO  [org.jboss.modules] JBoss Modules version 1.2.0.CR1
RHQ Storage Node               (pid 20375  ) is ✔ running
RHQ Server                     (pid 20560  ) is ✔ running
JBossAS Java VM child process  (pid 20560  ) is ✔ running
RHQ Agent                      (pid 20779  ) is ✔ running 
michael@grace-mint ~/projects/rhq/dev-container/rhq-server/bin $ echo $?
0
michael@grace-mint ~/projects/rhq/dev-container/rhq-server/bin $ kill -9 20779
michael@grace-mint ~/projects/rhq/dev-container/rhq-server/bin $ ./rhqctl status
00:30:37,791 INFO  [org.jboss.modules] JBoss Modules version 1.2.0.CR1
RHQ Storage Node               (pid 20375  ) is ✔ running
RHQ Server                     (pid 20560  ) is ✔ running
JBossAS Java VM child process  (pid 20560  ) is ✔ running
RHQ Agent                      (pid 20779  ) is ✘ down
michael@grace-mint ~/projects/rhq/dev-container/rhq-server/bin $ echo $?
1
michael@grace-mint ~/projects/rhq/dev-container/rhq-server/bin $
Comment 16 Heiko W. Rupp 2014-03-12 07:00:19 EDT
Moving to 4.11 after talking to Michael.
Comment 17 John Mazzitelli 2014-03-12 16:30:06 EDT
commit to master: ff40f15 from pull request: https://github.com/rhq-project/rhq/pull/3
Comment 18 Heiko W. Rupp 2014-07-21 06:13:37 EDT
Bulk closing of RHQ 4.11 issues, now that RHQ 4.12 is out.

If you find an issue with those, please open a new BZ, linking to the old one.

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