Skip to content

Commit 0010fae

Browse files
Harikrishna Patnalakoushik-das
Harikrishna Patnala
authored and
koushik-das
committed
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.
1 parent 80ac885 commit 0010fae

7 files changed

Lines changed: 36 additions & 97 deletions

File tree

‎api/src/com/cloud/resource/ResourceService.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ public interface ResourceService {
6464

6565
boolean deleteCluster(DeleteClusterCmd cmd);
6666

67-
Cluster updateCluster(Cluster cluster, String clusterType, String hypervisor, String allocationState, String managedstate,Float memoryOvercommitRatio, Float cpuOvercommitRatio);
67+
Cluster updateCluster(Cluster cluster, String clusterType, String hypervisor, String allocationState, String managedstate);
6868

6969
List<? extends Host> discoverHosts(AddHostCmd cmd) throws IllegalArgumentException, DiscoveryException, InvalidParameterValueException;
7070

‎api/src/org/apache/cloudstack/api/ApiConstants.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@ public class ApiConstants {
4848
public static final String COMPONENT = "component";
4949
public static final String CPU_NUMBER = "cpunumber";
5050
public static final String CPU_SPEED = "cpuspeed";
51-
public static final String CPU_OVERCOMMIT_RATIO="cpuovercommitratio";
5251
public static final String CREATED = "created";
5352
public static final String CUSTOMIZED = "customized";
5453
public static final String CUSTOMIZED_IOPS = "customizediops";
@@ -142,7 +141,6 @@ public class ApiConstants {
142141
public static final String MAX = "max";
143142
public static final String MAX_SNAPS = "maxsnaps";
144143
public static final String MEMORY = "memory";
145-
public static final String MEMORY_OVERCOMMIT_RATIO="memoryovercommitratio";
146144
public static final String MODE = "mode";
147145
public static final String NAME = "name";
148146
public static final String METHOD_NAME = "methodname";

‎api/src/org/apache/cloudstack/api/command/admin/cluster/AddClusterCmd.java‎

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -81,12 +81,6 @@ public class AddClusterCmd extends BaseCmd {
8181
@Parameter(name = ApiConstants.VSM_IPADDRESS, type = CommandType.STRING, required = false, description = "the ipaddress of the VSM associated with this cluster")
8282
private String vsmipaddress;
8383

84-
@Parameter (name=ApiConstants.CPU_OVERCOMMIT_RATIO, type = CommandType.STRING, required = false , description = "value of the cpu overcommit ratio, defaults to 1")
85-
private String cpuOvercommitRatio;
86-
87-
@Parameter(name = ApiConstants.MEMORY_OVERCOMMIT_RATIO, type = CommandType.STRING, required = false, description = "value of the default memory overcommit ratio, defaults to 1")
88-
private String memoryOvercommitRatio;
89-
9084
@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)")
9185
private String vSwitchTypeGuestTraffic;
9286

@@ -181,26 +175,9 @@ public void setAllocationState(String allocationState) {
181175
this.allocationState = allocationState;
182176
}
183177

184-
public Float getCpuOvercommitRatio (){
185-
if(cpuOvercommitRatio != null){
186-
return Float.parseFloat(cpuOvercommitRatio);
187-
}
188-
return 1.0f;
189-
}
190-
191-
public Float getMemoryOvercommitRatio(){
192-
if (memoryOvercommitRatio != null){
193-
return Float.parseFloat(memoryOvercommitRatio);
194-
}
195-
return 1.0f;
196-
}
197-
198178
@Override
199179
public void execute(){
200180
try {
201-
if (getMemoryOvercommitRatio().compareTo(1f) < 0 || getCpuOvercommitRatio().compareTo(1f) < 0) {
202-
throw new InvalidParameterValueException("cpu and memory overcommit ratios should be greater than or equal to one");
203-
}
204181
List<? extends Cluster> result = _resourceService.discoverCluster(this);
205182
ListResponse<ClusterResponse> response = new ListResponse<ClusterResponse>();
206183
List<ClusterResponse> clusterResponses = new ArrayList<ClusterResponse>();

‎api/src/org/apache/cloudstack/api/command/admin/cluster/UpdateClusterCmd.java‎

Lines changed: 2 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -54,12 +54,6 @@ public class UpdateClusterCmd extends BaseCmd {
5454
@Parameter(name=ApiConstants.MANAGED_STATE, type=CommandType.STRING, description="whether this cluster is managed by cloudstack")
5555
private String managedState;
5656

57-
@Parameter(name=ApiConstants.CPU_OVERCOMMIT_RATIO, type = CommandType.STRING, description = "Value of cpu overcommit ratio")
58-
private String cpuovercommitratio;
59-
60-
@Parameter(name=ApiConstants.MEMORY_OVERCOMMIT_RATIO, type = CommandType.STRING, description = "Value of memory overcommit ratio")
61-
private String memoryovercommitratio;
62-
6357

6458
public String getClusterName() {
6559
return clusterName;
@@ -107,40 +101,14 @@ public void setManagedstate(String managedstate) {
107101
this.managedState = managedstate;
108102
}
109103

110-
public Float getCpuOvercommitRatio (){
111-
if(cpuovercommitratio != null){
112-
return Float.parseFloat(cpuovercommitratio);
113-
}
114-
return null;
115-
}
116-
117-
public Float getMemoryOvercommitRaito (){
118-
if (memoryovercommitratio != null){
119-
return Float.parseFloat(memoryovercommitratio);
120-
}
121-
return null;
122-
}
123-
124104
@Override
125105
public void execute(){
126106
Cluster cluster = _resourceService.getCluster(getId());
127107
if (cluster == null) {
128108
throw new InvalidParameterValueException("Unable to find the cluster by id=" + getId());
129109
}
130-
if (getMemoryOvercommitRaito() !=null){
131-
if ((getMemoryOvercommitRaito().compareTo(1f) < 0)) {
132-
throw new InvalidParameterValueException("Memory overcommit ratio should be greater than or equal to one");
133-
}
134-
}
135-
136-
if (getCpuOvercommitRatio() !=null){
137-
if (getCpuOvercommitRatio().compareTo(1f) < 0) {
138-
throw new InvalidParameterValueException("Cpu overcommit ratio should be greater than or equal to one");
139-
}
140-
}
141-
142-
Cluster result = _resourceService.updateCluster(cluster, getClusterType(), getHypervisor(), getAllocationState(), getManagedstate(), getMemoryOvercommitRaito(), getCpuOvercommitRatio());
143-
if (result != null) {
110+
Cluster result = _resourceService.updateCluster(cluster, getClusterType(), getHypervisor(), getAllocationState(), getManagedstate());
111+
if (result != null) {
144112
ClusterResponse clusterResponse = _responseGenerator.createClusterResponse(cluster, false);
145113
clusterResponse.setResponseName(getCommandName());
146114
this.setResponseObject(clusterResponse);

‎server/src/com/cloud/configuration/ConfigurationManagerImpl.java‎

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
313313
private long _defaultPageSize = Long.parseLong(Config.DefaultPageSize.getDefaultValue());
314314
protected Set<String> configValuesForValidation;
315315
private Set<String> weightBasedParametersForValidation;
316+
private Set<String> overprovisioningFactorsForValidation;
316317

317318
@Override
318319
public boolean configure(final String name, final Map<String, Object> params) throws ConfigurationException {
@@ -326,6 +327,7 @@ public boolean configure(final String name, final Map<String, Object> params) th
326327

327328
populateConfigValuesForValidationSet();
328329
weightBasedParametersForValidation();
330+
overProvisioningFactorsForValidation();
329331
return true;
330332
}
331333

@@ -373,6 +375,13 @@ private void weightBasedParametersForValidation() {
373375

374376
}
375377

378+
private void overProvisioningFactorsForValidation() {
379+
overprovisioningFactorsForValidation = new HashSet<String>();
380+
overprovisioningFactorsForValidation.add(Config.MemOverprovisioningFactor.key());
381+
overprovisioningFactorsForValidation.add(Config.CPUOverprovisioningFactor.key());
382+
overprovisioningFactorsForValidation.add(Config.StorageOverprovisioningFactor.key());
383+
}
384+
376385
@Override
377386
public boolean start() {
378387

@@ -688,11 +697,6 @@ private String validateConfigurationValue(String name, String value, String scop
688697
s_logger.error("Invalid scope id provided for the parameter " + name);
689698
return "Invalid scope id provided for the parameter " + name;
690699
}
691-
if ((name.equalsIgnoreCase("cpu.overprovisioning.factor") || name
692-
.equalsIgnoreCase("mem.overprovisioning.factor")) && value == null) {
693-
s_logger.error("value cannot be null for cpu.overprovisioning.factor/mem.overprovisioning.factor");
694-
return "value cannot be null for cpu.overprovisioning.factor/mem.overprovisioning.factor";
695-
}
696700
}
697701

698702
Class<?> type = c.getType();
@@ -701,9 +705,26 @@ private String validateConfigurationValue(String name, String value, String scop
701705
if (type.equals(Boolean.class)) {
702706
return "Please enter either 'true' or 'false'.";
703707
}
708+
if (overprovisioningFactorsForValidation.contains(name)) {
709+
String msg = "value cannot be null for the parameter " + name;
710+
s_logger.error(msg);
711+
return msg;
712+
}
704713
return null;
705714
}
715+
706716
value = value.trim();
717+
try {
718+
if (overprovisioningFactorsForValidation.contains(name) && (Float.parseFloat(value) < 1f)) {
719+
String msg = name + " should be greater than or equal to 1";
720+
s_logger.error(msg);
721+
throw new InvalidParameterValueException(msg);
722+
}
723+
} catch (NumberFormatException e) {
724+
String msg = "There was an error trying to parse the float value for: " + name;
725+
s_logger.error(msg);
726+
throw new InvalidParameterValueException(msg);
727+
}
707728

708729
if (type.equals(Boolean.class)) {
709730
if (!(value.equals("true") || value.equals("false"))) {

‎server/src/com/cloud/resource/ResourceManagerImpl.java‎

Lines changed: 6 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import javax.inject.Inject;
3131
import javax.naming.ConfigurationException;
3232

33+
import com.cloud.server.ConfigurationServer;
3334
import org.apache.log4j.Logger;
3435
import org.springframework.stereotype.Component;
3536

@@ -214,6 +215,8 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager,
214215
PlannerHostReservationDao _plannerHostReserveDao;
215216
@Inject
216217
protected DedicatedResourceDao _dedicatedDao;
218+
@Inject
219+
protected ConfigurationServer _configServer;
217220

218221
protected List<? extends Discoverer> _discoverers;
219222

@@ -479,8 +482,8 @@ public List<? extends Cluster> discoverCluster(AddClusterCmd cmd) throws Illegal
479482
clusterId = cluster.getId();
480483
result.add(cluster);
481484

482-
ClusterDetailsVO cluster_detail_cpu = new ClusterDetailsVO(clusterId, "cpuOvercommitRatio", Float.toString(cmd.getCpuOvercommitRatio()));
483-
ClusterDetailsVO cluster_detail_ram = new ClusterDetailsVO(clusterId, "memoryOvercommitRatio", Float.toString(cmd.getMemoryOvercommitRatio()));
485+
ClusterDetailsVO cluster_detail_cpu = new ClusterDetailsVO(clusterId, "cpuOvercommitRatio", _configServer.getConfigValue(Config.CPUOverprovisioningFactor.key(), null, null));
486+
ClusterDetailsVO cluster_detail_ram = new ClusterDetailsVO(clusterId, "memoryOvercommitRatio", _configServer.getConfigValue(Config.MemOverprovisioningFactor.key(), null, null));
484487
_clusterDetailsDao.persist(cluster_detail_cpu);
485488
_clusterDetailsDao.persist(cluster_detail_ram);
486489

@@ -495,20 +498,6 @@ public List<? extends Cluster> discoverCluster(AddClusterCmd cmd) throws Illegal
495498
details.put("password", password);
496499
_clusterDetailsDao.persist(cluster.getId(), details);
497500

498-
_clusterDetailsDao.persist(cluster_detail_cpu);
499-
_clusterDetailsDao.persist(cluster_detail_ram);
500-
// create a new entry only if the overcommit ratios are greater than 1.
501-
if (cmd.getCpuOvercommitRatio().compareTo(1f) > 0) {
502-
cluster_detail_cpu = new ClusterDetailsVO(clusterId, "cpuOvercommitRatio", Float.toString(cmd.getCpuOvercommitRatio()));
503-
_clusterDetailsDao.persist(cluster_detail_cpu);
504-
}
505-
506-
507-
if(cmd.getMemoryOvercommitRatio().compareTo(1f) > 0) {
508-
cluster_detail_ram = new ClusterDetailsVO(clusterId, "memoryOvercommitRatio", Float.toString(cmd.getMemoryOvercommitRatio()));
509-
_clusterDetailsDao.persist(cluster_detail_ram);
510-
}
511-
512501
boolean success = false;
513502
try {
514503
try {
@@ -998,8 +987,7 @@ public boolean deleteCluster(DeleteClusterCmd cmd) {
998987

999988
@Override
1000989
@DB
1001-
public Cluster updateCluster(Cluster clusterToUpdate, String clusterType, String hypervisor, String allocationState, String managedstate,
1002-
Float memoryovercommitratio, Float cpuovercommitratio) {
990+
public Cluster updateCluster(Cluster clusterToUpdate, String clusterType, String hypervisor, String allocationState, String managedstate) {
1003991

1004992
ClusterVO cluster = (ClusterVO) clusterToUpdate;
1005993
// Verify cluster information and update the cluster if needed
@@ -1065,19 +1053,6 @@ public Cluster updateCluster(Cluster clusterToUpdate, String clusterType, String
10651053
}
10661054
}
10671055

1068-
1069-
if (memoryovercommitratio != null) {
1070-
ClusterDetailsVO memory_detail = _clusterDetailsDao.findDetail(cluster.getId(),"memoryOvercommitRatio");
1071-
memory_detail.setValue(Float.toString(memoryovercommitratio));
1072-
_clusterDetailsDao.update(memory_detail.getId(),memory_detail);
1073-
}
1074-
1075-
if (cpuovercommitratio != null) {
1076-
ClusterDetailsVO cpu_detail = _clusterDetailsDao.findDetail(cluster.getId(),"cpuOvercommitRatio");
1077-
cpu_detail.setValue(Float.toString(cpuovercommitratio));
1078-
_clusterDetailsDao.update(cpu_detail.getId(),cpu_detail);
1079-
}
1080-
10811056
if (doUpdate) {
10821057
Transaction txn = Transaction.currentTxn();
10831058
try {

‎server/test/com/cloud/resource/MockResourceManagerImpl.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ public boolean deleteCluster(DeleteClusterCmd cmd) {
107107
*/
108108
@Override
109109
public Cluster updateCluster(Cluster cluster, String clusterType, String hypervisor, String allocationState,
110-
String managedstate, Float memoryOvercommitRaito, Float cpuOvercommitRatio) {
110+
String managedstate) {
111111
// TODO Auto-generated method stub
112112
return null;
113113
}

0 commit comments

Comments
 (0)