Applied, thanks. --Sheng
On Tue, Jun 19, 2012 at 12:05 PM, Vijayendra Bhamidipati <vijayendra.bhamidip...@citrix.com> wrote: > Have attached the patch to bug CS-15173 (patch file is > 0001-CS-15173-Additional-Cluster-is-allowed-to-add-with-t.patch). > > Regards, > Vijay > > -----Original Message----- > From: Vijayendra Bhamidipati [mailto:vijayendra.bhamidip...@citrix.com] > Sent: Monday, June 18, 2012 5:28 PM > To: cloudstack-dev@incubator.apache.org > Cc: Alena Prokharchyk; Kelven Yang; Chiradeep Vittal; Sheng Yang > Subject: Patch for review and merge into Apache master: Bug CS-15173 > > Hi Team, > > Please find attached the ASF master git patch for Bug CS-15173 > (http://bugs.cloudstack.org/browse/CS-15173 ). Also, pasting the contents of > the patch below for quicker review. > > Regards, > Vijay > > > > From 89753db2ef5411e81de6c28d57794d8acda87efd Mon Sep 17 00:00:00 2001 > From: Vijayendra Bhamidipati <vijayendra.bhamidip...@citrix.com> > Date: Mon, 18 Jun 2012 16:41:06 -0700 > Subject: [PATCH] CS-15173: Additional Cluster is allowed to add with the same > VSM IPaddress as the previous cluster > > Description: > > Restricting association of a Cisco Nexus VSM to a single > cluster. > --- > api/src/com/cloud/api/commands/AddClusterCmd.java | 11 +++- > api/src/com/cloud/resource/ResourceService.java | 3 +- > .../com/cloud/resource/ResourceManagerImpl.java | 73 > +++++++++++++------- > .../hypervisor/vmware/mo/HypervisorHostHelper.java | 10 +++- > 4 files changed, 68 insertions(+), 29 deletions(-) > > diff --git a/api/src/com/cloud/api/commands/AddClusterCmd.java > b/api/src/com/cloud/api/commands/AddClusterCmd.java > index 6688dee..39fa63d 100755 > --- a/api/src/com/cloud/api/commands/AddClusterCmd.java > +++ b/api/src/com/cloud/api/commands/AddClusterCmd.java > @@ -31,8 +31,10 @@ import com.cloud.api.ServerApiException; import > com.cloud.api.response.ClusterResponse; > import com.cloud.api.response.ListResponse; > import com.cloud.exception.DiscoveryException; > +import com.cloud.exception.ResourceInUseException; > import com.cloud.org.Cluster; > import com.cloud.user.Account; > +import com.cloud.utils.IdentityProxy; > ^M > @Implementation(description="Adds a new cluster", > responseObject=ClusterResponse.class)^M > public class AddClusterCmd extends BaseCmd {^M @@ -166,7 +168,14 @@ public > class AddClusterCmd extends BaseCmd { > this.setResponseObject(response);^M > } catch (DiscoveryException ex) {^M > s_logger.warn("Exception: ", ex);^M > - throw new ServerApiException(BaseCmd.INTERNAL_ERROR, > ex.getMessage());^M > + throw new ServerApiException(BaseCmd.INTERNAL_ERROR, > ex.getMessage()); > + } catch (ResourceInUseException ex) { > + s_logger.warn("Exception: ", ex); > + ServerApiException e = new > ServerApiException(BaseCmd.INTERNAL_ERROR, ex.getMessage()); > + for (IdentityProxy proxyObj : ex.getIdProxyList()) { > + e.addProxyObject(proxyObj.getTableName(), > proxyObj.getValue(), proxyObj.getidFieldName()); > + } > + throw e; > }^M > }^M > } > diff --git a/api/src/com/cloud/resource/ResourceService.java > b/api/src/com/cloud/resource/ResourceService.java > index d971a40..1065453 100755 > --- a/api/src/com/cloud/resource/ResourceService.java > +++ b/api/src/com/cloud/resource/ResourceService.java > @@ -31,6 +31,7 @@ import com.cloud.api.commands.UpdateHostCmd; > import com.cloud.api.commands.UpdateHostPasswordCmd; > import com.cloud.exception.DiscoveryException; > import com.cloud.exception.InvalidParameterValueException; > +import com.cloud.exception.ResourceInUseException; > import com.cloud.host.Host; > import com.cloud.hypervisor.Hypervisor.HypervisorType; > import com.cloud.org.Cluster; > @@ -61,7 +62,7 @@ public interface ResourceService { > * @throws IllegalArgumentException > * @throws DiscoveryException > */ > - List<? extends Cluster> discoverCluster(AddClusterCmd cmd) throws > IllegalArgumentException, DiscoveryException; > + List<? extends Cluster> discoverCluster(AddClusterCmd cmd) throws > + IllegalArgumentException, DiscoveryException, ResourceInUseException; > > boolean deleteCluster(DeleteClusterCmd cmd); > > diff --git a/server/src/com/cloud/resource/ResourceManagerImpl.java > b/server/src/com/cloud/resource/ResourceManagerImpl.java > index 3380972..357dca3 100755 > --- a/server/src/com/cloud/resource/ResourceManagerImpl.java > +++ b/server/src/com/cloud/resource/ResourceManagerImpl.java > @@ -79,6 +79,7 @@ import com.cloud.exception.AgentUnavailableException; > import com.cloud.exception.DiscoveryException; > import com.cloud.exception.InvalidParameterValueException; > import com.cloud.exception.PermissionDeniedException; > +import com.cloud.exception.ResourceInUseException; > import com.cloud.ha.HighAvailabilityManager; > import com.cloud.ha.HighAvailabilityManager.WorkType; > import com.cloud.host.DetailVO; > @@ -324,7 +325,7 @@ public class ResourceManagerImpl implements > ResourceManager, ResourceService, Ma > > @DB > @Override > - public List<? extends Cluster> discoverCluster(AddClusterCmd cmd) throws > IllegalArgumentException, DiscoveryException { > + public List<? extends Cluster> discoverCluster(AddClusterCmd cmd) > + throws IllegalArgumentException, DiscoveryException, > + ResourceInUseException { > long dcId = cmd.getZoneId(); > long podId = cmd.getPodId(); > String clusterName = cmd.getClusterName(); @@ -433,6 +434,7 @@ public > class ResourceManagerImpl implements ResourceManager, ResourceService, Ma > clusterId = cluster.getId(); > result.add(cluster); > > + // Check if we're associating a Cisco Nexus VSM with a vmware > cluster. > if (hypervisorType == HypervisorType.VMware && > > Boolean.parseBoolean(_configDao.getValue(Config.VmwareUseNexusVSwitch.toString()))) > { > String vsmIp = cmd.getVSMIpaddress(); @@ -450,33 +452,52 @@ > public class ResourceManagerImpl implements ResourceManager, ResourceService, > Ma > _clusterDao.remove(clusterId); > throw new CloudRuntimeException(msg); > } > - // persist credentials to database > - CiscoNexusVSMDeviceVO vsm = new > CiscoNexusVSMDeviceVO(vsmIp, vsmUser, vsmPassword); > > - Transaction txn = Transaction.currentTxn(); > - try { > - txn.start(); > - vsm = _vsmDao.persist(vsm); > - txn.commit(); > - } catch (Exception e) { > - txn.rollback(); > - s_logger.error("Failed to persist Cisco Nexus 1000v > VSM details to database. Exception: " + e.getMessage()); > - // Removing the cluster record which was added > already because the persistence of Nexus VSM credentials has failed. > - _clusterDao.remove(clusterId); > - throw new CloudRuntimeException(e.getMessage()); > - } > + Transaction txn; > > - ClusterVSMMapVO connectorObj = new > ClusterVSMMapVO(clusterId, vsm.getId()); > - txn = Transaction.currentTxn(); > - try { > - txn.start(); > - _clusterVSMDao.persist(connectorObj); > - txn.commit(); > - } catch (Exception e) { > - txn.rollback(); > - s_logger.error("Failed to associate Cisco Nexus 1000v > VSM with cluster: " + clusterName + ". Exception: " + e.getMessage()); > - _clusterDao.remove(clusterId); > - throw new CloudRuntimeException(e.getMessage()); > + // If VSM already exists and is mapped to a cluster, fail > this operation. > + CiscoNexusVSMDeviceVO vsm = > _vsmDao.getVSMbyIpaddress(vsmIp); > + if(vsm != null) { > + List<ClusterVSMMapVO> clusterList = > _clusterVSMDao.listByVSMId(vsm.getId()); > + if (clusterList != null && !clusterList.isEmpty()) { > + s_logger.error("Failed to add cluster: > specified Nexus VSM is already associated with another cluster"); > + _clusterDao.remove(clusterId); > + ResourceInUseException ex = new > ResourceInUseException("Failed to add cluster: specified Nexus VSM is already > associated with another cluster with specified Id"); > + ex.addProxyObject("cluster", > clusterList.get(0).getClusterId(), "clusterId"); > + throw ex; > + } > + } > + // persist credentials to database if the VSM entry is > not already in the db. > + if (_vsmDao.getVSMbyIpaddress(vsmIp) == null) { > + vsm = new CiscoNexusVSMDeviceVO(vsmIp, vsmUser, > vsmPassword); > + txn = Transaction.currentTxn(); > + try { > + txn.start(); > + vsm = _vsmDao.persist(vsm); > + txn.commit(); > + } catch (Exception e) { > + txn.rollback(); > + s_logger.error("Failed to persist Cisco Nexus > 1000v VSM details to database. Exception: " + e.getMessage()); > + // Removing the cluster record which was > added already because the persistence of Nexus VSM credentials has failed. > + _clusterDao.remove(clusterId); > + throw new > CloudRuntimeException(e.getMessage()); > + } > + } > + // Create a mapping between the cluster and the vsm. > + vsm = _vsmDao.getVSMbyIpaddress(vsmIp); > + if (vsm != null) { > + ClusterVSMMapVO connectorObj = new > ClusterVSMMapVO(clusterId, vsm.getId()); > + txn = Transaction.currentTxn(); > + try { > + txn.start(); > + _clusterVSMDao.persist(connectorObj); > + txn.commit(); > + } catch (Exception e) { > + txn.rollback(); > + s_logger.error("Failed to associate Cisco > Nexus 1000v VSM with cluster: " + clusterName + ". Exception: " + > e.getMessage()); > + _clusterDao.remove(clusterId); > + throw new > CloudRuntimeException(e.getMessage()); > + } > } > } else { > String msg; > diff --git > a/vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java > b/vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java > index 4437519..cfbdc56 100755 > --- a/vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java > +++ b/vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelpe > +++ r.java > @@ -194,6 +194,7 @@ public class HypervisorHostHelper { > netconfClient.disconnect(); > s_logger.debug("Disconnected Nexus 1000v session."); > } > + throw new CloudRuntimeException(msg); > } > > List<Pair<OperationType, String>> params = new > ArrayList<Pair<OperationType, String>>(); @@ -212,6 +213,7 @@ public class > HypervisorHostHelper { > netconfClient.disconnect(); > s_logger.debug("Disconnected Nexus 1000v session."); > } > + throw new CloudRuntimeException(msg); > } > } > > @@ -230,6 +232,7 @@ public class HypervisorHostHelper { > netconfClient.disconnect(); > s_logger.debug("Disconnected Nexus 1000v session."); > } > + throw new CloudRuntimeException(msg); > } > > try { > @@ -241,7 +244,8 @@ public class HypervisorHostHelper { > catch(CloudRuntimeException e) { > msg = "Failed to associate policy map " + policyName + " with > port profile " + networkName > + ". Exception: " + e.toString(); > - s_logger.error(msg); > + s_logger.error(msg); > + throw new CloudRuntimeException(msg); > } finally { > if (netconfClient != null) { > netconfClient.disconnect(); @@ -301,6 +305,7 @@ public class > HypervisorHostHelper { > netconfClient.disconnect(); > s_logger.debug("Disconnected Nexus 1000v session."); > } > + throw new CloudRuntimeException(msg); > } > > try { > @@ -315,6 +320,7 @@ public class HypervisorHostHelper { > netconfClient.disconnect(); > s_logger.debug("Disconnected Nexus 1000v session."); > } > + throw new CloudRuntimeException(msg); > } > } > } > @@ -352,6 +358,7 @@ public class HypervisorHostHelper { > netconfClient.disconnect(); > s_logger.debug("Disconnected Nexus 1000v session."); > } > + throw new CloudRuntimeException(msg); > } > > try { > @@ -364,6 +371,7 @@ public class HypervisorHostHelper { > netconfClient.disconnect(); > s_logger.debug("Disconnected Nexus 1000v session."); > } > + throw new CloudRuntimeException(msg); > } > }
