diff --git a/framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaManagerImpl.java b/framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaManagerImpl.java index 5afef8bc95b6..01bb9bc7e7a0 100644 --- a/framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaManagerImpl.java +++ b/framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaManagerImpl.java @@ -54,6 +54,7 @@ import org.apache.cloudstack.quota.vo.QuotaTariffUsageVO; import org.apache.cloudstack.quota.vo.QuotaTariffVO; import org.apache.cloudstack.quota.vo.QuotaUsageVO; +import org.apache.cloudstack.usage.UsageTypes; import org.apache.cloudstack.usage.UsageUnitTypes; import org.apache.cloudstack.utils.bytescale.ByteScaleUtils; import org.apache.cloudstack.utils.jsinterpreter.JsInterpreter; @@ -366,6 +367,11 @@ protected boolean shouldCalculateUsageRecord(AccountVO accountVO, UsageVO usageR usageRecord.toString(usageAggregationTimeZone), accountVO.reflectionToString()); return false; } + if (usageRecord.getUsageType() == UsageTypes.VOLUME && usageRecord.getVmInstanceId() != null) { + logger.debug("Considering usage record [{}] as calculated and skipping it because it represents the period " + + "a volume has remained attached to an instance, which Quota does not handle.", usageRecord.toString(usageAggregationTimeZone)); + return false; + } return true; } diff --git a/framework/quota/src/test/java/org/apache/cloudstack/quota/QuotaManagerImplTest.java b/framework/quota/src/test/java/org/apache/cloudstack/quota/QuotaManagerImplTest.java index 1e08e7d7fc00..5f9a9d3c0222 100644 --- a/framework/quota/src/test/java/org/apache/cloudstack/quota/QuotaManagerImplTest.java +++ b/framework/quota/src/test/java/org/apache/cloudstack/quota/QuotaManagerImplTest.java @@ -26,12 +26,15 @@ import java.util.List; import java.util.Map; +import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.framework.config.impl.ConfigDepotImpl; import org.apache.cloudstack.quota.activationrule.presetvariables.Domain; import org.apache.cloudstack.quota.activationrule.presetvariables.GenericPresetVariable; import org.apache.cloudstack.quota.activationrule.presetvariables.PresetVariableHelper; import org.apache.cloudstack.quota.activationrule.presetvariables.PresetVariables; import org.apache.cloudstack.quota.activationrule.presetvariables.Tariff; import org.apache.cloudstack.quota.activationrule.presetvariables.Value; +import org.apache.cloudstack.quota.constant.QuotaConfig; import org.apache.cloudstack.quota.constant.QuotaTypes; import org.apache.cloudstack.quota.dao.QuotaTariffDao; import org.apache.cloudstack.quota.dao.QuotaTariffUsageDao; @@ -39,10 +42,13 @@ import org.apache.cloudstack.quota.vo.QuotaTariffUsageVO; import org.apache.cloudstack.quota.vo.QuotaTariffVO; import org.apache.cloudstack.quota.vo.QuotaUsageVO; +import org.apache.cloudstack.usage.UsageTypes; import org.apache.cloudstack.usage.UsageUnitTypes; import org.apache.cloudstack.utils.bytescale.ByteScaleUtils; import org.apache.cloudstack.utils.jsinterpreter.JsInterpreter; +import org.junit.After; import org.junit.Assert; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.InjectMocks; @@ -95,8 +101,21 @@ public class QuotaManagerImplTest { @Mock QuotaTariffUsageDao quotaTariffUsageDaoMock; + @Mock + ConfigDepotImpl configDepotImplMock; + SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss"); + @Before + public void setup() { + ConfigKey.init(configDepotImplMock); + } + + @After + public void tearDown() { + ConfigKey.init(null); + } + @Test public void isLockableTestValidateAccountTypes() { List lockablesAccountTypes = Arrays.asList(Account.Type.NORMAL, Account.Type.DOMAIN_ADMIN); @@ -113,6 +132,49 @@ public void isLockableTestValidateAccountTypes() { }); } + @Test + public void shouldCalculateUsageRecordTestQuotaIsDisabledForAccountReturnFalse() { + Mockito.doReturn(1L).when(accountVoMock).getAccountId(); + Mockito.doReturn("false").when(configDepotImplMock).getConfigStringValue(Mockito.eq(QuotaConfig.QuotaAccountEnabled.key()), Mockito.eq(ConfigKey.Scope.Account), + Mockito.eq(1L)); + + boolean result = quotaManagerImplSpy.shouldCalculateUsageRecord(accountVoMock, usageVoMock); + + Assert.assertFalse(result); + } + + @Test + public void shouldCalculateUsageRecordTestQuotaIsEnabledForAccountAndUsageRecordIsVolumeWithVmInstanceIdReturnFalse() { + Mockito.doReturn(1L).when(accountVoMock).getAccountId(); + Mockito.doReturn(UsageTypes.VOLUME).when(usageVoMock).getUsageType(); + Mockito.doReturn(1L).when(usageVoMock).getVmInstanceId(); + + boolean result = quotaManagerImplSpy.shouldCalculateUsageRecord(accountVoMock, usageVoMock); + + Assert.assertFalse(result); + } + + @Test + public void shouldCalculateUsageRecordTestQuotaIsEnabledForAccountAndUsageRecordIsVolumeWithoutVmInstanceIdReturnTrue() { + Mockito.doReturn(1L).when(accountVoMock).getAccountId(); + Mockito.doReturn(UsageTypes.VOLUME).when(usageVoMock).getUsageType(); + Mockito.doReturn(null).when(usageVoMock).getVmInstanceId(); + + boolean result = quotaManagerImplSpy.shouldCalculateUsageRecord(accountVoMock, usageVoMock); + + Assert.assertTrue(result); + } + + @Test + public void shouldCalculateUsageRecordTestQuotaIsEnabledForAccountAndUsageRecordIsNotVolumeReturnTrue() { + Mockito.doReturn(1L).when(accountVoMock).getAccountId(); + Mockito.doReturn(UsageTypes.RUNNING_VM).when(usageVoMock).getUsageType(); + + boolean result = quotaManagerImplSpy.shouldCalculateUsageRecord(accountVoMock, usageVoMock); + + Assert.assertTrue(result); + } + @Test public void getPendingUsageRecordsForQuotaAggregationTestNullListReturnNull() { Mockito.doReturn(pairMock).when(usageDaoMock).listUsageRecordsPendingForQuotaAggregation(Mockito.anyLong(), Mockito.anyLong());