Applygin fix from commit ID aaeadc5c44e3fe16a1deea5348b085b08b5f4f4d

Sheng Yang changed 2 classes, ut only one was related to the bug CLOUDSTACK-7605.
I applied the changed on the routerslist, used during the deployment of the virtual routers.

Tested Advanced Zone against the simulator. 69 happy tests in place
This commit is contained in:
Wilder Rodrigues 2014-09-30 14:18:52 +02:00 committed by wilderrodrigues
parent c81b3380df
commit db86bdfb2c
3 changed files with 125 additions and 137 deletions

View File

@ -100,8 +100,7 @@ public class RouterDeploymentDefinition {
protected boolean isPublicNetwork;
protected PublicIp sourceNatIp;
protected RouterDeploymentDefinition(final Network guestNetwork, final DeployDestination dest,
final Account owner, final Map<Param, Object> params, final boolean isRedundant) {
protected RouterDeploymentDefinition(final Network guestNetwork, final DeployDestination dest, final Account owner, final Map<Param, Object> params, final boolean isRedundant) {
this.guestNetwork = guestNetwork;
this.dest = dest;
@ -111,98 +110,103 @@ public class RouterDeploymentDefinition {
}
public Long getOfferingId() {
return this.offeringId;
return offeringId;
}
public Vpc getVpc() {
return null;
}
public Network getGuestNetwork() {
return guestNetwork;
}
public DeployDestination getDest() {
return dest;
}
public Account getOwner() {
return owner;
}
public Map<Param, Object> getParams() {
return params;
}
public boolean isRedundant() {
return isRedundant;
}
public DeploymentPlan getPlan() {
return plan;
}
public boolean isVpcRouter() {
return false;
}
public Pod getPod() {
return dest.getPod();
}
public Long getPodId() {
return dest.getPod() == null ? null : dest.getPod().getId();
}
public List<DomainRouterVO> getRouters() {
return routers;
}
public VirtualRouterProvider getVirtualProvider() {
return this.vrProvider;
return vrProvider;
}
public boolean isBasic() {
return this.dest.getDataCenter().getNetworkType() == NetworkType.Basic;
return dest.getDataCenter().getNetworkType() == NetworkType.Basic;
}
public boolean isPublicNetwork() {
return this.isPublicNetwork;
return isPublicNetwork;
}
public PublicIp getSourceNatIP() {
return this.sourceNatIp;
return sourceNatIp;
}
protected void generateDeploymentPlan() {
final long dcId = this.dest.getDataCenter().getId();
final long dcId = dest.getDataCenter().getId();
Long podId = null;
if (this.isBasic()) {
if (this.dest.getPod() == null) {
if (isBasic()) {
if (dest.getPod() == null) {
throw new CloudRuntimeException("Pod id is expected in deployment destination");
}
podId = this.dest.getPod().getId();
podId = dest.getPod().getId();
}
this.plan = new DataCenterDeployment(dcId, podId, null, null, null, null);
plan = new DataCenterDeployment(dcId, podId, null, null, null, null);
}
public List<DomainRouterVO> deployVirtualRouter()
throws InsufficientCapacityException,
ConcurrentOperationException, ResourceUnavailableException {
public List<DomainRouterVO> deployVirtualRouter() throws InsufficientCapacityException, ConcurrentOperationException, ResourceUnavailableException {
this.findOrDeployVirtualRouter();
findOrDeployVirtualRouter();
return nwHelper.startRouters(this);
}
@DB
protected void findOrDeployVirtualRouter()
throws ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException {
protected void findOrDeployVirtualRouter() throws ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException {
try {
this.lock();
this.checkPreconditions();
lock();
checkPreconditions();
// dest has pod=null, for Basic Zone findOrDeployVRs for all Pods
final List<DeployDestination> destinations = findDestinations();
for (final DeployDestination destination : destinations) {
this.dest = destination;
this.planDeploymentRouters();
this.generateDeploymentPlan();
this.executeDeployment();
dest = destination;
generateDeploymentPlan();
executeDeployment();
}
} finally {
this.unlock();
unlock();
}
}
@ -211,30 +215,25 @@ public class RouterDeploymentDefinition {
if (lock == null) {
throw new ConcurrentOperationException("Unable to lock network " + guestNetwork.getId());
}
this.tableLockId = lock.getId();
tableLockId = lock.getId();
}
protected void unlock() {
if (this.tableLockId != null) {
networkDao.releaseFromLockTable(this.tableLockId);
if (tableLockId != null) {
networkDao.releaseFromLockTable(tableLockId);
if (logger.isDebugEnabled()) {
logger.debug("Lock is released for network id " + this.tableLockId
+ " as a part of router startup in " + dest);
logger.debug("Lock is released for network id " + tableLockId + " as a part of router startup in " + dest);
}
}
}
protected void checkPreconditions() throws ResourceUnavailableException {
if (guestNetwork.getState() != Network.State.Implemented &&
guestNetwork.getState() != Network.State.Setup &&
guestNetwork.getState() != Network.State.Implementing) {
throw new ResourceUnavailableException("Network is not yet fully implemented: " + guestNetwork,
Network.class, this.guestNetwork.getId());
if (guestNetwork.getState() != Network.State.Implemented && guestNetwork.getState() != Network.State.Setup && guestNetwork.getState() != Network.State.Implementing) {
throw new ResourceUnavailableException("Network is not yet fully implemented: " + guestNetwork, Network.class, guestNetwork.getId());
}
if (guestNetwork.getTrafficType() != TrafficType.Guest) {
throw new ResourceUnavailableException("Network is not type Guest as expected: " + guestNetwork,
Network.class, this.guestNetwork.getId());
throw new ResourceUnavailableException("Network is not type Guest as expected: " + guestNetwork, Network.class, guestNetwork.getId());
}
}
@ -242,8 +241,9 @@ public class RouterDeploymentDefinition {
// dest has pod=null, for Basic Zone findOrDeployVRs for all Pods
final List<DeployDestination> destinations = new ArrayList<DeployDestination>();
// for basic zone, if 'dest' has pod set to null then this is network restart scenario otherwise it is a vm deployment scenario
if (this.isBasic() && dest.getPod() == null) {
// for basic zone, if 'dest' has pod set to null then this is network
// restart scenario otherwise it is a vm deployment scenario
if (isBasic() && dest.getPod() == null) {
// Find all pods in the data center with running or starting user vms
final long dcId = dest.getDataCenter().getId();
final List<HostPodVO> pods = listByDataCenterIdVMTypeAndStates(dcId, VirtualMachine.Type.User, VirtualMachine.State.Starting, VirtualMachine.State.Running);
@ -261,7 +261,7 @@ public class RouterDeploymentDefinition {
// Add virtualRouters to the routers, this avoids the situation when
// all routers are skipped and VirtualRouterElement throws exception
this.routers.addAll(virtualRouters);
routers.addAll(virtualRouters);
// If List size is one, we already have a starting or running VR, skip deployment
if (virtualRouters.size() == 1) {
@ -280,43 +280,42 @@ public class RouterDeploymentDefinition {
protected int getNumberOfRoutersToDeploy() {
// TODO Are we sure this makes sense? Somebody said 5 was too many?
if (this.routers.size() >= 5) {
if (routers.size() >= 5) {
logger.error("Too many redundant routers!");
}
// If old network is redundant but new is single router, then routers.size() = 2 but routerCount = 1
// If old network is redundant but new is single router, then
// routers.size() = 2 but routerCount = 1
int routersExpected = 1;
if (this.isRedundant) {
if (isRedundant) {
routersExpected = 2;
}
return routersExpected < this.routers.size() ?
0 : routersExpected - this.routers.size();
return routersExpected < routers.size() ? 0 : routersExpected - routers.size();
}
protected void setupAccountOwner() {
if (networkModel.isNetworkSystem(guestNetwork) || guestNetwork.getGuestType() == Network.GuestType.Shared) {
this.owner = accountMgr.getAccount(Account.ACCOUNT_ID_SYSTEM);
owner = accountMgr.getAccount(Account.ACCOUNT_ID_SYSTEM);
}
}
/**
* It executes last pending tasks to prepare the deployment and checks the deployment
* can proceed. If it can't it return false
* It executes last pending tasks to prepare the deployment and checks the
* deployment can proceed. If it can't it return false
*
* @return if the deployment can proceed
*/
protected boolean prepareDeployment() {
this.setupAccountOwner();
setupAccountOwner();
// Check if public network has to be set on VR
this.isPublicNetwork = networkModel.isProviderSupportServiceInNetwork(
guestNetwork.getId(), Service.SourceNat, Provider.VirtualRouter);
isPublicNetwork = networkModel.isProviderSupportServiceInNetwork(guestNetwork.getId(), Service.SourceNat, Provider.VirtualRouter);
boolean canProceed = true;
if (this.isRedundant && !this.isPublicNetwork) {
if (isRedundant && !isPublicNetwork) {
// TODO Shouldn't be this throw an exception instead of log error and empty list of routers
logger.error("Didn't support redundant virtual router without public network!");
this.routers = new ArrayList<>();
routers = new ArrayList<DomainRouterVO>();
canProceed = false;
}
@ -324,39 +323,39 @@ public class RouterDeploymentDefinition {
}
/**
* Executes preparation and deployment of the routers. After this method ends, {@link this#routers}
* should have all of the deployed routers ready for start, and no more.
* Executes preparation and deployment of the routers. After this method
* ends, {@link this#routers} should have all of the deployed routers ready
* for start, and no more.
*
* @throws ConcurrentOperationException
* @throws InsufficientCapacityException
* @throws ResourceUnavailableException
*/
protected void executeDeployment()
throws ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException {
protected void executeDeployment() throws ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException {
// Check current redundant routers, if possible(all routers are
// stopped), reset the priority
planDeploymentRouters();
setupPriorityOfRedundantRouter();
//Check current redundant routers, if possible(all routers are stopped), reset the priority
this.setupPriorityOfRedundantRouter();
if (this.getNumberOfRoutersToDeploy() > 0 && this.prepareDeployment()) {
this.findVirtualProvider();
this.findOfferingId();
this.findSourceNatIP();
this.deployAllVirtualRouters();
if (getNumberOfRoutersToDeploy() > 0 && prepareDeployment()) {
findVirtualProvider();
findOfferingId();
findSourceNatIP();
deployAllVirtualRouters();
}
}
protected void findSourceNatIP() throws InsufficientAddressCapacityException, ConcurrentOperationException {
this.sourceNatIp = null;
if (this.isPublicNetwork) {
this.sourceNatIp = this.ipAddrMgr.assignSourceNatIpAddressToGuestNetwork(
this.owner,this.guestNetwork);
sourceNatIp = null;
if (isPublicNetwork) {
sourceNatIp = ipAddrMgr.assignSourceNatIpAddressToGuestNetwork(owner, guestNetwork);
}
}
protected void findOfferingId() {
Long networkOfferingId = networkOfferingDao.findById(guestNetwork.getNetworkOfferingId()).getServiceOfferingId();
if (networkOfferingId != null) {
this.offeringId = networkOfferingId;
offeringId = networkOfferingId;
}
}
@ -364,42 +363,36 @@ public class RouterDeploymentDefinition {
// Check if providers are supported in the physical networks
final Type type = Type.VirtualRouter;
final Long physicalNetworkId = networkModel.getPhysicalNetworkId(guestNetwork);
final PhysicalNetworkServiceProvider provider =
physicalProviderDao.findByServiceProvider(physicalNetworkId, type.toString());
final PhysicalNetworkServiceProvider provider = physicalProviderDao.findByServiceProvider(physicalNetworkId, type.toString());
if (provider == null) {
throw new CloudRuntimeException(
String.format("Cannot find service provider %s in physical network %s",
type.toString(), physicalNetworkId));
throw new CloudRuntimeException(String.format("Cannot find service provider %s in physical network %s", type.toString(), physicalNetworkId));
}
this.vrProvider = vrProviderDao.findByNspIdAndType(provider.getId(), type);
if (this.vrProvider == null) {
throw new CloudRuntimeException(
String.format("Cannot find virtual router provider %s as service provider %s",
type.toString(), provider.getId()));
vrProvider = vrProviderDao.findByNspIdAndType(provider.getId(), type);
if (vrProvider == null) {
throw new CloudRuntimeException(String.format("Cannot find virtual router provider %s as service provider %s", type.toString(), provider.getId()));
}
}
protected void deployAllVirtualRouters()
throws ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException {
int routersToDeploy = this.getNumberOfRoutersToDeploy();
for(int i = 0; i < routersToDeploy; i++) {
// Don't start the router as we are holding the network lock that needs to be released at the end of router allocation
DomainRouterVO router = this.nwHelper.deployRouter(this, false);
protected void deployAllVirtualRouters() throws ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException {
int routersToDeploy = getNumberOfRoutersToDeploy();
for (int i = 0; i < routersToDeploy; i++) {
// Don't start the router as we are holding the network lock that
// needs to be released at the end of router allocation
DomainRouterVO router = nwHelper.deployRouter(this, false);
if (router != null) {
this.routerDao.addRouterToGuestNetwork(router, this.guestNetwork);
this.routers.add(router);
routerDao.addRouterToGuestNetwork(router, guestNetwork);
//Fix according to changes by Sheng Yang in commit ID cb4513379996b262ae378daf00c6388c6b7313cf
routers.add(router);
}
}
}
/**
* Lists all pods given a Data Center Id, a {@link VirtualMachine.Type} and a list of
* {@link VirtualMachine.State}
*
* Lists all pods given a Data Center Id, a {@link VirtualMachine.Type} and
* a list of {@link VirtualMachine.State}
* @param id
* @param type
* @param states
@ -419,28 +412,25 @@ public class RouterDeploymentDefinition {
final SearchCriteria<HostPodVO> sc = podIdSearch.create();
sc.setParameters("dc", id);
sc.setJoinParameters("vmInstanceSearch", "type", type);
sc.setJoinParameters("vmInstanceSearch", "states", (Object[])states);
sc.setJoinParameters("vmInstanceSearch", "states", (Object[]) states);
return podDao.search(sc, null);
}
protected void planDeploymentRouters() {
if (this.isBasic()) {
this.routers = routerDao.listByNetworkAndPodAndRole(this.guestNetwork.getId(),
this.getPodId(), Role.VIRTUAL_ROUTER);
if (isBasic()) {
routers.addAll(routerDao.listByNetworkAndPodAndRole(guestNetwork.getId(), getPodId(), Role.VIRTUAL_ROUTER));
} else {
this.routers = routerDao.listByNetworkAndRole(this.guestNetwork.getId(),
Role.VIRTUAL_ROUTER);
routers.addAll(routerDao.listByNetworkAndRole(guestNetwork.getId(), Role.VIRTUAL_ROUTER));
}
}
/**
* Routers need reset if at least one of the routers is not redundant or stopped.
*
* @return
* Routers need reset if at least one of the routers is not redundant or
* stopped.
*/
protected boolean routersNeedReset() {
boolean needReset = true;
for (final DomainRouterVO router : this.routers) {
for (final DomainRouterVO router : routers) {
if (!router.getIsRedundantRouter() || router.getState() != VirtualMachine.State.Stopped) {
needReset = false;
break;
@ -451,18 +441,17 @@ public class RouterDeploymentDefinition {
}
/**
* Only for redundant deployment and if any routers needed reset, we shall reset all
* routers priorities
* Only for redundant deployment and if any routers needed reset, we shall
* reset all routers priorities
*/
protected void setupPriorityOfRedundantRouter() {
if (this.isRedundant && this.routersNeedReset()) {
for (final DomainRouterVO router : this.routers) {
// getUpdatedPriority() would update the value later
router.setPriority(0);
router.setIsPriorityBumpUp(false);
routerDao.update(router.getId(), router);
}
if (isRedundant && routersNeedReset()) {
for (final DomainRouterVO router : routers) {
// getUpdatedPriority() would update the value later
router.setPriority(0);
router.setIsPriorityBumpUp(false);
routerDao.update(router.getId(), router);
}
}
}
}

View File

@ -53,9 +53,7 @@ public class VpcRouterDeploymentDefinition extends RouterDeploymentDefinition {
protected Vpc vpc;
protected VpcRouterDeploymentDefinition(final Vpc vpc, final DeployDestination dest, final Account owner,
final Map<Param, Object> params, final boolean isRedundant) {
protected VpcRouterDeploymentDefinition(final Vpc vpc, final DeployDestination dest, final Account owner, final Map<Param, Object> params, final boolean isRedundant) {
super(null, dest, owner, params, isRedundant);
@ -64,7 +62,7 @@ public class VpcRouterDeploymentDefinition extends RouterDeploymentDefinition {
@Override
public Vpc getVpc() {
return this.vpc;
return vpc;
}
@Override
@ -83,16 +81,15 @@ public class VpcRouterDeploymentDefinition extends RouterDeploymentDefinition {
if (vpcLock == null) {
throw new ConcurrentOperationException("Unable to lock vpc " + vpc.getId());
}
this.tableLockId = vpcLock.getId();
tableLockId = vpcLock.getId();
}
@Override
protected void unlock() {
if (this.tableLockId != null) {
vpcDao.releaseFromLockTable(this.tableLockId);
if (tableLockId != null) {
vpcDao.releaseFromLockTable(tableLockId);
if (logger.isDebugEnabled()) {
logger.debug("Lock is released for vpc id " + this.tableLockId
+ " as a part of router startup in " + dest);
logger.debug("Lock is released for vpc id " + tableLockId + " as a part of router startup in " + dest);
}
}
}
@ -105,14 +102,15 @@ public class VpcRouterDeploymentDefinition extends RouterDeploymentDefinition {
@Override
protected List<DeployDestination> findDestinations() {
final List<DeployDestination> destinations = new ArrayList<>();
destinations.add(this.dest);
destinations.add(dest);
return destinations;
}
@Override
protected int getNumberOfRoutersToDeploy() {
// TODO Should we make our changes here in order to enable Redundant Router for VPC?
return this.routers.isEmpty() ? 1 : 0;
// TODO Should we make our changes here in order to enable Redundant
// Router for VPC?
return routers.isEmpty() ? 1 : 0;
}
/**
@ -128,12 +126,13 @@ public class VpcRouterDeploymentDefinition extends RouterDeploymentDefinition {
@Override
protected void setupPriorityOfRedundantRouter() {
// Nothing to do for now
// TODO Shouldn't we add this behavior once Redundant Router works for Vpc too
// TODO Shouldn't we add this behavior once Redundant Router works for
// Vpc too
}
@Override
protected void findSourceNatIP() throws InsufficientAddressCapacityException, ConcurrentOperationException {
this.sourceNatIp = vpcMgr.assignSourceNatIpAddressToVpc(this.owner, vpc);
sourceNatIp = vpcMgr.assignSourceNatIpAddressToVpc(owner, vpc);
}
@Override
@ -145,8 +144,8 @@ public class VpcRouterDeploymentDefinition extends RouterDeploymentDefinition {
if (provider == null) {
throw new CloudRuntimeException("Cannot find service provider " + Type.VPCVirtualRouter.toString() + " in physical network " + pNtwk.getId());
}
this.vrProvider = vrProviderDao.findByNspIdAndType(provider.getId(), Type.VPCVirtualRouter);
if (this.vrProvider != null) {
vrProvider = vrProviderDao.findByNspIdAndType(provider.getId(), Type.VPCVirtualRouter);
if (vrProvider != null) {
break;
}
}
@ -156,28 +155,28 @@ public class VpcRouterDeploymentDefinition extends RouterDeploymentDefinition {
protected void findOfferingId() {
Long vpcOfferingId = vpcOffDao.findById(vpc.getVpcOfferingId()).getServiceOfferingId();
if (vpcOfferingId != null) {
this.offeringId = vpcOfferingId;
offeringId = vpcOfferingId;
}
}
@Override
protected void deployAllVirtualRouters()
throws ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException {
protected void deployAllVirtualRouters() throws ConcurrentOperationException, InsufficientCapacityException,
ResourceUnavailableException {
DomainRouterVO router = this.nwHelper.deployRouter(this, true);
DomainRouterVO router = nwHelper.deployRouter(this, true);
if (router != null) {
this.routers.add(router);
routers.add(router);
}
}
@Override
protected void planDeploymentRouters() {
this.routers = this.routerDao.listByVpcId(this.vpc.getId());
routers = routerDao.listByVpcId(vpc.getId());
}
@Override
protected void generateDeploymentPlan() {
this.plan = new DataCenterDeployment(this.dest.getDataCenter().getId());
plan = new DataCenterDeployment(dest.getDataCenter().getId());
}
}

View File

@ -525,9 +525,9 @@ public class RouterDeploymentDefinitionTest extends RouterDeploymentDefinitionTe
verify(deploymentUT, times(1)).lock();
verify(deploymentUT, times(1)).checkPreconditions();
verify(deploymentUT, times(1)).findDestinations();
verify(deploymentUT, times(2)).planDeploymentRouters();
verify(deploymentUT, times(2)).generateDeploymentPlan();
verify(deploymentUT, times(2)).executeDeployment();
//verify(deploymentUT, times(2)).planDeploymentRouters();
verify(deploymentUT, times(1)).unlock();
}