DaanHoogland commented on a change in pull request #5862:
URL: https://github.com/apache/cloudstack/pull/5862#discussion_r795459338
##########
File path:
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHAMonitor.java
##########
@@ -35,121 +35,189 @@
public class KVMHAMonitor extends KVMHABase implements Runnable {
private static final Logger s_logger =
Logger.getLogger(KVMHAMonitor.class);
- private final Map<String, NfsStoragePool> storagePool = new
ConcurrentHashMap<>();
+ private final Map<String, NfsStoragePool> nfsstoragePool = new
ConcurrentHashMap<>();
+ private final Map<String, RbdStoragePool> rbdstoragePool = new
ConcurrentHashMap<>();
+ private final NfsStoragePool nfsStoragePool = null;
+ private final RbdStoragePool rbdStoragePool = null;
private final boolean rebootHostAndAlertManagementOnHeartbeatTimeout;
private final String hostPrivateIp;
- public KVMHAMonitor(NfsStoragePool pool, String host, String scriptPath) {
+ public KVMHAMonitor(NfsStoragePool pool, RbdStoragePool rbdpool, String
host, String scriptPath, String scriptPathRbd) {
if (pool != null) {
- storagePool.put(pool._poolUUID, pool);
+ nfsstoragePool.put(pool._poolUUID, pool);
+ }else if (rbdpool != null) {
+ rbdstoragePool.put(rbdpool._poolUUID, rbdpool);
}
+ configureHeartBeatPath(scriptPath, scriptPathRbd);
hostPrivateIp = host;
- configureHeartBeatPath(scriptPath);
-
_heartBeatUpdateTimeout =
AgentPropertiesFileHandler.getPropertyValue(AgentProperties.HEARTBEAT_UPDATE_TIMEOUT);
rebootHostAndAlertManagementOnHeartbeatTimeout =
AgentPropertiesFileHandler.getPropertyValue(AgentProperties.REBOOT_HOST_AND_ALERT_MANAGEMENT_ON_HEARTBEAT_TIMEOUT);
}
- private static synchronized void configureHeartBeatPath(String scriptPath)
{
+ private static synchronized void configureHeartBeatPath(String scriptPath,
String scriptPathRbd) {
KVMHABase.s_heartBeatPath = scriptPath;
+ KVMHABase.s_heartBeatPathRbd = scriptPathRbd;
}
public void addStoragePool(NfsStoragePool pool) {
- synchronized (storagePool) {
- storagePool.put(pool._poolUUID, pool);
+ synchronized (nfsstoragePool) {
+ nfsstoragePool.put(pool._poolUUID, pool);
+ }
+ }
+
+ public void addStoragePool(RbdStoragePool pool) {
+ synchronized (rbdstoragePool) {
+ rbdstoragePool.put(pool._poolUUID, pool);
}
}
public void removeStoragePool(String uuid) {
- synchronized (storagePool) {
- NfsStoragePool pool = storagePool.get(uuid);
+ synchronized (nfsstoragePool) {
+ NfsStoragePool pool = nfsstoragePool.get(uuid);
+ if (pool != null) {
+ Script.runSimpleBashScript("umount " + pool._mountDestPath);
+ nfsstoragePool.remove(uuid);
+ }
+ }
+ }
+
+ public void removeRbdStoragePool(String uuid) {
+ synchronized (rbdstoragePool) {
+ RbdStoragePool pool = rbdstoragePool.get(uuid);
if (pool != null) {
Script.runSimpleBashScript("umount " + pool._mountDestPath);
- storagePool.remove(uuid);
+ rbdstoragePool.remove(uuid);
}
}
}
public List<NfsStoragePool> getStoragePools() {
- synchronized (storagePool) {
- return new ArrayList<>(storagePool.values());
+ synchronized (nfsstoragePool) {
+ return new ArrayList<>(nfsstoragePool.values());
}
}
- public NfsStoragePool getStoragePool(String uuid) {
- synchronized (storagePool) {
- return storagePool.get(uuid);
+ public List<RbdStoragePool> getRbdStoragePools() {
+ synchronized (rbdstoragePool) {
+ return new ArrayList<>(rbdstoragePool.values());
}
}
- protected void runHeartBeat() {
- synchronized (storagePool) {
- Set<String> removedPools = new HashSet<>();
- for (String uuid : storagePool.keySet()) {
- NfsStoragePool primaryStoragePool = storagePool.get(uuid);
- StoragePool storage;
- try {
- Connect conn = LibvirtConnection.getConnection();
- storage = conn.storagePoolLookupByUUIDString(uuid);
- if (storage == null || storage.getInfo().state !=
StoragePoolState.VIR_STORAGE_POOL_RUNNING) {
- if (storage == null) {
- s_logger.debug(String.format("Libvirt storage pool
[%s] not found, removing from HA list.", uuid));
- } else {
- s_logger.debug(String.format("Libvirt storage pool
[%s] found, but not running, removing from HA list.", uuid));
- }
-
- removedPools.add(uuid);
- continue;
- }
+ public NfsStoragePool getStoragePool(String uuid) {
+ synchronized (nfsstoragePool) {
+ return nfsstoragePool.get(uuid);
+ }
+ }
- s_logger.debug(String.format("Found NFS storage pool [%s]
in libvirt, continuing.", uuid));
+ public RbdStoragePool getRbdStoragePool(String uuid) {
+ synchronized (rbdstoragePool) {
+ return rbdstoragePool.get(uuid);
+ }
+ }
- } catch (LibvirtException e) {
- s_logger.debug(String.format("Failed to lookup libvirt
storage pool [%s].", uuid), e);
+ protected void runHeartBeat() {
+ if (nfsstoragePool != null && !nfsstoragePool.isEmpty()) {
+ synchronized (nfsstoragePool) {
+ Set<String> removedPools = new HashSet<>();
+ for (String uuid : nfsstoragePool.keySet()) {
+ NfsStoragePool nfsStoragePool = nfsstoragePool.get(uuid);
+ runHeartbeatToPool(nfsStoragePool, rbdStoragePool, uuid,
removedPools);
+ continue;
+ }
- if (e.toString().contains("pool not found")) {
- s_logger.debug(String.format("Removing pool [%s] from
HA monitor since it was deleted.", uuid));
- removedPools.add(uuid);
- continue;
+ if (!removedPools.isEmpty()) {
+ for (String uuid : removedPools) {
+ removeStoragePool(uuid);
}
+ }
+ }
+ }
+ if (rbdstoragePool != null && !rbdstoragePool.isEmpty()) {
+ synchronized (rbdstoragePool) {
+ Set<String> removedPools = new HashSet<>();
+ for (String uuid : rbdstoragePool.keySet()) {
+ RbdStoragePool rbdStoragePool = rbdstoragePool.get(uuid);
+ runHeartbeatToPool(nfsStoragePool, rbdStoragePool, uuid,
removedPools);
+ continue;
}
- String result = null;
- for (int i = 1; i <= _heartBeatUpdateMaxTries; i++) {
- Script cmd = createHeartBeatCommand(primaryStoragePool,
hostPrivateIp, true);
- result = cmd.execute();
-
- s_logger.debug(String.format("The command (%s), to the
pool [%s], has the result [%s].", cmd.toString(), uuid, result));
-
- if (result != null) {
- s_logger.warn(String.format("Write heartbeat for pool
[%s] failed: %s; try: %s of %s.", uuid, result, i, _heartBeatUpdateMaxTries));
- try {
- Thread.sleep(_heartBeatUpdateRetrySleep);
- } catch (InterruptedException e) {
- s_logger.debug("[IGNORED] Interrupted between
heartbeat retries.", e);
- }
- } else {
- break;
+ if (!removedPools.isEmpty()) {
+ for (String uuid : removedPools) {
+ removeRbdStoragePool(uuid);
}
-
}
+ }
+ }
+ }
- if (result != null &&
rebootHostAndAlertManagementOnHeartbeatTimeout) {
- s_logger.warn(String.format("Write heartbeat for pool [%s]
failed: %s; stopping cloudstack-agent.", uuid, result));
- Script cmd = createHeartBeatCommand(primaryStoragePool,
null, false);
- result = cmd.execute();
+ private Set<String> runHeartbeatToPool(NfsStoragePool nfsStoragePool,
RbdStoragePool rbdStoragePool, String uuid, Set<String> removedPools) {
+ StoragePool storage;
+ try {
+ Connect conn = LibvirtConnection.getConnection();
+ storage = conn.storagePoolLookupByUUIDString(uuid);
+ if (storage == null || storage.getInfo().state !=
StoragePoolState.VIR_STORAGE_POOL_RUNNING) {
+ if (storage == null) {
+ s_logger.debug(String.format("Libvirt storage pool [%s]
not found, removing from HA list.", uuid));
+ } else {
+ s_logger.debug(String.format("Libvirt storage pool [%s]
was found, but it is not running, removing it from HA list.", uuid));
Review comment:
not as strict on logging as @GutoVeronezi but one suggestion (making it
only once)
```suggestion
s_logger.debug(String.format("Libvirt storage pool [%s]
was found, but it is not running, removing it from the HA list.", uuid));
```
##########
File path:
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHAMonitor.java
##########
@@ -35,121 +35,221 @@
public class KVMHAMonitor extends KVMHABase implements Runnable {
private static final Logger s_logger =
Logger.getLogger(KVMHAMonitor.class);
- private final Map<String, NfsStoragePool> storagePool = new
ConcurrentHashMap<>();
+ private final Map<String, NfsStoragePool> nfsstoragePool = new
ConcurrentHashMap<>();
+ private final Map<String, RbdStoragePool> rbdstoragePool = new
ConcurrentHashMap<>();
private final boolean rebootHostAndAlertManagementOnHeartbeatTimeout;
private final String hostPrivateIp;
- public KVMHAMonitor(NfsStoragePool pool, String host, String scriptPath) {
+ public KVMHAMonitor(NfsStoragePool pool, RbdStoragePool rbdpool, String
host, String scriptPath, String scriptPathRbd) {
if (pool != null) {
- storagePool.put(pool._poolUUID, pool);
+ nfsstoragePool.put(pool._poolUUID, pool);
+ }else if (rbdpool != null) {
+ rbdstoragePool.put(rbdpool._poolUUID, rbdpool);
}
+ configureHeartBeatPath(scriptPath, scriptPathRbd);
hostPrivateIp = host;
- configureHeartBeatPath(scriptPath);
-
_heartBeatUpdateTimeout =
AgentPropertiesFileHandler.getPropertyValue(AgentProperties.HEARTBEAT_UPDATE_TIMEOUT);
rebootHostAndAlertManagementOnHeartbeatTimeout =
AgentPropertiesFileHandler.getPropertyValue(AgentProperties.REBOOT_HOST_AND_ALERT_MANAGEMENT_ON_HEARTBEAT_TIMEOUT);
}
- private static synchronized void configureHeartBeatPath(String scriptPath)
{
+ private static synchronized void configureHeartBeatPath(String scriptPath,
String scriptPathRbd) {
KVMHABase.s_heartBeatPath = scriptPath;
+ KVMHABase.s_heartBeatPathRbd = scriptPathRbd;
}
public void addStoragePool(NfsStoragePool pool) {
- synchronized (storagePool) {
- storagePool.put(pool._poolUUID, pool);
+ synchronized (nfsstoragePool) {
+ nfsstoragePool.put(pool._poolUUID, pool);
+ }
+ }
+
+ public void addStoragePool(RbdStoragePool pool) {
+ synchronized (rbdstoragePool) {
+ rbdstoragePool.put(pool._poolUUID, pool);
}
}
public void removeStoragePool(String uuid) {
- synchronized (storagePool) {
- NfsStoragePool pool = storagePool.get(uuid);
+ synchronized (nfsstoragePool) {
+ NfsStoragePool pool = nfsstoragePool.get(uuid);
if (pool != null) {
Script.runSimpleBashScript("umount " + pool._mountDestPath);
- storagePool.remove(uuid);
+ nfsstoragePool.remove(uuid);
+ }
+ }
+ }
+
+ public void removeRbdStoragePool(String uuid) {
+ synchronized (rbdstoragePool) {
+ RbdStoragePool pool = rbdstoragePool.get(uuid);
+ if (pool != null) {
+ Script.runSimpleBashScript("umount " + pool._mountDestPath);
+ rbdstoragePool.remove(uuid);
}
}
}
public List<NfsStoragePool> getStoragePools() {
- synchronized (storagePool) {
- return new ArrayList<>(storagePool.values());
+ synchronized (nfsstoragePool) {
+ return new ArrayList<>(nfsstoragePool.values());
+ }
+ }
+
+ public List<RbdStoragePool> getRbdStoragePools() {
+ synchronized (rbdstoragePool) {
+ return new ArrayList<>(rbdstoragePool.values());
}
}
public NfsStoragePool getStoragePool(String uuid) {
- synchronized (storagePool) {
- return storagePool.get(uuid);
+ synchronized (nfsstoragePool) {
+ return nfsstoragePool.get(uuid);
+ }
+ }
+
+ public RbdStoragePool getRbdStoragePool(String uuid) {
+ synchronized (rbdstoragePool) {
+ return rbdstoragePool.get(uuid);
}
}
protected void runHeartBeat() {
- synchronized (storagePool) {
- Set<String> removedPools = new HashSet<>();
- for (String uuid : storagePool.keySet()) {
- NfsStoragePool primaryStoragePool = storagePool.get(uuid);
- StoragePool storage;
- try {
- Connect conn = LibvirtConnection.getConnection();
- storage = conn.storagePoolLookupByUUIDString(uuid);
- if (storage == null || storage.getInfo().state !=
StoragePoolState.VIR_STORAGE_POOL_RUNNING) {
- if (storage == null) {
- s_logger.debug(String.format("Libvirt storage pool
[%s] not found, removing from HA list.", uuid));
- } else {
- s_logger.debug(String.format("Libvirt storage pool
[%s] found, but not running, removing from HA list.", uuid));
+ if(nfsstoragePool != null && !nfsstoragePool.isEmpty()) {
+ synchronized (nfsstoragePool) {
+ Set<String> removedPools = new HashSet<>();
+ for (String uuid : nfsstoragePool.keySet()) {
+ NfsStoragePool primaryStoragePool =
nfsstoragePool.get(uuid);
+ StoragePool storage;
+ try {
+ Connect conn = LibvirtConnection.getConnection();
+ storage = conn.storagePoolLookupByUUIDString(uuid);
+ if (storage == null || storage.getInfo().state !=
StoragePoolState.VIR_STORAGE_POOL_RUNNING) {
+ if (storage == null) {
+ s_logger.debug(String.format("Libvirt storage
pool [%s] not found, removing from HA list.", uuid));
+ } else {
+ s_logger.debug(String.format("Libvirt storage
pool [%s] found, but not running, removing from HA list.", uuid));
+ }
+
+ removedPools.add(uuid);
+ continue;
+ }
+
+ s_logger.debug(String.format("Found NFS storage pool
[%s] in libvirt, continuing.", uuid));
+
+ } catch (LibvirtException e) {
+ s_logger.debug(String.format("Failed to lookup libvirt
storage pool [%s].", uuid), e);
+
+ if (e.toString().contains("pool not found")) {
+ s_logger.debug(String.format("Removing pool [%s]
from HA monitor since it was deleted.", uuid));
+ removedPools.add(uuid);
+ continue;
}
- removedPools.add(uuid);
- continue;
}
- s_logger.debug(String.format("Found NFS storage pool [%s]
in libvirt, continuing.", uuid));
+ String result = null;
+ for (int i = 1; i <= _heartBeatUpdateMaxTries; i++) {
+ Script cmd =
createHeartBeatCommand(primaryStoragePool, hostPrivateIp, true);
+ result = cmd.execute();
+
+ s_logger.debug(String.format("The command (%s), to the
pool [%s], has the result [%s].", cmd.toString(), uuid, result));
- } catch (LibvirtException e) {
- s_logger.debug(String.format("Failed to lookup libvirt
storage pool [%s].", uuid), e);
+ if (result != null) {
+ s_logger.warn(String.format("Write heartbeat for
pool [%s] failed: %s; try: %s of %s.", uuid, result, i,
_heartBeatUpdateMaxTries));
+ try {
+ Thread.sleep(_heartBeatUpdateRetrySleep);
+ } catch (InterruptedException e) {
+ s_logger.debug("[IGNORED] Interrupted between
heartbeat retries.", e);
+ }
+ } else {
+ break;
+ }
- if (e.toString().contains("pool not found")) {
- s_logger.debug(String.format("Removing pool [%s] from
HA monitor since it was deleted.", uuid));
- removedPools.add(uuid);
- continue;
}
+ if (result != null &&
rebootHostAndAlertManagementOnHeartbeatTimeout) {
+ s_logger.warn(String.format("Write heartbeat for pool
[%s] failed: %s; stopping cloudstack-agent.", uuid, result));
+ Script cmd =
createHeartBeatCommand(primaryStoragePool, null, false);
+ result = cmd.execute();
+ }
}
- String result = null;
- for (int i = 1; i <= _heartBeatUpdateMaxTries; i++) {
- Script cmd = createHeartBeatCommand(primaryStoragePool,
hostPrivateIp, true);
- result = cmd.execute();
+ if (!removedPools.isEmpty()) {
+ for (String uuid : removedPools) {
+ removeStoragePool(uuid);
+ }
+ }
+ }
+ }
- s_logger.debug(String.format("The command (%s), to the
pool [%s], has the result [%s].", cmd.toString(), uuid, result));
+ if (rbdstoragePool != null && !rbdstoragePool.isEmpty()) {
Review comment:
I agree with @GutoVeronezi and you could do more than just the
`runHeartbeatToPool()`. The rest of the block can be parameterised with the
synchronisation object, `nfsstoragePool` and `rbdstoragePool` respectively.
`obj.getClass()` to get the type and instantiate the and get the object in the
loop, or add the type ars parameter as well.
no blocker, just a suggestion!
--
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]
