From 0010faee4bac8ebb32ceb95ca6ff6b02b4013cba Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Thu, 25 Jul 2013 12:47:50 +0530 Subject: [PATCH] Fix for CLOUDSTACK-2536 Removing memoryovercommitratio and cpuovercommitratio parameters from addCluster and updateCluster APIs, since these can be configurable using updateConfiguration API at cluster level. By default while creating cluster these values are taken from global configuration parameters. --- .../com/cloud/resource/ResourceService.java | 2 +- .../apache/cloudstack/api/ApiConstants.java | 2 - .../command/admin/cluster/AddClusterCmd.java | 23 ------------ .../admin/cluster/UpdateClusterCmd.java | 36 +----------------- .../ConfigurationManagerImpl.java | 31 +++++++++++++--- .../cloud/resource/ResourceManagerImpl.java | 37 +++---------------- .../resource/MockResourceManagerImpl.java | 2 +- 7 files changed, 36 insertions(+), 97 deletions(-) diff --git a/api/src/com/cloud/resource/ResourceService.java b/api/src/com/cloud/resource/ResourceService.java index 25298cc6251..f1080b82488 100755 --- a/api/src/com/cloud/resource/ResourceService.java +++ b/api/src/com/cloud/resource/ResourceService.java @@ -64,7 +64,7 @@ public interface ResourceService { boolean deleteCluster(DeleteClusterCmd cmd); - Cluster updateCluster(Cluster cluster, String clusterType, String hypervisor, String allocationState, String managedstate,Float memoryOvercommitRatio, Float cpuOvercommitRatio); + Cluster updateCluster(Cluster cluster, String clusterType, String hypervisor, String allocationState, String managedstate); List discoverHosts(AddHostCmd cmd) throws IllegalArgumentException, DiscoveryException, InvalidParameterValueException; diff --git a/api/src/org/apache/cloudstack/api/ApiConstants.java b/api/src/org/apache/cloudstack/api/ApiConstants.java index 0550a39a0ba..121403fa064 100755 --- a/api/src/org/apache/cloudstack/api/ApiConstants.java +++ b/api/src/org/apache/cloudstack/api/ApiConstants.java @@ -48,7 +48,6 @@ public class ApiConstants { public static final String COMPONENT = "component"; public static final String CPU_NUMBER = "cpunumber"; public static final String CPU_SPEED = "cpuspeed"; - public static final String CPU_OVERCOMMIT_RATIO="cpuovercommitratio"; public static final String CREATED = "created"; public static final String CUSTOMIZED = "customized"; public static final String CUSTOMIZED_IOPS = "customizediops"; @@ -142,7 +141,6 @@ public class ApiConstants { public static final String MAX = "max"; public static final String MAX_SNAPS = "maxsnaps"; public static final String MEMORY = "memory"; - public static final String MEMORY_OVERCOMMIT_RATIO="memoryovercommitratio"; public static final String MODE = "mode"; public static final String NAME = "name"; public static final String METHOD_NAME = "methodname"; diff --git a/api/src/org/apache/cloudstack/api/command/admin/cluster/AddClusterCmd.java b/api/src/org/apache/cloudstack/api/command/admin/cluster/AddClusterCmd.java index c6ca9bc673b..c8b012a4824 100644 --- a/api/src/org/apache/cloudstack/api/command/admin/cluster/AddClusterCmd.java +++ b/api/src/org/apache/cloudstack/api/command/admin/cluster/AddClusterCmd.java @@ -81,12 +81,6 @@ public class AddClusterCmd extends BaseCmd { @Parameter(name = ApiConstants.VSM_IPADDRESS, type = CommandType.STRING, required = false, description = "the ipaddress of the VSM associated with this cluster") private String vsmipaddress; - @Parameter (name=ApiConstants.CPU_OVERCOMMIT_RATIO, type = CommandType.STRING, required = false , description = "value of the cpu overcommit ratio, defaults to 1") - private String cpuOvercommitRatio; - - @Parameter(name = ApiConstants.MEMORY_OVERCOMMIT_RATIO, type = CommandType.STRING, required = false, description = "value of the default memory overcommit ratio, defaults to 1") - private String memoryOvercommitRatio; - @Parameter(name = ApiConstants.VSWITCH_TYPE_GUEST_TRAFFIC, type = CommandType.STRING, required = false, description = "Type of virtual switch used for guest traffic in the cluster. Allowed values are, vmwaresvs (for VMware standard vSwitch) and vmwaredvs (for VMware distributed vSwitch)") private String vSwitchTypeGuestTraffic; @@ -181,26 +175,9 @@ public class AddClusterCmd extends BaseCmd { this.allocationState = allocationState; } - public Float getCpuOvercommitRatio (){ - if(cpuOvercommitRatio != null){ - return Float.parseFloat(cpuOvercommitRatio); - } - return 1.0f; - } - - public Float getMemoryOvercommitRatio(){ - if (memoryOvercommitRatio != null){ - return Float.parseFloat(memoryOvercommitRatio); - } - return 1.0f; - } - @Override public void execute(){ try { - if (getMemoryOvercommitRatio().compareTo(1f) < 0 || getCpuOvercommitRatio().compareTo(1f) < 0) { - throw new InvalidParameterValueException("cpu and memory overcommit ratios should be greater than or equal to one"); - } List result = _resourceService.discoverCluster(this); ListResponse response = new ListResponse(); List clusterResponses = new ArrayList(); diff --git a/api/src/org/apache/cloudstack/api/command/admin/cluster/UpdateClusterCmd.java b/api/src/org/apache/cloudstack/api/command/admin/cluster/UpdateClusterCmd.java index a14f9055211..34d05c0a821 100644 --- a/api/src/org/apache/cloudstack/api/command/admin/cluster/UpdateClusterCmd.java +++ b/api/src/org/apache/cloudstack/api/command/admin/cluster/UpdateClusterCmd.java @@ -54,12 +54,6 @@ public class UpdateClusterCmd extends BaseCmd { @Parameter(name=ApiConstants.MANAGED_STATE, type=CommandType.STRING, description="whether this cluster is managed by cloudstack") private String managedState; - @Parameter(name=ApiConstants.CPU_OVERCOMMIT_RATIO, type = CommandType.STRING, description = "Value of cpu overcommit ratio") - private String cpuovercommitratio; - - @Parameter(name=ApiConstants.MEMORY_OVERCOMMIT_RATIO, type = CommandType.STRING, description = "Value of memory overcommit ratio") - private String memoryovercommitratio; - public String getClusterName() { return clusterName; @@ -107,40 +101,14 @@ public class UpdateClusterCmd extends BaseCmd { this.managedState = managedstate; } - public Float getCpuOvercommitRatio (){ - if(cpuovercommitratio != null){ - return Float.parseFloat(cpuovercommitratio); - } - return null; - } - - public Float getMemoryOvercommitRaito (){ - if (memoryovercommitratio != null){ - return Float.parseFloat(memoryovercommitratio); - } - return null; - } - @Override public void execute(){ Cluster cluster = _resourceService.getCluster(getId()); if (cluster == null) { throw new InvalidParameterValueException("Unable to find the cluster by id=" + getId()); } - if (getMemoryOvercommitRaito() !=null){ - if ((getMemoryOvercommitRaito().compareTo(1f) < 0)) { - throw new InvalidParameterValueException("Memory overcommit ratio should be greater than or equal to one"); - } - } - - if (getCpuOvercommitRatio() !=null){ - if (getCpuOvercommitRatio().compareTo(1f) < 0) { - throw new InvalidParameterValueException("Cpu overcommit ratio should be greater than or equal to one"); - } - } - - Cluster result = _resourceService.updateCluster(cluster, getClusterType(), getHypervisor(), getAllocationState(), getManagedstate(), getMemoryOvercommitRaito(), getCpuOvercommitRatio()); - if (result != null) { + Cluster result = _resourceService.updateCluster(cluster, getClusterType(), getHypervisor(), getAllocationState(), getManagedstate()); + if (result != null) { ClusterResponse clusterResponse = _responseGenerator.createClusterResponse(cluster, false); clusterResponse.setResponseName(getCommandName()); this.setResponseObject(clusterResponse); diff --git a/server/src/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/com/cloud/configuration/ConfigurationManagerImpl.java index 9bc8efd465b..b2dedb3681f 100755 --- a/server/src/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/com/cloud/configuration/ConfigurationManagerImpl.java @@ -313,6 +313,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati private long _defaultPageSize = Long.parseLong(Config.DefaultPageSize.getDefaultValue()); protected Set configValuesForValidation; private Set weightBasedParametersForValidation; + private Set overprovisioningFactorsForValidation; @Override public boolean configure(final String name, final Map params) throws ConfigurationException { @@ -326,6 +327,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati populateConfigValuesForValidationSet(); weightBasedParametersForValidation(); + overProvisioningFactorsForValidation(); return true; } @@ -373,6 +375,13 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati } + private void overProvisioningFactorsForValidation() { + overprovisioningFactorsForValidation = new HashSet(); + overprovisioningFactorsForValidation.add(Config.MemOverprovisioningFactor.key()); + overprovisioningFactorsForValidation.add(Config.CPUOverprovisioningFactor.key()); + overprovisioningFactorsForValidation.add(Config.StorageOverprovisioningFactor.key()); + } + @Override public boolean start() { @@ -688,11 +697,6 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati s_logger.error("Invalid scope id provided for the parameter " + name); return "Invalid scope id provided for the parameter " + name; } - if ((name.equalsIgnoreCase("cpu.overprovisioning.factor") || name - .equalsIgnoreCase("mem.overprovisioning.factor")) && value == null) { - s_logger.error("value cannot be null for cpu.overprovisioning.factor/mem.overprovisioning.factor"); - return "value cannot be null for cpu.overprovisioning.factor/mem.overprovisioning.factor"; - } } Class type = c.getType(); @@ -701,9 +705,26 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati if (type.equals(Boolean.class)) { return "Please enter either 'true' or 'false'."; } + if (overprovisioningFactorsForValidation.contains(name)) { + String msg = "value cannot be null for the parameter " + name; + s_logger.error(msg); + return msg; + } return null; } + value = value.trim(); + try { + if (overprovisioningFactorsForValidation.contains(name) && (Float.parseFloat(value) < 1f)) { + String msg = name + " should be greater than or equal to 1"; + s_logger.error(msg); + throw new InvalidParameterValueException(msg); + } + } catch (NumberFormatException e) { + String msg = "There was an error trying to parse the float value for: " + name; + s_logger.error(msg); + throw new InvalidParameterValueException(msg); + } if (type.equals(Boolean.class)) { if (!(value.equals("true") || value.equals("false"))) { diff --git a/server/src/com/cloud/resource/ResourceManagerImpl.java b/server/src/com/cloud/resource/ResourceManagerImpl.java index 342b9fd03dc..c0187702e71 100755 --- a/server/src/com/cloud/resource/ResourceManagerImpl.java +++ b/server/src/com/cloud/resource/ResourceManagerImpl.java @@ -30,6 +30,7 @@ import javax.ejb.Local; import javax.inject.Inject; import javax.naming.ConfigurationException; +import com.cloud.server.ConfigurationServer; import org.apache.log4j.Logger; import org.springframework.stereotype.Component; @@ -214,6 +215,8 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager, PlannerHostReservationDao _plannerHostReserveDao; @Inject protected DedicatedResourceDao _dedicatedDao; + @Inject + protected ConfigurationServer _configServer; protected List _discoverers; @@ -479,8 +482,8 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager, clusterId = cluster.getId(); result.add(cluster); - ClusterDetailsVO cluster_detail_cpu = new ClusterDetailsVO(clusterId, "cpuOvercommitRatio", Float.toString(cmd.getCpuOvercommitRatio())); - ClusterDetailsVO cluster_detail_ram = new ClusterDetailsVO(clusterId, "memoryOvercommitRatio", Float.toString(cmd.getMemoryOvercommitRatio())); + ClusterDetailsVO cluster_detail_cpu = new ClusterDetailsVO(clusterId, "cpuOvercommitRatio", _configServer.getConfigValue(Config.CPUOverprovisioningFactor.key(), null, null)); + ClusterDetailsVO cluster_detail_ram = new ClusterDetailsVO(clusterId, "memoryOvercommitRatio", _configServer.getConfigValue(Config.MemOverprovisioningFactor.key(), null, null)); _clusterDetailsDao.persist(cluster_detail_cpu); _clusterDetailsDao.persist(cluster_detail_ram); @@ -495,20 +498,6 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager, details.put("password", password); _clusterDetailsDao.persist(cluster.getId(), details); - _clusterDetailsDao.persist(cluster_detail_cpu); - _clusterDetailsDao.persist(cluster_detail_ram); - // create a new entry only if the overcommit ratios are greater than 1. - if (cmd.getCpuOvercommitRatio().compareTo(1f) > 0) { - cluster_detail_cpu = new ClusterDetailsVO(clusterId, "cpuOvercommitRatio", Float.toString(cmd.getCpuOvercommitRatio())); - _clusterDetailsDao.persist(cluster_detail_cpu); - } - - - if(cmd.getMemoryOvercommitRatio().compareTo(1f) > 0) { - cluster_detail_ram = new ClusterDetailsVO(clusterId, "memoryOvercommitRatio", Float.toString(cmd.getMemoryOvercommitRatio())); - _clusterDetailsDao.persist(cluster_detail_ram); - } - boolean success = false; try { try { @@ -998,8 +987,7 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager, @Override @DB - public Cluster updateCluster(Cluster clusterToUpdate, String clusterType, String hypervisor, String allocationState, String managedstate, - Float memoryovercommitratio, Float cpuovercommitratio) { + public Cluster updateCluster(Cluster clusterToUpdate, String clusterType, String hypervisor, String allocationState, String managedstate) { ClusterVO cluster = (ClusterVO) clusterToUpdate; // Verify cluster information and update the cluster if needed @@ -1065,19 +1053,6 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager, } } - - if (memoryovercommitratio != null) { - ClusterDetailsVO memory_detail = _clusterDetailsDao.findDetail(cluster.getId(),"memoryOvercommitRatio"); - memory_detail.setValue(Float.toString(memoryovercommitratio)); - _clusterDetailsDao.update(memory_detail.getId(),memory_detail); - } - - if (cpuovercommitratio != null) { - ClusterDetailsVO cpu_detail = _clusterDetailsDao.findDetail(cluster.getId(),"cpuOvercommitRatio"); - cpu_detail.setValue(Float.toString(cpuovercommitratio)); - _clusterDetailsDao.update(cpu_detail.getId(),cpu_detail); - } - if (doUpdate) { Transaction txn = Transaction.currentTxn(); try { diff --git a/server/test/com/cloud/resource/MockResourceManagerImpl.java b/server/test/com/cloud/resource/MockResourceManagerImpl.java index 2c66134f98d..6beb0648573 100644 --- a/server/test/com/cloud/resource/MockResourceManagerImpl.java +++ b/server/test/com/cloud/resource/MockResourceManagerImpl.java @@ -107,7 +107,7 @@ public class MockResourceManagerImpl extends ManagerBase implements ResourceMana */ @Override public Cluster updateCluster(Cluster cluster, String clusterType, String hypervisor, String allocationState, - String managedstate, Float memoryOvercommitRaito, Float cpuOvercommitRatio) { + String managedstate) { // TODO Auto-generated method stub return null; }