This is an automated email from the ASF dual-hosted git repository.
gutoveronezi pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/main by this push:
new 28385be Fix metrics stats for VMs not running (#5633)
28385be is described below
commit 28385be60998852f1419c5a77d3e0bba6e069807
Author: José Flauzino <jose.wilson...@gmail.com>
AuthorDate: Mon Dec 6 11:06:10 2021 -0300
Fix metrics stats for VMs not running (#5633)
* Fix metrics stats for VMs that are not running
* Improves the way to get vmIdsToRemoveStats
* Improves test
Co-authored-by: José Flauzino <j...@scclouds.com.br>
---
.../src/main/java/com/cloud/vm/dao/UserVmDao.java | 6 ++
.../main/java/com/cloud/vm/dao/UserVmDaoImpl.java | 31 +++++++++--
.../main/java/com/cloud/server/StatsCollector.java | 32 ++++++++++-
.../main/java/com/cloud/vm/UserVmManagerImpl.java | 7 +++
.../java/com/cloud/server/StatsCollectorTest.java | 65 ++++++++++++++++++++++
.../AccountManagerImplVolumeDeleteEventTest.java | 4 ++
6 files changed, 139 insertions(+), 6 deletions(-)
diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/UserVmDao.java
b/engine/schema/src/main/java/com/cloud/vm/dao/UserVmDao.java
index 55d8fa0..c53936f 100644
--- a/engine/schema/src/main/java/com/cloud/vm/dao/UserVmDao.java
+++ b/engine/schema/src/main/java/com/cloud/vm/dao/UserVmDao.java
@@ -61,6 +61,12 @@ public interface UserVmDao extends GenericDao<UserVmVO,
Long> {
public List<UserVmVO> listRunningByHostId(long hostId);
/**
+ * List all running VMs.
+ * @return the list of all VM instances that are running.
+ */
+ public List<UserVmVO> listAllRunning();
+
+ /**
* List user vm instances with virtualized networking (i.e. not direct
attached networking) for the given account and datacenter
* @param accountId will search for vm instances belonging to this account
* @return the list of vm instances owned by the account in the given data
center that have virtualized networking (not direct attached networking)
diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/UserVmDaoImpl.java
b/engine/schema/src/main/java/com/cloud/vm/dao/UserVmDaoImpl.java
index a903c33..9cfa7a7 100644
--- a/engine/schema/src/main/java/com/cloud/vm/dao/UserVmDaoImpl.java
+++ b/engine/schema/src/main/java/com/cloud/vm/dao/UserVmDaoImpl.java
@@ -67,6 +67,7 @@ public class UserVmDaoImpl extends GenericDaoBase<UserVmVO,
Long> implements Use
protected SearchBuilder<UserVmVO> LastHostSearch;
protected SearchBuilder<UserVmVO> HostUpSearch;
protected SearchBuilder<UserVmVO> HostRunningSearch;
+ protected SearchBuilder<UserVmVO> RunningSearch;
protected SearchBuilder<UserVmVO> StateChangeSearch;
protected SearchBuilder<UserVmVO> AccountHostSearch;
@@ -134,9 +135,8 @@ public class UserVmDaoImpl extends GenericDaoBase<UserVmVO,
Long> implements Use
HostSearch.and("host", HostSearch.entity().getHostId(),
SearchCriteria.Op.EQ);
HostSearch.done();
- LastHostSearch = createSearchBuilder();
+ LastHostSearch =
createSearchBuilderWithStateCriteria(SearchCriteria.Op.EQ);
LastHostSearch.and("lastHost",
LastHostSearch.entity().getLastHostId(), SearchCriteria.Op.EQ);
- LastHostSearch.and("state", LastHostSearch.entity().getState(),
SearchCriteria.Op.EQ);
LastHostSearch.done();
HostUpSearch = createSearchBuilder();
@@ -144,11 +144,13 @@ public class UserVmDaoImpl extends
GenericDaoBase<UserVmVO, Long> implements Use
HostUpSearch.and("states", HostUpSearch.entity().getState(),
SearchCriteria.Op.NIN);
HostUpSearch.done();
- HostRunningSearch = createSearchBuilder();
+ HostRunningSearch =
createSearchBuilderWithStateCriteria(SearchCriteria.Op.EQ);
HostRunningSearch.and("host", HostRunningSearch.entity().getHostId(),
SearchCriteria.Op.EQ);
- HostRunningSearch.and("state", HostRunningSearch.entity().getState(),
SearchCriteria.Op.EQ);
HostRunningSearch.done();
+ RunningSearch =
createSearchBuilderWithStateCriteria(SearchCriteria.Op.EQ);
+ RunningSearch.done();
+
AccountPodSearch = createSearchBuilder();
AccountPodSearch.and("account",
AccountPodSearch.entity().getAccountId(), SearchCriteria.Op.EQ);
AccountPodSearch.and("pod",
AccountPodSearch.entity().getPodIdToDeployIn(), SearchCriteria.Op.EQ);
@@ -209,6 +211,19 @@ public class UserVmDaoImpl extends
GenericDaoBase<UserVmVO, Long> implements Use
assert _updateTimeAttr != null : "Couldn't get this updateTime
attribute";
}
+ /**
+ * Creates an {@link com.cloud.vm.UserVmVO UserVmVO} search builder with a
+ * {@link com.cloud.utils.db.SearchCriteria.Op SearchCriteria.Op} condition
+ * to the 'state' criteria already included.
+ * @param searchCriteria the {@link com.cloud.utils.db.SearchCriteria.Op
SearchCriteria.Op} to 'state' criteria.
+ * @return the {@link com.cloud.vm.UserVmVO UserVmVO} search builder.
+ */
+ protected SearchBuilder<UserVmVO>
createSearchBuilderWithStateCriteria(SearchCriteria.Op searchCriteria) {
+ SearchBuilder<UserVmVO> genericSearchBuilderWithStateCriteria =
createSearchBuilder();
+ genericSearchBuilderWithStateCriteria.and("state",
genericSearchBuilderWithStateCriteria.entity().getState(), searchCriteria);
+ return genericSearchBuilderWithStateCriteria;
+ }
+
@Override
public List<UserVmVO> listByAccountAndPod(long accountId, long podId) {
SearchCriteria<UserVmVO> sc = AccountPodSearch.create();
@@ -299,6 +314,14 @@ public class UserVmDaoImpl extends
GenericDaoBase<UserVmVO, Long> implements Use
}
@Override
+ public List<UserVmVO> listAllRunning() {
+ SearchCriteria<UserVmVO> sc = RunningSearch.create();
+ sc.setParameters("state", State.Running);
+
+ return listBy(sc);
+ }
+
+ @Override
public List<UserVmVO> listVirtualNetworkInstancesByAcctAndNetwork(long
accountId, long networkId) {
SearchCriteria<UserVmVO> sc = AccountDataCenterVirtualSearch.create();
diff --git a/server/src/main/java/com/cloud/server/StatsCollector.java
b/server/src/main/java/com/cloud/server/StatsCollector.java
index c14f88c..9b193c0 100644
--- a/server/src/main/java/com/cloud/server/StatsCollector.java
+++ b/server/src/main/java/com/cloud/server/StatsCollector.java
@@ -238,7 +238,7 @@ public class StatsCollector extends ManagerBase implements
ComponentMethodInterc
@Inject
private ClusterDao _clusterDao;
@Inject
- private UserVmDao _userVmDao;
+ protected UserVmDao _userVmDao;
@Inject
private VolumeDao _volsDao;
@Inject
@@ -291,7 +291,7 @@ public class StatsCollector extends ManagerBase implements
ComponentMethodInterc
private ImageStoreDetailsUtil imageStoreDetailsUtil;
private ConcurrentHashMap<Long, HostStats> _hostStats = new
ConcurrentHashMap<Long, HostStats>();
- private final ConcurrentHashMap<Long, VmStats> _VmStats = new
ConcurrentHashMap<Long, VmStats>();
+ protected ConcurrentHashMap<Long, VmStats> _VmStats = new
ConcurrentHashMap<Long, VmStats>();
private final Map<String, VolumeStats> _volumeStats = new
ConcurrentHashMap<String, VolumeStats>();
private ConcurrentHashMap<Long, StorageStats> _storageStats = new
ConcurrentHashMap<Long, StorageStats>();
private ConcurrentHashMap<Long, StorageStats> _storagePoolStats = new
ConcurrentHashMap<Long, StorageStats>();
@@ -619,6 +619,8 @@ public class StatsCollector extends ManagerBase implements
ComponentMethodInterc
}
}
+ cleanUpVirtualMachineStats();
+
} catch (Throwable t) {
s_logger.error("Error trying to retrieve VM stats", t);
}
@@ -1486,6 +1488,32 @@ public class StatsCollector extends ManagerBase
implements ComponentMethodInterc
}
/**
+ * Removes stats for a given virtual machine.
+ * @param vmId ID of the virtual machine to remove stats.
+ */
+ public void removeVirtualMachineStats(Long vmId) {
+ s_logger.debug(String.format("Removing stats from VM with ID: %s
.",vmId));
+ _VmStats.remove(vmId);
+ }
+
+ /**
+ * Removes stats of virtual machines that are not running from memory.
+ */
+ protected void cleanUpVirtualMachineStats() {
+ List<Long> allRunningVmIds = new ArrayList<Long>();
+ for (UserVmVO vm : _userVmDao.listAllRunning()) {
+ allRunningVmIds.add(vm.getId());
+ }
+
+ List<Long> vmIdsToRemoveStats = new ArrayList<Long>(_VmStats.keySet());
+ vmIdsToRemoveStats.removeAll(allRunningVmIds);
+
+ for (Long vmId : vmIdsToRemoveStats) {
+ removeVirtualMachineStats(vmId);
+ }
+ }
+
+ /**
* Sends host metrics to a configured InfluxDB host. The metrics respects
the following specification.</br>
* <b>Tags:</b>vm_id, uuid, instance_name, data_center_id, host_id</br>
* <b>Fields:</b>memory_total_kb, memory_internal_free_kbs,
memory_target_kbs, cpu_utilization, cpus, network_write_kb, disk_read_iops,
disk_read_kbs, disk_write_iops, disk_write_kbs
diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
index da6b55b..07d1f2b 100644
--- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
+++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
@@ -269,6 +269,7 @@ import com.cloud.resource.ResourceManager;
import com.cloud.resource.ResourceState;
import com.cloud.server.ManagementService;
import com.cloud.server.ResourceTag;
+import com.cloud.server.StatsCollector;
import com.cloud.service.ServiceOfferingVO;
import com.cloud.service.dao.ServiceOfferingDao;
import com.cloud.service.dao.ServiceOfferingDetailsDao;
@@ -552,6 +553,8 @@ public class UserVmManagerImpl extends ManagerBase
implements UserVmManager, Vir
@Autowired
@Qualifier("networkHelper")
protected NetworkHelper nwHelper;
+ @Inject
+ private StatsCollector statsCollector;
private ScheduledExecutorService _executor = null;
private ScheduledExecutorService _vmIpFetchExecutor = null;
@@ -5001,6 +5004,8 @@ public class UserVmManagerImpl extends ManagerBase
implements UserVmManager, Vir
throw new InvalidParameterValueException("unable to find a virtual
machine with id " + vmId);
}
+ statsCollector.removeVirtualMachineStats(vmId);
+
_userDao.findById(userId);
boolean status = false;
try {
@@ -5306,6 +5311,8 @@ public class UserVmManagerImpl extends ManagerBase
implements UserVmManager, Vir
return vm;
}
+ statsCollector.removeVirtualMachineStats(vmId);
+
boolean status;
State vmState = vm.getState();
diff --git a/server/src/test/java/com/cloud/server/StatsCollectorTest.java
b/server/src/test/java/com/cloud/server/StatsCollectorTest.java
index 70416e8..4ee2099 100644
--- a/server/src/test/java/com/cloud/server/StatsCollectorTest.java
+++ b/server/src/test/java/com/cloud/server/StatsCollectorTest.java
@@ -21,9 +21,11 @@ package com.cloud.server;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.TimeUnit;
import org.influxdb.InfluxDB;
@@ -34,6 +36,8 @@ import org.influxdb.dto.Point;
import org.junit.Assert;
import org.junit.Test;
import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
import org.mockito.Mockito;
import org.powermock.api.mockito.PowerMockito;
import org.powermock.core.classloader.annotations.PrepareForTest;
@@ -44,6 +48,9 @@ import com.cloud.agent.api.VmDiskStatsEntry;
import com.cloud.server.StatsCollector.ExternalStatsProtocol;
import com.cloud.user.VmDiskStatisticsVO;
import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.vm.UserVmVO;
+import com.cloud.vm.VmStats;
+import com.cloud.vm.dao.UserVmDao;
import com.tngtech.java.junit.dataprovider.DataProvider;
import com.tngtech.java.junit.dataprovider.DataProviderRunner;
@@ -51,6 +58,8 @@ import com.tngtech.java.junit.dataprovider.DataProviderRunner;
@PowerMockRunnerDelegate(DataProviderRunner.class)
@PrepareForTest({InfluxDBFactory.class, BatchPoints.class})
public class StatsCollectorTest {
+
+ @InjectMocks
private StatsCollector statsCollector = Mockito.spy(new StatsCollector());
private static final int GRAPHITE_DEFAULT_PORT = 2003;
@@ -60,6 +69,18 @@ public class StatsCollectorTest {
private static final String DEFAULT_DATABASE_NAME = "cloudstack";
+ @Mock
+ ConcurrentHashMap<Long, VmStats> vmStatsMock;
+
+ @Mock
+ VmStats singleVmStatsMock;
+
+ @Mock
+ UserVmDao userVmDaoMock;
+
+ @Mock
+ UserVmVO userVmVOMock;
+
@Test
public void createInfluxDbConnectionTest() {
configureAndTestCreateInfluxDbConnection(true);
@@ -218,4 +239,48 @@ public class StatsCollectorTest {
boolean result = statsCollector.areAllDiskStatsZero(vmDiskStatsEntry);
Assert.assertEquals(expected, result);
}
+
+ @Test
+ public void removeVirtualMachineStatsTestRemoveOneVmStats() {
+ Mockito.doReturn(new
Object()).when(vmStatsMock).remove(Mockito.anyLong());
+
+ statsCollector.removeVirtualMachineStats(1l);
+
+ Mockito.verify(vmStatsMock,
Mockito.times(1)).remove(Mockito.anyLong());
+ }
+
+ @Test
+ public void cleanUpVirtualMachineStatsTestDoNothing() {
+ Mockito.doReturn(new
ArrayList<>()).when(userVmDaoMock).listAllRunning();
+ Mockito.doReturn(new ConcurrentHashMap<Long, VmStats>(new
HashMap<>()).keySet())
+ .when(vmStatsMock).keySet();
+
+ statsCollector.cleanUpVirtualMachineStats();
+
+ Mockito.verify(statsCollector,
Mockito.never()).removeVirtualMachineStats(Mockito.anyLong());
+ }
+
+ @Test
+ public void cleanUpVirtualMachineStatsTestRemoveOneVmStats() {
+ Mockito.doReturn(new
ArrayList<>()).when(userVmDaoMock).listAllRunning();
+ Mockito.doReturn(1l).when(userVmVOMock).getId();
+ Mockito.doReturn(new ConcurrentHashMap<Long, VmStats>(Map.of(1l,
singleVmStatsMock)).keySet())
+ .when(vmStatsMock).keySet();
+
+ statsCollector.cleanUpVirtualMachineStats();
+
+ Mockito.verify(vmStatsMock,
Mockito.times(1)).remove(Mockito.anyLong());
+ }
+
+ @Test
+ public void cleanUpVirtualMachineStatsTestRemoveOnlyOneVmStats() {
+ Mockito.doReturn(1l).when(userVmVOMock).getId();
+
Mockito.doReturn(Arrays.asList(userVmVOMock)).when(userVmDaoMock).listAllRunning();
+ Mockito.doReturn(new ConcurrentHashMap<Long, VmStats>(Map.of(1l,
singleVmStatsMock, 2l, singleVmStatsMock)).keySet())
+ .when(vmStatsMock).keySet();
+
+ statsCollector.cleanUpVirtualMachineStats();
+
+ Mockito.verify(vmStatsMock,
Mockito.times(1)).remove(Mockito.anyLong());
+ }
}
diff --git
a/server/src/test/java/com/cloud/user/AccountManagerImplVolumeDeleteEventTest.java
b/server/src/test/java/com/cloud/user/AccountManagerImplVolumeDeleteEventTest.java
index ce0e796..37c48ad 100644
---
a/server/src/test/java/com/cloud/user/AccountManagerImplVolumeDeleteEventTest.java
+++
b/server/src/test/java/com/cloud/user/AccountManagerImplVolumeDeleteEventTest.java
@@ -54,6 +54,7 @@ import com.cloud.event.UsageEventVO;
import com.cloud.exception.AgentUnavailableException;
import com.cloud.exception.CloudException;
import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.server.StatsCollector;
import com.cloud.service.ServiceOfferingVO;
import com.cloud.storage.Volume.Type;
import com.cloud.storage.VolumeVO;
@@ -75,6 +76,8 @@ public class AccountManagerImplVolumeDeleteEventTest extends
AccountManagetImplT
@Mock
private UserVmManager userVmManager;
+ @Mock
+ private StatsCollector statsCollectorMock;
Map<String, Object> oldFields = new HashMap<>();
UserVmVO vm = mock(UserVmVO.class);
@@ -201,6 +204,7 @@ public class AccountManagerImplVolumeDeleteEventTest
extends AccountManagetImplT
// volume.
public void runningVMRootVolumeUsageEvent()
throws SecurityException, IllegalArgumentException,
ReflectiveOperationException, AgentUnavailableException,
ConcurrentOperationException, CloudException {
+
Mockito.doNothing().when(statsCollectorMock).removeVirtualMachineStats(Mockito.anyLong());
Mockito.lenient().when(_vmMgr.destroyVm(nullable(Long.class),
nullable(Boolean.class))).thenReturn(vm);
List<UsageEventVO> emittedEvents =
deleteUserAccountRootVolumeUsageEvents(false);
UsageEventVO event = emittedEvents.get(0);
