You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by ah...@apache.org on 2013/06/10 02:54:00 UTC
[11/50] [abbrv] git commit: updated refs/heads/vmsync to aaa16af
CLOUDSTACK-2805 [VMware] addVmwareDc fails with NPE when non-existent DC name is passed to the API
Made vcenter as required parameter to addVmwareDc API.
Removed stale parameter url from addVmwareDc API.
Improved Exception handling, logging remote exception details.
Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/58b57ca5
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/58b57ca5
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/58b57ca5
Branch: refs/heads/vmsync
Commit: 58b57ca5dbe514c94469a28d2ebaf0474550ecec
Parents: 89fa121
Author: Sateesh Chodapuneedi <sa...@apache.org>
Authored: Thu Jun 6 15:33:10 2013 +0530
Committer: Sateesh Chodapuneedi <sa...@apache.org>
Committed: Thu Jun 6 15:33:10 2013 +0530
----------------------------------------------------------------------
.../org/apache/cloudstack/api/ApiConstants.java | 1 +
.../vmware/dao/VmwareDatacenterDaoImpl.java | 4 +-
.../vmware/manager/VmwareManagerImpl.java | 102 ++++++-------------
.../api/command/admin/zone/AddVmwareDcCmd.java | 8 +-
.../vmware/VmwareDatacenterApiUnitTest.java | 4 +-
5 files changed, 42 insertions(+), 77 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/58b57ca5/api/src/org/apache/cloudstack/api/ApiConstants.java
----------------------------------------------------------------------
diff --git a/api/src/org/apache/cloudstack/api/ApiConstants.java b/api/src/org/apache/cloudstack/api/ApiConstants.java
index a0cd0f5..ab1402c 100755
--- a/api/src/org/apache/cloudstack/api/ApiConstants.java
+++ b/api/src/org/apache/cloudstack/api/ApiConstants.java
@@ -405,6 +405,7 @@ public class ApiConstants {
public static final String VSM_CONFIG_MODE = "vsmconfigmode";
public static final String VSM_CONFIG_STATE = "vsmconfigstate";
public static final String VSM_DEVICE_STATE = "vsmdevicestate";
+ public static final String VCENTER = "vcenter";
public static final String ADD_VSM_FLAG = "addvsmflag";
public static final String END_POINT = "endpoint";
public static final String REGION_ID = "regionid";
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/58b57ca5/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/dao/VmwareDatacenterDaoImpl.java
----------------------------------------------------------------------
diff --git a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/dao/VmwareDatacenterDaoImpl.java b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/dao/VmwareDatacenterDaoImpl.java
index 8324e93..9f5796a 100644
--- a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/dao/VmwareDatacenterDaoImpl.java
+++ b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/dao/VmwareDatacenterDaoImpl.java
@@ -75,7 +75,7 @@ public class VmwareDatacenterDaoImpl extends GenericDaoBase<VmwareDatacenterVO,
@Override
public List<VmwareDatacenterVO> getVmwareDatacenterByNameAndVcenter(String name, String vCenterHost) {
- SearchCriteria<VmwareDatacenterVO> sc = guidSearch.create();
+ SearchCriteria<VmwareDatacenterVO> sc = nameVcSearch.create();
sc.setParameters("name", name);
sc.setParameters("vCenterHost", vCenterHost);
return search(sc, null);
@@ -83,7 +83,7 @@ public class VmwareDatacenterDaoImpl extends GenericDaoBase<VmwareDatacenterVO,
@Override
public List<VmwareDatacenterVO> listVmwareDatacenterByName(String name) {
- SearchCriteria<VmwareDatacenterVO> sc = guidSearch.create();
+ SearchCriteria<VmwareDatacenterVO> sc = nameSearch.create();
sc.setParameters("name", name);
return search(sc, null);
}
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/58b57ca5/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java
----------------------------------------------------------------------
diff --git a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java
index 664651e..a604392 100755
--- a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java
+++ b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java
@@ -21,7 +21,7 @@ import java.io.IOException;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;
-import java.net.URLDecoder;
+import java.rmi.RemoteException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
@@ -38,8 +38,6 @@ import javax.naming.ConfigurationException;
import org.apache.cloudstack.api.command.admin.zone.AddVmwareDcCmd;
import org.apache.cloudstack.api.command.admin.zone.RemoveVmwareDcCmd;
-import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
-import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreRole;
import org.apache.log4j.Logger;
import com.cloud.agent.AgentManager;
@@ -50,8 +48,6 @@ import com.cloud.agent.api.Answer;
import com.cloud.agent.api.Command;
import com.cloud.agent.api.StartupCommand;
import com.cloud.agent.api.StartupRoutingCommand;
-import com.cloud.agent.api.storage.MigrateVolumeAnswer;
-import com.cloud.agent.api.to.VolumeTO;
import com.cloud.cluster.ClusterManager;
import com.cloud.configuration.Config;
import com.cloud.configuration.dao.ConfigurationDao;
@@ -80,7 +76,6 @@ import com.cloud.hypervisor.vmware.dao.LegacyZoneDao;
import com.cloud.hypervisor.vmware.dao.VmwareDatacenterDao;
import com.cloud.hypervisor.vmware.dao.VmwareDatacenterZoneMapDao;
import com.cloud.hypervisor.vmware.mo.CustomFieldConstants;
-import com.cloud.hypervisor.vmware.mo.CustomFieldsManagerMO;
import com.cloud.hypervisor.vmware.mo.DatacenterMO;
import com.cloud.hypervisor.vmware.mo.DiskControllerType;
import com.cloud.hypervisor.vmware.mo.HostFirewallSystemMO;
@@ -89,7 +84,6 @@ import com.cloud.hypervisor.vmware.mo.HypervisorHostHelper;
import com.cloud.hypervisor.vmware.mo.TaskMO;
import com.cloud.hypervisor.vmware.mo.VirtualEthernetCardType;
import com.cloud.hypervisor.vmware.mo.VmwareHostType;
-import com.cloud.utils.ssh.SshHelper;
import com.cloud.hypervisor.vmware.resource.VmwareContextFactory;
import com.cloud.hypervisor.vmware.util.VmwareClient;
import com.cloud.hypervisor.vmware.util.VmwareContext;
@@ -103,15 +97,10 @@ import com.cloud.serializer.GsonHelper;
import com.cloud.server.ConfigurationServer;
import com.cloud.storage.JavaStorageLayer;
import com.cloud.storage.StorageLayer;
-import com.cloud.storage.StoragePool;
-import com.cloud.storage.VolumeVO;
-import com.cloud.storage.dao.VolumeDao;
import com.cloud.storage.secondary.SecondaryStorageVmManager;
import com.cloud.utils.FileUtil;
import com.cloud.utils.NumbersUtil;
import com.cloud.utils.Pair;
-import com.cloud.utils.UriUtils;
-import com.cloud.utils.component.Manager;
import com.cloud.utils.component.ManagerBase;
import com.cloud.utils.concurrency.NamedThreadFactory;
import com.cloud.utils.db.DB;
@@ -121,8 +110,6 @@ import com.cloud.utils.exception.CloudRuntimeException;
import com.cloud.utils.script.Script;
import com.cloud.utils.ssh.SshHelper;
import com.cloud.vm.DomainRouterVO;
-import com.cloud.vm.VMInstanceVO;
-import com.cloud.vm.dao.VMInstanceDao;
import com.google.gson.Gson;
import com.vmware.vim25.AboutInfo;
import com.vmware.vim25.HostConnectSpec;
@@ -530,7 +517,7 @@ public class VmwareManagerImpl extends ManagerBase implements VmwareManager, Vmw
_configServer.updateKeyPairs();
s_logger.info("Copy System VM patch ISO file to secondary storage. source ISO: " + srcIso.getAbsolutePath() +
- ", destination: " + destIso.getAbsolutePath());
+ ", destination: " + destIso.getAbsolutePath());
try {
FileUtil.copyfile(srcIso, destIso);
} catch(IOException e) {
@@ -579,7 +566,7 @@ public class VmwareManagerImpl extends ManagerBase implements VmwareManager, Vmw
assert(isoFile != null);
if(!isoFile.exists()) {
- s_logger.error("Unable to locate systemvm.iso in your setup at " + isoFile.toString());
+ s_logger.error("Unable to locate systemvm.iso in your setup at " + isoFile.toString());
}
return isoFile;
}
@@ -596,7 +583,7 @@ public class VmwareManagerImpl extends ManagerBase implements VmwareManager, Vmw
}
assert(keyFile != null);
if(!keyFile.exists()) {
- s_logger.error("Unable to locate id_rsa.cloud in your setup at " + keyFile.toString());
+ s_logger.error("Unable to locate id_rsa.cloud in your setup at " + keyFile.toString());
}
return keyFile;
}
@@ -914,9 +901,9 @@ public class VmwareManagerImpl extends ManagerBase implements VmwareManager, Vmw
public VmwareDatacenterVO addVmwareDatacenter(AddVmwareDcCmd cmd) throws ResourceInUseException {
VmwareDatacenterVO vmwareDc = null;
Long zoneId = cmd.getZoneId();
- String url = cmd.getUrl();
String userName = cmd.getUsername();
String password = cmd.getPassword();
+ String vCenterHost = cmd.getVcenter();
String vmwareDcName = cmd.getName();
// Zone validation
@@ -928,25 +915,26 @@ public class VmwareManagerImpl extends ManagerBase implements VmwareManager, Vmw
throw new CloudRuntimeException("Zone " + zoneId + " is already associated with a VMware datacenter.");
}
- // Validate url and get uri
- URI uri = getUri(url);
-
- // Validate username and password and DC name
+ // Validate username, password, VMware DC name and vCenter
if (userName == null) {
- throw new InvalidParameterValueException("Invalid parameter username.");
+ throw new InvalidParameterValueException("Missing or invalid parameter username.");
}
if (password == null) {
- throw new InvalidParameterValueException("Invalid parameter password.");
+ throw new InvalidParameterValueException("Missing or invalid parameter username.");
}
if (vmwareDcName == null) {
- throw new InvalidParameterValueException("Invalid parameter name. Please provide valid VMware datacenter name.");
+ throw new InvalidParameterValueException("Missing or invalid parameter name. Please provide valid VMware datacenter name.");
+ }
+
+ if (vCenterHost == null) {
+ throw new InvalidParameterValueException("Missing or invalid parameter name. " +
+ "Please provide valid VMware vCenter server's IP address or fully qualified domain name.");
}
// Check if DC is already part of zone
// In that case vmware_data_center table should have the DC
- String vCenterHost = uri.getHost();
vmwareDc = _vmwareDcDao.getVmwareDatacenterByGuid(vmwareDcName + "@" + vCenterHost);
if (vmwareDc != null) {
throw new ResourceInUseException("This DC is already part of other CloudStack zone(s). Cannot add this DC to more zones.");
@@ -963,14 +951,13 @@ public class VmwareManagerImpl extends ManagerBase implements VmwareManager, Vmw
context = VmwareContextFactory.create(vCenterHost, userName, password);
// Check if DC exists on vCenter
- try {
- dcMo = new DatacenterMO(context, vmwareDcName);
- } catch(Throwable t) {
- String msg = "Unable to find DC " + vmwareDcName + " in vCenter " + vCenterHost;
+ dcMo = new DatacenterMO(context, vmwareDcName);
+ dcMor = dcMo.getMor();
+ if (dcMor == null) {
+ String msg = "Unable to find VMware DC " + vmwareDcName + " in vCenter " + vCenterHost + ". ";
s_logger.error(msg);
- throw new DiscoveryException(msg);
+ throw new InvalidParameterValueException(msg);
}
- assert (dcMo != null);
// Check if DC is already associated with another cloudstack deployment
// Get custom field property cloud.zone over this DC
@@ -1020,9 +1007,13 @@ public class VmwareManagerImpl extends ManagerBase implements VmwareManager, Vmw
}
dcMo.setCustomFieldValue(CustomFieldConstants.CLOUD_ZONE, "true");
- } catch (Exception e) {
- String msg = "VMware DC discovery failed due to : " + VmwareHelper.getExceptionMessage(e);
- s_logger.error(msg);
+ } catch (Throwable e) {
+ String msg = "Failed to add VMware DC to zone ";
+ if (e instanceof RemoteException) {
+ msg = "Encountered remote exception at vCenter. " + VmwareHelper.getExceptionMessage(e);
+ } else {
+ msg += "due to : " + e.getMessage();
+ }
throw new CloudRuntimeException(msg);
} finally {
if (context != null)
@@ -1032,6 +1023,7 @@ public class VmwareManagerImpl extends ManagerBase implements VmwareManager, Vmw
return vmwareDc;
}
+
@Override
public boolean removeVmwareDatacenter(RemoveVmwareDcCmd cmd) throws ResourceInUseException {
Long zoneId = cmd.getZoneId();
@@ -1070,8 +1062,8 @@ public class VmwareManagerImpl extends ManagerBase implements VmwareManager, Vmw
_vmwareDcZoneMapDao.remove(vmwareDcZoneMap.getId());
txn.commit();
} catch (Exception e) {
- s_logger.info("Caught exception when trying to delete VMware datacenter record.." + e.getMessage());
- throw new CloudRuntimeException("Failed to delete VMware datacenter ");
+ s_logger.info("Caught exception when trying to delete VMware datacenter record." + e.getMessage());
+ throw new CloudRuntimeException("Failed to delete VMware datacenter.");
}
// Construct context
@@ -1083,7 +1075,7 @@ public class VmwareManagerImpl extends ManagerBase implements VmwareManager, Vmw
try {
dcMo = new DatacenterMO(context, vmwareDcName);
} catch(Throwable t) {
- String msg = "able to find DC " + vmwareDcName + " in vCenter " + vCenterHost;
+ String msg = "Unable to find DC " + vmwareDcName + " in vCenter " + vCenterHost;
s_logger.error(msg);
throw new DiscoveryException(msg);
}
@@ -1095,7 +1087,7 @@ public class VmwareManagerImpl extends ManagerBase implements VmwareManager, Vmw
s_logger.info("Sucessfully reset custom field property cloud.zone over DC " + vmwareDcName);
} catch (Exception e) {
String msg = "Unable to reset custom field property cloud.zone over DC " + vmwareDcName
- + " due to : " + VmwareHelper.getExceptionMessage(e);
+ + " due to : " + VmwareHelper.getExceptionMessage(e);
s_logger.error(msg);
throw new CloudRuntimeException(msg);
} finally {
@@ -1111,7 +1103,7 @@ public class VmwareManagerImpl extends ManagerBase implements VmwareManager, Vmw
DataCenterVO zone = _dcDao.findById(zoneId);
if (zone == null) {
InvalidParameterValueException ex = new InvalidParameterValueException(
- "Can't find zone by the id specified");
+ "Can't find zone by the id specified.");
throw ex;
}
@@ -1122,38 +1114,10 @@ public class VmwareManagerImpl extends ManagerBase implements VmwareManager, Vmw
for (ClusterVO cluster : clusters) {
if (cluster.getHypervisorType().equals(HypervisorType.VMware)) {
throw new ResourceInUseException("Zone has one or more clusters."
- + " Can't " + errStr + " which already has clusters.");
- }
- }
- }
- }
-
- private URI getUri(String url) throws InvalidParameterValueException {
- if (url == null) {
- throw new InvalidParameterValueException("Invalid url.");
- }
-
- URI uri = null;
- try {
- uri = new URI(UriUtils.encodeURIComponent(url));
- if (uri.getScheme() == null) {
- throw new InvalidParameterValueException(
- "uri.scheme is null " + url
- + ", add http:// as a prefix");
- } else if (uri.getScheme().equalsIgnoreCase("http")) {
- if (uri.getHost() == null
- || uri.getHost().equalsIgnoreCase("")
- || uri.getPath() == null
- || uri.getPath().equalsIgnoreCase("")) {
- throw new InvalidParameterValueException(
- "Your host and/or path is wrong. Make sure it's of the format http://hostname/path");
+ + " Can't " + errStr + " which already has clusters.");
}
}
- } catch (URISyntaxException e) {
- throw new InvalidParameterValueException(url
- + " is not a valid uri");
}
- return uri;
}
@Override
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/58b57ca5/plugins/hypervisors/vmware/src/org/apache/cloudstack/api/command/admin/zone/AddVmwareDcCmd.java
----------------------------------------------------------------------
diff --git a/plugins/hypervisors/vmware/src/org/apache/cloudstack/api/command/admin/zone/AddVmwareDcCmd.java b/plugins/hypervisors/vmware/src/org/apache/cloudstack/api/command/admin/zone/AddVmwareDcCmd.java
index fde96c8..317452b 100644
--- a/plugins/hypervisors/vmware/src/org/apache/cloudstack/api/command/admin/zone/AddVmwareDcCmd.java
+++ b/plugins/hypervisors/vmware/src/org/apache/cloudstack/api/command/admin/zone/AddVmwareDcCmd.java
@@ -48,8 +48,8 @@ public class AddVmwareDcCmd extends BaseCmd {
@Parameter(name=ApiConstants.NAME, type=CommandType.STRING, required=true, description="Name of VMware datacenter to be added to specified zone.")
private String name;
- @Parameter(name=ApiConstants.URL, type=CommandType.STRING, required=false, description="The URL of vCenter.")
- private String url;
+ @Parameter(name=ApiConstants.VCENTER, type=CommandType.STRING, required=true, description="The name/ip of vCenter. Make sure it is IP address or full qualified domain name for host running vCenter server.")
+ private String vCenter;
@Parameter(name=ApiConstants.USERNAME, type=CommandType.STRING, required=false, description="The Username required to connect to resource.")
private String username;
@@ -64,8 +64,8 @@ public class AddVmwareDcCmd extends BaseCmd {
return name;
}
- public String getUrl() {
- return url;
+ public String getVcenter() {
+ return vCenter;
}
public String getUsername() {
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/58b57ca5/plugins/hypervisors/vmware/test/com/cloud/hypervisor/vmware/VmwareDatacenterApiUnitTest.java
----------------------------------------------------------------------
diff --git a/plugins/hypervisors/vmware/test/com/cloud/hypervisor/vmware/VmwareDatacenterApiUnitTest.java b/plugins/hypervisors/vmware/test/com/cloud/hypervisor/vmware/VmwareDatacenterApiUnitTest.java
index 688b93c..de08c93 100644
--- a/plugins/hypervisors/vmware/test/com/cloud/hypervisor/vmware/VmwareDatacenterApiUnitTest.java
+++ b/plugins/hypervisors/vmware/test/com/cloud/hypervisor/vmware/VmwareDatacenterApiUnitTest.java
@@ -214,7 +214,7 @@ public class VmwareDatacenterApiUnitTest {
Mockito.when(_vmwareDcZoneMapDao.findByZoneId(1L)).thenReturn(null);
Mockito.when(_vmwareDcZoneMapDao.expunge(1L)).thenReturn(true);
Mockito.when(addCmd.getZoneId()).thenReturn(1L);
- Mockito.when(addCmd.getUrl()).thenReturn(url);
+ Mockito.when(addCmd.getVcenter()).thenReturn(vCenterHost);
Mockito.when(addCmd.getUsername()).thenReturn(user);
Mockito.when(addCmd.getPassword()).thenReturn(password);
Mockito.when(addCmd.getName()).thenReturn(vmwareDcName);
@@ -265,7 +265,7 @@ public class VmwareDatacenterApiUnitTest {
//@Test(expected = InvalidParameterValueException.class)
public void testAddVmwareDcWithNullUrl() throws ResourceInUseException, IllegalArgumentException, DiscoveryException, Exception {
- Mockito.when(addCmd.getUrl()).thenReturn(null);
+ Mockito.when(addCmd.getVcenter()).thenReturn(null);
_vmwareDatacenterService.addVmwareDatacenter(addCmd);
}