From 8b341b0639b1947a71df29d409948eb96e238499 Mon Sep 17 00:00:00 2001 From: Sheng Yang Date: Wed, 19 Feb 2014 19:12:07 -0800 Subject: [PATCH] CLOUDSTACK-6047: Add generic wrapper for group answer needed commands --- .../cloud/agent/api/routing/GroupAnswer.java | 2 +- .../agent/api/routing/IpAssocCommand.java | 4 + .../agent/api/routing/IpAssocVpcCommand.java | 6 + .../api/routing/NetworkElementCommand.java | 4 + .../api/routing/SetFirewallRulesCommand.java | 9 +- .../api/routing/SetNetworkACLCommand.java | 11 +- .../SetPortForwardingRulesCommand.java | 10 +- .../SetPortForwardingRulesVpcCommand.java | 4 +- .../api/routing/SetStaticNatRulesCommand.java | 9 +- .../api/routing/SetStaticRouteCommand.java | 13 +- .../VirtualRoutingResource.java | 218 +++++------------- 11 files changed, 118 insertions(+), 172 deletions(-) diff --git a/core/src/com/cloud/agent/api/routing/GroupAnswer.java b/core/src/com/cloud/agent/api/routing/GroupAnswer.java index 0917a7b52f8..293934bce3a 100644 --- a/core/src/com/cloud/agent/api/routing/GroupAnswer.java +++ b/core/src/com/cloud/agent/api/routing/GroupAnswer.java @@ -34,7 +34,7 @@ public class GroupAnswer extends Answer { this.results = results; } - String[] getResults() { + public String[] getResults() { return results; } } diff --git a/core/src/com/cloud/agent/api/routing/IpAssocCommand.java b/core/src/com/cloud/agent/api/routing/IpAssocCommand.java index df5d54a8c88..fe6ab6fef73 100644 --- a/core/src/com/cloud/agent/api/routing/IpAssocCommand.java +++ b/core/src/com/cloud/agent/api/routing/IpAssocCommand.java @@ -38,4 +38,8 @@ public class IpAssocCommand extends NetworkElementCommand { return ipAddresses; } + @Override + public int getAnswersCount() { + return ipAddresses.length; + } } diff --git a/core/src/com/cloud/agent/api/routing/IpAssocVpcCommand.java b/core/src/com/cloud/agent/api/routing/IpAssocVpcCommand.java index 3e5566a700c..d4996a5321e 100644 --- a/core/src/com/cloud/agent/api/routing/IpAssocVpcCommand.java +++ b/core/src/com/cloud/agent/api/routing/IpAssocVpcCommand.java @@ -26,4 +26,10 @@ public class IpAssocVpcCommand extends IpAssocCommand { public IpAssocVpcCommand(IpAddressTO[] ips) { super(ips); } + + @Override + public int getAnswersCount() { + //Count private gateway to maximum value + return ipAddresses.length * 2; + } } diff --git a/core/src/com/cloud/agent/api/routing/NetworkElementCommand.java b/core/src/com/cloud/agent/api/routing/NetworkElementCommand.java index 205784bb923..f13037c65c6 100644 --- a/core/src/com/cloud/agent/api/routing/NetworkElementCommand.java +++ b/core/src/com/cloud/agent/api/routing/NetworkElementCommand.java @@ -62,4 +62,8 @@ public abstract class NetworkElementCommand extends Command { public void setRouterAccessIp(String routerAccessIp) { this.routerAccessIp = routerAccessIp; } + + public int getAnswersCount() { + return 1; + } } diff --git a/core/src/com/cloud/agent/api/routing/SetFirewallRulesCommand.java b/core/src/com/cloud/agent/api/routing/SetFirewallRulesCommand.java index f7ac052d72c..36771b9d0cc 100644 --- a/core/src/com/cloud/agent/api/routing/SetFirewallRulesCommand.java +++ b/core/src/com/cloud/agent/api/routing/SetFirewallRulesCommand.java @@ -16,12 +16,12 @@ // under the License. package com.cloud.agent.api.routing; +import com.cloud.agent.api.to.FirewallRuleTO; + import java.util.HashSet; import java.util.List; import java.util.Set; -import com.cloud.agent.api.to.FirewallRuleTO; - /** * * AccessDetails allow different components to put in information about @@ -92,4 +92,9 @@ public class SetFirewallRulesCommand extends NetworkElementCommand { return result; } + + @Override + public int getAnswersCount() { + return rules.length; + } } diff --git a/core/src/com/cloud/agent/api/routing/SetNetworkACLCommand.java b/core/src/com/cloud/agent/api/routing/SetNetworkACLCommand.java index 7edcdf38f17..0b9fec5155f 100644 --- a/core/src/com/cloud/agent/api/routing/SetNetworkACLCommand.java +++ b/core/src/com/cloud/agent/api/routing/SetNetworkACLCommand.java @@ -17,14 +17,14 @@ package com.cloud.agent.api.routing; +import com.cloud.agent.api.to.NetworkACLTO; +import com.cloud.agent.api.to.NicTO; + import java.util.Arrays; import java.util.Collections; import java.util.Comparator; import java.util.List; -import com.cloud.agent.api.to.NetworkACLTO; -import com.cloud.agent.api.to.NicTO; - public class SetNetworkACLCommand extends NetworkElementCommand { NetworkACLTO[] rules; NicTO nic; @@ -97,4 +97,9 @@ public class SetNetworkACLCommand extends NetworkElementCommand { public NicTO getNic() { return nic; } + + @Override + public int getAnswersCount() { + return rules.length; + } } diff --git a/core/src/com/cloud/agent/api/routing/SetPortForwardingRulesCommand.java b/core/src/com/cloud/agent/api/routing/SetPortForwardingRulesCommand.java index a7bf37f83ef..d93ccaf7201 100644 --- a/core/src/com/cloud/agent/api/routing/SetPortForwardingRulesCommand.java +++ b/core/src/com/cloud/agent/api/routing/SetPortForwardingRulesCommand.java @@ -16,10 +16,10 @@ // under the License. package com.cloud.agent.api.routing; -import java.util.List; - import com.cloud.agent.api.to.PortForwardingRuleTO; +import java.util.List; + public class SetPortForwardingRulesCommand extends NetworkElementCommand { PortForwardingRuleTO[] rules; @@ -37,4 +37,10 @@ public class SetPortForwardingRulesCommand extends NetworkElementCommand { public PortForwardingRuleTO[] getRules() { return rules; } + + @Override + public int getAnswersCount() { + return rules.length; + } } + diff --git a/core/src/com/cloud/agent/api/routing/SetPortForwardingRulesVpcCommand.java b/core/src/com/cloud/agent/api/routing/SetPortForwardingRulesVpcCommand.java index 613ae5d7985..1bce50dbddf 100644 --- a/core/src/com/cloud/agent/api/routing/SetPortForwardingRulesVpcCommand.java +++ b/core/src/com/cloud/agent/api/routing/SetPortForwardingRulesVpcCommand.java @@ -16,10 +16,10 @@ // under the License. package com.cloud.agent.api.routing; -import java.util.List; - import com.cloud.agent.api.to.PortForwardingRuleTO; +import java.util.List; + public class SetPortForwardingRulesVpcCommand extends SetPortForwardingRulesCommand { protected SetPortForwardingRulesVpcCommand() { } diff --git a/core/src/com/cloud/agent/api/routing/SetStaticNatRulesCommand.java b/core/src/com/cloud/agent/api/routing/SetStaticNatRulesCommand.java index a38bf5f22ff..64c76605765 100644 --- a/core/src/com/cloud/agent/api/routing/SetStaticNatRulesCommand.java +++ b/core/src/com/cloud/agent/api/routing/SetStaticNatRulesCommand.java @@ -16,10 +16,10 @@ // under the License. package com.cloud.agent.api.routing; -import java.util.List; - import com.cloud.agent.api.to.StaticNatRuleTO; +import java.util.List; + public class SetStaticNatRulesCommand extends NetworkElementCommand { StaticNatRuleTO[] rules; @@ -44,4 +44,9 @@ public class SetStaticNatRulesCommand extends NetworkElementCommand { public Long getVpcId() { return vpcId; } + + @Override + public int getAnswersCount() { + return rules.length; + } } diff --git a/core/src/com/cloud/agent/api/routing/SetStaticRouteCommand.java b/core/src/com/cloud/agent/api/routing/SetStaticRouteCommand.java index 26800a115a1..1e4f0c86e0f 100644 --- a/core/src/com/cloud/agent/api/routing/SetStaticRouteCommand.java +++ b/core/src/com/cloud/agent/api/routing/SetStaticRouteCommand.java @@ -17,14 +17,14 @@ package com.cloud.agent.api.routing; -import java.util.HashSet; -import java.util.List; -import java.util.Set; - import com.cloud.network.vpc.StaticRoute; import com.cloud.network.vpc.StaticRouteProfile; import com.cloud.utils.net.NetUtils; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + public class SetStaticRouteCommand extends NetworkElementCommand { StaticRouteProfile[] staticRoutes; @@ -59,4 +59,9 @@ public class SetStaticRouteCommand extends NetworkElementCommand { result[0] = toAdd.toArray(new String[toAdd.size()]); return result; } + + @Override + public int getAnswersCount() { + return staticRoutes.length; + } } diff --git a/core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java b/core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java index f66b03064c8..49a5c0a7d6b 100755 --- a/core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java +++ b/core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java @@ -231,14 +231,44 @@ public class VirtualRoutingResource { } } - private Answer applyConfigSingle(NetworkElementCommand cmd, List cfg) { - for (ConfigItem c : cfg) { + private Answer applyConfig(NetworkElementCommand cmd, List cfg) { + int answersCount = cmd.getAnswersCount(); + assert (cfg.size() <= answersCount) : "Why there are more commands than answers?"; + + if (cfg.size() == 1 && answersCount == 1) { + ConfigItem c = cfg.get(0); ExecutionResult result = _vrDeployer.executeInVR(cmd.getRouterAccessIp(), c.getScript(), c.getArgs()); - if (!result.isSuccess()) { - return new Answer(cmd, false, result.getDetails()); - } + return new Answer(cmd, result.isSuccess(), result.getDetails()); } - return new Answer(cmd); + + ExecutionResult[] results = new ExecutionResult[answersCount]; + String[] resultsString = new String[answersCount]; + boolean finalResult = true; + int i = 0, j; + for (ConfigItem c : cfg) { + results[i] = _vrDeployer.executeInVR(cmd.getRouterAccessIp(), c.getScript(), c.getArgs()); + if (c.getInfo() != null) { + if (results[i].isSuccess()) { + results[i].setDetails(c.getInfo() + " - success: " + results[i].getDetails()); + } else { + results[i].setDetails(c.getInfo() + " - failed: " + results[i].getDetails()); + } + } + i ++; + } + i = 0; j = 0; + while (j < answersCount) { + resultsString[j] = results[i].getDetails(); + if (!results[i].isSuccess()) { + finalResult = false; + } + // Fill the resultsString with the last result of execution, mostly in 1:n + if (i < cfg.size() - 1) { + i ++; + } + j ++; + } + return new GroupAnswer(cmd, finalResult, answersCount, resultsString); } private List generateConfig(VpnUsersCfgCommand cmd) { @@ -259,7 +289,7 @@ public class VirtualRoutingResource { private Answer execute(VpnUsersCfgCommand cmd) { List cfg = generateConfig(cmd); - return applyConfigSingle(cmd, cfg); + return applyConfig(cmd, cfg); } private List generateConfig(RemoteAccessVpnCfgCommand cmd) { @@ -288,7 +318,7 @@ public class VirtualRoutingResource { private Answer execute(RemoteAccessVpnCfgCommand cmd) { List cfg = generateConfig(cmd); - return applyConfigSingle(cmd, cfg); + return applyConfig(cmd, cfg); } private List generateConfig(SetFirewallRulesCommand cmd) { @@ -332,27 +362,8 @@ public class VirtualRoutingResource { } private Answer execute(SetFirewallRulesCommand cmd) { - int rulesCount = cmd.getRules().length; - String[] results = new String[rulesCount]; - String routerAccessIp = cmd.getRouterAccessIp(); - - if (routerAccessIp == null) { - return new GroupAnswer(cmd, false, rulesCount, results); - } - List cfg = generateConfig(cmd); - ConfigItem c = cfg.get(0); - ExecutionResult result = _vrDeployer.executeInVR(routerAccessIp, c.getScript(), c.getArgs()); - - if (!result.isSuccess()) { - //FIXME - in the future we have to process each rule separately; now we temporarily set every rule to be false if single rule fails - for (int i = 0; i < results.length; i++) { - results[i] = "Failed: " + result.getDetails(); - } - return new GroupAnswer(cmd, false, rulesCount, results); - } - return new GroupAnswer(cmd, true, rulesCount, results); - + return applyConfig(cmd, cfg); } private List generateConfig(SetPortForwardingRulesCommand cmd) { @@ -373,24 +384,8 @@ public class VirtualRoutingResource { } private Answer execute(SetPortForwardingRulesCommand cmd) { - int rulesCount = cmd.getRules().length; - String[] results = new String[rulesCount]; - int i = 0; - boolean endResult = true; List cfg = generateConfig(cmd); - - for (ConfigItem c : cfg) { - ExecutionResult result = _vrDeployer.executeInVR(cmd.getRouterAccessIp(), c.getScript(), c.getArgs()); - - if (!result.isSuccess()) { - results[i++] = "Failed"; - endResult = false; - } else { - results[i++] = null; - } - } - - return new GroupAnswer(cmd, endResult, rulesCount, results); + return applyConfig(cmd, cfg); } private List generateConfig(SetStaticNatRulesCommand cmd) { @@ -424,24 +419,9 @@ public class VirtualRoutingResource { return cfg; } - private GroupAnswer execute(SetStaticNatRulesCommand cmd) { - String[] results = new String[cmd.getRules().length]; - int i = 0; - boolean endResult = true; - + private Answer execute(SetStaticNatRulesCommand cmd) { List cfg = generateConfig(cmd); - for (ConfigItem c : cfg) { - ExecutionResult result = _vrDeployer.executeInVR(cmd.getRouterAccessIp(), c.getScript(), c.getArgs()); - - if (!result.isSuccess()) { - results[i++] = "Failed"; - endResult = false; - } else { - results[i++] = null; - } - } - - return new GroupAnswer(cmd, endResult, cmd.getRules().length, results); + return applyConfig(cmd, cfg); } private List generateConfig(LoadBalancerConfigCommand cmd) { @@ -542,7 +522,7 @@ public class VirtualRoutingResource { protected Answer execute(VmDataCommand cmd) { List cfg = generateConfig(cmd); - return applyConfigSingle(cmd, cfg); + return applyConfig(cmd, cfg); } private List generateConfig(SavePasswordCommand cmd) { @@ -560,7 +540,7 @@ public class VirtualRoutingResource { protected Answer execute(final SavePasswordCommand cmd) { List cfg = generateConfig(cmd); - return applyConfigSingle(cmd, cfg); + return applyConfig(cmd, cfg); } private List generateConfig(DhcpEntryCommand cmd) { @@ -599,7 +579,7 @@ public class VirtualRoutingResource { protected Answer execute(final DhcpEntryCommand cmd) { List cfg = generateConfig(cmd); - return applyConfigSingle(cmd, cfg); + return applyConfig(cmd, cfg); } private List generateConfig(CreateIpAliasCommand cmd) { @@ -617,7 +597,7 @@ public class VirtualRoutingResource { protected Answer execute(final CreateIpAliasCommand cmd) { List cfg = generateConfig(cmd); - return applyConfigSingle(cmd, cfg); + return applyConfig(cmd, cfg); } private List generateConfig(DeleteIpAliasCommand cmd) { @@ -641,7 +621,7 @@ public class VirtualRoutingResource { protected Answer execute(final DeleteIpAliasCommand cmd) { List cfg = generateConfig(cmd); - return applyConfigSingle(cmd, cfg); + return applyConfig(cmd, cfg); } private List generateConfig(DnsMasqConfigCommand cmd) { @@ -659,7 +639,7 @@ public class VirtualRoutingResource { protected Answer execute(final DnsMasqConfigCommand cmd) { List cfg = generateConfig(cmd); - return applyConfigSingle(cmd, cfg); + return applyConfig(cmd, cfg); } private CheckS2SVpnConnectionsAnswer execute(CheckS2SVpnConnectionsCommand cmd) { @@ -688,7 +668,7 @@ public class VirtualRoutingResource { protected Answer execute(BumpUpPriorityCommand cmd) { List cfg = generateConfig(cmd); - return applyConfigSingle(cmd, cfg); + return applyConfig(cmd, cfg); } protected Answer execute(GetDomRVersionCmd cmd) { @@ -754,7 +734,7 @@ public class VirtualRoutingResource { protected Answer execute(Site2SiteVpnCfgCommand cmd) { List cfg = generateConfig(cmd); - return applyConfigSingle(cmd, cfg); + return applyConfig(cmd, cfg); } protected List generateConfig(SetMonitorServiceCommand cmd) { @@ -774,7 +754,7 @@ public class VirtualRoutingResource { protected Answer execute(SetMonitorServiceCommand cmd) { List cfg = generateConfig(cmd); - return applyConfigSingle(cmd, cfg); + return applyConfig(cmd, cfg); } protected List generateConfig(SetupGuestNetworkCommand cmd) { @@ -819,7 +799,7 @@ public class VirtualRoutingResource { protected Answer execute(SetupGuestNetworkCommand cmd) { List cfg = generateConfig(cmd); - return applyConfigSingle(cmd, cfg); + return applyConfig(cmd, cfg); } protected List generateConfig(SetNetworkACLCommand cmd) { @@ -856,22 +836,9 @@ public class VirtualRoutingResource { return cfg; } - private GroupAnswer execute(SetNetworkACLCommand cmd) { - int rulesCount = cmd.getRules().length; - String[] results = new String[rulesCount]; - + private Answer execute(SetNetworkACLCommand cmd) { List cfg = generateConfig(cmd); - ConfigItem c = cfg.get(0); - final ExecutionResult result = _vrDeployer.executeInVR(cmd.getRouterAccessIp(), c.getScript(), c.getArgs()); - - if (!result.isSuccess()) { - for (int i = 0; i < results.length; i++) { - results[i] = "Failed"; - } - return new GroupAnswer(cmd, false, rulesCount, results); - } - - return new GroupAnswer(cmd, true, rulesCount, results); + return applyConfig(cmd, cfg); } protected List generateConfig(SetSourceNatCommand cmd) { @@ -891,7 +858,7 @@ public class VirtualRoutingResource { protected Answer execute(SetSourceNatCommand cmd) { List cfg = generateConfig(cmd); - return applyConfigSingle(cmd, cfg); + return applyConfig(cmd, cfg); } protected List generateConfig(SetPortForwardingRulesVpcCommand cmd) { @@ -911,45 +878,14 @@ public class VirtualRoutingResource { return cfg; } - private GroupAnswer execute(SetPortForwardingRulesVpcCommand cmd) { - String[] results = new String[cmd.getRules().length]; - int i = 0; - - boolean endResult = true; + private Answer execute(SetPortForwardingRulesVpcCommand cmd) { List cfg = generateConfig(cmd); - for (ConfigItem c : cfg) { - ExecutionResult result = _vrDeployer.executeInVR(cmd.getRouterAccessIp(), c.getScript(), c.getArgs()); - - if (!result.isSuccess()) { - results[i++] = "Failed"; - endResult = false; - } else { - results[i++] = null; - } - } - return new GroupAnswer(cmd, endResult, cmd.getRules().length, results); + return applyConfig(cmd, cfg); } - public GroupAnswer execute(IpAssocVpcCommand cmd) { - boolean finalResult = true; - String[] results = new String[cmd.getIpAddresses().length]; - for (int i = 0; i < cmd.getIpAddresses().length; i ++) { - results[i] = "Failed"; - } - - int i = 0; + public Answer execute(IpAssocVpcCommand cmd) { List cfg = generateConfig(cmd); - for (ConfigItem c : cfg) { - ExecutionResult result = _vrDeployer.executeInVR(cmd.getRouterAccessIp(), c.getScript(), c.getArgs()); - if (!result.isSuccess()) { - results[i++] = c.getInfo() + " failed: " + result.getDetails(); - finalResult = false; - break; - } - - results[i++] = c.getInfo() + " - success "; - } - return new GroupAnswer(cmd, finalResult, cmd.getIpAddresses().length, results); + return applyConfig(cmd, cfg); } protected List generateConfig(SetStaticRouteCommand cmd) { @@ -969,22 +905,9 @@ public class VirtualRoutingResource { return cfg; } - private GroupAnswer execute(SetStaticRouteCommand cmd) { - int rulesCount = cmd.getStaticRoutes().length; - String[] results = new String[rulesCount]; - + private Answer execute(SetStaticRouteCommand cmd) { List cfg = generateConfig(cmd); - ConfigItem c = cfg.get(0); - final ExecutionResult result = _vrDeployer.executeInVR(cmd.getRouterAccessIp(), c.getScript(), c.getArgs()); - - if (!result.isSuccess()) { - for (int i = 0; i < results.length; i++) { - results[i] = "Failed"; - } - return new GroupAnswer(cmd, false, rulesCount, results); - } - - return new GroupAnswer(cmd, true, rulesCount, results); + return applyConfig(cmd, cfg); } protected List generateConfig(IpAssocCommand cmd) { @@ -1067,25 +990,8 @@ public class VirtualRoutingResource { } public Answer execute(IpAssocCommand cmd) { - boolean finalResult = true; - String[] results = new String[cmd.getIpAddresses().length]; - for (int i = 0; i < results.length; i++) { - results[i] = "Failed"; - } - - int i = 0; List cfg = generateConfig(cmd); - for (ConfigItem c : cfg) { - ExecutionResult result = _vrDeployer.executeInVR(cmd.getRouterAccessIp(), c.getScript(), c.getArgs()); - if (result.isSuccess()) { - results[i++] = c.getInfo() + " - success"; - } else { - results[i++] = c.getInfo() + " - failed:" + result.getDetails(); - finalResult = false; - break; - } - } - return new GroupAnswer(cmd, finalResult, cmd.getIpAddresses().length, results); + return applyConfig(cmd, cfg); } public boolean configure(final String name, final Map params) throws ConfigurationException {