Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper (apache#8547)

This PR fixes bug introduced in apache#8502. Timeout for script execution was set to 60 ms instead of 60s which resulted in host not getting UEFI enabled. This is a blocker for 4.19 release.

We do this by introducing a new agent parameter `agent.script.timeout` (default - 60 seconds) to use as a timeout for the script checking host's UEFI status.

We also externalize the timeout for the ReadyCommand by introducing a new global setting `ready.command.wait` (default - 60 seconds).

For ModifyStoragePoolCommand, we don't externalize the timeout to avoid confusion for the user. Since, the required timeout can vary depending on the provider in use and we are only setting the wait for default host listener for now. Instead, we reuse the global `wait` setting by dividing it by `5` making the default value of 6 minutes (1800/5 = 360s) for ModifyStoragePoolCommand.

Note: the actual time, the MS waits is twice the wait set for a Command. Check reference code below.
https://github.com/apache/cloudstack/blob/19250403e645c76f60b17aa4aeb4dc915f5ca206/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentAttache.java#L406-L442

Author: vishesh92

agent/conf/agent.properties

@@ -5,9 +5,9 @@
 # to you under the Apache License, Version 2.0 (the
 # "License"); you may not use this file except in compliance
 # with the License.  You may obtain a copy of the License at
-# 
+#
 #   http://www.apache.org/licenses/LICENSE-2.0
-# 
+#
 # Unless required by applicable law or agreed to in writing,
 # software distributed under the License is distributed on an
 # "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
@@ -378,6 +378,10 @@ iscsi.session.cleanup.enabled=false
 # If the time is exceeded shutdown will be forced.
 #stop.script.timeout=120
 
+# Time (in seconds) to wait for scripts to complete.
+# This is currently used only while checking if the host supports UEFI.
+#agent.script.timeout=60
+
 # Definition of VMs video model type.
 #vm.video.hardware=
 

agent/src/main/java/com/cloud/agent/properties/AgentProperties.java

@@ -659,6 +659,14 @@ public class AgentProperties{
      */
     public static final Property<Integer> STOP_SCRIPT_TIMEOUT = new Property<>("stop.script.timeout", 120);
 
+    /**
+     * Time (in seconds) to wait for scripts to complete.<br>
+     * This is currently used only while checking if the host supports UEFI.<br>
+     * Data type: Integer.<br>
+     * Default value: <code>60</code>
+     */
+    public static final Property<Integer> AGENT_SCRIPT_TIMEOUT = new Property<>("agent.script.timeout", 60);
+
     /**
      * Definition of VMs video model type.<br>
      * Data type: String.<br>

engine/components-api/src/main/java/com/cloud/agent/AgentManager.java

@@ -47,6 +47,9 @@ public interface AgentManager {
                             "according to the hosts health check results",
                     true, ConfigKey.Scope.Cluster, null);
 
+    ConfigKey<Integer> ReadyCommandWait = new ConfigKey<Integer>("Advanced", Integer.class, "ready.command.wait",
+            "60", "Time in seconds to wait for Ready command to return", true);
+
     public enum TapAgentsAction {
         Add, Del, Contains,
     }

engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java

@@ -596,7 +596,7 @@ protected AgentAttache notifyMonitorsOfConnection(final AgentAttache attache, fi
 
         final Long dcId = host.getDataCenterId();
         final ReadyCommand ready = new ReadyCommand(dcId, host.getId(), NumbersUtil.enableHumanReadableSizes);
-        ready.setWait(60);
+        ready.setWait(ReadyCommandWait.value());
         final Answer answer = easySend(hostId, ready);
         if (answer == null || !answer.getResult()) {
             // this is tricky part for secondary storage
@@ -1838,7 +1838,7 @@ public String getConfigComponentName() {
     @Override
     public ConfigKey<?>[] getConfigKeys() {
         return new ConfigKey<?>[] { CheckTxnBeforeSending, Workers, Port, Wait, AlertWait, DirectAgentLoadSize,
-                DirectAgentPoolSize, DirectAgentThreadCap, EnableKVMAutoEnableDisable };
+                DirectAgentPoolSize, DirectAgentThreadCap, EnableKVMAutoEnableDisable, ReadyCommandWait };
     }
 
     protected class SetHostParamsListener implements Listener {

engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java

@@ -57,6 +57,12 @@
 
 public class DefaultHostListener implements HypervisorHostListener {
     private static final Logger s_logger = Logger.getLogger(DefaultHostListener.class);
+
+    /**
+     * Wait time for modify storage pool command to complete. We should wait for 5 minutes for the command to complete.
+     * This should ideally be externalised as a global configuration parameter in the future (See #8506).
+     **/
+    private final int modifyStoragePoolCommandWait = 300; // 5 minutes
     @Inject
     AgentManager agentMgr;
     @Inject
@@ -84,7 +90,6 @@ public class DefaultHostListener implements HypervisorHostListener {
     @Inject
     NetworkDao networkDao;
 
-
     @Override
     public boolean hostAdded(long hostId) {
         return true;
@@ -121,7 +126,9 @@ private NicTO createNicTOFromNetworkAndOffering(NetworkVO networkVO, NetworkOffe
     public boolean hostConnect(long hostId, long poolId) throws StorageConflictException {
         StoragePool pool = (StoragePool) this.dataStoreMgr.getDataStore(poolId, DataStoreRole.Primary);
         ModifyStoragePoolCommand cmd = new ModifyStoragePoolCommand(true, pool);
-        cmd.setWait(60);
+        cmd.setWait(modifyStoragePoolCommandWait);
+        s_logger.debug(String.format("Sending modify storage pool command to agent: %d for storage pool: %d with timeout %d seconds",
+                hostId, poolId, cmd.getWait()));
         final Answer answer = agentMgr.easySend(hostId, cmd);
 
         if (answer == null) {

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtReadyCommandWrapper.java

@@ -25,6 +25,8 @@
 import com.cloud.agent.api.Answer;
 import com.cloud.agent.api.ReadyAnswer;
 import com.cloud.agent.api.ReadyCommand;
+import com.cloud.agent.properties.AgentProperties;
+import com.cloud.agent.properties.AgentPropertiesFileHandler;
 import com.cloud.host.Host;
 import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
 import com.cloud.resource.CommandWrapper;
@@ -51,11 +53,12 @@ public Answer execute(final ReadyCommand command, final LibvirtComputingResource
 
     private boolean hostSupportsUefi(boolean isUbuntuHost) {
         String cmd = "rpm -qa | grep -i ovmf";
+        int timeout = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.AGENT_SCRIPT_TIMEOUT) * 1000; // Get property value & convert to milliseconds
         if (isUbuntuHost) {
             cmd = "dpkg -l ovmf";
         }
-        s_logger.debug("Running command : " + cmd);
-        int result = Script.runSimpleBashScriptForExitValue(cmd, 60, false);
+        s_logger.debug("Running command : [" + cmd + "] with timeout : " + timeout + " ms");
+        int result = Script.runSimpleBashScriptForExitValue(cmd, timeout, false);
         s_logger.debug("Got result : " + result);
         return result == 0;
     }

utils/src/main/java/com/cloud/utils/script/Script.java

@@ -531,6 +531,19 @@ public static int runSimpleBashScriptForExitValueAvoidLogging(String command) {
         return runSimpleBashScriptForExitValue(command, 0, true);
     }
 
+    /**
+     * Executes a bash script and returns the exit value of the script.
+     *
+     * @param command
+     *         The bash command to be executed.
+     * @param timeout
+     *         The maximum time (in milliseconds) that the script is allowed to run before it is forcibly terminated.
+     * @param avoidLogging
+     *         If set to true, some logging is avoided.
+     *
+     * @return The exit value of the script. Returns -1 if the result is null or empty, or if it cannot be parsed into
+     *         an integer which can happen in case of a timeout.
+     */
     public static int runSimpleBashScriptForExitValue(String command, int timeout, boolean avoidLogging) {
 
         Script s = new Script("/bin/bash", timeout);