Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
shwstppr merged PR #8547: URL: https://github.com/apache/cloudstack/pull/8547 -- 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
shwstppr commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1911904477 @GutoVeronezi are your concerns with this PR addressed? I'll proceed with 4.19RC4 if no objections -- 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
blueorangutan commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1911759801 [SF] Trillian test result (tid-8947) Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server r8 Total time taken: 48946 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8547-t8947-vmware-67u3.zip Smoke tests completed. 119 look OK, 2 have errors, 0 did not run Only failed and skipped tests results shown below: Test | Result | Time (s) | Test File --- | --- | --- | --- ContextSuite context=TestAccounts>:setup | `Error` | 0.00 | test_accounts.py ContextSuite context=TestAddVmToSubDomain>:setup | `Error` | 0.00 | test_accounts.py test_DeleteDomain | `Error` | 9.30 | test_accounts.py test_forceDeleteDomain | `Failure` | 9.18 | test_accounts.py ContextSuite context=TestRemoveUserFromAccount>:setup | `Error` | 10.38 | test_accounts.py ContextSuite context=TestTemplateHierarchy>:setup | `Error` | 12.21 | test_accounts.py test_02_balanced_drs_algorithm | `Error` | 429.31 | test_cluster_drs.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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
blueorangutan commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1911642867 [SF] Trillian test result (tid-8948) Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7 Total time taken: 43265 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8547-t8948-kvm-centos7.zip Smoke tests completed. 121 look OK, 0 have errors, 0 did not run Only failed and skipped tests results shown below: Test | Result | Time (s) | Test File --- | --- | --- | --- -- 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
blueorangutan commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1911638525 [SF] Trillian test result (tid-8946) Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7 Total time taken: 43130 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8547-t8946-xenserver-71.zip Smoke tests completed. 121 look OK, 0 have errors, 0 did not run Only failed and skipped tests results shown below: Test | Result | Time (s) | Test File --- | --- | --- | --- -- 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
blueorangutan commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1911201217 [SF] Trillian test result (tid-8942) Environment: kvm-alma9 (x2), Advanced Networking with Mgmt server a9 Total time taken: 55621 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8547-t8942-kvm-alma9.zip Smoke tests completed. 121 look OK, 0 have errors, 0 did not run Only failed and skipped tests results shown below: Test | Result | Time (s) | Test File --- | --- | --- | --- -- 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
blueorangutan commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1911106294 [SF] Trillian test result (tid-8941) Environment: kvm-ubuntu22 (x2), Advanced Networking with Mgmt server u22 Total time taken: 50172 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8547-t8941-kvm-ubuntu22.zip Smoke tests completed. 121 look OK, 0 have errors, 0 did not run Only failed and skipped tests results shown below: Test | Result | Time (s) | Test File --- | --- | --- | --- -- 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
blueorangutan commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1910872929 @vishesh92 a [SL] Trillian-Jenkins matrix job (centos7 mgmt + xenserver71, rocky8 mgmt + vmware67u3, centos7 mgmt + kvmcentos7) 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
vishesh92 commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1910870129 @blueorangutan test matrix -- 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
blueorangutan commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1910854722 Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8443 -- 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
blueorangutan commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1910754646 @vishesh92 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
vishesh92 commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1910754120 @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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
GutoVeronezi commented on code in PR #8547:
URL: https://github.com/apache/cloudstack/pull/8547#discussion_r1466747036
##
engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java:
##
@@ -121,7 +122,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(Wait.value() / 5);
Review Comment:
In contrast with taking a value and dividing it by a random number,
externalizing a new configuration would be the best option. We will not
externalize every command timeout right now, so it would not be a problem. I
agree a more robust solution, like what I mentioned in
https://github.com/apache/cloudstack/pull/8502#discussion_r1450506118, is
better; however, we have to discuss it first.
As last resort, if you do not want to externalize a new configuration,
creating a constant with javadoc for this case is better than deriving the
value.
--
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-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
vishesh92 commented on code in PR #8547:
URL: https://github.com/apache/cloudstack/pull/8547#discussion_r1466712708
##
engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java:
##
@@ -121,7 +122,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(Wait.value() / 5);
Review Comment:
Why do you think externalizing is the best choice?
If we externalize this, we will be externalizing it for the default host
listener only. Meaning the configuration parameter would be something like
`command.modify.storage.pool.default.host.listener.timeout` since this won't be
used by other listeners. And if we keep on externalizing in this way,
eventually we will have a lot of configuration settings. We have around 550
Commands. Add listeners and providers on top, we will probably end up with more
than a thousand new settings this way.
I externalized ReadyCommand, because I could verify that not much is being
done to process it.
IMO, this is not the best solution as of now, but it's better than nothing.
I would prefer to actually have a better solution out of #8506 and use that
instead.
Let me know if this is okay for now. If it's not, I will just remove the
wait for `ModifyStoragePoolCommand`.
--
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-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
vishesh92 commented on code in PR #8547:
URL: https://github.com/apache/cloudstack/pull/8547#discussion_r1466712708
##
engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java:
##
@@ -121,7 +122,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(Wait.value() / 5);
Review Comment:
Why do you think externalizing is the best choice?
If we externalize this, we will be externalizing it for the default host
listener only. Meaning the configuration parameter would be something like
`command.modify.storage.pool.default.host.listener.timeout` since this won't be
used by other listeners. And if we keep on externalizing in this way,
eventually we will have a lot of configuration settings. We have around 550
Commands. Add listeners and providers on top, we will probably end up with more
than a thousand new settings this way.
I externalized ReadyCommand, because I could verify that not much is being
done to process it.
IMO, this is not the best solution as of now, but it's better than nothing.
I would prefer to actually have a better solution out of #8506 and use that
instead.
Let me know if this is okay for now. If it's not, I will just remove the
wait for `ModifyStoragePoolCommand` for now.
--
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-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
vishesh92 commented on code in PR #8547:
URL: https://github.com/apache/cloudstack/pull/8547#discussion_r1466712708
##
engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java:
##
@@ -121,7 +122,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(Wait.value() / 5);
Review Comment:
Why do you think externalizing is the best choice?
If we externalize this, we will be externalizing it for the default host
listener only. Meaning the configuration parameter would be something like
`command.modify.storage.pool.default.listener.timeout` since this won't be used
by other listeners. And if we keep on externalizing in this way, eventually we
will have a lot of configuration settings. We have around 550 Commands. Add
listeners and providers on top, we will probably end up with more than a
thousand new settings this way.
I externalized ReadyCommand, because I could verify that not much is being
done to process it.
IMO, this is not the best solution as of now, but it's better than nothing.
I would prefer to actually have a better solution out of #8506 and use that
instead.
Let me know if this is okay for now. If it's not, I will just remove the
wait for `ModifyStoragePoolCommand` for now.
--
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-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
GutoVeronezi commented on code in PR #8547:
URL: https://github.com/apache/cloudstack/pull/8547#discussion_r1466678435
##
engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java:
##
@@ -121,7 +122,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(Wait.value() / 5);
Review Comment:
In this case, externalizing a new configuration is the best choice.
--
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-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
vishesh92 commented on code in PR #8547:
URL: https://github.com/apache/cloudstack/pull/8547#discussion_r149536
##
engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java:
##
@@ -121,7 +122,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(Wait.value() / 5);
Review Comment:
It's more about the worst case scenario. It will wait for the specified time
only if the agent is stuck because of some reason. Before #8502, if the wait
was set to 18000, this command would have waited for 10 hours (5 hours *2).
This solution doesn't completely solve the the problem but reduces the issues
to some extent until issues #8506 or #8011 is resolved.
--
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-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
GutoVeronezi commented on code in PR #8547:
URL: https://github.com/apache/cloudstack/pull/8547#discussion_r1466636892
##
engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java:
##
@@ -121,7 +122,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(Wait.value() / 5);
Review Comment:
User also might increase `wait` to 18000 and we would have 1 hour here. It
does not make sense to use an unfounded math function to derive a value;
please, either use the same configuration or externalize a new one.
--
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-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
vishesh92 commented on code in PR #8547:
URL: https://github.com/apache/cloudstack/pull/8547#discussion_r1466620107
##
engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java:
##
@@ -121,7 +122,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(Wait.value() / 5);
Review Comment:
Yes. Reasoning behind this is that user might decrease the wait in some
scenario. It's quite unlikely but if the user sets it to 5 min, we will wait at
least 1 minute here.
P.S. - the actual wait time is twice the value set for wait. Check the note
in the description.
--
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-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
GutoVeronezi commented on code in PR #8547:
URL: https://github.com/apache/cloudstack/pull/8547#discussion_r1466604442
##
engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java:
##
@@ -121,7 +122,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(Wait.value() / 5);
Review Comment:
It still does not make sense. Why 5? Just to match 6 minutes when the
default `wait` is 1800?
--
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-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
vishesh92 commented on code in PR #8547:
URL: https://github.com/apache/cloudstack/pull/8547#discussion_r1466578342
##
engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java:
##
@@ -121,7 +122,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(Wait.value() / 5);
Review Comment:
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.
--
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-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
vishesh92 commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1910486420 > I see #8502 was a blocker, but why this one is a blocker? I used 60 ms instead of 6 ms as timeout for the script. This resulted in all hosts having UEFI status as false. > @vishesh92, please, improve the description of the PR to reflect the real proposal (which is different from issue #8506), mentioning PR #8502 and explaining why it is a blocker. I have updated the PR description. -- 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
GutoVeronezi commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1910406928 @vishesh92, please, improve the description of the PR to reflect the real proposal (which is different from issue #8506), mentioning PR #8502 and explaining why it is a blocker. -- 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
GutoVeronezi commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1910397215 > I would like to get this PR merged for now since it's a blocker for unblock 4.19 release. Let me know what you think? I see #8502 was a blocker, but why this one is a blocker? -- 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
GutoVeronezi commented on code in PR #8547:
URL: https://github.com/apache/cloudstack/pull/8547#discussion_r1466511764
##
engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java:
##
@@ -121,7 +122,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(Wait.value() / 5);
Review Comment:
Why divide by 5?
--
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-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
GutoVeronezi commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1910393568 > @GutoVeronezi would it make sense to keep the issue open for further work... Sure do, because the issue is not being fixed by this PR. > ... are you -1 on the PR changes as well? No. Indeed, I proposed the externalization of the timeout settings, vide https://github.com/apache/cloudstack/pull/8502#discussion_r1450410628 (and the author of the PR was against it at first: https://github.com/apache/cloudstack/pull/8502#discussion_r1452036208). My point is that this PR does not even touch the essence of the issue; therefore, saying it solves the issue does not make sense. > binding does not apply to PRs. Thanks, @DaanHoogland. I think I missed this detail from the docs. > @GutoVeronezi , I suppose you are referring to the solution mentioned in this comment: [#8502 (comment)](https://github.com/apache/cloudstack/pull/8502#discussion_r1450506118) ? > > If so, this is a good idea and would obsolete this PR, but not without implementation. Do you have code ready to replace the code here? No, there is no code ready yet; before implementing something every each way, we should discuss the pros and cons. That is the purpose of the issue. -- 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
vishesh92 commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1910385336 > -1 (binding) > > @vishesh92 this PR does not fix #8506. The issue and the thread it mentions is related to a generic solution, which was not even discussed. @GutoVeronezi I agree that this PR does not fix #8506. We need to revisit the timeouts for Commands on both the MS (#8506) & agents (#8011 - Agents don't even consider timeout which is set in the Commands). The main purpose of #8502 was to fix the failures we were seeing in e2e tests, but I introduced another bug (because of using 60ms instead of 60s for the script's timeout). In this PR, I wanted to fix the bug I introduced and address your concerns at least for the changes I did in my previous PR (#8502). Regarding externalizing timeouts for all commands, IMO we shouldn't try to fix it for now. I would like to do that but it would be a very big change and it will require more discussion and planning because the same `Command` can processed by different providers depending on an environment and the required timeout can be different depending on the scenario and the setup. I would like to get this PR merged for now since it's a blocker for unblock 4.19 release. Let me know what you think? -- 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
DaanHoogland commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1910311087 > -1 (binding) binding does not apply to PRs. That said, if you have objections and a way forward we will not merge (on any solidly argued -1) > @vishesh92 this PR does not fix #8506. The issue and the thread it mentions is related to a generic solution, which was not even discussed. @GutoVeronezi , I suppose you are referring to the solution mentioned in this comment: https://github.com/apache/cloudstack/pull/8502#discussion_r1450506118 ? If so, this is a good idea and would obsolete this PR, but not without implementation. Do you have code ready to replace the code here? -- 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
shwstppr commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1910255944 > -1 (binding) > > @vishesh92 this PR does not fix #8506. The issue and the thread it mentions is related to a generic solution, which was not even discussed. @GutoVeronezi would it make sense to keep the issue open for further work and continue with this PR or are you -1 on the PR changes as well? -- 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
blueorangutan commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1910228840 Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8440 -- 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
GutoVeronezi commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1910217325 -1 (binding) @vishesh92 this PR does not fix #8506. The issue and the thread it mentions is related to a generic solution, which was not even discussed. -- 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
blueorangutan commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1910098591 @rohityadavcloud 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
rohityadavcloud commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1910096991 @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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
blueorangutan commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1909654668 [SF] Trillian test result (tid-8937) Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server r8 Total time taken: 53125 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8547-t8937-vmware-67u3.zip Smoke tests completed. 120 look OK, 1 have errors, 0 did not run Only failed and skipped tests results shown below: Test | Result | Time (s) | Test File --- | --- | --- | --- test_list_vms_metrics_history | `Error` | 15.12 | test_metrics_api.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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
blueorangutan commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1909645152 [SF] Trillian test result (tid-8938) Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7 Total time taken: 52708 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8547-t8938-kvm-centos7.zip Smoke tests completed. 120 look OK, 1 have errors, 0 did not run Only failed and skipped tests results shown below: Test | Result | Time (s) | Test File --- | --- | --- | --- test_08_upgrade_kubernetes_ha_cluster | `Failure` | 940.82 | test_kubernetes_clusters.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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
blueorangutan commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1909627594 @weizhouapache a [SL] Trillian-Jenkins test job (alma9 mgmt + kvm-alma9) 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
weizhouapache commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1909626532 @blueorangutan test alma9 kvm-alma9 -- 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
blueorangutan commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1909609349 @weizhouapache a [SL] Trillian-Jenkins test job (ubuntu22 mgmt + kvm-ubuntu22) 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
weizhouapache commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1909606981 @blueorangutan test ubuntu22 kvm-ubuntu22 -- 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
vishesh92 commented on code in PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#discussion_r1465880884 ## agent/conf/agent.properties: ## @@ -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 not used for every script execution. Review Comment: ```suggestion # This is currently used only while checking if the host supports UEFI. ``` -- 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
vishesh92 commented on code in PR #8547:
URL: https://github.com/apache/cloudstack/pull/8547#discussion_r1465880741
##
agent/src/main/java/com/cloud/agent/properties/AgentProperties.java:
##
@@ -659,6 +659,14 @@ public class AgentProperties{
*/
public static final Property STOP_SCRIPT_TIMEOUT = new
Property<>("stop.script.timeout", 120);
+/**
+ * Time (in seconds) to wait for scripts to complete.
+ * This is not used for every script execution.
Review Comment:
```suggestion
* This is currently used only while checking if the host supports
UEFI.
```
--
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-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
blueorangutan commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1909375195 [SF] Trillian test result (tid-8936) Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7 Total time taken: 41623 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8547-t8936-xenserver-71.zip Smoke tests completed. 120 look OK, 1 have errors, 0 did not run Only failed and skipped tests results shown below: Test | Result | Time (s) | Test File --- | --- | --- | --- test_02_trigger_shutdown | `Failure` | 341.90 | test_safe_shutdown.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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
blueorangutan commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1908590059 @vishesh92 a [SL] Trillian-Jenkins matrix job (centos7 mgmt + xenserver71, rocky8 mgmt + vmware67u3, centos7 mgmt + kvmcentos7) 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
vishesh92 commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1908587823 @blueorangutan test matrix -- 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
blueorangutan commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1908584147 Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8435 -- 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
blueorangutan commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1908462547 @vishesh92 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
vishesh92 commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1908460969 @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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
harikrishna-patnala commented on code in PR #8547:
URL: https://github.com/apache/cloudstack/pull/8547#discussion_r1464718607
##
agent/src/main/java/com/cloud/agent/properties/AgentProperties.java:
##
@@ -659,6 +659,14 @@ public class AgentProperties{
*/
public static final Property STOP_SCRIPT_TIMEOUT = new
Property<>("stop.script.timeout", 120);
+/**
+ * Time (in seconds) to wait for scripts to complete.
+ * This is not used for every script execution.
+ * Data type: Integer.
+ * Default value: 60
+ */
+public static final Property AGENT_SCRIPT_TIMEOUT = new
Property<>("agent.script.timeout", 60);
Review Comment:
yeah correct but that has to be known to the operator right, we normally put
them in the properties file and comment them
eg.
# Instance Conversion from Vmware to KVM through virt-v2v. Enable verbose
mode
# virtv2v.verbose.enabled=false
##
agent/src/main/java/com/cloud/agent/properties/AgentProperties.java:
##
@@ -659,6 +659,14 @@ public class AgentProperties{
*/
public static final Property STOP_SCRIPT_TIMEOUT = new
Property<>("stop.script.timeout", 120);
+/**
+ * Time (in seconds) to wait for scripts to complete.
+ * This is not used for every script execution.
+ * Data type: Integer.
+ * Default value: 60
+ */
+public static final Property AGENT_SCRIPT_TIMEOUT = new
Property<>("agent.script.timeout", 60);
Review Comment:
yeah correct but that has to be known to the operator right, we normally put
them in the properties file and comment them
eg.
```
# Instance Conversion from Vmware to KVM through virt-v2v. Enable verbose
mode
# virtv2v.verbose.enabled=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-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
vishesh92 commented on code in PR #8547:
URL: https://github.com/apache/cloudstack/pull/8547#discussion_r1464709838
##
agent/src/main/java/com/cloud/agent/properties/AgentProperties.java:
##
@@ -659,6 +659,14 @@ public class AgentProperties{
*/
public static final Property STOP_SCRIPT_TIMEOUT = new
Property<>("stop.script.timeout", 120);
+/**
+ * Time (in seconds) to wait for scripts to complete.
+ * This is not used for every script execution.
+ * Data type: Integer.
+ * Default value: 60
+ */
+public static final Property AGENT_SCRIPT_TIMEOUT = new
Property<>("agent.script.timeout", 60);
Review Comment:
We don't need to. If the user adds it in the file, it will get picked up.
--
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-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
blueorangutan commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1907824569 Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8430 -- 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
harikrishna-patnala commented on code in PR #8547:
URL: https://github.com/apache/cloudstack/pull/8547#discussion_r1464654622
##
agent/src/main/java/com/cloud/agent/properties/AgentProperties.java:
##
@@ -659,6 +659,14 @@ public class AgentProperties{
*/
public static final Property STOP_SCRIPT_TIMEOUT = new
Property<>("stop.script.timeout", 120);
+/**
+ * Time (in seconds) to wait for scripts to complete.
+ * This is not used for every script execution.
+ * Data type: Integer.
+ * Default value: 60
+ */
+public static final Property AGENT_SCRIPT_TIMEOUT = new
Property<>("agent.script.timeout", 60);
Review Comment:
@vishesh92 we need to add this in agent.properties to make it configurable
--
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-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
sureshanaparti commented on code in PR #8547:
URL: https://github.com/apache/cloudstack/pull/8547#discussion_r1464645992
##
engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java:
##
@@ -121,7 +122,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(Wait.value() / 5);
+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() / 5));
Review Comment:
```suggestion
hostId, poolId, cmd.getWait()));
```
--
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-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
sureshanaparti commented on code in PR #8547:
URL: https://github.com/apache/cloudstack/pull/8547#discussion_r1464610998
##
engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java:
##
@@ -121,7 +122,7 @@ 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(Wait.value()/5);
Review Comment:
```suggestion
cmd.setWait(Wait.value() / 5);
```
--
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-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
shwstppr commented on code in PR #8547:
URL: https://github.com/apache/cloudstack/pull/8547#discussion_r1464583734
##
engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java:
##
@@ -121,7 +122,7 @@ 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(Wait.value()/5);
Review Comment:
@vishesh92 do we need to add some logging here?
--
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-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
blueorangutan commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1907705902 [SF] Trillian test result (tid-8924) Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server r8 Total time taken: 48332 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8547-t8924-vmware-67u3.zip Smoke tests completed. 120 look OK, 1 have errors, 0 did not run Only failed and skipped tests results shown below: Test | Result | Time (s) | Test File --- | --- | --- | --- test_02_balanced_drs_algorithm | `Failure` | 135.16 | test_cluster_drs.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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
blueorangutan commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1907700139 @vishesh92 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
vishesh92 commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1907698821 @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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
blueorangutan commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1907692851 [SF] Trillian test result (tid-8923) Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7 Total time taken: 48002 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8547-t8923-xenserver-71.zip Smoke tests completed. 120 look OK, 1 have errors, 0 did not run Only failed and skipped tests results shown below: Test | Result | Time (s) | Test File --- | --- | --- | --- test_08_upgrade_kubernetes_ha_cluster | `Failure` | 681.44 | test_kubernetes_clusters.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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
blueorangutan commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1907670272 [SF] Trillian test result (tid-8925) Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7 Total time taken: 47048 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8547-t8925-kvm-centos7.zip Smoke tests completed. 121 look OK, 0 have errors, 0 did not run Only failed and skipped tests results shown below: Test | Result | Time (s) | Test File --- | --- | --- | --- -- 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
vishesh92 commented on code in PR #8547:
URL: https://github.com/apache/cloudstack/pull/8547#discussion_r1464515044
##
engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java:
##
@@ -121,7 +135,7 @@ 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.value());
Review Comment:
Use (wait.timeout / 6) here.
--
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-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
vishesh92 commented on code in PR #8547:
URL: https://github.com/apache/cloudstack/pull/8547#discussion_r1464511864
##
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtReadyCommandWrapper.java:
##
@@ -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.LIBVIRT_HOST_UEFI_SCRIPT_TIMEOUT)
* 1000; // Get property value & convert to milliseconds
Review Comment:
Add a common timeout for all scripts here.
--
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-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
weizhouapache commented on code in PR #8547:
URL: https://github.com/apache/cloudstack/pull/8547#discussion_r1463889096
##
engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java:
##
@@ -55,8 +57,12 @@
import javax.inject.Inject;
import java.util.List;
-public class DefaultHostListener implements HypervisorHostListener {
+public class DefaultHostListener implements HypervisorHostListener,
Configurable {
private static final Logger s_logger =
Logger.getLogger(DefaultHostListener.class);
+ConfigKey ModifyStoragePoolCommandWait = new
ConfigKey("Advanced", Integer.class,
+"modify.storage.pool.command.wait", "60",
+"Time in seconds to wait for ModifyStoragePoolCommand command to
return", true);
Review Comment:
What about
agent.startup.command.timeout
agent.startup.script.timeout
?
--
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-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
blueorangutan commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1906776782 @vishesh92 a [SL] Trillian-Jenkins matrix job (centos7 mgmt + xenserver71, rocky8 mgmt + vmware67u3, centos7 mgmt + kvmcentos7) 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
vishesh92 commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1906771640 @blueorangutan test matrix -- 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
vishesh92 commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1906770531 @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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
blueorangutan commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1906398026 @vishesh92 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
vishesh92 commented on code in PR #8547:
URL: https://github.com/apache/cloudstack/pull/8547#discussion_r1463528354
##
engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java:
##
@@ -55,8 +57,12 @@
import javax.inject.Inject;
import java.util.List;
-public class DefaultHostListener implements HypervisorHostListener {
+public class DefaultHostListener implements HypervisorHostListener,
Configurable {
private static final Logger s_logger =
Logger.getLogger(DefaultHostListener.class);
+ConfigKey ModifyStoragePoolCommandWait = new
ConfigKey("Advanced", Integer.class,
+"modify.storage.pool.command.wait", "60",
+"Time in seconds to wait for ModifyStoragePoolCommand command to
return", true);
Review Comment:
I agree with you. But generic config might not work. e.g.
`storage.command.wait` as 60 seconds will work for `ModifyStoragePoolCommand`
but not might be appropriate for other storage related commands. It needs to be
specific to a `Command`.
IMO, it would be better to have something which allows the user to set
configuration if required like `command..wait` and wait is set
only if the value is set. If it's not set, then some default value is used.
--
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-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
vishesh92 commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1906394104 @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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
sureshanaparti commented on code in PR #8547:
URL: https://github.com/apache/cloudstack/pull/8547#discussion_r1463275271
##
engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java:
##
@@ -55,8 +57,12 @@
import javax.inject.Inject;
import java.util.List;
-public class DefaultHostListener implements HypervisorHostListener {
+public class DefaultHostListener implements HypervisorHostListener,
Configurable {
private static final Logger s_logger =
Logger.getLogger(DefaultHostListener.class);
+ConfigKey ModifyStoragePoolCommandWait = new
ConfigKey("Advanced", Integer.class,
+"modify.storage.pool.command.wait", "60",
+"Time in seconds to wait for ModifyStoragePoolCommand command to
return", true);
Review Comment:
CS have lot of such cmds, maybe not good idea to add a config at cmd level.
(later, might end up one config for each cmd). generic configs, eg.
storage.command.wait / host.command.wait, based on the category with
appropriate descriptions might be good.
--
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-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
weizhouapache commented on code in PR #8547:
URL: https://github.com/apache/cloudstack/pull/8547#discussion_r1463264662
##
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtReadyCommandWrapper.java:
##
@@ -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.LIBVIRT_HOST_UEFI_SCRIPT_TIMEOUT)
* 1000; // Get property value & convert to milliseconds
if (isUbuntuHost) {
cmd = "dpkg -l ovmf";
}
s_logger.debug("Running command : " + cmd);
Review Comment:
add timeout value to debug message ?
--
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-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
vishesh92 commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1906020407 @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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
blueorangutan commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1906004990 @vishesh92 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Externalise a few timeouts & fix timeout for hostSupportsUefi in libvirt ready command wrapper [cloudstack]
vishesh92 commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1906002371 @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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
