You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by abhinandanprateek <gi...@git.apache.org> on 2016/09/20 07:34:40 UTC
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
GitHub user abhinandanprateek opened a pull request:
https://github.com/apache/cloudstack/pull/1678
CLOUDSTACK-9503: Increased the VR script timeout. Most of the changes\u2026
\u2026 are about converting int/long time values to Duration.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/shapeblue/cloudstack vr_timeout
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1678.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1678
----
commit e90511dfb7ee2cdfdd4981e2fed59a5bef8773f4
Author: Abhinandan Prateek <ap...@apache.org>
Date: 2016-09-20T07:30:46Z
CLOUDSTACK-9503: Increased the VR script timeout. Most of the changes are about converting int/long time values to Duration.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by abhinandanprateek <gi...@git.apache.org>.
Github user abhinandanprateek commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80207993
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java ---
@@ -1319,7 +1320,7 @@ public Answer executeRequest(final Command cmd) {
public synchronized boolean destroyTunnelNetwork(final String bridge) {
findOrCreateTunnelNetwork(bridge);
- final Script cmd = new Script(_ovsTunnelPath, _timeout, s_logger);
+ final Script cmd = new Script(_ovsTunnelPath, _timeout.getMillis(), s_logger);
--- End diff --
Scripts is an external package, hope they will upgrade to Duration after Java 1.8.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80064319
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java ---
@@ -3229,7 +3230,7 @@ protected boolean post_default_network_rules(final Connect conn, final String vm
final String brname = intf.getBrName();
final String vif = intf.getDevName();
- final Script cmd = new Script(_securityGroupPath, _timeout, s_logger);
+ final Script cmd = new Script(_securityGroupPath, _timeout.getMillis(), s_logger);
--- End diff --
Please consider adding an overridden version of the `Script(String, int, Logger)` constructor that accepts `Duration` to encapsulate this type conversion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1678: CLOUDSTACK-9503: Increased the VR script timeout. Mo...
Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:
https://github.com/apache/cloudstack/pull/1678
Packaging result: \u2716centos6 \u2716centos7 \u2716debian. JID-8
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80064208
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java ---
@@ -2377,7 +2378,7 @@ public Type getType() {
}
private Map<String, String> getVersionStrings() {
- final Script command = new Script(_versionstringpath, _timeout, s_logger);
+ final Script command = new Script(_versionstringpath, _timeout.getMillis(), s_logger);
--- End diff --
Please consider adding an overridden version of the `Script(String, int, Logger)` constructor that accepts `Duration` to encapsulate this type conversion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1678: CLOUDSTACK-9503: Increased the VR script timeout. Mo...
Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:
https://github.com/apache/cloudstack/pull/1678
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by abhinandanprateek <gi...@git.apache.org>.
Github user abhinandanprateek commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80427974
--- Diff: core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java ---
@@ -374,9 +375,9 @@ private Answer execute(AggregationControlCommand cmd) {
FileConfigItem fileConfigItem = new FileConfigItem(VRScripts.CONFIG_CACHE_LOCATION, cfgFileName, sb.toString());
ScriptConfigItem scriptConfigItem = new ScriptConfigItem(VRScripts.VR_CFG, "-c " + VRScripts.CONFIG_CACHE_LOCATION + cfgFileName);
// 120s is the minimal timeout
- int timeout = answerCounts * _eachTimeout;
- if (timeout < 120) {
- timeout = 120;
+ Duration timeout = _eachTimeout.withDurationAdded(_eachTimeout.getStandardSeconds(), answerCounts);
--- End diff --
@jburwell the java Duration that has multipliedBy is available from Java 8. Right now we are using joda.time.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by abhinandanprateek <gi...@git.apache.org>.
Github user abhinandanprateek commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80404569
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPvlanSetupCommandWrapper.java ---
@@ -87,7 +88,7 @@ public Answer execute(final PvlanSetupCommand command, final LibvirtComputingRes
} else if (command.getType() == PvlanSetupCommand.Type.VM) {
final String ovsPvlanVmPath = libvirtComputingResource.getOvsPvlanVmPath();
- final Script script = new Script(ovsPvlanVmPath, timeout, s_logger);
+ final Script script = new Script(ovsPvlanVmPath, timeout.getMillis(), s_logger);
--- End diff --
@jburwell yes you are right, we should modify the script class to take duration as timeout instead of long.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80064597
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtOvsCreateTunnelCommandWrapper.java ---
@@ -40,14 +40,11 @@ public Answer execute(final OvsCreateTunnelCommand command, final LibvirtComputi
try {
if (!libvirtComputingResource.findOrCreateTunnelNetwork(bridge)) {
s_logger.debug("Error during bridge setup");
- return new OvsCreateTunnelAnswer(command, false,
- "Cannot create network", bridge);
+ return new OvsCreateTunnelAnswer(command, false, "Cannot create network", bridge);
}
- libvirtComputingResource.configureTunnelNetwork(command.getNetworkId(), command.getFrom(),
- command.getNetworkName());
-
- final Script scriptCommand = new Script(libvirtComputingResource.getOvsTunnelPath(), libvirtComputingResource.getTimeout(), s_logger);
+ libvirtComputingResource.configureTunnelNetwork(command.getNetworkId(), command.getFrom(), command.getNetworkName());
+ final Script scriptCommand = new Script(libvirtComputingResource.getOvsTunnelPath(), libvirtComputingResource.getTimeout().getMillis(), s_logger);
--- End diff --
Please consider adding an overridden version of the `Script(String, int, Logger)` constructor that accepts `Duration` to encapsulate this type conversion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80065456
--- Diff: plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java ---
@@ -1265,11 +1266,11 @@ private ExecutionResult prepareNetworkElementCommand(IpAssocCommand cmd) {
@Override
public ExecutionResult executeInVR(String routerIP, String script, String args) {
- return executeInVR(routerIP, script, args, 120);
+ return executeInVR(routerIP, script, args, Duration.standardSeconds(120L));
--- End diff --
Please consider extracting ``Duration.standardSeconds(120L)`` to a constant to clarify intent
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by abhinandanprateek <gi...@git.apache.org>.
Github user abhinandanprateek commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80208194
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtModifySshKeysCommandWrapper.java ---
@@ -108,7 +108,7 @@ public Answer execute(final ModifySshKeysCommand command, final LibvirtComputing
result = "Write file " + sshprvkeypath + ":" + e.toString();
s_logger.debug(result);
}
- final Script script = new Script("chmod", libvirtComputingResource.getTimeout(), s_logger);
+ final Script script = new Script("chmod", libvirtComputingResource.getTimeout().getMillis(), s_logger);
--- End diff --
Scripts is an external package, hope they will upgrade to Duration after Java 1.8.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80064254
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java ---
@@ -3150,7 +3151,7 @@ protected long getMemoryFreeInKBs(Domain dm) throws LibvirtException {
}
private boolean canBridgeFirewall(final String prvNic) {
- final Script cmd = new Script(_securityGroupPath, _timeout, s_logger);
+ final Script cmd = new Script(_securityGroupPath, _timeout.getMillis(), s_logger);
--- End diff --
Please consider adding an overridden version of the `Script(String, int, Logger)` constructor that accepts `Duration` to encapsulate this type conversion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1678: CLOUDSTACK-9503: Increased the VR script timeout. Mo...
Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:
https://github.com/apache/cloudstack/pull/1678
Packaging result: \u2714centos6 \u2714centos7 \u2714debian. JID-84
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80064278
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java ---
@@ -3170,7 +3171,7 @@ public boolean destroyNetworkRulesForVM(final Connect conn, final String vmName)
final InterfaceDef intf = intfs.get(0);
vif = intf.getDevName();
}
- final Script cmd = new Script(_securityGroupPath, _timeout, s_logger);
+ final Script cmd = new Script(_securityGroupPath, _timeout.getMillis(), s_logger);
--- End diff --
Please consider adding an overridden version of the `Script(String, int, Logger)` constructor that accepts `Duration` to encapsulate this type conversion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80064404
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java ---
@@ -3317,7 +3318,7 @@ public boolean cleanupRules() {
if (!_canBridgeFirewall) {
return false;
}
- final Script cmd = new Script(_securityGroupPath, _timeout, s_logger);
+ final Script cmd = new Script(_securityGroupPath, _timeout.getMillis(), s_logger);
--- End diff --
Please consider adding an overridden version of the `Script(String, int, Logger)` constructor that accepts `Duration` to encapsulate this type conversion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80064380
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java ---
@@ -3300,7 +3301,7 @@ public boolean configureNetworkRulesVMSecondaryIP(final Connect conn, final Stri
return false;
}
- final Script cmd = new Script(_securityGroupPath, _timeout, s_logger);
+ final Script cmd = new Script(_securityGroupPath, _timeout.getMillis(), s_logger);
--- End diff --
Please consider adding an overridden version of the `Script(String, int, Logger)` constructor that accepts `Duration` to encapsulate this type conversion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by abhinandanprateek <gi...@git.apache.org>.
Github user abhinandanprateek commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80208151
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java ---
@@ -3300,7 +3301,7 @@ public boolean configureNetworkRulesVMSecondaryIP(final Connect conn, final Stri
return false;
}
- final Script cmd = new Script(_securityGroupPath, _timeout, s_logger);
+ final Script cmd = new Script(_securityGroupPath, _timeout.getMillis(), s_logger);
--- End diff --
Scripts is an external package, hope they will upgrade to Duration after Java 1.8.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80063605
--- Diff: plugins/hypervisors/hyperv/src/com/cloud/hypervisor/hyperv/resource/HypervDirectConnectResource.java ---
@@ -636,7 +637,7 @@ public ExecutionResult executeInVR(final String routerIP, final String script, f
try {
result = SshHelper.sshExecute(routerIP, DEFAULT_DOMR_SSHPORT, "root", getSystemVMKeyFile(), null, "/opt/cloud/bin/" + script + " " + args,
- 60000, 60000, timeout * 1000);
+ 60000, 60000, (int)timeout.getMillis());
--- End diff --
Please consider defining an override of the ``SshHelper.sshExecute`` method that accepts a ``Duration`` in order to encapsulate this conversion. We should also consider deprecating the version that accepts ``int``.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1678: CLOUDSTACK-9503: Increased the VR script timeout. Mo...
Posted by abhinandanprateek <gi...@git.apache.org>.
Github user abhinandanprateek commented on the issue:
https://github.com/apache/cloudstack/pull/1678
@jburwell updated Script to use Duration as timeout.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80065874
--- Diff: plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java ---
@@ -1660,18 +1661,18 @@ public StopAnswer execute(final StopCommand cmd) {
@Override
public ExecutionResult executeInVR(final String routerIP, final String script, final String args) {
// Timeout is 120 seconds by default
- return executeInVR(routerIP, script, args, 120);
+ return executeInVR(routerIP, script, args, Duration.standardSeconds(120L));
}
@Override
- public ExecutionResult executeInVR(final String routerIP, final String script, final String args, final int timeout) {
+ public ExecutionResult executeInVR(final String routerIP, final String script, final String args, final Duration timeout) {
Pair<Boolean, String> result;
String cmdline = "/opt/cloud/bin/router_proxy.sh " + script + " " + routerIP + " " + args;
// semicolon need to be escape for bash
cmdline = cmdline.replaceAll(";", "\\\\;");
try {
s_logger.debug("Executing command in VR: " + cmdline);
- result = SshHelper.sshExecute(_host.getIp(), 22, _username, null, _password.peek(), cmdline, 60000, 60000, timeout * 1000);
+ result = SshHelper.sshExecute(_host.getIp(), 22, _username, null, _password.peek(), cmdline, 60000, 60000, (int)timeout.getMillis());
--- End diff --
Please consider adding an overridden version of the `SshHelper.sshExecute(String, int, String, String, ??, String, int, int, int)` method that accepts `Duration` to encapsulate this type conversion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by abhinandanprateek <gi...@git.apache.org>.
Github user abhinandanprateek commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80208171
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java ---
@@ -3327,7 +3328,7 @@ public boolean cleanupRules() {
}
public String getRuleLogsForVms() {
- final Script cmd = new Script(_securityGroupPath, _timeout, s_logger);
+ final Script cmd = new Script(_securityGroupPath, _timeout.getMillis(), s_logger);
--- End diff --
Scripts is an external package, hope they will upgrade to Duration after Java 1.8.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80061842
--- Diff: core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java ---
@@ -374,9 +375,9 @@ private Answer execute(AggregationControlCommand cmd) {
FileConfigItem fileConfigItem = new FileConfigItem(VRScripts.CONFIG_CACHE_LOCATION, cfgFileName, sb.toString());
ScriptConfigItem scriptConfigItem = new ScriptConfigItem(VRScripts.VR_CFG, "-c " + VRScripts.CONFIG_CACHE_LOCATION + cfgFileName);
// 120s is the minimal timeout
- int timeout = answerCounts * _eachTimeout;
- if (timeout < 120) {
- timeout = 120;
+ Duration timeout = Duration.standardSeconds(answerCounts * _eachTimeout.getStandardSeconds());
--- End diff --
To avoid any potential unit conversion issues, consider using the ``Duration. multipliedBy(int)` method.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1678: CLOUDSTACK-9503: Increased the VR script timeout. Mo...
Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:
https://github.com/apache/cloudstack/pull/1678
@jburwell a Trillian-Jenkins test job (centos6 mgmt + kvm) has been kicked to run smoke tests
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by abhinandanprateek <gi...@git.apache.org>.
Github user abhinandanprateek commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80208027
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java ---
@@ -2377,7 +2378,7 @@ public Type getType() {
}
private Map<String, String> getVersionStrings() {
- final Script command = new Script(_versionstringpath, _timeout, s_logger);
+ final Script command = new Script(_versionstringpath, _timeout.getMillis(), s_logger);
--- End diff --
Scripts is an external package, hope they will upgrade to Duration after Java 1.8.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80064103
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java ---
@@ -1226,7 +1227,7 @@ private boolean checkOvsNetwork(final String networkName) {
return true;
}
- final Script command = new Script("/bin/sh", _timeout);
+ final Script command = new Script("/bin/sh", _timeout.getMillis());
--- End diff --
Please consider adding an overridden version of the `Script(String, int)` constructor that accepts `Duration` to encapsulate this type conversion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by abhinandanprateek <gi...@git.apache.org>.
Github user abhinandanprateek commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80208251
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtOvsVpcPhysicalTopologyConfigCommandWrapper.java ---
@@ -36,7 +36,7 @@
@Override
public Answer execute(final OvsVpcPhysicalTopologyConfigCommand command, final LibvirtComputingResource libvirtComputingResource) {
try {
- final Script scriptCommand = new Script(libvirtComputingResource.getOvsTunnelPath(), libvirtComputingResource.getTimeout(), s_logger);
+ final Script scriptCommand = new Script(libvirtComputingResource.getOvsTunnelPath(), libvirtComputingResource.getTimeout().getMillis(), s_logger);
--- End diff --
Scripts is an external package, hope they will upgrade to Duration after Java 1.8.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80065801
--- Diff: plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java ---
@@ -1280,7 +1281,7 @@ public ExecutionResult executeInVR(String routerIP, String script, String args,
try {
VmwareManager mgr = getServiceContext().getStockObject(VmwareManager.CONTEXT_STOCK_NAME);
result = SshHelper.sshExecute(routerIP, DefaultDomRSshPort, "root", mgr.getSystemVMKeyFile(), null, "/opt/cloud/bin/" + script + " " + args,
- 60000, 60000, timeout * 1000);
+ 60000, 60000, (int)timeout.getMillis());
--- End diff --
Please consider adding an overridden version of the `SshHelper.sshExecute(String, int, String, String, ??, String, int, int, int)` method that accepts `Duration` to encapsulate this type conversion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by abhinandanprateek <gi...@git.apache.org>.
Github user abhinandanprateek commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80208062
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java ---
@@ -3150,7 +3151,7 @@ protected long getMemoryFreeInKBs(Domain dm) throws LibvirtException {
}
private boolean canBridgeFirewall(final String prvNic) {
- final Script cmd = new Script(_securityGroupPath, _timeout, s_logger);
+ final Script cmd = new Script(_securityGroupPath, _timeout.getMillis(), s_logger);
--- End diff --
Scripts is an external package, hope they will upgrade to Duration after Java 1.8.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80063935
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java ---
@@ -279,12 +280,12 @@
@Override
public ExecutionResult executeInVR(final String routerIp, final String script, final String args) {
- return executeInVR(routerIp, script, args, _timeout / 1000);
+ return executeInVR(routerIp, script, args, _timeout);
}
@Override
- public ExecutionResult executeInVR(final String routerIp, final String script, final String args, final int timeout) {
- final Script command = new Script(_routerProxyPath, timeout * 1000, s_logger);
+ public ExecutionResult executeInVR(final String routerIp, final String script, final String args, final Duration milli) {
+ final Script command = new Script(_routerProxyPath, milli.getMillis(), s_logger);
--- End diff --
Please consider adding an overridden version of the `Script(String, int, Logger)` constructor that accepts a `Duration` instance to encapsulate this conversion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80064432
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java ---
@@ -3327,7 +3328,7 @@ public boolean cleanupRules() {
}
public String getRuleLogsForVms() {
- final Script cmd = new Script(_securityGroupPath, _timeout, s_logger);
+ final Script cmd = new Script(_securityGroupPath, _timeout.getMillis(), s_logger);
--- End diff --
Please consider adding an overridden version of the `Script(String, int, Logger)` constructor that accepts `Duration` to encapsulate this type conversion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80064464
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtModifySshKeysCommandWrapper.java ---
@@ -52,7 +52,7 @@ public Answer execute(final ModifySshKeysCommand command, final LibvirtComputing
String result = null;
if (!sshKeysDir.exists()) {
// Change permissions for the 700
- final Script script = new Script("mkdir", libvirtComputingResource.getTimeout(), s_logger);
+ final Script script = new Script("mkdir", libvirtComputingResource.getTimeout().getMillis(), s_logger);
--- End diff --
Please consider adding an overridden version of the `Script(String, int, Logger)` constructor that accepts `Duration` to encapsulate this type conversion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1678: CLOUDSTACK-9503: Increased the VR script timeout. Mo...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on the issue:
https://github.com/apache/cloudstack/pull/1678
@abhinandanprateek it appears that there is a Findbugs issue that needs to be addressed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80064364
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java ---
@@ -3273,7 +3274,7 @@ public boolean addNetworkRules(final String vmName, final String vmId, final Str
}
final String newRules = rules.replace(" ", ";");
- final Script cmd = new Script(_securityGroupPath, _timeout, s_logger);
+ final Script cmd = new Script(_securityGroupPath, _timeout.getMillis(), s_logger);
--- End diff --
Please consider adding an overridden version of the `Script(String, int, Logger)` constructor that accepts `Duration` to encapsulate this type conversion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80064168
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java ---
@@ -1319,7 +1320,7 @@ public Answer executeRequest(final Command cmd) {
public synchronized boolean destroyTunnelNetwork(final String bridge) {
findOrCreateTunnelNetwork(bridge);
- final Script cmd = new Script(_ovsTunnelPath, _timeout, s_logger);
+ final Script cmd = new Script(_ovsTunnelPath, _timeout.getMillis(), s_logger);
--- End diff --
Please consider adding an overridden version of the `Script(String, int, Logger)` constructor that accepts `Duration` to encapsulate this type conversion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1678: CLOUDSTACK-9503: Increased the VR script timeout. Mo...
Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1678
@abhinandanprateek please fix noredist/vmware issue:
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.2:compile (default-compile) on project cloud-plugin-hypervisor-vmware: Compilation failure: Compilation failure:
[ERROR] /jenkins/workspace/acs-pr-centos6-pkg-builder/dist/rpmbuild/BUILD/cloudstack-4.9.1.0-SNAPSHOT/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java:[1269,15] error: no suitable method found for executeInVR(String,String,String,int)
[ERROR] method VmwareResource.executeInVR(String,String,String,Duration) is not applicable
[ERROR] (actual argument int cannot be converted to Duration by method invocation conversion)
[ERROR] method VmwareResource.executeInVR(String,String,String) is not applicable
[ERROR] (actual and formal argument lists differ in length)
[ERROR] /jenkins/workspace/acs-pr-centos6-pkg-builder/dist/rpmbuild/BUILD/cloudstack-4.9.1.0-SNAPSHOT/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java:[1284,41] error: cannot find symbol
[ERROR] -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException
[ERROR]
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR] mvn <goals> -rf :cloud-plugin-hypervisor-vmware
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1678: CLOUDSTACK-9503: Increased the VR script timeout. Mo...
Posted by abhinandanprateek <gi...@git.apache.org>.
Github user abhinandanprateek commented on the issue:
https://github.com/apache/cloudstack/pull/1678
@rhtyd fixed it, not sure why that was not caught by jenkins build.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1678: CLOUDSTACK-9503: Increased the VR script timeout. Mo...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on the issue:
https://github.com/apache/cloudstack/pull/1678
@abhinandanprateek how difficult would it be to change the base branch of this PR to 4.8?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by abhinandanprateek <gi...@git.apache.org>.
Github user abhinandanprateek commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80208208
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtOvsCreateTunnelCommandWrapper.java ---
@@ -40,14 +40,11 @@ public Answer execute(final OvsCreateTunnelCommand command, final LibvirtComputi
try {
if (!libvirtComputingResource.findOrCreateTunnelNetwork(bridge)) {
s_logger.debug("Error during bridge setup");
- return new OvsCreateTunnelAnswer(command, false,
- "Cannot create network", bridge);
+ return new OvsCreateTunnelAnswer(command, false, "Cannot create network", bridge);
}
- libvirtComputingResource.configureTunnelNetwork(command.getNetworkId(), command.getFrom(),
- command.getNetworkName());
-
- final Script scriptCommand = new Script(libvirtComputingResource.getOvsTunnelPath(), libvirtComputingResource.getTimeout(), s_logger);
+ libvirtComputingResource.configureTunnelNetwork(command.getNetworkId(), command.getFrom(), command.getNetworkName());
+ final Script scriptCommand = new Script(libvirtComputingResource.getOvsTunnelPath(), libvirtComputingResource.getTimeout().getMillis(), s_logger);
--- End diff --
Scripts is an external package, hope they will upgrade to Duration after Java 1.8.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80064777
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtOvsVpcPhysicalTopologyConfigCommandWrapper.java ---
@@ -36,7 +36,7 @@
@Override
public Answer execute(final OvsVpcPhysicalTopologyConfigCommand command, final LibvirtComputingResource libvirtComputingResource) {
try {
- final Script scriptCommand = new Script(libvirtComputingResource.getOvsTunnelPath(), libvirtComputingResource.getTimeout(), s_logger);
+ final Script scriptCommand = new Script(libvirtComputingResource.getOvsTunnelPath(), libvirtComputingResource.getTimeout().getMillis(), s_logger);
--- End diff --
Please consider adding an overridden version of the `Script(String, int, Logger)` constructor that accepts `Duration` to encapsulate this type conversion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by abhinandanprateek <gi...@git.apache.org>.
Github user abhinandanprateek commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80208159
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java ---
@@ -3317,7 +3318,7 @@ public boolean cleanupRules() {
if (!_canBridgeFirewall) {
return false;
}
- final Script cmd = new Script(_securityGroupPath, _timeout, s_logger);
+ final Script cmd = new Script(_securityGroupPath, _timeout.getMillis(), s_logger);
--- End diff --
Scripts is an external package, hope they will upgrade to Duration after Java 1.8.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80065183
--- Diff: plugins/hypervisors/ovm3/src/main/java/com/cloud/hypervisor/ovm3/resources/Ovm3VirtualRoutingResource.java ---
@@ -38,7 +40,7 @@
private final Logger logger = Logger
.getLogger(Ovm3VirtualRoutingResource.class);
private String domRCloudPath = "/opt/cloud/bin/";
- private int vrTimeout = 600;
+ private Duration vrTimeout = Duration.standardSeconds(600L);
--- End diff --
Please consider extracting ``Duration.standardSeconds(600L)`` to constant to clarify the intent.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by abhinandanprateek <gi...@git.apache.org>.
Github user abhinandanprateek commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80208042
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java ---
@@ -2973,7 +2974,7 @@ public Domain getDomain(final Connect conn, final String vmName) throws LibvirtE
}
private String executeBashScript(final String script) {
- final Script command = new Script("/bin/bash", _timeout, s_logger);
+ final Script command = new Script("/bin/bash", _timeout.getMillis(), s_logger);
--- End diff --
Scripts is an external package, hope they will upgrade to Duration after Java 1.8.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1678: CLOUDSTACK-9503: Increased the VR script timeout. Mo...
Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1678
@blueorangutan package
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1678: CLOUDSTACK-9503: Increased the VR script timeout. Mo...
Posted by abhinandanprateek <gi...@git.apache.org>.
Github user abhinandanprateek commented on the issue:
https://github.com/apache/cloudstack/pull/1678
@jburwell there was some issue with jenkins, force pushing PR resolved it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80064540
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtModifySshKeysCommandWrapper.java ---
@@ -108,7 +108,7 @@ public Answer execute(final ModifySshKeysCommand command, final LibvirtComputing
result = "Write file " + sshprvkeypath + ":" + e.toString();
s_logger.debug(result);
}
- final Script script = new Script("chmod", libvirtComputingResource.getTimeout(), s_logger);
+ final Script script = new Script("chmod", libvirtComputingResource.getTimeout().getMillis(), s_logger);
--- End diff --
Please consider adding an overridden version of the `Script(String, int, Logger)` constructor that accepts `Duration` to encapsulate this type conversion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1678: CLOUDSTACK-9503: Increased the VR script timeout. Mo...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on the issue:
https://github.com/apache/cloudstack/pull/1678
@abhinandanprateek this fix seems like a good fix for 4.8 users as well which I would like to get included in 4.8.2.0, as well as, 4.9.1.0 and 4.10.0.0. Could rebase this PR and change the base branch to 4.8?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by abhinandanprateek <gi...@git.apache.org>.
Github user abhinandanprateek commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80208282
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPvlanSetupCommandWrapper.java ---
@@ -58,11 +59,11 @@ public Answer execute(final PvlanSetupCommand command, final LibvirtComputingRes
String result = null;
try {
final String guestBridgeName = libvirtComputingResource.getGuestBridgeName();
- final int timeout = libvirtComputingResource.getTimeout();
+ final Duration timeout = libvirtComputingResource.getTimeout();
if (command.getType() == PvlanSetupCommand.Type.DHCP) {
final String ovsPvlanDhcpHostPath = libvirtComputingResource.getOvsPvlanDhcpHostPath();
- final Script script = new Script(ovsPvlanDhcpHostPath, timeout, s_logger);
+ final Script script = new Script(ovsPvlanDhcpHostPath, timeout.getMillis(), s_logger);
--- End diff --
Scripts is an external package, hope they will upgrade to Duration after Java 1.8.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80062899
--- Diff: core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java ---
@@ -374,9 +375,9 @@ private Answer execute(AggregationControlCommand cmd) {
FileConfigItem fileConfigItem = new FileConfigItem(VRScripts.CONFIG_CACHE_LOCATION, cfgFileName, sb.toString());
ScriptConfigItem scriptConfigItem = new ScriptConfigItem(VRScripts.VR_CFG, "-c " + VRScripts.CONFIG_CACHE_LOCATION + cfgFileName);
// 120s is the minimal timeout
- int timeout = answerCounts * _eachTimeout;
- if (timeout < 120) {
- timeout = 120;
+ Duration timeout = Duration.standardSeconds(answerCounts * _eachTimeout.getStandardSeconds());
+ if (timeout.getStandardSeconds() < 120L) {
+ timeout = Duration.standardSeconds(120L);
--- End diff --
Consider using the previously defined constant here for ensured consistency between he check and the value used.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80404113
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPvlanSetupCommandWrapper.java ---
@@ -87,7 +88,7 @@ public Answer execute(final PvlanSetupCommand command, final LibvirtComputingRes
} else if (command.getType() == PvlanSetupCommand.Type.VM) {
final String ovsPvlanVmPath = libvirtComputingResource.getOvsPvlanVmPath();
- final Script script = new Script(ovsPvlanVmPath, timeout, s_logger);
+ final Script script = new Script(ovsPvlanVmPath, timeout.getMillis(), s_logger);
--- End diff --
@abhinandanprateek the `Script` class is in the `com.cloud.utils.script` which is part of the project. What prevents us from adding a constructor that class?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80408774
--- Diff: core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java ---
@@ -374,9 +375,9 @@ private Answer execute(AggregationControlCommand cmd) {
FileConfigItem fileConfigItem = new FileConfigItem(VRScripts.CONFIG_CACHE_LOCATION, cfgFileName, sb.toString());
ScriptConfigItem scriptConfigItem = new ScriptConfigItem(VRScripts.VR_CFG, "-c " + VRScripts.CONFIG_CACHE_LOCATION + cfgFileName);
// 120s is the minimal timeout
- int timeout = answerCounts * _eachTimeout;
- if (timeout < 120) {
- timeout = 120;
+ Duration timeout = _eachTimeout.withDurationAdded(_eachTimeout.getStandardSeconds(), answerCounts);
--- End diff --
Why use `withDurationAdded` rather `multipliedBy`? Passing `_eachTimeout` into the method is an indication that `multipliedBy` may be a more concise expression of intent.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by abhinandanprateek <gi...@git.apache.org>.
Github user abhinandanprateek commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80207975
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java ---
@@ -1226,7 +1227,7 @@ private boolean checkOvsNetwork(final String networkName) {
return true;
}
- final Script command = new Script("/bin/sh", _timeout);
+ final Script command = new Script("/bin/sh", _timeout.getMillis());
--- End diff --
Scripts is an external package, hope they will upgrade to Duration after Java 1.8.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1678: CLOUDSTACK-9503: Increased the VR script timeout. Mo...
Posted by abhinandanprateek <gi...@git.apache.org>.
Github user abhinandanprateek commented on the issue:
https://github.com/apache/cloudstack/pull/1678
@rhtyd thanks for sharing nordist repo. I have sanitized the vmware plugin too.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by abhinandanprateek <gi...@git.apache.org>.
Github user abhinandanprateek commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80207999
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java ---
@@ -1373,7 +1374,7 @@ public synchronized boolean configureTunnelNetwork(final long networkId,
}
}
if (!configured) {
- final Script cmd = new Script(_ovsTunnelPath, _timeout, s_logger);
+ final Script cmd = new Script(_ovsTunnelPath, _timeout.getMillis(), s_logger);
--- End diff --
Scripts is an external package, hope they will upgrade to Duration after Java 1.8.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80064229
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java ---
@@ -2973,7 +2974,7 @@ public Domain getDomain(final Connect conn, final String vmName) throws LibvirtE
}
private String executeBashScript(final String script) {
- final Script command = new Script("/bin/bash", _timeout, s_logger);
+ final Script command = new Script("/bin/bash", _timeout.getMillis(), s_logger);
--- End diff --
Please consider adding an overridden version of the `Script(String, int, Logger)` constructor that accepts `Duration` to encapsulate this type conversion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by abhinandanprateek <gi...@git.apache.org>.
Github user abhinandanprateek commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80207941
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java ---
@@ -279,12 +280,12 @@
@Override
public ExecutionResult executeInVR(final String routerIp, final String script, final String args) {
- return executeInVR(routerIp, script, args, _timeout / 1000);
+ return executeInVR(routerIp, script, args, _timeout);
}
@Override
- public ExecutionResult executeInVR(final String routerIp, final String script, final String args, final int timeout) {
- final Script command = new Script(_routerProxyPath, timeout * 1000, s_logger);
+ public ExecutionResult executeInVR(final String routerIp, final String script, final String args, final Duration milli) {
+ final Script command = new Script(_routerProxyPath, milli.getMillis(), s_logger);
--- End diff --
Scripts is an external package, hope they will upgrade to Duration after Java 1.8.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by abhinandanprateek <gi...@git.apache.org>.
Github user abhinandanprateek commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80208106
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java ---
@@ -3229,7 +3230,7 @@ protected boolean post_default_network_rules(final Connect conn, final String vm
final String brname = intf.getBrName();
final String vif = intf.getDevName();
- final Script cmd = new Script(_securityGroupPath, _timeout, s_logger);
+ final Script cmd = new Script(_securityGroupPath, _timeout.getMillis(), s_logger);
--- End diff --
Scripts is an external package, hope they will upgrade to Duration after Java 1.8.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80064180
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java ---
@@ -1373,7 +1374,7 @@ public synchronized boolean configureTunnelNetwork(final long networkId,
}
}
if (!configured) {
- final Script cmd = new Script(_ovsTunnelPath, _timeout, s_logger);
+ final Script cmd = new Script(_ovsTunnelPath, _timeout.getMillis(), s_logger);
--- End diff --
Please consider adding an overridden version of the `Script(String, int, Logger)` constructor that accepts `Duration` to encapsulate this type conversion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1678: CLOUDSTACK-9503: Increased the VR script timeout. Mo...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on the issue:
https://github.com/apache/cloudstack/pull/1678
@abhinandanprateek could you please create a ticket to refactor `Script` to use `Duration` internally? Also, are there any component tests that need to be run in addition to smoke tests to verify this bug fix?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80064807
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtOvsVpcRoutingPolicyConfigCommandWrapper.java ---
@@ -36,7 +36,7 @@
@Override
public Answer execute(final OvsVpcRoutingPolicyConfigCommand command, final LibvirtComputingResource libvirtComputingResource) {
try {
- final Script scriptCommand = new Script(libvirtComputingResource.getOvsTunnelPath(), libvirtComputingResource.getTimeout(), s_logger);
+ final Script scriptCommand = new Script(libvirtComputingResource.getOvsTunnelPath(), libvirtComputingResource.getTimeout().getMillis(), s_logger);
--- End diff --
Please consider adding an overridden version of the `Script(String, int, Logger)` constructor that accepts `Duration` to encapsulate this type conversion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by abhinandanprateek <gi...@git.apache.org>.
Github user abhinandanprateek commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80208135
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java ---
@@ -3273,7 +3274,7 @@ public boolean addNetworkRules(final String vmName, final String vmId, final Str
}
final String newRules = rules.replace(" ", ";");
- final Script cmd = new Script(_securityGroupPath, _timeout, s_logger);
+ final Script cmd = new Script(_securityGroupPath, _timeout.getMillis(), s_logger);
--- End diff --
Scripts is an external package, hope they will upgrade to Duration after Java 1.8.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1678: CLOUDSTACK-9503: Increased the VR script timeout. Mo...
Posted by abhinandanprateek <gi...@git.apache.org>.
Github user abhinandanprateek commented on the issue:
https://github.com/apache/cloudstack/pull/1678
@jburwell ticket filed to refactor Script and SshHelper to use Duration. https://issues.apache.org/jira/browse/CLOUDSTACK-9508
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1678: CLOUDSTACK-9503: Increased the VR script timeout. Mo...
Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1678
@abhinandanprateek error:
[INFO] Compiling 51 source files to /jenkins/workspace/acs-pr-centos6-pkg-builder/dist/rpmbuild/BUILD/cloudstack-4.9.1.0-SNAPSHOT/plugins/hypervisors/vmware/target/classes
[INFO] -------------------------------------------------------------
[ERROR] COMPILATION ERROR :
[INFO] -------------------------------------------------------------
[ERROR] /jenkins/workspace/acs-pr-centos6-pkg-builder/dist/rpmbuild/BUILD/cloudstack-4.9.1.0-SNAPSHOT/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java:[1284,41] error: cannot find symbol
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1678: CLOUDSTACK-9503: Increased the VR script timeout. Mo...
Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:
https://github.com/apache/cloudstack/pull/1678
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1678: CLOUDSTACK-9503: Increased the VR script timeout. Mo...
Posted by abhinandanprateek <gi...@git.apache.org>.
Github user abhinandanprateek commented on the issue:
https://github.com/apache/cloudstack/pull/1678
@jburwell Created a new PR for 4.8 https://github.com/apache/cloudstack/pull/1745, closing this one.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by abhinandanprateek <gi...@git.apache.org>.
Github user abhinandanprateek commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80208295
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPvlanSetupCommandWrapper.java ---
@@ -87,7 +88,7 @@ public Answer execute(final PvlanSetupCommand command, final LibvirtComputingRes
} else if (command.getType() == PvlanSetupCommand.Type.VM) {
final String ovsPvlanVmPath = libvirtComputingResource.getOvsPvlanVmPath();
- final Script script = new Script(ovsPvlanVmPath, timeout, s_logger);
+ final Script script = new Script(ovsPvlanVmPath, timeout.getMillis(), s_logger);
--- End diff --
Scripts is an external package, hope they will upgrade to Duration after Java 1.8.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1678: CLOUDSTACK-9503: Increased the VR script timeout. Mo...
Posted by karuturi <gi...@git.apache.org>.
Github user karuturi commented on the issue:
https://github.com/apache/cloudstack/pull/1678
I think the build timed out.
> [INFO] --- findbugs-maven-plugin:3.0.3:findbugs (findbugs) @ cloud-plugin-network-bigswitch ---
> [INFO] Fork Value is true
> Build timed out (after 96 minutes). Marking the build as aborted.
> Build was aborted
>
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80064306
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java ---
@@ -3197,7 +3198,7 @@ public boolean defaultNetworkRules(final Connect conn, final String vmName, fina
final String brname = intf.getBrName();
final String vif = intf.getDevName();
- final Script cmd = new Script(_securityGroupPath, _timeout, s_logger);
+ final Script cmd = new Script(_securityGroupPath, _timeout.getMillis(), s_logger);
--- End diff --
Please consider adding an overridden version of the `Script(String, int, Logger)` constructor that accepts `Duration` to encapsulate this type conversion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1678: CLOUDSTACK-9503: Increased the VR script timeout. Mo...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on the issue:
https://github.com/apache/cloudstack/pull/1678
@blueorangutan test centos6 kvm
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by abhinandanprateek <gi...@git.apache.org>.
Github user abhinandanprateek commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80208231
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtOvsDestroyTunnelCommandWrapper.java ---
@@ -42,7 +42,7 @@ public Answer execute(final OvsDestroyTunnelCommand command, final LibvirtComput
return new Answer(command, false, "No network found");
}
- final Script scriptCommand = new Script(libvirtComputingResource.getOvsTunnelPath(), libvirtComputingResource.getTimeout(), s_logger);
+ final Script scriptCommand = new Script(libvirtComputingResource.getOvsTunnelPath(), libvirtComputingResource.getTimeout().getMillis(), s_logger);
--- End diff --
Scripts is an external package, hope they will upgrade to Duration after Java 1.8.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1678: CLOUDSTACK-9503: Increased the VR script timeout. Mo...
Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:
https://github.com/apache/cloudstack/pull/1678
Packaging result: \u2716centos6 \u2716centos7 \u2716debian. JID-12
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by abhinandanprateek <gi...@git.apache.org>.
Github user abhinandanprateek commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80208089
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java ---
@@ -3197,7 +3198,7 @@ public boolean defaultNetworkRules(final Connect conn, final String vmName, fina
final String brname = intf.getBrName();
final String vif = intf.getDevName();
- final Script cmd = new Script(_securityGroupPath, _timeout, s_logger);
+ final Script cmd = new Script(_securityGroupPath, _timeout.getMillis(), s_logger);
--- End diff --
Scripts is an external package, hope they will upgrade to Duration after Java 1.8.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by abhinandanprateek <gi...@git.apache.org>.
Github user abhinandanprateek commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80208121
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java ---
@@ -3255,7 +3256,7 @@ public boolean configureDefaultNetworkRulesForSystemVm(final Connect conn, final
return false;
}
- final Script cmd = new Script(_securityGroupPath, _timeout, s_logger);
+ final Script cmd = new Script(_securityGroupPath, _timeout.getMillis(), s_logger);
--- End diff --
Scripts is an external package, hope they will upgrade to Duration after Java 1.8.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80064873
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPvlanSetupCommandWrapper.java ---
@@ -58,11 +59,11 @@ public Answer execute(final PvlanSetupCommand command, final LibvirtComputingRes
String result = null;
try {
final String guestBridgeName = libvirtComputingResource.getGuestBridgeName();
- final int timeout = libvirtComputingResource.getTimeout();
+ final Duration timeout = libvirtComputingResource.getTimeout();
if (command.getType() == PvlanSetupCommand.Type.DHCP) {
final String ovsPvlanDhcpHostPath = libvirtComputingResource.getOvsPvlanDhcpHostPath();
- final Script script = new Script(ovsPvlanDhcpHostPath, timeout, s_logger);
+ final Script script = new Script(ovsPvlanDhcpHostPath, timeout.getMillis(), s_logger);
--- End diff --
Please consider adding an overridden version of the `Script(String, int, Logger)` constructor that accepts `Duration` to encapsulate this type conversion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by abhinandanprateek <gi...@git.apache.org>.
Github user abhinandanprateek commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80208075
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java ---
@@ -3170,7 +3171,7 @@ public boolean destroyNetworkRulesForVM(final Connect conn, final String vmName)
final InterfaceDef intf = intfs.get(0);
vif = intf.getDevName();
}
- final Script cmd = new Script(_securityGroupPath, _timeout, s_logger);
+ final Script cmd = new Script(_securityGroupPath, _timeout.getMillis(), s_logger);
--- End diff --
Scripts is an external package, hope they will upgrade to Duration after Java 1.8.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80062812
--- Diff: core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java ---
@@ -374,9 +375,9 @@ private Answer execute(AggregationControlCommand cmd) {
FileConfigItem fileConfigItem = new FileConfigItem(VRScripts.CONFIG_CACHE_LOCATION, cfgFileName, sb.toString());
ScriptConfigItem scriptConfigItem = new ScriptConfigItem(VRScripts.VR_CFG, "-c " + VRScripts.CONFIG_CACHE_LOCATION + cfgFileName);
// 120s is the minimal timeout
- int timeout = answerCounts * _eachTimeout;
- if (timeout < 120) {
- timeout = 120;
+ Duration timeout = Duration.standardSeconds(answerCounts * _eachTimeout.getStandardSeconds());
+ if (timeout.getStandardSeconds() < 120L) {
--- End diff --
Please consider extract ``120L`` to a constant of type ``Duration`` to intent more clear and provide further unit conversion safety (e.g. MINIMUM_VR_RESTART_TIMEOUT).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1678: CLOUDSTACK-9503: Increased the VR script timeout. Mo...
Posted by abhinandanprateek <gi...@git.apache.org>.
Github user abhinandanprateek commented on the issue:
https://github.com/apache/cloudstack/pull/1678
@karuturi Jenkins fail on find-bug: [ERROR] Failed to execute goal org.codehaus.mojo:findbugs-maven-plugin:3.0.3:findbugs (findbugs) on project cloud-plugin-network-bigswitch: Execution findbugs of goal org.codehaus.mojo:findbugs-maven-plugin:3.0.3:findbugs failed: Java returned: 143
I did not make any changes there, do you know if it is failing elsewhere too ? Or it is just some spurious failure.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80064346
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java ---
@@ -3255,7 +3256,7 @@ public boolean configureDefaultNetworkRulesForSystemVm(final Connect conn, final
return false;
}
- final Script cmd = new Script(_securityGroupPath, _timeout, s_logger);
+ final Script cmd = new Script(_securityGroupPath, _timeout.getMillis(), s_logger);
--- End diff --
Please consider adding an overridden version of the `Script(String, int, Logger)` constructor that accepts `Duration` to encapsulate this type conversion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by abhinandanprateek <gi...@git.apache.org>.
Github user abhinandanprateek commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80208262
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtOvsVpcRoutingPolicyConfigCommandWrapper.java ---
@@ -36,7 +36,7 @@
@Override
public Answer execute(final OvsVpcRoutingPolicyConfigCommand command, final LibvirtComputingResource libvirtComputingResource) {
try {
- final Script scriptCommand = new Script(libvirtComputingResource.getOvsTunnelPath(), libvirtComputingResource.getTimeout(), s_logger);
+ final Script scriptCommand = new Script(libvirtComputingResource.getOvsTunnelPath(), libvirtComputingResource.getTimeout().getMillis(), s_logger);
--- End diff --
Scripts is an external package, hope they will upgrade to Duration after Java 1.8.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80064723
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtOvsDestroyTunnelCommandWrapper.java ---
@@ -42,7 +42,7 @@ public Answer execute(final OvsDestroyTunnelCommand command, final LibvirtComput
return new Answer(command, false, "No network found");
}
- final Script scriptCommand = new Script(libvirtComputingResource.getOvsTunnelPath(), libvirtComputingResource.getTimeout(), s_logger);
+ final Script scriptCommand = new Script(libvirtComputingResource.getOvsTunnelPath(), libvirtComputingResource.getTimeout().getMillis(), s_logger);
--- End diff --
Please consider adding an overridden version of the `Script(String, int, Logger)` constructor that accepts `Duration` to encapsulate this type conversion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1678: CLOUDSTACK-9503: Increased the VR script timeout. Mo...
Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1678
@abhinandanprateek jenkins does not do noredist builds
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1678#discussion_r80064905
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPvlanSetupCommandWrapper.java ---
@@ -87,7 +88,7 @@ public Answer execute(final PvlanSetupCommand command, final LibvirtComputingRes
} else if (command.getType() == PvlanSetupCommand.Type.VM) {
final String ovsPvlanVmPath = libvirtComputingResource.getOvsPvlanVmPath();
- final Script script = new Script(ovsPvlanVmPath, timeout, s_logger);
+ final Script script = new Script(ovsPvlanVmPath, timeout.getMillis(), s_logger);
--- End diff --
Please consider adding an overridden version of the `Script(String, int, Logger)` constructor that accepts `Duration` to encapsulate this type conversion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1678: CLOUDSTACK-9503: Increased the VR script time...
Posted by abhinandanprateek <gi...@git.apache.org>.
Github user abhinandanprateek closed the pull request at:
https://github.com/apache/cloudstack/pull/1678
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---