etan39
/
cloudstack
mirror of https://github.com/apache/cloudstack.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Validate disk offering IOPS normal and maximum read/write values (#3681) 

* Validate API IOPS normal and maximum read/write values.

Ensures that normal read/write cannot be greater than Maximum
read/write. Additionally, it was added a global settings
'iops.maximum.rate.length'.

'iops.maximum.rate.length' sets the maximum IOPS read/write length
(seconds) accepted; thus, preventing irrealistic values for a disk
offering (e.g. hours or days of burst IOPS). The default value is 0
(zero) and allows any IOPS maximum rate length. Example:
iops.maximum.rate.length = 3600 sets the maximum IOPS length
accepted for a disk offering as 3600 seconds (60 minutes).

* Fix log String.format message from %s to %d

* Add bytes rate validation

* Refactoring to cover Read/Write Bytes and IOPS length validation

* Fix "copy-paste" issue with bytes write rate max length
This commit is contained in:
Gabriel Beims Bräscher 2020-03-13 15:48:45 -03:00 committed by GitHub
parent c44539fd00
commit 4ca69ac152
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 127 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

74
server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
View File

@ -401,6 +401,20 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
public static final ConfigKey<Boolean> SystemVMUseLocalStorage = new ConfigKey<Boolean>(Boolean.class, "system.vm.use.local.storage", "Advanced", "false",
"Indicates whether to use local storage pools or shared storage pools for system VMs.", false, ConfigKey.Scope.Zone, null);
public final static ConfigKey<Long> BYTES_MAX_READ_LENGTH= new ConfigKey<Long>(Long.class, "vm.disk.bytes.maximum.read.length", "Advanced", "0",
"Maximum Bytes read burst duration (seconds). If '0' (zero) then does not check for maximum burst length.", true, ConfigKey.Scope.Global, null);
public final static ConfigKey<Long> BYTES_MAX_WRITE_LENGTH = new ConfigKey<Long>(Long.class, "vm.disk.bytes.maximum.write.length", "Advanced", "0",
"Maximum Bytes write burst duration (seconds). If '0' (zero) then does not check for maximum burst length.", true, ConfigKey.Scope.Global, null);
public final static ConfigKey<Long> IOPS_MAX_READ_LENGTH = new ConfigKey<Long>(Long.class, "vm.disk.iops.maximum.read.length", "Advanced", "0",
"Maximum IOPS read burst duration (seconds). If '0' (zero) then does not check for maximum burst length.", true, ConfigKey.Scope.Global, null);
public final static ConfigKey<Long> IOPS_MAX_WRITE_LENGTH = new ConfigKey<Long>(Long.class, "vm.disk.iops.maximum.write.length", "Advanced", "0",
"Maximum IOPS write burst duration (seconds). If '0' (zero) then does not check for maximum burst length.", true, ConfigKey.Scope.Global, null);
private static final String IOPS_READ_RATE = "IOPS Read";
private static final String IOPS_WRITE_RATE = "IOPS Write";
private static final String BYTES_READ_RATE = "Bytes Read";
private static final String BYTES_WRITE_RATE = "Bytes Write";
private static final String DefaultForSystemVmsForPodIpRange = "0";
private static final String DefaultVlanForPodIpRange = Vlan.UNTAGGED.toString();
@ -3012,6 +3026,13 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
final Integer hypervisorSnapshotReserve = cmd.getHypervisorSnapshotReserve();
final String cacheMode = cmd.getCacheMode();
validateMaxRateEqualsOrGreater(iopsReadRate, iopsReadRateMax, IOPS_READ_RATE);
validateMaxRateEqualsOrGreater(iopsWriteRate, iopsWriteRateMax, IOPS_WRITE_RATE);
validateMaxRateEqualsOrGreater(bytesReadRate, bytesReadRateMax, BYTES_READ_RATE);
validateMaxRateEqualsOrGreater(bytesWriteRate, bytesWriteRateMax, BYTES_WRITE_RATE);
validateMaximumIopsAndBytesLength(iopsReadRateMaxLength, iopsWriteRateMaxLength, bytesReadRateMaxLength, bytesWriteRateMaxLength);
final Long userId = CallContext.current().getCallingUserId();
return createDiskOffering(userId, domainIds, zoneIds, name, description, provisioningType, numGibibytes, tags, isCustomized,
localStorageRequired, isDisplayOfferingEnabled, isCustomizedIops, minIops,
@ -3020,6 +3041,57 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
hypervisorSnapshotReserve, cacheMode);
}
/**
* Validates rate offerings, being flexible about which rate is being validated (e.g. read/write Bytes, read/write IOPS).</br>
* It throws InvalidParameterValueException if normal rate is greater than maximum rate
*/
protected void validateMaxRateEqualsOrGreater(Long normalRate, Long maxRate, String rateType) {
if (normalRate != null && maxRate != null && maxRate < normalRate) {
throw new InvalidParameterValueException(
String.format("%s rate (%d) cannot be greater than %s maximum rate (%d)", rateType, normalRate, rateType, maxRate));
}
}
/**
* Throws InvalidParameterValueException if At least one of the VM disk Bytes/IOPS Read/Write length are smaller than the respective disk offering max length.</br>
* It will ignore verification in case of default values (zero):
* <ul>
* <li>vm.disk.bytes.maximum.read.length = 0</li>
* <li>vm.disk.bytes.maximum.write.length = 0</li>
* <li>vm.disk.iops.maximum.read.length = 0</li>
* <li>vm.disk.iops.maximum.write.length = 0</li>
* </ul>
*/
protected void validateMaximumIopsAndBytesLength(final Long iopsReadRateMaxLength, final Long iopsWriteRateMaxLength, Long bytesReadRateMaxLength, Long bytesWriteRateMaxLength) {
if (IOPS_MAX_READ_LENGTH.value() != null && IOPS_MAX_READ_LENGTH.value() != 0l) {
if (iopsReadRateMaxLength != null && iopsReadRateMaxLength > IOPS_MAX_READ_LENGTH.value()) {
throw new InvalidParameterValueException(String.format("IOPS read max length (%d seconds) cannot be greater than vm.disk.iops.maximum.read.length (%d seconds)",
iopsReadRateMaxLength, IOPS_MAX_READ_LENGTH.value()));
}
}
if (IOPS_MAX_WRITE_LENGTH.value() != null && IOPS_MAX_WRITE_LENGTH.value() != 0l) {
if (iopsWriteRateMaxLength != null && iopsWriteRateMaxLength > IOPS_MAX_WRITE_LENGTH.value()) {
throw new InvalidParameterValueException(String.format("IOPS write max length (%d seconds) cannot be greater than vm.disk.iops.maximum.write.length (%d seconds)",
iopsWriteRateMaxLength, IOPS_MAX_WRITE_LENGTH.value()));
}
}
if (BYTES_MAX_READ_LENGTH.value() != null && BYTES_MAX_READ_LENGTH.value() != 0l) {
if (bytesReadRateMaxLength != null && bytesReadRateMaxLength > BYTES_MAX_READ_LENGTH.value()) {
throw new InvalidParameterValueException(String.format("Bytes read max length (%d seconds) cannot be greater than vm.disk.bytes.maximum.read.length (%d seconds)",
bytesReadRateMaxLength, BYTES_MAX_READ_LENGTH.value()));
}
}
if (BYTES_MAX_WRITE_LENGTH.value() != null && BYTES_MAX_WRITE_LENGTH.value() != 0l) {
if (bytesWriteRateMaxLength != null && bytesWriteRateMaxLength > BYTES_MAX_WRITE_LENGTH.value()) {
throw new InvalidParameterValueException(String.format("Bytes write max length (%d seconds) cannot be greater than vm.disk.bytes.maximum.write.length (%d seconds)",
bytesWriteRateMaxLength, BYTES_MAX_WRITE_LENGTH.value()));
}
}
}
@Override
@ActionEvent(eventType = EventTypes.EVENT_DISK_OFFERING_EDIT, eventDescription = "updating disk offering")
public DiskOffering updateDiskOffering(final UpdateDiskOfferingCmd cmd) {
@ -6306,6 +6378,6 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
@Override
public ConfigKey<?>[] getConfigKeys() {
return new ConfigKey<?>[] {SystemVMUseLocalStorage};
return new ConfigKey<?>[] {SystemVMUseLocalStorage, IOPS_MAX_READ_LENGTH, IOPS_MAX_WRITE_LENGTH, BYTES_MAX_READ_LENGTH, BYTES_MAX_WRITE_LENGTH};
}
}

80
server/src/test/java/com/cloud/configuration/ConfigurationManagerTest.java
View File

@ -19,11 +19,11 @@ package com.cloud.configuration;
import static org.hamcrest.CoreMatchers.is;
import static org.junit.Assert.assertThat;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyBoolean;
import static org.mockito.Matchers.anyInt;
import static org.mockito.Matchers.anyLong;
import static org.mockito.Matchers.anyString;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
@ -166,6 +166,8 @@ public class ConfigurationManagerTest {
VlanVO vlan = new VlanVO(Vlan.VlanType.VirtualNetwork, "vlantag", "vlangateway", "vlannetmask", 1L, "iprange", 1L, 1L, null, null, null);
private static final String MAXIMUM_DURATION_ALLOWED = "3600";
@Mock
Network network;
@Mock
@ -280,9 +282,8 @@ public class ConfigurationManagerTest {
when(configurationMgr._accountVlanMapDao.listAccountVlanMapsByAccount(anyLong())).thenReturn(null);
DataCenterVO dc =
new DataCenterVO(UUID.randomUUID().toString(), "test", "8.8.8.8", null, "10.0.0.1", null, "10.0.0.1/24", null, null, NetworkType.Advanced, null, null, true,
true, null, null);
DataCenterVO dc = new DataCenterVO(UUID.randomUUID().toString(), "test", "8.8.8.8", null, "10.0.0.1", null, "10.0.0.1/24", null, null, NetworkType.Advanced, null, null,
true, true, null, null);
when(configurationMgr._zoneDao.findById(anyLong())).thenReturn(dc);
List<IPAddressVO> ipAddressList = new ArrayList<IPAddressVO>();
@ -324,9 +325,8 @@ public class ConfigurationManagerTest {
accountVlanMaps.add(accountVlanMap);
when(configurationMgr._accountVlanMapDao.listAccountVlanMapsByVlan(anyLong())).thenReturn(accountVlanMaps);
DataCenterVO dc =
new DataCenterVO(UUID.randomUUID().toString(), "test", "8.8.8.8", null, "10.0.0.1", null, "10.0.0.1/24", null, null, NetworkType.Advanced, null, null, true,
true, null, null);
DataCenterVO dc = new DataCenterVO(UUID.randomUUID().toString(), "test", "8.8.8.8", null, "10.0.0.1", null, "10.0.0.1/24", null, null, NetworkType.Advanced, null, null,
true, true, null, null);
when(configurationMgr._zoneDao.findById(anyLong())).thenReturn(dc);
List<IPAddressVO> ipAddressList = new ArrayList<IPAddressVO>();
@ -351,8 +351,7 @@ public class ConfigurationManagerTest {
when(configurationMgr._accountVlanMapDao.listAccountVlanMapsByVlan(anyLong())).thenReturn(null);
// public ip range belongs to zone of type basic
DataCenterVO dc =
new DataCenterVO(UUID.randomUUID().toString(), "test", "8.8.8.8", null, "10.0.0.1", null, "10.0.0.1/24", null, null, NetworkType.Basic, null, null, true,
DataCenterVO dc = new DataCenterVO(UUID.randomUUID().toString(), "test", "8.8.8.8", null, "10.0.0.1", null, "10.0.0.1/24", null, null, NetworkType.Basic, null, null, true,
true, null, null);
when(configurationMgr._zoneDao.findById(anyLong())).thenReturn(dc);
@ -377,9 +376,8 @@ public class ConfigurationManagerTest {
when(configurationMgr._accountVlanMapDao.listAccountVlanMapsByAccount(anyLong())).thenReturn(null);
DataCenterVO dc =
new DataCenterVO(UUID.randomUUID().toString(), "test", "8.8.8.8", null, "10.0.0.1", null, "10.0.0.1/24", null, null, NetworkType.Advanced, null, null, true,
true, null, null);
DataCenterVO dc = new DataCenterVO(UUID.randomUUID().toString(), "test", "8.8.8.8", null, "10.0.0.1", null, "10.0.0.1/24", null, null, NetworkType.Advanced, null, null,
true, true, null, null);
when(configurationMgr._zoneDao.findById(anyLong())).thenReturn(dc);
// one of the ip addresses of the range is allocated to different account
@ -489,11 +487,7 @@ public class ConfigurationManagerTest {
public void searchForNetworkOfferingsTest() {
NetworkOfferingJoinVO forVpcOfferingJoinVO = new NetworkOfferingJoinVO();
forVpcOfferingJoinVO.setForVpc(true);
List<NetworkOfferingJoinVO> offerings = Arrays.asList(
new NetworkOfferingJoinVO(),
new NetworkOfferingJoinVO(),
forVpcOfferingJoinVO
);
List<NetworkOfferingJoinVO> offerings = Arrays.asList(new NetworkOfferingJoinVO(), new NetworkOfferingJoinVO(), forVpcOfferingJoinVO);
Mockito.when(networkOfferingJoinDao.createSearchCriteria()).thenReturn(Mockito.mock(SearchCriteria.class));
Mockito.when(networkOfferingJoinDao.search(Mockito.any(SearchCriteria.class), Mockito.any(Filter.class))).thenReturn(offerings);
@ -558,9 +552,9 @@ public class ConfigurationManagerTest {
configurationMgr.validateStaticNatServiceCapablities(staticNatServiceCapabilityMap);
} catch (InvalidParameterValueException e) {
Assert.assertTrue(
e.getMessage(),
e.getMessage().contains(
"Capability " + Capability.AssociatePublicIP.getName() + " can only be set when capability " + Capability.ElasticIp.getName() + " is true"));
e.getMessage(),
e.getMessage().contains(
"Capability " + Capability.AssociatePublicIP.getName() + " can only be set when capability " + Capability.ElasticIp.getName() + " is true"));
caught = true;
}
Assert.assertTrue("should not be accepted", caught);
@ -858,7 +852,6 @@ public class ConfigurationManagerTest {
doThrow(new InvalidParameterValueException("Exception from Mock: endIPv6 is not in ip6cidr indicated network!")).when(configurationMgr._networkModel).checkIp6Parameters("2001:db8:0:f101::a", "2001:db9:0:f101::2", "2001:db8:0:f101::1", "2001:db8:0:f101::0/64");
doThrow(new InvalidParameterValueException("ip6Gateway and ip6Cidr should be defined when startIPv6/endIPv6 are passed in")).when(configurationMgr._networkModel).checkIp6Parameters(Mockito.anyString(), Mockito.anyString(), Mockito.isNull(String.class), Mockito.isNull(String.class));
configurationMgr.hasSameSubnet(false, null, null, null, null, null, null, true, "2001:db8:0:f101::1", "2001:db8:0:f101::0/64", "2001:db8:0:f101::2", "2001:db8:0:f101::a", ipV6Network);
Assert.assertTrue(result);
try {
@ -883,7 +876,7 @@ public class ConfigurationManagerTest {
try {
configurationMgr.hasSameSubnet(false, null, null, null, null, null, null, true, "2001:db8:0:f101::1", "2001:db8:0:f101::0/64", "2001:db8:0:f101::a", "2001:db9:0:f101::2", ipV6Network);
Assert.fail();
} catch(InvalidParameterValueException e){
} catch(InvalidParameterValueException e) {
Assert.assertEquals(e.getMessage(), "Exception from Mock: endIPv6 is not in ip6cidr indicated network!");
}
@ -910,4 +903,39 @@ public class ConfigurationManagerTest {
public void testGetVlanNumberFromUriUntagged() {
Assert.assertEquals("untagged", configurationMgr.getVlanNumberFromUri("vlan://untagged"));
}
@Test
public void validateMaxRateEqualsOrGreaterTestAllGood() {
configurationMgr.validateMaxRateEqualsOrGreater(1l, 2l, "IOPS Read");
}
@Test(expected = InvalidParameterValueException.class)
public void validateMaxRateEqualsOrGreaterTestNormalRateGreaterThanMax() {
configurationMgr.validateMaxRateEqualsOrGreater(3l, 2l, "IOPS Read");
}
@Test
public void validateMaxRateNull() {
configurationMgr.validateMaxRateEqualsOrGreater(3l, null, "IOPS Read");
}
@Test
public void validateNormalRateNull() {
configurationMgr.validateMaxRateEqualsOrGreater(null, 3l, "IOPS Read");
}
@Test
public void validateAllNull() {
configurationMgr.validateMaxRateEqualsOrGreater(null, 3l, "IOPS Read");
}
@Test
public void validateMaximumIopsAndBytesLengthTestAllNull() {
configurationMgr.validateMaximumIopsAndBytesLength(null, null, null, null);
}
@Test
public void validateMaximumIopsAndBytesLengthTestDefaultLengthConfigs() {
configurationMgr.validateMaximumIopsAndBytesLength(36000l, 36000l, 36000l, 36000l);
}
}