Bug 994250

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 $

We should revisit this.

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?

Created attachment 839282 [details]
Earlier patch rebased to 4.10 master.

Here's the rebased version.

Ah, that patch is missing the changes to the rhq-server.sh. Too late today to do that, but I'll continue..

Created attachment 842792 [details]
Earlier patch rebased to 4.10 master.

Fixed the rhq-server.sh as well (status & clean commands) in this patch.

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.

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.

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

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

(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.

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.

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.

Where do we stand here? Michael, could you please have a look?

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 $

Moving to 4.11 after talking to Michael.

commit to master: ff40f15 from pull request: https://github.com/rhq-project/rhq/pull/3

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.