Repository: cloudstack
Updated Branches:
  refs/heads/master 56e37243b -> e78bb1d22


Aggregate command cleanup is not required for the virtual router as we
already cleanup in the finish.

And consequently dies if somebody tries to test with assertions enabled.


Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/dbc7d803
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/dbc7d803
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/dbc7d803

Branch: refs/heads/master
Commit: dbc7d803291547f50d00dde0c37490c2005b7fdc
Parents: 56e3724
Author: Hugo Trippaers <[email protected]>
Authored: Wed Jul 16 12:40:50 2014 +0200
Committer: Hugo Trippaers <[email protected]>
Committed: Wed Jul 16 16:24:54 2014 +0200

----------------------------------------------------------------------
 .../virtualnetwork/VirtualRoutingResource.java  |  13 +-
 .../network/element/VirtualRouterElement.java   | 184 +++++++++----------
 .../router/VirtualNetworkApplianceManager.java  |  11 +-
 .../VirtualNetworkApplianceManagerImpl.java     |   4 -
 .../MockVpcVirtualNetworkApplianceManager.java  |  24 ++-
 5 files changed, 106 insertions(+), 130 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/dbc7d803/core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java
----------------------------------------------------------------------
diff --git 
a/core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java 
b/core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java
index 1fa5bf2..00501e3 100755
--- 
a/core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java
+++ 
b/core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java
@@ -361,7 +361,7 @@ public class VirtualRoutingResource {
 
             Queue<NetworkElementCommand> queue = new LinkedBlockingQueue<>();
             _vrAggregateCommandsSet.put(routerName, queue);
-            return new Answer(cmd);
+            return new Answer(cmd, true, "Command aggregation started");
         } else if (action == Action.Finish) {
             Queue<NetworkElementCommand> queue = 
_vrAggregateCommandsSet.get(routerName);
             int answerCounts = 0;
@@ -402,20 +402,11 @@ public class VirtualRoutingResource {
                     return new Answer(cmd, false, result.getDetails());
                 }
 
-                return new Answer(cmd);
+                return new Answer(cmd, true, "Command aggregation finished");
             } finally {
                 queue.clear();
                 _vrAggregateCommandsSet.remove(routerName);
             }
-        } else if (action == Action.Cleanup) {
-            assert (_vrAggregateCommandsSet.containsKey(routerName));
-            Queue<NetworkElementCommand> queue = 
_vrAggregateCommandsSet.get(routerName);
-            if (queue != null) {
-                queue.clear();
-            }
-            _vrAggregateCommandsSet.remove(routerName);
-
-            return new Answer(cmd);
         }
         return new Answer(cmd, false, "Fail to recongize aggregation action " 
+ action.toString());
     }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/dbc7d803/server/src/com/cloud/network/element/VirtualRouterElement.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/network/element/VirtualRouterElement.java 
b/server/src/com/cloud/network/element/VirtualRouterElement.java
index 3b3cacd..3fcf578 100755
--- a/server/src/com/cloud/network/element/VirtualRouterElement.java
+++ b/server/src/com/cloud/network/element/VirtualRouterElement.java
@@ -16,6 +16,26 @@
 // under the License.
 package com.cloud.network.element;
 
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import javax.ejb.Local;
+import javax.inject.Inject;
+
+import org.apache.log4j.Logger;
+
+import com.google.gson.Gson;
+
+import org.apache.cloudstack.api.command.admin.router.ConfigureOvsElementCmd;
+import 
org.apache.cloudstack.api.command.admin.router.ConfigureVirtualRouterElementCmd;
+import 
org.apache.cloudstack.api.command.admin.router.CreateVirtualRouterElementCmd;
+import org.apache.cloudstack.api.command.admin.router.ListOvsElementsCmd;
+import 
org.apache.cloudstack.api.command.admin.router.ListVirtualRouterElementsCmd;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+
 import com.cloud.agent.api.to.LoadBalancerTO;
 import com.cloud.configuration.ConfigurationManager;
 import com.cloud.dc.DataCenter;
@@ -81,32 +101,16 @@ import com.cloud.vm.VirtualMachine.State;
 import com.cloud.vm.VirtualMachineProfile;
 import com.cloud.vm.dao.DomainRouterDao;
 import com.cloud.vm.dao.UserVmDao;
-import com.google.gson.Gson;
-import org.apache.cloudstack.api.command.admin.router.ConfigureOvsElementCmd;
-import 
org.apache.cloudstack.api.command.admin.router.ConfigureVirtualRouterElementCmd;
-import 
org.apache.cloudstack.api.command.admin.router.CreateVirtualRouterElementCmd;
-import org.apache.cloudstack.api.command.admin.router.ListOvsElementsCmd;
-import 
org.apache.cloudstack.api.command.admin.router.ListVirtualRouterElementsCmd;
-import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
-import org.apache.log4j.Logger;
-
-import javax.ejb.Local;
-import javax.inject.Inject;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
 
 @Local(value = {NetworkElement.class, FirewallServiceProvider.class,
-    DhcpServiceProvider.class, UserDataServiceProvider.class,
-    StaticNatServiceProvider.class, LoadBalancingServiceProvider.class,
-    PortForwardingServiceProvider.class, IpDeployer.class,
-    RemoteAccessVPNServiceProvider.class, NetworkMigrationResponder.class})
+        DhcpServiceProvider.class, UserDataServiceProvider.class,
+        StaticNatServiceProvider.class, LoadBalancingServiceProvider.class,
+        PortForwardingServiceProvider.class, IpDeployer.class,
+        RemoteAccessVPNServiceProvider.class, NetworkMigrationResponder.class})
 public class VirtualRouterElement extends AdapterBase implements 
VirtualRouterElementService, DhcpServiceProvider,
-        UserDataServiceProvider, SourceNatServiceProvider, 
StaticNatServiceProvider, FirewallServiceProvider,
-        LoadBalancingServiceProvider, PortForwardingServiceProvider, 
RemoteAccessVPNServiceProvider, IpDeployer,
-        NetworkMigrationResponder, AggregatedCommandExecutor {
+UserDataServiceProvider, SourceNatServiceProvider, StaticNatServiceProvider, 
FirewallServiceProvider,
+LoadBalancingServiceProvider, PortForwardingServiceProvider, 
RemoteAccessVPNServiceProvider, IpDeployer,
+NetworkMigrationResponder, AggregatedCommandExecutor {
     private static final Logger s_logger = 
Logger.getLogger(VirtualRouterElement.class);
     public static final AutoScaleCounterType AutoScaleCounterCpu = new 
AutoScaleCounterType("cpu");
     public static final AutoScaleCounterType AutoScaleCounterMemory = new 
AutoScaleCounterType("memory");
@@ -179,7 +183,7 @@ public class VirtualRouterElement extends AdapterBase 
implements VirtualRouterEl
 
     @Override
     public boolean implement(Network network, NetworkOffering offering, 
DeployDestination dest, ReservationContext context) throws 
ResourceUnavailableException,
-        ConcurrentOperationException, InsufficientCapacityException {
+    ConcurrentOperationException, InsufficientCapacityException {
 
         if (offering.isSystemOnly()) {
             return false;
@@ -205,7 +209,7 @@ public class VirtualRouterElement extends AdapterBase 
implements VirtualRouterEl
 
     @Override
     public boolean prepare(Network network, NicProfile nic, 
VirtualMachineProfile vm, DeployDestination dest, ReservationContext context)
-        throws ConcurrentOperationException, InsufficientCapacityException, 
ResourceUnavailableException {
+            throws ConcurrentOperationException, 
InsufficientCapacityException, ResourceUnavailableException {
         if (vm.getType() != VirtualMachine.Type.User || vm.getHypervisorType() 
== HypervisorType.BareMetal) {
             return false;
         }
@@ -225,8 +229,8 @@ public class VirtualRouterElement extends AdapterBase 
implements VirtualRouterEl
         @SuppressWarnings("unchecked")
         VirtualMachineProfile uservm = vm;
         List<DomainRouterVO> routers =
-            _routerMgr.deployVirtualRouterInGuestNetwork(network, dest, 
_accountMgr.getAccount(network.getAccountId()), uservm.getParameters(),
-                offering.getRedundantRouter());
+                _routerMgr.deployVirtualRouterInGuestNetwork(network, dest, 
_accountMgr.getAccount(network.getAccountId()), uservm.getParameters(),
+                        offering.getRedundantRouter());
         if ((routers == null) || (routers.size() == 0)) {
             throw new ResourceUnavailableException("Can't find at least one 
running router!", DataCenter.class, network.getDataCenterId());
         }
@@ -239,14 +243,14 @@ public class VirtualRouterElement extends AdapterBase 
implements VirtualRouterEl
             List<DomainRouterVO> routers = 
_routerDao.listByNetworkAndRole(config.getId(), Role.VIRTUAL_ROUTER);
             if (routers == null || routers.isEmpty()) {
                 s_logger.debug("Virtual router elemnt doesn't need to apply 
firewall rules on the backend; virtual " + "router doesn't exist in the network 
" +
-                    config.getId());
+                        config.getId());
                 return true;
             }
 
             if (rules != null && rules.size() == 1) {
                 // for VR no need to add default egress rule to DENY traffic
                 if (rules.get(0).getTrafficType() == 
FirewallRule.TrafficType.Egress && rules.get(0).getType() == 
FirewallRule.FirewallRuleType.System &&
-                    !_networkMgr.getNetworkEgressDefaultPolicy(config.getId()))
+                        
!_networkMgr.getNetworkEgressDefaultPolicy(config.getId()))
                     return true;
             }
 
@@ -274,7 +278,7 @@ public class VirtualRouterElement extends AdapterBase 
implements VirtualRouterEl
             boolean matchedEndChar = false;
             if (str.length() < 2)
                 return false; // atleast one numeric and one char. example:
-                              // 3h
+            // 3h
             char strEnd = str.toCharArray()[str.length() - 1];
             for (char c : endChar.toCharArray()) {
                 if (strEnd == c) {
@@ -287,7 +291,7 @@ public class VirtualRouterElement extends AdapterBase 
implements VirtualRouterEl
                 return false;
         }
         try {
-            int i = Integer.parseInt(number);
+            Integer.parseInt(number);
         } catch (NumberFormatException e) {
             return false;
         }
@@ -323,21 +327,14 @@ public class VirtualRouterElement extends AdapterBase 
implements VirtualRouterEl
 
                 }
             } else if 
(StickinessMethodType.AppCookieBased.getName().equalsIgnoreCase(stickinessPolicy.getMethodName()))
 {
-                /*
-                 * FORMAT : appsession <cookie> len <length> timeout <holdtime>
-                 * [request-learn] [prefix] [mode
-                 * <path-parameters|query-string>]
-                 */
-                /* example: appsession JSESSIONID len 52 timeout 3h */
-                String cookieName = null; // optional
                 String length = null; // optional
                 String holdTime = null; // optional
 
                 for (Pair<String, String> paramKV : paramsList) {
                     String key = paramKV.first();
                     String value = paramKV.second();
-                    if ("cookie-name".equalsIgnoreCase(key))
-                        cookieName = value;
+                    if ("cookie-name".equalsIgnoreCase(key)) {
+                    }
                     if ("length".equalsIgnoreCase(key))
                         length = value;
                     if ("holdtime".equalsIgnoreCase(key))
@@ -403,7 +400,7 @@ public class VirtualRouterElement extends AdapterBase 
implements VirtualRouterEl
             List<DomainRouterVO> routers = 
_routerDao.listByNetworkAndRole(network.getId(), Role.VIRTUAL_ROUTER);
             if (routers == null || routers.isEmpty()) {
                 s_logger.debug("Virtual router elemnt doesn't need to apply 
vpn users on the backend; virtual router" + " doesn't exist in the network " +
-                    network.getId());
+                        network.getId());
                 return null;
             }
             return _routerMgr.applyVpnUsers(network, users, routers);
@@ -466,7 +463,7 @@ public class VirtualRouterElement extends AdapterBase 
implements VirtualRouterEl
             List<DomainRouterVO> routers = 
_routerDao.listByNetworkAndRole(network.getId(), Role.VIRTUAL_ROUTER);
             if (routers == null || routers.isEmpty()) {
                 s_logger.debug("Virtual router elemnt doesn't need to 
associate ip addresses on the backend; virtual " + "router doesn't exist in the 
network " +
-                    network.getId());
+                        network.getId());
                 return true;
             }
 
@@ -493,61 +490,61 @@ public class VirtualRouterElement extends AdapterBase 
implements VirtualRouterEl
         method = new LbStickinessMethod(StickinessMethodType.LBCookieBased, 
"This is loadbalancer cookie based stickiness method.");
         method.addParam("cookie-name", false, "Cookie name passed in http 
header by the LB to the client.", false);
         method.addParam("mode", false, "Valid values: insert, rewrite, prefix. 
Default value: insert.  In the insert mode cookie will be created"
-            + " by the LB. In other modes, cookie will be created by the 
server and LB modifies it.", false);
+                + " by the LB. In other modes, cookie will be created by the 
server and LB modifies it.", false);
         method.addParam("nocache", false, "This option is recommended in 
conjunction with the insert mode when there is a cache between the client"
-            + " and HAProxy, as it ensures that a cacheable response will be 
tagged non-cacheable if  a cookie needs "
-            + "to be inserted. This is important because if all persistence 
cookies are added on a cacheable home page"
-            + " for instance, then all customers will then fetch the page from 
an outer cache and will all share the "
-            + "same persistence cookie, leading to one server receiving much 
more traffic than others. See also the " + "insert and postonly options. ", 
true);
+                + " and HAProxy, as it ensures that a cacheable response will 
be tagged non-cacheable if  a cookie needs "
+                + "to be inserted. This is important because if all 
persistence cookies are added on a cacheable home page"
+                + " for instance, then all customers will then fetch the page 
from an outer cache and will all share the "
+                + "same persistence cookie, leading to one server receiving 
much more traffic than others. See also the " + "insert and postonly options. 
", true);
         method.addParam("indirect", false, "When this option is specified in 
insert mode, cookies will only be added when the server was not reached"
-            + " after a direct access, which means that only when a server is 
elected after applying a load-balancing algorithm,"
-            + " or after a redispatch, then the cookie  will be inserted. If 
the client has all the required information"
-            + " to connect to the same server next time, no further cookie 
will be inserted. In all cases, when the "
-            + "indirect option is used in insert mode, the cookie is always 
removed from the requests transmitted to "
-            + "the server. The persistence mechanism then becomes totally 
transparent from the application point of view.", true);
+                + " after a direct access, which means that only when a server 
is elected after applying a load-balancing algorithm,"
+                + " or after a redispatch, then the cookie  will be inserted. 
If the client has all the required information"
+                + " to connect to the same server next time, no further cookie 
will be inserted. In all cases, when the "
+                + "indirect option is used in insert mode, the cookie is 
always removed from the requests transmitted to "
+                + "the server. The persistence mechanism then becomes totally 
transparent from the application point of view.", true);
         method.addParam("postonly", false, "This option ensures that cookie 
insertion will only be performed on responses to POST requests. It is an"
-            + " alternative to the nocache option, because POST responses are 
not cacheable, so this ensures that the "
-            + "persistence cookie will never get cached.Since most sites do 
not need any sort of persistence before the"
-            + " first POST which generally is a login request, this is a very 
efficient method to optimize caching "
-            + "without risking to find a persistence cookie in the cache. See 
also the insert and nocache options.", true);
+                + " alternative to the nocache option, because POST responses 
are not cacheable, so this ensures that the "
+                + "persistence cookie will never get cached.Since most sites 
do not need any sort of persistence before the"
+                + " first POST which generally is a login request, this is a 
very efficient method to optimize caching "
+                + "without risking to find a persistence cookie in the cache. 
See also the insert and nocache options.", true);
         method.addParam("domain", false, "This option allows to specify the 
domain at which a cookie is inserted. It requires exactly one parameter:"
-            + " a valid domain name. If the domain begins with a dot, the 
browser is allowed to use it for any host "
-            + "ending with that name. It is also possible to specify several 
domain names by invoking this option multiple"
-            + " times. Some browsers might have small limits on the number of 
domains, so be careful when doing that. "
-            + "For the record, sending 10 domains to MSIE 6 or Firefox 2 works 
as expected.", false);
+                + " a valid domain name. If the domain begins with a dot, the 
browser is allowed to use it for any host "
+                + "ending with that name. It is also possible to specify 
several domain names by invoking this option multiple"
+                + " times. Some browsers might have small limits on the number 
of domains, so be careful when doing that. "
+                + "For the record, sending 10 domains to MSIE 6 or Firefox 2 
works as expected.", false);
         methodList.add(method);
 
         method =
-            new LbStickinessMethod(StickinessMethodType.AppCookieBased,
-                "This is App session based sticky method. Define session 
stickiness on an existing application cookie. "
-                    + "It can be used only for a specific http traffic");
+                new LbStickinessMethod(StickinessMethodType.AppCookieBased,
+                        "This is App session based sticky method. Define 
session stickiness on an existing application cookie. "
+                                + "It can be used only for a specific http 
traffic");
         method.addParam("cookie-name", false, "This is the name of the cookie 
used by the application and which LB will "
-            + "have to learn for each new session. Default value: Auto 
geneared based on ip", false);
+                + "have to learn for each new session. Default value: Auto 
geneared based on ip", false);
         method.addParam("length", false, "This is the max number of characters 
that will be memorized and checked in " + "each cookie value. Default 
value:52", false);
         method.addParam("holdtime", false, "This is the time after which the 
cookie will be removed from memory if unused. The value should be in "
-            + "the format Example : 20s or 30m  or 4h or 5d . only seconds(s), 
minutes(m) hours(h) and days(d) are valid,"
-            + " cannot use th combinations like 20h30m. Default value:3h ", 
false);
+                + "the format Example : 20s or 30m  or 4h or 5d . only 
seconds(s), minutes(m) hours(h) and days(d) are valid,"
+                + " cannot use th combinations like 20h30m. Default value:3h 
", false);
         method.addParam(
-            "request-learn",
-            false,
-            "If this option is specified, then haproxy will be able to learn 
the cookie found in the request in case the server does not specify any in 
response. This is typically what happens with PHPSESSID cookies, or when 
haproxy's session expires before the application's session and the correct 
server is selected. It is recommended to specify this option to improve 
reliability",
-            true);
+                "request-learn",
+                false,
+                "If this option is specified, then haproxy will be able to 
learn the cookie found in the request in case the server does not specify any 
in response. This is typically what happens with PHPSESSID cookies, or when 
haproxy's session expires before the application's session and the correct 
server is selected. It is recommended to specify this option to improve 
reliability",
+                true);
         method.addParam(
-            "prefix",
-            false,
-            "When this option is specified, haproxy will match on the cookie 
prefix (or URL parameter prefix). "
-                + "The appsession value is the data following this prefix. 
Example : appsession ASPSESSIONID len 64 timeout 3h prefix  This will match the 
cookie ASPSESSIONIDXXXX=XXXXX, the appsession value will be XXXX=XXXXX.",
-            true);
+                "prefix",
+                false,
+                "When this option is specified, haproxy will match on the 
cookie prefix (or URL parameter prefix). "
+                        + "The appsession value is the data following this 
prefix. Example : appsession ASPSESSIONID len 64 timeout 3h prefix  This will 
match the cookie ASPSESSIONIDXXXX=XXXXX, the appsession value will be 
XXXX=XXXXX.",
+                        true);
         method.addParam("mode", false, "This option allows to change the URL 
parser mode. 2 modes are currently supported : - path-parameters "
-            + ": The parser looks for the appsession in the path parameters 
part (each parameter is separated by a semi-colon), "
-            + "which is convenient for JSESSIONID for example.This is the 
default mode if the option is not set. - query-string :"
-            + " In this mode, the parser will look for the appsession in the 
query string.", false);
+                + ": The parser looks for the appsession in the path 
parameters part (each parameter is separated by a semi-colon), "
+                + "which is convenient for JSESSIONID for example.This is the 
default mode if the option is not set. - query-string :"
+                + " In this mode, the parser will look for the appsession in 
the query string.", false);
         methodList.add(method);
 
         method = new LbStickinessMethod(StickinessMethodType.SourceBased, 
"This is source based Stickiness method, " + "it can be used for any type of 
protocol.");
         method.addParam("tablesize", false, "Size of table to store source ip 
addresses. example: tablesize=200k or 300m" + " or 400g. Default value:200k", 
false);
         method.addParam("expire", false, "Entry in source ip table will expire 
after expire duration. units can be s,m,h,d ."
-            + " example: expire=30m 20s 50h 4d. Default value:3h", false);
+                + " example: expire=30m 20s 50h 4d. Default value:3h", false);
         methodList.add(method);
 
         Gson gson = new Gson();
@@ -622,7 +619,7 @@ public class VirtualRouterElement extends AdapterBase 
implements VirtualRouterEl
             List<DomainRouterVO> routers = 
_routerDao.listByNetworkAndRole(config.getId(), Role.VIRTUAL_ROUTER);
             if (routers == null || routers.isEmpty()) {
                 s_logger.debug("Virtual router elemnt doesn't need to apply 
static nat on the backend; virtual " + "router doesn't exist in the network " +
-                    config.getId());
+                        config.getId());
                 return true;
             }
 
@@ -749,7 +746,7 @@ public class VirtualRouterElement extends AdapterBase 
implements VirtualRouterEl
         OvsProviderVO element = _ovsProviderDao.findById(cmd.getId());
         if (element == null) {
             s_logger.debug("Can't find Ovs element with network service 
provider id "
-                + cmd.getId());
+                    + cmd.getId());
             return null;
         }
 
@@ -763,7 +760,7 @@ public class VirtualRouterElement extends AdapterBase 
implements VirtualRouterEl
     public VirtualRouterProvider addElement(Long nspId, Type providerType) {
         if (!(providerType == Type.VirtualRouter || providerType == 
Type.VPCVirtualRouter)) {
             throw new InvalidParameterValueException("Element " + getName() + 
" supports only providerTypes: " + Type.VirtualRouter.toString() + " and " +
-                Type.VPCVirtualRouter);
+                    Type.VPCVirtualRouter);
         }
         VirtualRouterProviderVO element = 
_vrProviderDao.findByNspIdAndType(nspId, providerType);
         if (element != null) {
@@ -781,7 +778,7 @@ public class VirtualRouterElement extends AdapterBase 
implements VirtualRouterEl
             List<DomainRouterVO> routers = 
_routerDao.listByNetworkAndRole(network.getId(), Role.VIRTUAL_ROUTER);
             if (routers == null || routers.isEmpty()) {
                 s_logger.debug("Virtual router elemnt doesn't need to apply 
firewall rules on the backend; virtual " + "router doesn't exist in the network 
" +
-                    network.getId());
+                        network.getId());
                 return true;
             }
 
@@ -806,7 +803,7 @@ public class VirtualRouterElement extends AdapterBase 
implements VirtualRouterEl
 
     @Override
     public boolean shutdownProviderInstances(PhysicalNetworkServiceProvider 
provider, ReservationContext context) throws ConcurrentOperationException,
-        ResourceUnavailableException {
+    ResourceUnavailableException {
         VirtualRouterProviderVO element = 
_vrProviderDao.findByNspIdAndType(provider.getId(), getVirtualRouterProvider());
         if (element == null) {
             return true;
@@ -844,13 +841,13 @@ public class VirtualRouterElement extends AdapterBase 
implements VirtualRouterEl
 
     @Override
     public boolean release(Network network, NicProfile nic, 
VirtualMachineProfile vm, ReservationContext context) throws 
ConcurrentOperationException,
-        ResourceUnavailableException {
+    ResourceUnavailableException {
         return true;
     }
 
     @Override
     public boolean configDhcpSupportForSubnet(Network network, NicProfile nic, 
VirtualMachineProfile vm, DeployDestination dest, ReservationContext context)
-        throws ConcurrentOperationException, InsufficientCapacityException, 
ResourceUnavailableException {
+            throws ConcurrentOperationException, 
InsufficientCapacityException, ResourceUnavailableException {
         if (canHandle(network, Service.Dhcp)) {
             if (vm.getType() != VirtualMachine.Type.User) {
                 return false;
@@ -887,7 +884,7 @@ public class VirtualRouterElement extends AdapterBase 
implements VirtualRouterEl
 
     @Override
     public boolean addDhcpEntry(Network network, NicProfile nic, 
VirtualMachineProfile vm, DeployDestination dest, ReservationContext context)
-        throws ConcurrentOperationException, InsufficientCapacityException, 
ResourceUnavailableException {
+            throws ConcurrentOperationException, 
InsufficientCapacityException, ResourceUnavailableException {
         if (canHandle(network, Service.Dhcp)) {
             if (vm.getType() != VirtualMachine.Type.User) {
                 return false;
@@ -909,7 +906,7 @@ public class VirtualRouterElement extends AdapterBase 
implements VirtualRouterEl
 
     @Override
     public boolean addPasswordAndUserdata(Network network, NicProfile nic, 
VirtualMachineProfile vm, DeployDestination dest, ReservationContext context)
-        throws ConcurrentOperationException, InsufficientCapacityException, 
ResourceUnavailableException {
+            throws ConcurrentOperationException, 
InsufficientCapacityException, ResourceUnavailableException {
         if (canHandle(network, Service.UserData)) {
             if (vm.getType() != VirtualMachine.Type.User) {
                 return false;
@@ -940,7 +937,7 @@ public class VirtualRouterElement extends AdapterBase 
implements VirtualRouterEl
             publicNetwork = true;
         }
         boolean isPodBased =
-            (dest.getDataCenter().getNetworkType() == NetworkType.Basic || 
_networkMgr.isSecurityGroupSupportedInNetwork(network)) &&
+                (dest.getDataCenter().getNetworkType() == NetworkType.Basic || 
_networkMgr.isSecurityGroupSupportedInNetwork(network)) &&
                 network.getTrafficType() == TrafficType.Guest;
 
         List<DomainRouterVO> routers;
@@ -1118,12 +1115,7 @@ public class VirtualRouterElement extends AdapterBase 
implements VirtualRouterEl
 
     @Override
     public boolean cleanupAggregatedExecution(Network network, 
DeployDestination dest) throws ResourceUnavailableException {
-        List<DomainRouterVO> routers = getRouters(network, dest);
-
-        if ((routers == null) || (routers.size() == 0)) {
-            throw new ResourceUnavailableException("Can't find at least one 
router!", DataCenter.class, network.getDataCenterId());
-        }
-
-        return _routerMgr.cleanupAggregatedExecution(network, routers);
+        // The VR code already cleansup in the Finish routine using finally, 
lets not waste another command
+        return true;
     }
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/dbc7d803/server/src/com/cloud/network/router/VirtualNetworkApplianceManager.java
----------------------------------------------------------------------
diff --git 
a/server/src/com/cloud/network/router/VirtualNetworkApplianceManager.java 
b/server/src/com/cloud/network/router/VirtualNetworkApplianceManager.java
index 58ffb2f..85ce8b9 100644
--- a/server/src/com/cloud/network/router/VirtualNetworkApplianceManager.java
+++ b/server/src/com/cloud/network/router/VirtualNetworkApplianceManager.java
@@ -16,6 +16,11 @@
 // under the License.
 package com.cloud.network.router;
 
+import java.util.List;
+import java.util.Map;
+
+import org.apache.cloudstack.framework.config.ConfigKey;
+
 import com.cloud.deploy.DeployDestination;
 import com.cloud.exception.AgentUnavailableException;
 import com.cloud.exception.ConcurrentOperationException;
@@ -35,10 +40,6 @@ import com.cloud.utils.component.Manager;
 import com.cloud.vm.DomainRouterVO;
 import com.cloud.vm.NicProfile;
 import com.cloud.vm.VirtualMachineProfile;
-import org.apache.cloudstack.framework.config.ConfigKey;
-
-import java.util.List;
-import java.util.Map;
 
 /**
  * NetworkManager manages the network for the different end users.
@@ -137,6 +138,4 @@ public interface VirtualNetworkApplianceManager extends 
Manager, VirtualNetworkA
     public boolean prepareAggregatedExecution(Network network, 
List<DomainRouterVO> routers) throws AgentUnavailableException;
 
     public boolean completeAggregatedExecution(Network network, 
List<DomainRouterVO> routers) throws AgentUnavailableException;
-
-    public boolean cleanupAggregatedExecution(Network network, 
List<DomainRouterVO> routers) throws AgentUnavailableException;
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/dbc7d803/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java
----------------------------------------------------------------------
diff --git 
a/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java 
b/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java
index d8e3761..33d7cd7 100755
--- 
a/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java
+++ 
b/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java
@@ -4464,8 +4464,4 @@ VirtualMachineGuru, Listener, Configurable, 
StateListener<State, VirtualMachine.
         return aggregationExecution(Action.Finish, network, routers);
     }
 
-    @Override
-    public boolean cleanupAggregatedExecution(Network network, 
List<DomainRouterVO> routers) throws AgentUnavailableException {
-        return aggregationExecution(Action.Cleanup, network, routers);
-    }
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/dbc7d803/server/test/com/cloud/vpc/MockVpcVirtualNetworkApplianceManager.java
----------------------------------------------------------------------
diff --git 
a/server/test/com/cloud/vpc/MockVpcVirtualNetworkApplianceManager.java 
b/server/test/com/cloud/vpc/MockVpcVirtualNetworkApplianceManager.java
index cbed4ca..7a98a6d 100644
--- a/server/test/com/cloud/vpc/MockVpcVirtualNetworkApplianceManager.java
+++ b/server/test/com/cloud/vpc/MockVpcVirtualNetworkApplianceManager.java
@@ -17,6 +17,17 @@
 
 package com.cloud.vpc;
 
+import java.util.List;
+import java.util.Map;
+
+import javax.ejb.Local;
+import javax.naming.ConfigurationException;
+
+import org.springframework.stereotype.Component;
+
+import org.apache.cloudstack.api.command.admin.router.UpgradeRouterCmd;
+import org.apache.cloudstack.api.command.admin.router.UpgradeRouterTemplateCmd;
+
 import com.cloud.deploy.DeployDestination;
 import com.cloud.exception.AgentUnavailableException;
 import com.cloud.exception.ConcurrentOperationException;
@@ -44,14 +55,6 @@ import com.cloud.vm.DomainRouterVO;
 import com.cloud.vm.NicProfile;
 import com.cloud.vm.VirtualMachineProfile;
 import com.cloud.vm.VirtualMachineProfile.Param;
-import org.apache.cloudstack.api.command.admin.router.UpgradeRouterCmd;
-import org.apache.cloudstack.api.command.admin.router.UpgradeRouterTemplateCmd;
-import org.springframework.stereotype.Component;
-
-import javax.ejb.Local;
-import javax.naming.ConfigurationException;
-import java.util.List;
-import java.util.Map;
 
 @Component
 @Local(value = {VpcVirtualNetworkApplianceManager.class, 
VpcVirtualNetworkApplianceService.class})
@@ -429,11 +432,6 @@ public class MockVpcVirtualNetworkApplianceManager extends 
ManagerBase implement
     }
 
     @Override
-    public boolean cleanupAggregatedExecution(Network network, 
List<DomainRouterVO> routers) throws AgentUnavailableException {
-        return true;  //To change body of implemented methods use File | 
Settings | File Templates.
-    }
-
-    @Override
     public boolean startRemoteAccessVpn(RemoteAccessVpn vpn, VirtualRouter 
router) throws ResourceUnavailableException {
         // TODO Auto-generated method stub
         return false;

Reply via email to