Fix local storage deletion cases (#10231)

* Delete local storage properties in agent.properties during delete pool

* Fix stale entry when add local storage failed

* Smaller methods

* Comment added
This commit is contained in:
Harikrishna 2025-01-23 12:46:33 +05:30 committed by GitHub
parent 1e59f5cd0c
commit 20759187b3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 67 additions and 9 deletions

View File

@ -431,7 +431,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
protected static final String LOCAL_STORAGE_PATH = "local.storage.path";
protected static final String LOCAL_STORAGE_UUID = "local.storage.uuid";
protected static final String DEFAULT_LOCAL_STORAGE_PATH = "/var/lib/libvirt/images/";
public static final String DEFAULT_LOCAL_STORAGE_PATH = "/var/lib/libvirt/images";
protected List<String> localStoragePaths = new ArrayList<>();
protected List<String> localStorageUUIDs = new ArrayList<>();

View File

@ -22,12 +22,20 @@ package com.cloud.hypervisor.kvm.resource.wrapper;
import com.cloud.agent.api.Answer;
import com.cloud.agent.api.DeleteStoragePoolCommand;
import com.cloud.agent.api.to.StorageFilerTO;
import com.cloud.agent.dao.impl.PropertiesStorage;
import com.cloud.agent.properties.AgentProperties;
import com.cloud.agent.properties.AgentPropertiesFileHandler;
import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
import com.cloud.hypervisor.kvm.storage.KVMStoragePoolManager;
import com.cloud.resource.CommandWrapper;
import com.cloud.resource.ResourceWrapper;
import com.cloud.storage.Storage;
import com.cloud.utils.exception.CloudRuntimeException;
import java.util.Arrays;
import java.util.HashMap;
import java.util.stream.Collectors;
@ResourceWrapper(handles = DeleteStoragePoolCommand.class)
public final class LibvirtDeleteStoragePoolCommandWrapper extends CommandWrapper<DeleteStoragePoolCommand, Answer, LibvirtComputingResource> {
@Override
@ -35,15 +43,57 @@ public final class LibvirtDeleteStoragePoolCommandWrapper extends CommandWrapper
try {
// if getRemoveDatastore() is true, then we are dealing with managed storage and can skip the delete logic here
if (!command.getRemoveDatastore()) {
final StorageFilerTO pool = command.getPool();
final KVMStoragePoolManager storagePoolMgr = libvirtComputingResource.getStoragePoolMgr();
storagePoolMgr.deleteStoragePool(pool.getType(), pool.getUuid());
handleStoragePoolDeletion(command, libvirtComputingResource);
}
return new Answer(command);
} catch (final CloudRuntimeException e) {
return new Answer(command, false, e.toString());
}
}
private void handleStoragePoolDeletion(final DeleteStoragePoolCommand command, final LibvirtComputingResource libvirtComputingResource) {
final StorageFilerTO pool = command.getPool();
final KVMStoragePoolManager storagePoolMgr = libvirtComputingResource.getStoragePoolMgr();
storagePoolMgr.deleteStoragePool(pool.getType(), pool.getUuid());
if (isLocalStorageAndNotHavingDefaultPath(pool, libvirtComputingResource)) {
updateLocalStorageProperties(pool);
}
}
private boolean isLocalStorageAndNotHavingDefaultPath(final StorageFilerTO pool, final LibvirtComputingResource libvirtComputingResource) {
return Storage.StoragePoolType.Filesystem.equals(pool.getType())
&& !libvirtComputingResource.DEFAULT_LOCAL_STORAGE_PATH.equals(pool.getPath());
}
private void updateLocalStorageProperties(final StorageFilerTO pool) {
String localStoragePath = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.LOCAL_STORAGE_PATH);
String localStorageUuid = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.LOCAL_STORAGE_UUID);
String uuidToRemove = pool.getUuid();
String pathToRemove = pool.getPath();
if (localStorageUuid != null && uuidToRemove != null) {
localStorageUuid = Arrays.stream(localStorageUuid.split(","))
.filter(uuid -> !uuid.equals(uuidToRemove))
.collect(Collectors.joining(","));
}
if (localStoragePath != null && pathToRemove != null) {
localStoragePath = Arrays.stream(localStoragePath.split(","))
.filter(path -> !path.equals(pathToRemove))
.collect(Collectors.joining(","));
}
PropertiesStorage agentProperties = new PropertiesStorage();
agentProperties.configure("AgentProperties", new HashMap<String, Object>());
if (localStorageUuid != null) {
agentProperties.persist(AgentProperties.LOCAL_STORAGE_UUID.getName(), localStorageUuid);
}
if (localStoragePath != null) {
agentProperties.persist(AgentProperties.LOCAL_STORAGE_PATH.getName(), localStoragePath);
}
}
}

View File

@ -803,7 +803,9 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C
if (!(dc.isLocalStorageEnabled() || useLocalStorageForSystemVM)) {
return null;
}
DataStore store;
DataStore store = null;
DataStoreProvider provider = _dataStoreProviderMgr.getDefaultPrimaryDataStoreProvider();
DataStoreLifeCycle lifeCycle = provider.getDataStoreLifeCycle();
try {
String hostAddress = pInfo.getHost();
if (host.getHypervisorType() == Hypervisor.HypervisorType.VMware) {
@ -829,8 +831,6 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C
}
}
DataStoreProvider provider = _dataStoreProviderMgr.getDefaultPrimaryDataStoreProvider();
DataStoreLifeCycle lifeCycle = provider.getDataStoreLifeCycle();
if (pool == null) {
Map<String, Object> params = new HashMap<String, Object>();
String name = pInfo.getName() != null ? pInfo.getName() : createLocalStoragePoolName(host, pInfo);
@ -860,6 +860,14 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C
} catch (Exception e) {
s_logger.warn("Unable to setup the local storage pool for " + host, e);
try {
if (store != null) {
s_logger.debug(String.format("Trying to delete storage pool entry if exists %s", store));
lifeCycle.deleteDataStore(store);
}
} catch (Exception ex) {
s_logger.debug(String.format("Failed to clean up local storage pool: %s", ex.getMessage()));
}
throw new ConnectionException(true, "Unable to setup the local storage pool for " + host, e);
}