Merge pull request #12 from Jiri-Kremser/bug1073096

Bug 1073096 - 'rhqctl -h install' tries to install the components instead of showing a help

modules/enterprise/server/server-control/src/main/java/org/rhq/server/control/ControlCommand.java

@@ -31,6 +31,7 @@
 import java.io.OutputStream;
 import java.net.Socket;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 import java.util.Locale;
@@ -60,6 +61,7 @@ public abstract class ControlCommand {
     public static final String SERVER_OPTION = "server";
     public static final String STORAGE_OPTION = "storage";
     public static final String AGENT_OPTION = "agent";
+    public static final String HELP_OPTION = "--help";
     public static final String RHQ_AGENT_BASEDIR_PROP = "rhq.agent.basedir";
 
     protected static final String STORAGE_BASEDIR_NAME = "rhq-storage";
@@ -78,8 +80,10 @@ public abstract class ControlCommand {
     private ArrayList<Runnable> undoTasks = new ArrayList<Runnable>();
 
     protected void undo() {
-        Collections.reverse(undoTasks); // do the undo tasks in the reverse order in which they were added to the list
-        for (Runnable undoTask : undoTasks) {
+        // perform the undo on the snapshot of undoTasks, because of possible ConcurrentModificationException
+        List<Runnable> undoTasksCopy = new ArrayList<Runnable>(undoTasks);
+        Collections.reverse(undoTasksCopy); // do the undo tasks in the reverse order in which they were added to the list
+        for (Runnable undoTask : undoTasksCopy) {
             try {
                 undoTask.run();
             } catch (Throwable t) {
@@ -150,8 +154,14 @@ public int exec(String[] args) {
         Options options = getOptions();
         int rValue = RHQControl.EXIT_CODE_OK;
         try {
-            CommandLineParser parser = new PosixParser();
-            CommandLine cmdLine = parser.parse(options, args);
+            CommandLineParser parser = new RHQPosixParser(false);
+            CommandLine cmdLine = parser.parse(options, args, true);
+            if (!cmdLine.getArgList().isEmpty()) {
+                // there were some unrecognized args
+                System.out.println("Unrecognized arguments: " + cmdLine.getArgList());
+                printUsage();
+                return RHQControl.EXIT_CODE_INVALID_ARGUMENT;
+            }
             rValue = exec(cmdLine);
 
             if (rhqctlConfig != null) {

modules/enterprise/server/server-control/src/main/java/org/rhq/server/control/RHQControl.java

@@ -89,7 +89,10 @@ public int exec(String[] args) {
                 String commandName = findCommand(commands, args);
                 command = commands.get(commandName);
 
-                logWarningIfAgentRPMIsInstalled(command);
+                if (!isHelp(args)) {
+                    // don't wait for user to read the warning if this is just request for help
+                    logWarningIfAgentRPMIsInstalled(command);
+                }
 
                 validateInstallCommand(command, args);
 
@@ -183,15 +186,11 @@ public boolean accept(File dir, String name) {
     }
 
     private void validateInstallCommand(ControlCommand command, String[] args) {
-        if (!"install".equalsIgnoreCase(command.getName())) {
+        // just return if we're asking for help or if it is not an install command
+        if (!"install".equalsIgnoreCase(command.getName()) || isHelp(args)) {
             return;
         }
-
-        // just return if we're asking for help
         List<String> argsList = Arrays.asList(args);
-        if (argsList.contains("--help")) {
-            return;
-        }
 
         // don't perform validation for components not involved in the command
         boolean validateServer = argsList.contains("--server")
@@ -306,6 +305,15 @@ private String findCommand(Commands commands, String[] args) throws RHQControlEx
 
         return commandNames.get(0);
     }
+
+    private boolean isHelp(String[] args) {
+        for (String arg : args) {
+            if (ControlCommand.HELP_OPTION.equals(arg)) {
+                return true;
+            }
+        }
+        return false;
+    }
 
     private String[] getCommandLine(String cmd, String[] args) {
         String[] cmdLine = new String[args.length - 1];

modules/enterprise/server/server-control/src/main/java/org/rhq/server/control/RHQPosixParser.java

@@ -0,0 +1,53 @@
+/*
+ * RHQ Management Platform
+ * Copyright (C) 2005-2014 Red Hat, Inc.
+ * All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+package org.rhq.server.control;
+
+import java.util.ListIterator;
+
+import org.apache.commons.cli.ParseException;
+import org.apache.commons.cli.PosixParser;
+
+/**
+ * @author Jirka Kremser
+ * 
+ * This parser class throws an exception if it finds some unknown option. 
+ * Option in the commons-cli terminology is the command line token starting with '-' or '--')
+ *
+ */
+public class RHQPosixParser extends PosixParser {
+    private boolean ignoreUnrecognizedOption;
+
+    public RHQPosixParser() {
+        this(false);
+    }
+
+    public RHQPosixParser(final boolean ignoreUnrecognizedOption) {
+        this.ignoreUnrecognizedOption = ignoreUnrecognizedOption;
+    }
+
+    @Override
+    protected void processOption(final String arg, final ListIterator iter) throws ParseException {
+        boolean hasOption = getOptions().hasOption(arg);
+
+        if (!ignoreUnrecognizedOption && !hasOption) {
+            throw new ParseException("Unknown option: " + arg);
+        }
+        super.processOption(arg, iter);
+    }
+}

modules/enterprise/server/server-control/src/main/java/org/rhq/server/control/command/AbstractInstall.java

@@ -40,7 +40,11 @@
 
 import org.apache.commons.cli.CommandLine;
 import org.apache.commons.cli.CommandLineParser;
-import org.apache.commons.cli.PosixParser;
+import org.apache.commons.exec.DefaultExecuteResultHandler;
+import org.apache.commons.exec.DefaultExecutor;
+import org.apache.commons.exec.Executor;
+import org.apache.commons.exec.PumpStreamHandler;
+
 import org.jboss.as.controller.client.ModelControllerClient;
 import org.rhq.common.jbossas.client.controller.DeploymentJBossASClient;
 import org.rhq.common.jbossas.client.controller.MCCHelper;
@@ -52,6 +56,7 @@
 import org.rhq.server.control.RHQControl;
 import org.rhq.server.control.RHQControlException;
 import org.rhq.server.control.util.ExecutorAssist;
+import org.rhq.server.control.RHQPosixParser;
 
 /**
  * Common code for commands that perform installs. Basically shared code for Install and Upgrade commands.
@@ -822,7 +827,7 @@ protected void addUndoTaskToStopComponent(final String componentArgument) {
         addUndoTask(new ControlCommand.UndoTask("Stopping component: " + componentArgument) {
             public void performUndoWork() throws Exception {
                 Stop stopCommand = new Stop();
-                CommandLineParser parser = new PosixParser();
+                CommandLineParser parser = new RHQPosixParser(true);
                 CommandLine cmdLine = parser.parse(stopCommand.getOptions(), new String[] { componentArgument });
                 stopCommand.exec(cmdLine);
             }

modules/enterprise/server/server-control/src/main/java/org/rhq/server/control/command/Install.java

@@ -150,7 +150,7 @@ public void performUndoWork() throws Exception {
             if (startStorage || installServer) {
                 startedStorage = true;
                 Start startCommand = new Start();
-                rValue = Math.max(rValue, startCommand.exec(new String[] { "start", "--storage" }));
+                rValue = Math.max(rValue, startCommand.exec(new String[] { "--storage" }));
             }
 
             if (installServer) {
@@ -205,10 +205,10 @@ public void performUndoWork() throws Exception {
             if (!start && (startedStorage || startedServer)) {
                 Stop stopCommand = new Stop();
                 if (startedServer) {
-                    rValue = Math.max(rValue, stopCommand.exec(new String[] { "stop", "--server" }));
+                    rValue = Math.max(rValue, stopCommand.exec(new String[] { "--server" }));
                 }
                 if (startedStorage) {
-                    rValue = Math.max(rValue, stopCommand.exec(new String[] { "stop", "--storage" }));
+                    rValue = Math.max(rValue, stopCommand.exec(new String[] { "--storage" }));
                 }
             }
         }

modules/enterprise/server/server-control/src/main/java/org/rhq/server/control/command/Upgrade.java

@@ -218,7 +218,7 @@ public void performUndoWork() throws Exception {
                     Stop stopCommand = new Stop();
                     stopCommand.exec(new String[] { "stop", "--server" });
                     if (!commandLine.hasOption(RUN_DATA_MIGRATION)) {
-                        rValue = Math.max(rValue, stopCommand.exec(new String[] { "stop", "--storage" }));
+                        rValue = Math.max(rValue, stopCommand.exec(new String[] { "--storage" }));
                     }
                 }
             } catch (Throwable t) {