PAYARA-2162 Added evaluation of commands which erroneously trigger re…

nucleus/admin/util/src/main/java/com/sun/enterprise/admin/util/ClusterOperationUtil.java

@@ -38,6 +38,8 @@
  * holder.
  */
 
+// Portions Copyright [2017] [Payara Foundation and/or its affiliates]
+
 package com.sun.enterprise.admin.util;
 
 import com.sun.enterprise.admin.remote.RemoteRestAdminCommand;
@@ -56,17 +58,19 @@
 import org.glassfish.config.support.CommandTarget;
 import org.glassfish.hk2.api.ServiceLocator;
 import org.glassfish.internal.api.Target;
-import org.glassfish.logging.annotation.LogMessageInfo;
 
 /**
- *
+ * Utility Class to replicate commands to the cluster.
+ * 
  */
 public class ClusterOperationUtil {
     private static final Logger logger = AdminLoggerInfo.getLogger();
 
 
-    private static final LocalStringManagerImpl strings =
-                        new LocalStringManagerImpl(ClusterOperationUtil.class);
+    private static final LocalStringManagerImpl strings = new LocalStringManagerImpl(ClusterOperationUtil.class);
+
+    // PAYARA-2162 Commands which poll remote instances without setting values can trigger restart-required
+    private static final List<String> ALLOWED_COMMANDS = Arrays.asList("_get-runtime-info");
 
     //TODO : Begin temp fix for undoable commands
     private static List<Server> completedInstances = new ArrayList<Server>();
@@ -132,7 +136,7 @@ public static ActionReport.ExitCode replicateCommand(String commandName,
         InstanceStateService instanceState = habitat.getService(InstanceStateService.class);
         validateIntermediateDownloadDir(intermediateDownloadDir);
         RemoteInstanceCommandHelper rich = new RemoteInstanceCommandHelper(habitat);
-        Map<String, Future<InstanceCommandResult>> futures = new HashMap<String, Future<InstanceCommandResult>>();
+        Map<String, Future<InstanceCommandResult>> futures = new HashMap<>();
         try {
             for(Server svr : instancesForReplication) {
                 if (instanceState.getState(svr.getName()) == InstanceState.StateType.NEVER_STARTED) {
@@ -159,7 +163,9 @@ public static ActionReport.ExitCode replicateCommand(String commandName,
                     continue;
                 }               
                 Config scfg = svr.getConfig();
-                if (!Boolean.valueOf(scfg.getDynamicReconfigurationEnabled())) {
+                // PAYARA-2162 Restart Required is set erroneously when _get-runtime-info is called
+                if (!Boolean.valueOf(scfg.getDynamicReconfigurationEnabled()) 
+                        && !ALLOWED_COMMANDS.contains(commandName)) {
                     // Do not replicate to servers for which dynamic configuration is disabled
                     ActionReport aReport = context.getActionReport().addSubActionsReport();
                     aReport.setActionExitCode(ActionReport.ExitCode.WARNING);

nucleus/core/kernel/src/main/java/com/sun/enterprise/v3/admin/LocalStrings.properties

@@ -75,6 +75,8 @@ restart.server.failure=Got an exception trying to restart: {0}
 restart.server.init=Server restart initiated
 restart.server.noStartupInfo=Unable to restart.  Neither CLI or non-CLI startup info was found.  \
 Here is what we were looking for:\n{0}\n{1}
+restart.server.noPasswordFile=Unable to restart as the password file used to start the domain has been moved or deleted. \n\
+The following password file could not be found:\n{0}
 restart.server.asadminError=Error in Asadmin.  These 3 arguments must be present on the server's \
 original command line:\n-asadmin-classpath\n-asadmin-classname\n-asadmin-args\nCould not restart.
 restart.server.nonAsadminError=Internal Error in ASMain.  These 3 arguments were not set:\n\

nucleus/core/kernel/src/main/java/com/sun/enterprise/v3/admin/RestartServer.java

@@ -46,6 +46,7 @@
 import com.sun.enterprise.universal.process.JavaClassRunner;
 import com.sun.enterprise.universal.process.ProcessUtils;
 import com.sun.enterprise.util.StringUtils;
+import static com.sun.enterprise.util.StringUtils.ok;
 import org.glassfish.api.admin.AdminCommandContext;
 import org.glassfish.internal.api.Globals;
 import org.glassfish.embeddable.GlassFish;
@@ -60,21 +61,21 @@
 import javax.inject.Provider;
 
 /**
- * For non-verbose mode:
- * Stop this server, spawn a new JVM that will wait for this JVM to die.  The new JVM then starts the server again.
+ * For non-verbose mode: Stop this server, spawn a new JVM that will wait for this JVM to die. The new JVM then starts
+ * the server again.
  *
- * For verbose mode:
- * We want the asadmin console itself to do the respawning -- so just return a special int from
- * System.exit().  This tells asadmin to restart.
+ * For verbose mode: We want the asadmin console itself to do the respawning -- so just return a special int from
+ * System.exit(). This tells asadmin to restart.
  *
  * @author Byron Nevins
  */
 public class RestartServer {
+
     @Inject
     private Provider<GlassFish> glassfishProvider;
-    
+
     private final Lock stopLock = new ReentrantLock();
-    
+
     protected final void setDebug(Boolean b) {
         debug = b;
     }
@@ -90,19 +91,19 @@ protected final void setServerName(String serverNameIn) {
     /**
      * Restart of the application server :
      *
-     * All running services are stopped.
-     * LookupManager is flushed.
+     * All running services are stopped. LookupManager is flushed.
      *
      * Client code that started us should notice the special return value and restart us.
      */
     protected final void doExecute(AdminCommandContext context) {
         try {
             // unfortunately we can't rely on constructors with HK2...
-            if (registry == null)
+            if (registry == null) {
                 throw new NullPointerException(new LocalStringsImpl(getClass()).get("restart.server.internalError", "registry was not set"));
+            }
 
             init(context);
-            
+
             // get the GlassFish object - we have to wait in case startup is still in progress
             // This is a temporary work-around until HK2 supports waiting for the service to 
             // show up in the ServiceLocator. 
@@ -111,16 +112,15 @@ protected final void doExecute(AdminCommandContext context) {
                 Thread.sleep(1000);
                 gfKernel = glassfishProvider.get();
             }
-            
+
             if (!supervised) {
                 // do it now while we still have the Logging service running...
                 reincarnate();
             }
             prepareToExit();
             // else we just return a special int from System.exit()
             gfKernel.stop();
-        }
-        catch (Exception e) {
+        } catch (Exception e) {
             context.getLogger().severe(strings.get("restart.server.failure", e));
         } finally {
             stopLock.unlock();
@@ -137,10 +137,11 @@ private void prepareToExit() {
             public void run() {
                 stopLock.lock();
                 int ret = RESTART_NORMAL;
-                
-                if (debug != null)
+
+                if (debug != null) {
                     ret = debug ? RESTART_DEBUG_ON : RESTART_DEBUG_OFF;
-
+                }
+
                 System.exit(ret);
             }
         };
@@ -163,17 +164,20 @@ private void init(AdminCommandContext context) throws IOException {
 
     private void reincarnate() {
         try {
-            if (setupReincarnationWithAsadmin() || setupReincarnationWithOther())
-                doReincarnation();
-            else
-                logger.severe(strings.get("restart.server.noStartupInfo",
-                        strings.get("restart.server.asadminError"),
-                        strings.get("restart.server.nonAsadminError")));
-        }
-        catch (RDCException rdce) {
+            if (restartable()) {
+                if (setupReincarnationWithAsadmin() || setupReincarnationWithOther()) {
+                    doReincarnation();
+                } else {
+                    logger.severe(strings.get("restart.server.noStartupInfo",
+                            strings.get("restart.server.asadminError"),
+                            strings.get("restart.server.nonAsadminError")));
+                }
+            } else {
+                logger.severe(strings.get("restart.server.noPasswordFile", passwordFilePath));
+            }
+        } catch (RDCException rdce) {
             // already logged...
-        }
-        catch (Exception e) {
+        } catch (Exception e) {
             logger.severe(strings.get("restart.server.internalError", e));
         }
 
@@ -186,12 +190,11 @@ private void doReincarnation() throws RDCException {
 
             String[] props = normalProps;
 
-            if (Boolean.parseBoolean(System.getenv("AS_SUPER_DEBUG")))
+            if (Boolean.parseBoolean(System.getenv("AS_SUPER_DEBUG"))) {
                 props = debuggerProps;  // very very difficult to debug this stuff otherwise!
-
+            }
             new JavaClassRunner(classpath, props, classname, args);
-        }
-        catch (Exception e) {
+        } catch (Exception e) {
             logger.severe(strings.get("restart.server.jvmError", e));
             throw new RDCException();
         }
@@ -214,6 +217,31 @@ private boolean setupReincarnationWithOther() throws RDCException {
         return verify("restart.server.nonAsadminError");
     }
 
+    private boolean restartable() throws RDCException {
+        boolean restartable = true;
+        File passwordFile;
+        String asadminString = props.getProperty("-asadmin-args");
+        if (ok(asadminString) && asadminString.contains("--passwordfile")) {
+            String[] asadminArgs = asadminString.split(",,,");
+            for (int i = 0; i < asadminArgs.length; i++) {
+                if (asadminArgs[i].equals("--passwordfile")) {
+                    if ((i + 1) < asadminArgs.length && ok(asadminArgs[i + 1])) {
+                        passwordFilePath = asadminArgs[i + 1];
+                        break;
+                    }
+                }
+            }
+        } else {
+            // if there's no password file set, we're done here
+            return true;
+        }
+        passwordFile = new File(passwordFilePath);
+        if (ok(passwordFilePath)) {
+            restartable = passwordFile.canRead();
+        }
+        return restartable;
+    }
+
     private boolean verify(String errorStringKey) throws RDCException {
         // Either asadmin or non-asadmin startup params have been set -- check them!
         // THREE possible returns:
@@ -237,7 +265,9 @@ private boolean verify(String errorStringKey) throws RDCException {
 
     private void handleDebug() {
         if (debug == null) // nothing to do!
+        {
             return;
+        }
 
         stripDebugFromArgs();
         stripOperandFromArgs();
@@ -280,23 +310,27 @@ private void stripDebugFromArgs() {
             }
         }
 
-        if (indexOfDebug < 0)
+        if (indexOfDebug < 0) {
             return;
+        }
 
         int oldlen = args.length;
         int newlen = oldlen - 1;
 
-        if (twoArgs)
+        if (twoArgs) {
             --newlen;
+        }
 
         String[] newArgs = new String[newlen];
         int ctr = 0;
 
         for (int i = 0; i < oldlen; i++) {
-            if (i == indexOfDebug)
+            if (i == indexOfDebug) {
                 continue;
-            if (twoArgs && i == (indexOfDebug + 1))
+            }
+            if (twoArgs && i == (indexOfDebug + 1)) {
                 continue;
+            }
 
             newArgs[ctr++] = args[i];
         }
@@ -307,8 +341,9 @@ private void stripDebugFromArgs() {
     private void stripOperandFromArgs() {
         // remove the domain-name operand
         // it may not be here!
-        if (args.length < 2 || !StringUtils.ok(serverName))
+        if (args.length < 2 || !StringUtils.ok(serverName)) {
             return;
+        }
 
         int newlen = args.length - 1;
 
@@ -321,6 +356,7 @@ private void stripOperandFromArgs() {
 
     private boolean ok(String s) {
         return s != null && s.length() > 0;
+
     }
 
     // We use this simply to tell the difference between fatal errors and other
@@ -337,7 +373,9 @@ private static class RDCException extends Exception {
     private String argsString;
     private String[] args;
     private String serverName = "";
-    private static final LocalStringsImpl strings = new LocalStringsImpl(RestartServer.class);
+    private String passwordFilePath = "";
+    private static final LocalStringsImpl strings = new LocalStringsImpl(RestartServer.class
+    );
     /////////////             static variables               ///////////////////
     private static final String magicProperty = "-DAS_RESTART=" + ProcessUtils.getPid();
     private static final String[] normalProps = {magicProperty};

nucleus/core/kernel/src/main/java/com/sun/enterprise/v3/admin/RuntimeInfo.java

@@ -145,7 +145,7 @@ public Object run() throws Exception {
         checkDtrace();
         setDasName();
         top.addProperty("java.vm.name", System.getProperty("java.vm.name"));
-        setRestartable();
+//        setRestartable();
         reportMessage.append(Strings.get("runtime.info.debug", jpdaEnabled ? "enabled" : "not enabled"));
         report.setMessage(reportMessage.toString());
     }
@@ -170,54 +170,6 @@ private void setDasName() {
         }
     }
 
-    /**
-     * March 11 2011 -- See JIRA 16197
-     * Say the user started the server with a passwordfile arg.  After they started it
-     * they deleted the password file. If we don't do anything special restart-server
-     * will take down the server -- but it will not startup again.  The user will have no clue why.
-     * We can NOT tell the user directly because the restart server command is asynchronous
-     * (@Async annotation).
-     * So -- this method was added as a pre-flight check.  The client restart commands
-     * should run this command and check the restartable flag to make sure
-     * the restart doesn't fail because of a missing password file.
-     */
-    private void setRestartable() {
-        // false positive is MUCH better than false negative.  Err on the side of
-        // trying to restart if in doubt.  No harm can result from that.
-        restartable = true;
-        String passwordFile = null;
-
-        try {
-            Properties props = Globals.get(StartupContext.class).getArguments();
-            String argsString = props.getProperty("-asadmin-args");
-
-            if (ok(argsString) && argsString.indexOf("--passwordfile") >= 0) {
-                String[] args = argsString.split(",,,");
-
-                for (int i = 0; i < args.length; i++) {
-                    if (args[i].equals("--passwordfile")) {
-                        if ((i + 1) < args.length && ok(args[i + 1])) {
-                            passwordFile = args[i + 1];
-                        }
-                        break;
-                    }
-                }
-            }
-        }
-        catch (Exception e) {
-            // nothing to do, but I'll do this anyway because I'm paranoid
-            restartable = true;
-        }
-
-        if (ok(passwordFile)) {
-            // the --passwordfile is here -- so it had best point to a file that
-            // exists and can be read!  In all other cases -- restartable is true
-            File pwf = new File(passwordFile);
-            restartable = pwf.canRead();
-        }
-        top.addProperty("restartable", Boolean.toString(restartable));
-    }
-
     private int parsePort(String s) {
         //"-Xdebug -Xrunjdwp:transport=dt_socket,server=y,suspend=n,address=9009"
         int port = -1;