You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by "harikrishna-patnala (via GitHub)" <gi...@apache.org> on 2024/09/09 12:58:11 UTC
[PR] Introducing granular command timeouts global setting [cloudstack]
harikrishna-patnala opened a new pull request, #9659:
URL: https://github.com/apache/cloudstack/pull/9659
### Description
CloudStack has a global setting "wait" which controls the wait time by the management server for all the commands which it sends. There are also some command specific timeout settings which controls specific commands like "ready.command.wait", "kvm.storage.online.migration.wait", etc.
This PR simplifies these configurations by introducing a new parent global setting which serves as a single point to configure any command timeout. The new global setting is "commands. timeout", this timeout overrides the wait global config. This holds a comma separated key value pairs containing timeout for specific commands. For example: DhcpEntryCommand=600, SavePasswordCommand=300, VmDataCommand=300.
There are also some hardcoded values defined in the code base for few commands. so the order of precedence of all these timeouts will be
"commands.timeout" > Existing global settings > Hardcoded value for the command > global "wait"
<!--- Describe your changes in DETAIL - And how has behaviour functionally changed. -->
<!-- For new features, provide link to FS, dev ML discussion etc. -->
<!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
<!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
<!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
<!-- Fixes: # -->
<!--- ******************************************************************************* -->
<!--- NOTE: AUTOMATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. -->
<!--- PLEASE PUT AN 'X' in only **ONE** box -->
<!--- ******************************************************************************* -->
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
- [ ] build/CI
- [ ] test (unit or integration test code)
#### Feature/Enhancement Scale
- [ ] Major
- [ ] Minor
### Screenshots (if appropriate):
### How Has This Been Tested?
<!-- Please describe in detail how you tested your changes. -->
<!-- Include details of your testing environment, and the tests you ran to -->
#### How did you try to break this feature and the system with this change?
<!-- see how your change affects other areas of the code, etc. -->
<!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) document -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2362467733
<b>[SF] Trillian test result (tid-11518)</b>
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 71888 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9659-t11518-kvm-ol8.zip
Smoke tests completed. 138 look OK, 3 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_snapshot_usage | `Error` | 4.24 | test_usage.py
test_01_secure_vm_migration | `Error` | 135.20 | test_vm_life_cycle.py
test_01_secure_vm_migration | `Error` | 135.21 | test_vm_life_cycle.py
ContextSuite context=TestVnfTemplates>:setup | `Error` | 0.00 | test_vnf_templates.py
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2345278709
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2363553142
@harikrishna-patnala a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on code in PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#discussion_r1896654679
##########
engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java:
##########
@@ -424,15 +428,78 @@ private void setEmptyAnswers(final Commands commands, final Command[] cmds) {
}
}
+ protected int getTimeout(final Commands commands, int timeout) {
+ int result;
+ if (timeout > 0) {
+ result = timeout;
+ } else {
+ result = Wait.value();
+ }
+
+ int granularTimeout = getTimeoutFromGranularWaitTime(commands);
+ return (granularTimeout > 0) ? granularTimeout : result;
+ }
+
+ protected int getTimeoutFromGranularWaitTime(final Commands commands) {
+ int maxWait = 0;
+ if (MapUtils.isNotEmpty(_commandTimeouts)) {
+ for (final Command cmd : commands) {
+ String simpleCommandName = cmd.getClass().getSimpleName();
+ Integer commandTimeout = _commandTimeouts.get(simpleCommandName);
+ if (commandTimeout != null && commandTimeout > maxWait) {
+ maxWait = commandTimeout;
+ }
+ }
+ }
+
+ return maxWait;
+ }
+
+ private void initializeCommandTimeouts() {
+ String commandWaits = GranularWaitTimeForCommands.value().trim();
+ if (StringUtils.isNotEmpty(commandWaits)) {
+ try {
+ _commandTimeouts = getCommandTimeoutsMap(commandWaits);
+ logger.info(String.format("Timeouts for management server internal commands successfully initialized from global setting commands.timeout: %s", _commandTimeouts));
+ } catch (Exception e) {
+ logger.error("Error initializing command timeouts map: " + e.getMessage());
+ _commandTimeouts = new HashMap<>();
+ }
+ }
+ }
+
+ private Map<String, Integer> getCommandTimeoutsMap(String commandWaits) {
+ String[] commandPairs = commandWaits.split(",");
+ Map<String, Integer> commandTimeouts = new HashMap<>();
+
+ for (String commandPair : commandPairs) {
+ String[] parts = commandPair.trim().split("=");
+ if (parts.length == 2) {
+ String commandName = parts[0].trim();
+ int commandTimeout = Integer.parseInt(parts[1].trim());
+ commandTimeouts.put(commandName, commandTimeout);
+ } else {
+ logger.warn(String.format("Invalid format in commands.timeout global setting: %s", commandPair));
+ }
+ }
+ return commandTimeouts;
+ }
+
@Override
public Answer[] send(final Long hostId, final Commands commands, int timeout) throws AgentUnavailableException, OperationTimedoutException {
assert hostId != null : "Who's not checking the agent id before sending? ... (finger wagging)";
if (hostId == null) {
throw new AgentUnavailableException(-1);
}
- if (timeout <= 0) {
- timeout = Wait.value();
+ int wait = getTimeout(commands, timeout);
+ logger.debug(String.format("Wait time setting on %s is %d seconds", commands, wait));
+ for (Command cmd : commands) {
+ String simpleCommandName = cmd.getClass().getSimpleName();
+ Integer commandTimeout = _commandTimeouts.get(simpleCommandName);
+ if (commandTimeout != null) {
+ cmd.setWait(wait);
Review Comment:
I think anything is fine, I kept in debug because I observed some of the commands are not logging the time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2342942228
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 11092
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2339816297
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 11069
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2513588588
@harikrishna-patnala a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on code in PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#discussion_r1896650250
##########
engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java:
##########
@@ -424,15 +428,78 @@ private void setEmptyAnswers(final Commands commands, final Command[] cmds) {
}
}
+ protected int getTimeout(final Commands commands, int timeout) {
+ int result;
+ if (timeout > 0) {
+ result = timeout;
+ } else {
+ result = Wait.value();
+ }
+
+ int granularTimeout = getTimeoutFromGranularWaitTime(commands);
+ return (granularTimeout > 0) ? granularTimeout : result;
+ }
+
+ protected int getTimeoutFromGranularWaitTime(final Commands commands) {
+ int maxWait = 0;
+ if (MapUtils.isNotEmpty(_commandTimeouts)) {
+ for (final Command cmd : commands) {
+ String simpleCommandName = cmd.getClass().getSimpleName();
+ Integer commandTimeout = _commandTimeouts.get(simpleCommandName);
+ if (commandTimeout != null && commandTimeout > maxWait) {
+ maxWait = commandTimeout;
+ }
+ }
+ }
+
+ return maxWait;
+ }
+
+ private void initializeCommandTimeouts() {
+ String commandWaits = GranularWaitTimeForCommands.value().trim();
+ if (StringUtils.isNotEmpty(commandWaits)) {
+ try {
+ _commandTimeouts = getCommandTimeoutsMap(commandWaits);
+ logger.info(String.format("Timeouts for management server internal commands successfully initialized from global setting commands.timeout: %s", _commandTimeouts));
+ } catch (Exception e) {
Review Comment:
the getCommandTimeoutsMap parses the value from the global setting which is supposed to be in a specific format. Though we are already checking the format while setting value in the global setting, here I kept it as safe check not to fail the actual command and goes with default value / hardcoded value set on the command.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on code in PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#discussion_r1764931720
##########
engine/components-api/src/main/java/com/cloud/agent/AgentManager.java:
##########
@@ -50,6 +50,10 @@ public interface AgentManager {
ConfigKey<Integer> ReadyCommandWait = new ConfigKey<Integer>("Advanced", Integer.class, "ready.command.wait",
"60", "Time in seconds to wait for Ready command to return", true);
+ ConfigKey<String> GranularWaitTimeForCommands = new ConfigKey<>("Advanced", String.class, "commands.timeout", "",
+ "This timeout overrides the wait global config. This holds a comma separated key value pairs containing timeout (in seconds) for specific commands. " +
+ "For example: DhcpEntryCommand=600, SavePasswordCommand=300, VmDataCommand=300", true);
Review Comment:
Agreed @DaanHoogland, modified the code accordingly
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2574429518
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2348094173
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2340054168
@sureshanaparti a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2338180952
Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 11058
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2348153340
Packaging result [SF]: ✔️ el8 ✔️ el9 ✖️ debian ✔️ suse15. SL-JID 11107
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2363897389
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2514699074
> I am merging forward 4.19 on 4.20 and will merge this as soon as the conflicts are taken care of.
and the smoke tests return good results ;)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on code in PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#discussion_r1871289972
##########
engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java:
##########
@@ -424,15 +428,78 @@ private void setEmptyAnswers(final Commands commands, final Command[] cmds) {
}
}
+ protected int getTimeout(final Commands commands, int timeout) {
+ int result;
+ if (timeout > 0) {
+ result = timeout;
+ } else {
+ result = Wait.value();
+ }
+
+ int granularTimeout = getTimeoutFromGranularWaitTime(commands);
+ return (granularTimeout > 0) ? granularTimeout : result;
+ }
+
+ protected int getTimeoutFromGranularWaitTime(final Commands commands) {
+ int maxWait = 0;
+ if (MapUtils.isNotEmpty(_commandTimeouts)) {
+ for (final Command cmd : commands) {
+ String simpleCommandName = cmd.getClass().getSimpleName();
+ Integer commandTimeout = _commandTimeouts.get(simpleCommandName);
+ if (commandTimeout != null && commandTimeout > maxWait) {
+ maxWait = commandTimeout;
+ }
+ }
+ }
+
+ return maxWait;
+ }
+
+ private void initializeCommandTimeouts() {
+ String commandWaits = GranularWaitTimeForCommands.value().trim();
+ if (StringUtils.isNotEmpty(commandWaits)) {
+ try {
+ _commandTimeouts = getCommandTimeoutsMap(commandWaits);
+ logger.info(String.format("Timeouts for management server internal commands successfully initialized from global setting commands.timeout: %s", _commandTimeouts));
+ } catch (Exception e) {
+ logger.error("Error initializing command timeouts map: " + e.getMessage());
+ _commandTimeouts = new HashMap<>();
+ }
+ }
+ }
+
+ private Map<String, Integer> getCommandTimeoutsMap(String commandWaits) {
+ String[] commandPairs = commandWaits.split(",");
+ Map<String, Integer> commandTimeouts = new HashMap<>();
+
+ for (String commandPair : commandPairs) {
+ String[] parts = commandPair.trim().split("=");
+ if (parts.length == 2) {
+ String commandName = parts[0].trim();
+ int commandTimeout = Integer.parseInt(parts[1].trim());
Review Comment:
@harikrishna-patnala would it be better to handle null values and NumberFormatException here instead of handling any exception in the caller? It may allow ignoring a single wrong key-pair
##########
engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java:
##########
@@ -424,15 +428,78 @@ private void setEmptyAnswers(final Commands commands, final Command[] cmds) {
}
}
+ protected int getTimeout(final Commands commands, int timeout) {
+ int result;
+ if (timeout > 0) {
+ result = timeout;
+ } else {
+ result = Wait.value();
+ }
+
+ int granularTimeout = getTimeoutFromGranularWaitTime(commands);
+ return (granularTimeout > 0) ? granularTimeout : result;
+ }
+
+ protected int getTimeoutFromGranularWaitTime(final Commands commands) {
+ int maxWait = 0;
+ if (MapUtils.isNotEmpty(_commandTimeouts)) {
+ for (final Command cmd : commands) {
+ String simpleCommandName = cmd.getClass().getSimpleName();
+ Integer commandTimeout = _commandTimeouts.get(simpleCommandName);
+ if (commandTimeout != null && commandTimeout > maxWait) {
+ maxWait = commandTimeout;
+ }
+ }
+ }
+
+ return maxWait;
+ }
+
+ private void initializeCommandTimeouts() {
+ String commandWaits = GranularWaitTimeForCommands.value().trim();
+ if (StringUtils.isNotEmpty(commandWaits)) {
+ try {
+ _commandTimeouts = getCommandTimeoutsMap(commandWaits);
+ logger.info(String.format("Timeouts for management server internal commands successfully initialized from global setting commands.timeout: %s", _commandTimeouts));
+ } catch (Exception e) {
Review Comment:
why catch all?
##########
engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java:
##########
@@ -424,15 +428,78 @@ private void setEmptyAnswers(final Commands commands, final Command[] cmds) {
}
}
+ protected int getTimeout(final Commands commands, int timeout) {
+ int result;
+ if (timeout > 0) {
+ result = timeout;
+ } else {
+ result = Wait.value();
+ }
+
+ int granularTimeout = getTimeoutFromGranularWaitTime(commands);
+ return (granularTimeout > 0) ? granularTimeout : result;
+ }
+
+ protected int getTimeoutFromGranularWaitTime(final Commands commands) {
+ int maxWait = 0;
+ if (MapUtils.isNotEmpty(_commandTimeouts)) {
+ for (final Command cmd : commands) {
+ String simpleCommandName = cmd.getClass().getSimpleName();
+ Integer commandTimeout = _commandTimeouts.get(simpleCommandName);
+ if (commandTimeout != null && commandTimeout > maxWait) {
+ maxWait = commandTimeout;
+ }
+ }
+ }
+
+ return maxWait;
+ }
+
+ private void initializeCommandTimeouts() {
+ String commandWaits = GranularWaitTimeForCommands.value().trim();
+ if (StringUtils.isNotEmpty(commandWaits)) {
+ try {
+ _commandTimeouts = getCommandTimeoutsMap(commandWaits);
+ logger.info(String.format("Timeouts for management server internal commands successfully initialized from global setting commands.timeout: %s", _commandTimeouts));
+ } catch (Exception e) {
+ logger.error("Error initializing command timeouts map: " + e.getMessage());
+ _commandTimeouts = new HashMap<>();
+ }
+ }
+ }
+
+ private Map<String, Integer> getCommandTimeoutsMap(String commandWaits) {
+ String[] commandPairs = commandWaits.split(",");
+ Map<String, Integer> commandTimeouts = new HashMap<>();
+
+ for (String commandPair : commandPairs) {
+ String[] parts = commandPair.trim().split("=");
+ if (parts.length == 2) {
+ String commandName = parts[0].trim();
+ int commandTimeout = Integer.parseInt(parts[1].trim());
+ commandTimeouts.put(commandName, commandTimeout);
+ } else {
+ logger.warn(String.format("Invalid format in commands.timeout global setting: %s", commandPair));
+ }
+ }
+ return commandTimeouts;
+ }
+
@Override
public Answer[] send(final Long hostId, final Commands commands, int timeout) throws AgentUnavailableException, OperationTimedoutException {
assert hostId != null : "Who's not checking the agent id before sending? ... (finger wagging)";
if (hostId == null) {
throw new AgentUnavailableException(-1);
}
- if (timeout <= 0) {
- timeout = Wait.value();
+ int wait = getTimeout(commands, timeout);
+ logger.debug(String.format("Wait time setting on %s is %d seconds", commands, wait));
+ for (Command cmd : commands) {
+ String simpleCommandName = cmd.getClass().getSimpleName();
+ Integer commandTimeout = _commandTimeouts.get(simpleCommandName);
+ if (commandTimeout != null) {
+ cmd.setWait(wait);
Review Comment:
Do we need to log timeout for a specific command, maybe just in trace? It may help while troubleshooting
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2348095105
@harikrishna-patnala a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2350819793
<b>[SF] Trillian test result (tid-11470)</b>
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 61057 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9659-t11470-kvm-ol8.zip
Smoke tests completed. 138 look OK, 3 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_snapshot_usage | `Error` | 6.28 | test_usage.py
test_02_unsecure_vm_migration | `Error` | 448.61 | test_vm_life_cycle.py
test_03_secured_to_nonsecured_vm_migration | `Error` | 259.21 | test_vm_life_cycle.py
test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Failure` | 623.62 | test_vpc_redundant.py
test_05_rvpc_multi_tiers | `Failure` | 557.63 | test_vpc_redundant.py
test_05_rvpc_multi_tiers | `Error` | 557.65 | test_vpc_redundant.py
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on code in PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#discussion_r1896651022
##########
engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java:
##########
@@ -424,15 +428,78 @@ private void setEmptyAnswers(final Commands commands, final Command[] cmds) {
}
}
+ protected int getTimeout(final Commands commands, int timeout) {
+ int result;
+ if (timeout > 0) {
+ result = timeout;
+ } else {
+ result = Wait.value();
+ }
+
+ int granularTimeout = getTimeoutFromGranularWaitTime(commands);
+ return (granularTimeout > 0) ? granularTimeout : result;
+ }
+
+ protected int getTimeoutFromGranularWaitTime(final Commands commands) {
+ int maxWait = 0;
+ if (MapUtils.isNotEmpty(_commandTimeouts)) {
+ for (final Command cmd : commands) {
+ String simpleCommandName = cmd.getClass().getSimpleName();
+ Integer commandTimeout = _commandTimeouts.get(simpleCommandName);
+ if (commandTimeout != null && commandTimeout > maxWait) {
+ maxWait = commandTimeout;
+ }
+ }
+ }
+
+ return maxWait;
+ }
+
+ private void initializeCommandTimeouts() {
+ String commandWaits = GranularWaitTimeForCommands.value().trim();
+ if (StringUtils.isNotEmpty(commandWaits)) {
+ try {
+ _commandTimeouts = getCommandTimeoutsMap(commandWaits);
+ logger.info(String.format("Timeouts for management server internal commands successfully initialized from global setting commands.timeout: %s", _commandTimeouts));
+ } catch (Exception e) {
+ logger.error("Error initializing command timeouts map: " + e.getMessage());
+ _commandTimeouts = new HashMap<>();
+ }
+ }
+ }
+
+ private Map<String, Integer> getCommandTimeoutsMap(String commandWaits) {
+ String[] commandPairs = commandWaits.split(",");
+ Map<String, Integer> commandTimeouts = new HashMap<>();
+
+ for (String commandPair : commandPairs) {
+ String[] parts = commandPair.trim().split("=");
+ if (parts.length == 2) {
+ String commandName = parts[0].trim();
+ int commandTimeout = Integer.parseInt(parts[1].trim());
Review Comment:
sure let me see if this can be improved.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2574430690
@harikrishna-patnala a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2513493929
@harikrishna-patnala a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2516559746
> > @DaanHoogland, before merging this one, please check #8605.
> > This PR solves the same thing that #8605, but apparently that solution was ignored.
>
> ok @GutoVeronezi (and @harikrishna-patnala) , please discuss whether to merge or choose one of the two?
>
> As far as I can see the main difference is whether the configuration is done in a Configitem or in a separate table, is that right?
Thanks @GutoVeronezi for bringing this up. I wasn't aware of your earlier PR, so I independently worked on a solution to address this issue, which is now available here.
Yes @DaanHoogland both the solutions aim to resolve the same problem, though with slightly different approaches one with global setting and one with database table. I feel the current PR offers a more user-friendly experience and is already accessible through the UI. Additionally, it has gone through the development cycle, including reviews, testing, and validation, and is now ready to merge.
That said, @GutoVeronezi I want to ensure that any valuable aspects of your earlier work are not overlooked. If there are specific elements from your solution that you feel should be incorporated here, I'd be happy to discuss.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2338062724
@harikrishna-patnala a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2365041919
<b>[SF] Trillian test result (tid-11529)</b>
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 57863 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9659-t11529-kvm-ol8.zip
Smoke tests completed. 134 look OK, 0 have errors, 7 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
all_test_vpc_redundant | `Skipped` | --- | test_vpc_redundant.py
all_test_vpc_router_nics | `Skipped` | --- | test_vpc_router_nics.py
all_test_vpc_vpn | `Skipped` | --- | test_vpc_vpn.py
all_test_webhook_delivery | `Skipped` | --- | test_webhook_delivery.py
all_test_webhook_lifecycle | `Skipped` | --- | test_webhook_lifecycle.py
all_test_host_maintenance | `Skipped` | --- | test_host_maintenance.py
all_test_hostha_kvm | `Skipped` | --- | test_hostha_kvm.py
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2359913599
@harikrishna-patnala a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2359912888
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "GutoVeronezi (via GitHub)" <gi...@apache.org>.
GutoVeronezi commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2514770279
@DaanHoogland, before merging this one, please check #8605.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2574505056
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 12000
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2358284283
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2345280124
@sureshanaparti a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2338075085
## [Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/9659?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 4.05%. Comparing base [(`f8d8a9c`)](https://app.codecov.io/gh/apache/cloudstack/commit/f8d8a9c7b389ea77a3559b5d69560f8ac5b0e355?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`a666303`)](https://app.codecov.io/gh/apache/cloudstack/commit/a6663031b841335dd9fb28965b00172ce08b3e03?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 5 commits behind head on main.
> :exclamation: There is a different number of reports uploaded between BASE (f8d8a9c) and HEAD (a666303). Click for more details.
>
> <details><summary>HEAD has 1 upload less than BASE</summary>
>
>| Flag | BASE (f8d8a9c) | HEAD (a666303) |
>|------|------|------|
>|unittests|1|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #9659 +/- ##
============================================
- Coverage 15.77% 4.05% -11.73%
============================================
Files 5623 390 -5233
Lines 491611 32045 -459566
Branches 60012 5640 -54372
============================================
- Hits 77568 1298 -76270
+ Misses 405584 30599 -374985
+ Partials 8459 148 -8311
```
| [Flag](https://app.codecov.io/gh/apache/cloudstack/pull/9659/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [uitests](https://app.codecov.io/gh/apache/cloudstack/pull/9659/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `4.05% <ø> (-0.01%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/cloudstack/pull/9659/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/cloudstack/pull/9659?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2339680928
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2358287931
@harikrishna-patnala a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2342797981
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2513581566
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 11684
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2514818096
> @DaanHoogland, before merging this one, please check #8605.
>
> This PR solves the same thing that #8605, but apparently that solution was ignored.
ok @GutoVeronezi (and @harikrishna-patnala) , please discuss whether to merge or choose one of the two?
As far as I can see the main difference is whether the configuration is done in a Configitem or in a separate table, is that right?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2347294258
<b>[SF] Trillian test result (tid-11457)</b>
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 57345 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9659-t11457-kvm-ol8.zip
Smoke tests completed. 138 look OK, 3 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_UpdateConfigParamWithScope | `Error` | 0.08 | test_global_settings.py
test_01_snapshot_usage | `Error` | 6.26 | test_usage.py
test_02_unsecure_vm_migration | `Error` | 497.36 | test_vm_life_cycle.py
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2348587552
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#discussion_r1760707782
##########
engine/components-api/src/main/java/com/cloud/agent/AgentManager.java:
##########
@@ -50,6 +50,10 @@ public interface AgentManager {
ConfigKey<Integer> ReadyCommandWait = new ConfigKey<Integer>("Advanced", Integer.class, "ready.command.wait",
"60", "Time in seconds to wait for Ready command to return", true);
+ ConfigKey<String> GranularWaitTimeForCommands = new ConfigKey<>("Advanced", String.class, "commands.timeout", "",
+ "This timeout overrides the wait global config. This holds a comma separated key value pairs containing timeout (in seconds) for specific commands. " +
+ "For example: DhcpEntryCommand=600, SavePasswordCommand=300, VmDataCommand=300", true);
Review Comment:
I would not make this one dynamic as the whole array needs to be loaded on every command sent to every agent. this will strain the system. The array should be constructed on load and be in memory readily available from then on. I.E.
```suggestion
"For example: DhcpEntryCommand=600, SavePasswordCommand=300, VmDataCommand=300", false);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2358448210
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 11146
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2363552493
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2363686688
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 11162
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2338058862
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2342684869
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2342798901
@harikrishna-patnala a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on code in PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#discussion_r1904928463
##########
engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java:
##########
@@ -424,15 +428,78 @@ private void setEmptyAnswers(final Commands commands, final Command[] cmds) {
}
}
+ protected int getTimeout(final Commands commands, int timeout) {
+ int result;
+ if (timeout > 0) {
+ result = timeout;
+ } else {
+ result = Wait.value();
+ }
+
+ int granularTimeout = getTimeoutFromGranularWaitTime(commands);
+ return (granularTimeout > 0) ? granularTimeout : result;
+ }
+
+ protected int getTimeoutFromGranularWaitTime(final Commands commands) {
+ int maxWait = 0;
+ if (MapUtils.isNotEmpty(_commandTimeouts)) {
+ for (final Command cmd : commands) {
+ String simpleCommandName = cmd.getClass().getSimpleName();
+ Integer commandTimeout = _commandTimeouts.get(simpleCommandName);
+ if (commandTimeout != null && commandTimeout > maxWait) {
+ maxWait = commandTimeout;
+ }
+ }
+ }
+
+ return maxWait;
+ }
+
+ private void initializeCommandTimeouts() {
+ String commandWaits = GranularWaitTimeForCommands.value().trim();
+ if (StringUtils.isNotEmpty(commandWaits)) {
+ try {
+ _commandTimeouts = getCommandTimeoutsMap(commandWaits);
+ logger.info(String.format("Timeouts for management server internal commands successfully initialized from global setting commands.timeout: %s", _commandTimeouts));
+ } catch (Exception e) {
+ logger.error("Error initializing command timeouts map: " + e.getMessage());
+ _commandTimeouts = new HashMap<>();
+ }
+ }
+ }
+
+ private Map<String, Integer> getCommandTimeoutsMap(String commandWaits) {
+ String[] commandPairs = commandWaits.split(",");
+ Map<String, Integer> commandTimeouts = new HashMap<>();
+
+ for (String commandPair : commandPairs) {
+ String[] parts = commandPair.trim().split("=");
+ if (parts.length == 2) {
+ String commandName = parts[0].trim();
+ int commandTimeout = Integer.parseInt(parts[1].trim());
Review Comment:
updated the exception handling @shwstppr , please review again.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2342369414
<b>[SF] Trillian test result (tid-11439)</b>
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 52865 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9659-t11439-kvm-ol8.zip
Smoke tests completed. 139 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_UpdateConfigParamWithScope | `Error` | 0.08 | test_global_settings.py
test_01_snapshot_usage | `Error` | 4.24 | test_usage.py
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2339681951
@harikrishna-patnala a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2342687544
@harikrishna-patnala a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2348592098
@harikrishna-patnala a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2340051151
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2363898749
@harikrishna-patnala a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala merged PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2413111965
> @harikrishna-patnala should I mark this for 20.1 or 21?
This can go into 20.1 @DaanHoogland existing behavior won't change unless it is configured
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2515493652
<b>[SF] Trillian test result (tid-11833)</b>
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 52018 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9659-t11833-kvm-ol8.zip
Smoke tests completed. 140 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_secure_vm_migration | `Error` | 134.12 | test_vm_life_cycle.py
test_01_secure_vm_migration | `Error` | 134.12 | test_vm_life_cycle.py
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2513492031
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2513588145
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Introducing granular command timeouts global setting [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #9659:
URL: https://github.com/apache/cloudstack/pull/9659#issuecomment-2514697272
I am merging forward 4.19 on 4.20 and will merge this as soon as the conflicts are taken care of.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org