GutoVeronezi commented on a change in pull request #4978:
URL: https://github.com/apache/cloudstack/pull/4978#discussion_r666435432
##########
File path:
plugins/hypervisors/kvm/src/main/java/com/cloud/ha/KVMInvestigator.java
##########
@@ -77,57 +77,84 @@ public Status isAgentAlive(Host agent) {
return haManager.getHostStatus(agent);
}
- List<StoragePoolVO> clusterPools =
_storagePoolDao.listPoolsByCluster(agent.getClusterId());
- boolean hasNfs = false;
- for (StoragePoolVO pool : clusterPools) {
- if (pool.getPoolType() == StoragePoolType.NetworkFilesystem) {
- hasNfs = true;
- break;
- }
+ Status agentStatus = Status.Disconnected;
+ boolean hasNfs = isHostServedByNfsPool(agent);
+ if (hasNfs) {
+ agentStatus = checkAgentStatusViaNfs(agent);
+ s_logger.debug(String.format("Agent investigation was requested on
host %s. Agent status via NFS heartbeat is %s.", agent, agentStatus));
+ } else {
+ s_logger.debug(String.format("Agent investigation was requested on
host %s, but host has no NFS storage. Skipping investigation via NFS.", agent));
+ }
+
+ boolean isKvmHaWebserviceEnabled =
kvmHaHelper.isKvmHaWebserviceEnabled(agent);
+ if (isKvmHaWebserviceEnabled) {
+ agentStatus = kvmHaHelper.checkAgentStatusViaKvmHaAgent(agent,
agentStatus);
}
+
+ return agentStatus;
+ }
+
+ private boolean isHostServedByNfsPool(Host agent) {
+ boolean hasNfs = hasNfsPoolClusterWideForHost(agent);
if (!hasNfs) {
- List<StoragePoolVO> zonePools =
_storagePoolDao.findZoneWideStoragePoolsByHypervisor(agent.getDataCenterId(),
agent.getHypervisorType());
- for (StoragePoolVO pool : zonePools) {
- if (pool.getPoolType() == StoragePoolType.NetworkFilesystem) {
- hasNfs = true;
- break;
- }
+ hasNfs = hasNfsPoolZoneWideForHost(agent);
+ }
+ return hasNfs;
+ }
+
+ private boolean hasNfsPoolZoneWideForHost(Host agent) {
+ List<StoragePoolVO> zonePools =
_storagePoolDao.findZoneWideStoragePoolsByHypervisor(agent.getDataCenterId(),
agent.getHypervisorType());
+ for (StoragePoolVO pool : zonePools) {
+ if (pool.getPoolType() == StoragePoolType.NetworkFilesystem) {
+ return true;
}
}
- if (!hasNfs) {
- s_logger.warn(
- "Agent investigation was requested on host " + agent + ",
but host does not support investigation because it has no NFS storage. Skipping
investigation.");
- return Status.Disconnected;
+ return false;
Review comment:
We could use `stream` methods to simplify this implementation:
```java
return zonePools.stream().anyMatch(pool -> pool.getPoolType() ==
StoragePoolType.NetworkFilesystem);
```
Also, this code is repeated right below in `hasNfsPoolClusterWideForHost`,
we could extract it to a method and add unit tests.
##########
File path:
plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/kvm/ha/KVMHostActivityChecker.java
##########
@@ -213,6 +270,17 @@ protected boolean
verifyActivityOfStorageOnHost(HashMap<StoragePool, List<Volume
return poolVolMap;
}
+ private boolean isHostServedByNfsPool(Host agent) {
+ List<StoragePoolHostVO> storagesOnHost =
storagePoolHostDao.listByHostId(agent.getId());
+ for (StoragePoolHostVO storagePoolHostRef : storagesOnHost) {
+ StoragePoolVO storagePool =
this.storagePool.findById(storagePoolHostRef.getPoolId());
+ if (NFS_POOL_TYPE.contains(storagePool.getPoolType())) {
+ return true;
+ }
+ }
+ return false;
Review comment:
We could use `stream().anyMatch` here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
