Allow a service offering to set its own overcommit ratio

PendingReleaseNotes

@@ -91,3 +91,24 @@ example.ver.1 > example.ver.2:
     no device-mapper multipath configuration is required. A new
     Storage.StoragePoolType.NVMeTCP + MultipathNVMeOFAdapterBase /
     NVMeTCPAdapter on the KVM side back the new pool type.
+
+  * A service offering may now set its own overcommit ratio, overriding its cluster's, using the
+    cpuOvercommitRatio and memoryOvercommitRatio service offering details - the same detail keys
+    already used at cluster and VM scope. Absent, the cluster's ratio applies as before, so nothing
+    changes for existing offerings.
+
+    The value means the same thing at every scope: how far a VM's declared size is inflated
+    relative to what it really holds. 1 means not overcommitted at all, which is how an offering is
+    exempted from its cluster's overcommit. Values below 1 would mean reserving more than the VM
+    asked for and are rejected. An offering may declare a ratio higher than its cluster's; this is
+    logged.
+
+    An offering with a memory ratio of 1 on an overcommitted cluster is charged its full request
+    and keeps all of it: free page reporting and balloon auto-deflate are turned off for its VMs,
+    so the guest does not lend unused memory back to the host. The balloon device itself stays, so
+    memory statistics are unaffected. This is intended for infrastructure VMs that should not
+    participate in the overcommit that other workloads rely on.
+
+    Also fixes a latent accounting bug: capacity was charged raw when a VM started and stopped, but
+    recalculated with the VM's overcommit ratio applied. These agreed only while every VM shared
+    its cluster's ratio.

api/src/main/java/com/cloud/vm/VmDetailConstants.java

@@ -73,6 +73,7 @@ public interface VmDetailConstants {
     // Misc details for internal usage (not to be set/changed by user or admin)
     String CPU_OVER_COMMIT_RATIO = "cpuOvercommitRatio";
     String MEMORY_OVER_COMMIT_RATIO = "memoryOvercommitRatio";
+    String MEMORY_RECLAIM_DISABLED = "memoryReclaimDisabled";
     String MESSAGE_RESERVED_CAPACITY_FREED_FLAG = "Message.ReservedCapacityFreed.Flag";
     String DEPLOY_VM = "deployvm";
     String SSH_PUBLIC_KEY = "SSH.PublicKey";

engine/components-api/src/main/java/com/cloud/capacity/CapacityManager.java

@@ -162,6 +162,12 @@ boolean checkIfHostHasCapacity(Host host, Integer cpu, long ram, boolean checkFr
 
     void updateCapacityForHost(Host host);
 
+    /**
+     * Expresses a VM's request in the units capacity is counted in, taking account of any overcommit
+     * ratio its service offering sets in place of its cluster's.
+     */
+    long scaleRequestForOffering(long serviceOfferingId, long clusterId, long requested, boolean forCpu);
+
     /**
      * @param pool storage pool
      * @param templateForVmCreation template that will be used for vm creation

engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java

@@ -180,7 +180,6 @@
 import com.cloud.capacity.CapacityManager;
 import com.cloud.configuration.Resource;
 import com.cloud.dc.ClusterDetailsDao;
-import com.cloud.dc.ClusterDetailsVO;
 import com.cloud.dc.ClusterVO;
 import com.cloud.dc.DataCenter;
 import com.cloud.dc.DataCenterVO;
@@ -405,6 +404,8 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac
     @Inject
     private ClusterDetailsDao _clusterDetailsDao;
     @Inject
+    private VirtualMachineManagerOverCommit vmOverCommit;
+    @Inject
     private VMInstanceDetailsDao vmInstanceDetailsDao;
     @Inject
     private VolumeOrchestrationService volumeMgr;
@@ -1502,7 +1503,7 @@ public void orchestrateStart(final String vmUuid, final Map<VirtualMachineProfil
                 long destHostId = dest.getHost().getId();
                 vm.setPodIdToDeployIn(dest.getPod().getId());
                 final Long clusterId = dest.getCluster().getId();
-                updateOverCommitRatioForVmProfile(vmProfile, clusterId);
+                vmOverCommit.updateOverCommitRatioForVmProfile(vmProfile, clusterId);
 
                 StartAnswer startAnswer = null;
 
@@ -1854,27 +1855,6 @@ private void addToNetworkNameMap(long networkId, long dataCenterId, Map<Long, St
         networkToNetworkNameMap.put(networkVO.getId(), networkName);
     }
 
-    private void updateOverCommitRatioForVmProfile(VirtualMachineProfile vmProfile, long clusterId) {
-        final ClusterDetailsVO clusterDetailCpu = _clusterDetailsDao.findDetail(clusterId, VmDetailConstants.CPU_OVER_COMMIT_RATIO);
-        final ClusterDetailsVO clusterDetailRam = _clusterDetailsDao.findDetail(clusterId, VmDetailConstants.MEMORY_OVER_COMMIT_RATIO);
-        final float parsedClusterCpuDetailCpu = Float.parseFloat(clusterDetailCpu.getValue());
-        final float parsedClusterDetailRam = Float.parseFloat(clusterDetailRam.getValue());
-        VMInstanceDetailVO vmDetailCpu = vmInstanceDetailsDao.findDetail(vmProfile.getId(), VmDetailConstants.CPU_OVER_COMMIT_RATIO);
-        VMInstanceDetailVO vmDetailRam = vmInstanceDetailsDao.findDetail(vmProfile.getId(), VmDetailConstants.MEMORY_OVER_COMMIT_RATIO);
-
-        if ((vmDetailCpu == null && parsedClusterCpuDetailCpu > 1f) ||
-                (vmDetailCpu != null && Float.parseFloat(vmDetailCpu.getValue()) != parsedClusterCpuDetailCpu)) {
-            vmInstanceDetailsDao.addDetail(vmProfile.getId(), VmDetailConstants.CPU_OVER_COMMIT_RATIO, clusterDetailCpu.getValue(), true);
-        }
-        if ((vmDetailRam == null && parsedClusterDetailRam > 1f) ||
-                (vmDetailRam != null && Float.parseFloat(vmDetailRam.getValue()) != parsedClusterDetailRam)) {
-            vmInstanceDetailsDao.addDetail(vmProfile.getId(), VmDetailConstants.MEMORY_OVER_COMMIT_RATIO, clusterDetailRam.getValue(), true);
-        }
-
-        vmProfile.setCpuOvercommitRatio(Float.parseFloat(clusterDetailCpu.getValue()));
-        vmProfile.setMemoryOvercommitRatio(Float.parseFloat(clusterDetailRam.getValue()));
-    }
-
     /**
      * Setting pod id to null can result in migration of Volumes across pods. This is not desirable for VMs which
      * have a volume in Ready state (happens when a VM is shutdown and started again).
@@ -3178,7 +3158,7 @@ protected void migrate(final VMInstanceVO vm, final long srcHostId, final Deploy
         _networkMgr.prepareNicForMigration(profile, dest);
         volumeMgr.prepareForMigration(profile, dest);
         profile.setConfigDriveLabel(VmConfigDriveLabel.value());
-        updateOverCommitRatioForVmProfile(profile, dest.getHost().getClusterId());
+        vmOverCommit.updateOverCommitRatioForVmProfile(profile, dest.getHost().getClusterId());
 
         final VirtualMachineTO to = toVmTO(profile);
 
@@ -4215,7 +4195,7 @@ protected VirtualMachineTO prepVmSpecForUnmanageCmd(Long vmId, Long hostId) {
         final VirtualMachineTemplate template = _entityMgr.findByIdIncludingRemoved(VirtualMachineTemplate.class, vm.getTemplateId());
         Host host = _hostDao.findById(hostId);
         VirtualMachineProfileImpl vmProfile = new VirtualMachineProfileImpl(vm, template, offering, owner, null);
-        updateOverCommitRatioForVmProfile(vmProfile, host.getClusterId());
+        vmOverCommit.updateOverCommitRatioForVmProfile(vmProfile, host.getClusterId());
         final List<NicVO> nics = _nicsDao.listByVmId(vmProfile.getId());
         Collections.sort(nics, (nic1, nic2) -> {
             Long nicId1 = Long.valueOf(nic1.getDeviceId());

engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerOverCommit.java

@@ -0,0 +1,148 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package com.cloud.vm;
+
+import javax.inject.Inject;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+import com.cloud.dc.ClusterDetailsDao;
+import com.cloud.dc.ClusterDetailsVO;
+import com.cloud.service.dao.ServiceOfferingDetailsDao;
+import com.cloud.vm.dao.VMInstanceDetailsDao;
+
+/**
+ * Works out the overcommit ratio a VM runs at - its service offering's if it sets one, otherwise its
+ * cluster's - and records it on the VM, which is what capacity accounting and the hypervisor then read.
+ */
+public class VirtualMachineManagerOverCommit {
+    protected Logger logger = LogManager.getLogger(getClass());
+
+    @Inject
+    private ClusterDetailsDao clusterDetailsDao;
+    @Inject
+    private ServiceOfferingDetailsDao serviceOfferingDetailsDao;
+    @Inject
+    private VMInstanceDetailsDao vmInstanceDetailsDao;
+
+    public void updateOverCommitRatioForVmProfile(VirtualMachineProfile vmProfile, long clusterId) {
+        final ClusterDetailsVO clusterDetailCpu = clusterDetailsDao.findDetail(clusterId, VmDetailConstants.CPU_OVER_COMMIT_RATIO);
+        final ClusterDetailsVO clusterDetailRam = clusterDetailsDao.findDetail(clusterId, VmDetailConstants.MEMORY_OVER_COMMIT_RATIO);
+        final float clusterCpuRatio = Float.parseFloat(clusterDetailCpu.getValue());
+        final float clusterRamRatio = Float.parseFloat(clusterDetailRam.getValue());
+
+        final Float offeringCpuRatio = offeringOverCommitRatio(vmProfile, VmDetailConstants.CPU_OVER_COMMIT_RATIO, clusterCpuRatio);
+        final Float offeringRamRatio = offeringOverCommitRatio(vmProfile, VmDetailConstants.MEMORY_OVER_COMMIT_RATIO, clusterRamRatio);
+
+        final float cpuRatio = offeringCpuRatio != null ? offeringCpuRatio : clusterCpuRatio;
+        final float ramRatio = offeringRamRatio != null ? offeringRamRatio : clusterRamRatio;
+
+        persistOverCommitRatio(vmProfile.getId(), VmDetailConstants.CPU_OVER_COMMIT_RATIO, cpuRatio, clusterCpuRatio);
+        persistOverCommitRatio(vmProfile.getId(), VmDetailConstants.MEMORY_OVER_COMMIT_RATIO, ramRatio, clusterRamRatio);
+        persistMemoryReclaimFlag(vmProfile.getId(), offeringRamRatio);
+
+        vmProfile.setCpuOvercommitRatio(cpuRatio);
+        vmProfile.setMemoryOvercommitRatio(ramRatio);
+    }
+
+    /**
+     * The overcommit ratio a VM's service offering asks for, or null when it does not ask for one
+     * and should inherit its cluster's.
+     *
+     * An offering setting a ratio of 1 on an overcommitted cluster is how infrastructure VMs are
+     * kept off the overcommit: they are charged their full request and hold all of it.
+     */
+    protected Float offeringOverCommitRatio(VirtualMachineProfile vmProfile, String key, float clusterRatio) {
+        String offeringRatio = serviceOfferingDetailsDao.getDetail(vmProfile.getServiceOfferingId(), key);
+        if (offeringRatio == null) {
+            return null;
+        }
+        float ratio;
+        try {
+            ratio = Float.parseFloat(offeringRatio);
+        } catch (NumberFormatException e) {
+            logger.warn("Ignoring {} of [{}] on service offering {}: it is not a number.",
+                    key, offeringRatio, vmProfile.getServiceOfferingId());
+            return null;
+        }
+        if (ratio < 1) {
+            // rejected when the offering is created; only reachable by writing the detail directly
+            logger.warn("Ignoring {} of {} on service offering {}: it must be at least 1.",
+                    key, offeringRatio, vmProfile.getServiceOfferingId());
+            return null;
+        }
+        if (ratio > clusterRatio) {
+            logger.info("Service offering {} declares {} of {}, more than its cluster's {}, so its VMs are "
+                            + "oversubscribed further than the cluster default.",
+                    vmProfile.getServiceOfferingId(), key, ratio, clusterRatio);
+        }
+        return ratio;
+    }
+
+    /**
+     * Turns off returning the guest's unused pages to the host, for a VM whose offering has
+     * explicitly opted out of overcommit. Otherwise the exemption would hold in the books only.
+     *
+     * Only an explicit choice counts. An offering that says nothing inherits its cluster, and a
+     * cluster with no overcommit configured is the default everywhere - reading that as "pin the
+     * memory of every VM" would change behaviour for installations that opted into nothing.
+     *
+     * @param offeringRatio
+     *         the ratio the offering asked for, or null if it asked for nothing
+     */
+    protected void persistMemoryReclaimFlag(long vmId, Float offeringRatio) {
+        boolean disable = offeringRatio != null && offeringRatio <= 1f;
+        VMInstanceDetailVO existing = vmInstanceDetailsDao.findDetail(vmId, VmDetailConstants.MEMORY_RECLAIM_DISABLED);
+        if (disable && existing == null) {
+            vmInstanceDetailsDao.addDetail(vmId, VmDetailConstants.MEMORY_RECLAIM_DISABLED, "true", true);
+        } else if (!disable && existing != null) {
+            vmInstanceDetailsDao.removeDetail(vmId, VmDetailConstants.MEMORY_RECLAIM_DISABLED);
+        }
+    }
+
+    /**
+     * Records the ratio the VM is running at.
+     *
+     * Capacity is counted in cluster-overcommitted units, and the charge scales a VM by this detail,
+     * so it has to be present whenever the VM's ratio differs from its cluster's - otherwise the VM
+     * is charged as though it shared the cluster's ratio. It is also kept while the cluster is
+     * overcommitted at all, so that capacity accounting survives the cluster ratio being changed
+     * beneath a running VM.
+     */
+    protected void persistOverCommitRatio(long vmId, String key, float ratio, float clusterRatio) {
+        boolean needed = ratio != clusterRatio || clusterRatio > 1f;
+        VMInstanceDetailVO existing = vmInstanceDetailsDao.findDetail(vmId, key);
+        if (!needed) {
+            if (existing != null) {
+                vmInstanceDetailsDao.removeDetail(vmId, key);
+            }
+            return;
+        }
+        if (existing == null || parseRatioOrDefault(existing.getValue(), Float.NaN) != ratio) {
+            vmInstanceDetailsDao.addDetail(vmId, key, String.valueOf(ratio), true);
+        }
+    }
+
+    private float parseRatioOrDefault(String value, float fallback) {
+        try {
+            return Float.parseFloat(value);
+        } catch (NumberFormatException | NullPointerException e) {
+            return fallback;
+        }
+    }
+}

engine/orchestration/src/main/resources/META-INF/cloudstack/core/spring-engine-orchestration-core-context.xml

@@ -51,6 +51,8 @@
     <bean id="dataMigrationHelper"
           class="org.apache.cloudstack.engine.orchestration.DataMigrationUtility"/>
 
+    <bean id="virtualMachineManagerOverCommit" class="com.cloud.vm.VirtualMachineManagerOverCommit" />
+
     <bean id="clusteredVirtualMachineManagerImpl" class="com.cloud.vm.ClusteredVirtualMachineManagerImpl">
         <property name="hostAllocators" value="#{hostAllocatorsRegistry.registered}" />
         <property name="storagePoolAllocators"

engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java

@@ -108,7 +108,6 @@
 import com.cloud.api.query.dao.UserVmJoinDao;
 import com.cloud.api.query.vo.UserVmJoinVO;
 import com.cloud.dc.ClusterDetailsDao;
-import com.cloud.dc.ClusterDetailsVO;
 import com.cloud.dc.ClusterVO;
 import com.cloud.dc.DataCenter;
 import com.cloud.dc.DataCenterVO;
@@ -272,6 +271,8 @@ public class VirtualMachineManagerImplTest {
     @Mock
     private ClusterDetailsDao _clusterDetailsDao;
     @Mock
+    private VirtualMachineManagerOverCommit vmOverCommit;
+    @Mock
     private VMInstanceDetailsDao vmInstanceDetailsDao;
     @Mock
     private ItWorkDao _workDao;
@@ -1176,14 +1177,7 @@ public void testOrchestrateStartNonNullPodId() throws Exception {
 
         Cluster cluster = mock(Cluster.class);
         when(dest.getCluster()).thenReturn(cluster);
-        ClusterDetailsVO cluster_detail_cpu = mock(ClusterDetailsVO.class);
-        ClusterDetailsVO cluster_detail_ram = mock(ClusterDetailsVO.class);
         when(cluster.getId()).thenReturn(1L);
-        when(_clusterDetailsDao.findDetail(1L, VmDetailConstants.CPU_OVER_COMMIT_RATIO)).thenReturn(cluster_detail_cpu);
-        when(_clusterDetailsDao.findDetail(1L, VmDetailConstants.MEMORY_OVER_COMMIT_RATIO)).thenReturn(cluster_detail_ram);
-        when(vmInstanceDetailsDao.findDetail(anyLong(), Mockito.anyString())).thenReturn(null);
-        when(cluster_detail_cpu.getValue()).thenReturn("1.0");
-        when(cluster_detail_ram.getValue()).thenReturn("1.0");
         doReturn(false).when(virtualMachineManagerImpl).areAllVolumesAllocated(Mockito.anyLong());
 
         CallContext callContext = mock(CallContext.class);
@@ -1272,14 +1266,7 @@ public void testOrchestrateStartNullPodId() throws Exception {
 
         Cluster cluster = mock(Cluster.class);
         when(dest.getCluster()).thenReturn(cluster);
-        ClusterDetailsVO cluster_detail_cpu = mock(ClusterDetailsVO.class);
-        ClusterDetailsVO cluster_detail_ram = mock(ClusterDetailsVO.class);
         when(cluster.getId()).thenReturn(1L);
-        when(_clusterDetailsDao.findDetail(1L, VmDetailConstants.CPU_OVER_COMMIT_RATIO)).thenReturn(cluster_detail_cpu);
-        when(_clusterDetailsDao.findDetail(1L, VmDetailConstants.MEMORY_OVER_COMMIT_RATIO)).thenReturn(cluster_detail_ram);
-        when(vmInstanceDetailsDao.findDetail(anyLong(), Mockito.anyString())).thenReturn(null);
-        when(cluster_detail_cpu.getValue()).thenReturn("1.0");
-        when(cluster_detail_ram.getValue()).thenReturn("1.0");
         doReturn(true).when(virtualMachineManagerImpl).areAllVolumesAllocated(Mockito.anyLong());
 
         CallContext callContext = mock(CallContext.class);
@@ -1728,14 +1715,6 @@ public void testPrepVMSpecForUnmanageInstance() {
 
         when(hostMock.getClusterId()).thenReturn(clusterMockId);
 
-        // Mock cpuOvercommitRatio and ramOvercommitRatio
-        ClusterDetailsVO cpuOvercommitRatio = Mockito.mock(ClusterDetailsVO.class);
-        when(cpuOvercommitRatio.getValue()).thenReturn("1.0");
-        when(_clusterDetailsDao.findDetail(clusterMockId, VmDetailConstants.CPU_OVER_COMMIT_RATIO)).thenReturn(cpuOvercommitRatio);
-        ClusterDetailsVO ramOvercommitRatio = Mockito.mock(ClusterDetailsVO.class);
-        when(ramOvercommitRatio.getValue()).thenReturn("1.0");
-        when(_clusterDetailsDao.findDetail(clusterMockId, VmDetailConstants.MEMORY_OVER_COMMIT_RATIO)).thenReturn(ramOvercommitRatio);
-
         // Mock NICs
         List<NicVO> nics = new ArrayList<>();
         NicVO nic1 = Mockito.mock(NicVO.class);
@@ -1782,7 +1761,7 @@ public void testPrepVMSpecForUnmanageInstance() {
         // Assert
         assertNotNull(result);
         assertEquals(vmTO, result);
-        verify(_clusterDetailsDao, times(2)).findDetail(eq(clusterMockId), anyString());
+        verify(vmOverCommit).updateOverCommitRatioForVmProfile(any(VirtualMachineProfile.class), eq(clusterMockId));
         verify(vmInstanceDetailsDao).listDetailsKeyPairs(anyLong(), anyList());
     }