diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index 1dba76a5210..7e9c9d39c2b 100755 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -401,6 +401,20 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati public static final ConfigKey SystemVMUseLocalStorage = new ConfigKey(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 BYTES_MAX_READ_LENGTH= new ConfigKey(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 BYTES_MAX_WRITE_LENGTH = new ConfigKey(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 IOPS_MAX_READ_LENGTH = new ConfigKey(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 IOPS_MAX_WRITE_LENGTH = new ConfigKey(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).
+ * 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.
+ * It will ignore verification in case of default values (zero): + *
    + *
  • vm.disk.bytes.maximum.read.length = 0
    • + *
  • vm.disk.bytes.maximum.write.length = 0
    • + *
  • vm.disk.iops.maximum.read.length = 0
    • + *
  • vm.disk.iops.maximum.write.length = 0
    • + *
+ */ + 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}; } } diff --git a/server/src/test/java/com/cloud/configuration/ConfigurationManagerTest.java b/server/src/test/java/com/cloud/configuration/ConfigurationManagerTest.java index a7a9d2aae53..e6953296db0 100644 --- a/server/src/test/java/com/cloud/configuration/ConfigurationManagerTest.java +++ b/server/src/test/java/com/cloud/configuration/ConfigurationManagerTest.java @@ -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 ipAddressList = new ArrayList(); @@ -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 ipAddressList = new ArrayList(); @@ -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 offerings = Arrays.asList( - new NetworkOfferingJoinVO(), - new NetworkOfferingJoinVO(), - forVpcOfferingJoinVO - ); + List 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); + } }