This is an automated email from the ASF dual-hosted git repository.
pearl11594 pushed a commit to branch fr03-nsx-reorder-acl-rules
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit 85bf6f239b4eac0f5e1478f82e7dfb1d8455196e
Author: Vishesh <vishes...@gmail.com>
AuthorDate: Sat Jan 27 23:36:13 2024 +0530
Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt
ready command wrapper (#8547)
This PR fixes bug introduced in #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
---
agent/conf/agent.properties | 8 ++++++--
.../java/com/cloud/agent/properties/AgentProperties.java | 8 ++++++++
.../src/main/java/com/cloud/agent/AgentManager.java | 3 +++
.../main/java/com/cloud/agent/manager/AgentManagerImpl.java | 4 ++--
.../storage/datastore/provider/DefaultHostListener.java | 11 +++++++++--
.../kvm/resource/wrapper/LibvirtReadyCommandWrapper.java | 7 +++++--
utils/src/main/java/com/cloud/utils/script/Script.java | 13 +++++++++++++
7 files changed, 46 insertions(+), 8 deletions(-)
diff --git a/agent/conf/agent.properties b/agent/conf/agent.properties
index b15534a0be6..6adde7435ab 100644
--- a/agent/conf/agent.properties
+++ b/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=
diff --git
a/agent/src/main/java/com/cloud/agent/properties/AgentProperties.java
b/agent/src/main/java/com/cloud/agent/properties/AgentProperties.java
index 0e979d370f4..322a0ed2444 100644
--- a/agent/src/main/java/com/cloud/agent/properties/AgentProperties.java
+++ b/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>
diff --git
a/engine/components-api/src/main/java/com/cloud/agent/AgentManager.java
b/engine/components-api/src/main/java/com/cloud/agent/AgentManager.java
index 6ba0c3b4fa0..2182dfc542d 100644
--- a/engine/components-api/src/main/java/com/cloud/agent/AgentManager.java
+++ b/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,
}
diff --git
a/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java
b/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java
index d8671ed29df..606a902dce7 100644
---
a/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java
+++
b/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java
@@ -596,7 +596,7 @@ public class AgentManagerImpl extends ManagerBase
implements AgentManager, Handl
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 class AgentManagerImpl extends ManagerBase
implements AgentManager, Handl
@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 {
diff --git
a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java
b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java
index 90e8742c84d..a453f2d48c2 100644
---
a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java
+++
b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java
@@ -57,6 +57,12 @@ import java.util.List;
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 @@ public class DefaultHostListener implements
HypervisorHostListener {
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) {
diff --git
a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtReadyCommandWrapper.java
b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtReadyCommandWrapper.java
index 4df74decdea..c0089c0e3a6 100644
---
a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtReadyCommandWrapper.java
+++
b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtReadyCommandWrapper.java
@@ -25,6 +25,8 @@ import java.util.Map;
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 final class LibvirtReadyCommandWrapper extends
CommandWrapper<ReadyComman
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;
}
diff --git a/utils/src/main/java/com/cloud/utils/script/Script.java
b/utils/src/main/java/com/cloud/utils/script/Script.java
index b42acb2e3f4..6af08a981d1 100644
--- a/utils/src/main/java/com/cloud/utils/script/Script.java
+++ b/utils/src/main/java/com/cloud/utils/script/Script.java
@@ -531,6 +531,19 @@ public class Script implements Callable<String> {
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);
