Skip to content

Multi-Host support for NFS3 and iSCSI #64

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sandeeplocharla wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Multi-Host support for NFS3 and iSCSI #64

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8668b04
Multi-Host support for NFS3 and iSCSI
sandeeplocharla Jun 18, 2026
891d1ea
Addressed comments
sandeeplocharla Jun 29, 2026
d7651f3
Addressed comments
sandeeplocharla Jul 8, 2026
47868b9
Revert unwanted changes in file
sandeeplocharla Jul 10, 2026
30412d4
Revert unwanted changes in file
sandeeplocharla Jul 10, 2026
26892e9
Fixed review comments
sandeeplocharla Jul 13, 2026
d1ed46a
Fixed review comments
sandeeplocharla Jul 13, 2026
8df4bc9
Fixed more review comments
sandeeplocharla Jul 13, 2026
76bdf24
Fixed few more review comments
sandeeplocharla Jul 13, 2026
6af0e37
Removed a UT which no longer is necessary
sandeeplocharla Jul 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions ...rage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/ExportRule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,13 @@

package org.apache.cloudstack.storage.feign.model;

import java.util.List;

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.annotation.JsonProperty;
import java.util.List;
import com.fasterxml.jackson.annotation.JsonValue;

/**
* ExportRule
Expand Down Expand Up @@ -54,6 +57,7 @@ public enum ProtocolsEnum {
this.value = value;
}

@JsonValue
public String getValue() {
return value;
}
Expand All @@ -63,9 +67,13 @@ public String toString() {
return String.valueOf(value);
}

@JsonCreator
public static ProtocolsEnum fromValue(String text) {
if (text == null) {
return null;
}
Comment thread
sandeeplocharla marked this conversation as resolved.
for (ProtocolsEnum b : ProtocolsEnum.values()) {
if (String.valueOf(b.value).equals(text)) {
if (String.valueOf(b.value).equalsIgnoreCase(text)) {
return b;
}
}
Expand Down
186 changes: 136 additions & 50 deletions .../volume/ontap/src/main/java/org/apache/cloudstack/storage/listener/OntapHostListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,30 +19,38 @@

package org.apache.cloudstack.storage.listener;

import java.util.List;
import java.util.Map;

import javax.inject.Inject;

import com.cloud.agent.api.ModifyStoragePoolCommand;
import org.apache.cloudstack.engine.subsystem.api.storage.HypervisorHostListener;
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
import org.apache.cloudstack.storage.service.StorageStrategy;
import org.apache.cloudstack.storage.service.model.AccessGroup;
import org.apache.cloudstack.storage.service.model.ProtocolType;
import org.apache.cloudstack.storage.utils.OntapStorageConstants;
import org.apache.cloudstack.storage.utils.OntapStorageUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

import com.cloud.agent.AgentManager;
import com.cloud.agent.api.Answer;
import com.cloud.agent.api.ModifyStoragePoolAnswer;
import com.cloud.agent.api.ModifyStoragePoolCommand;
import com.cloud.agent.api.StoragePoolInfo;
import com.cloud.alert.AlertManager;
import com.cloud.host.Host;
import com.cloud.host.HostVO;
import com.cloud.host.dao.HostDao;
import com.cloud.hypervisor.Hypervisor;
import com.cloud.storage.StoragePool;
import com.cloud.storage.StoragePoolHostVO;
import com.cloud.storage.dao.StoragePoolHostDao;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.LogManager;
import com.cloud.agent.AgentManager;
import com.cloud.agent.api.Answer;
import com.cloud.agent.api.DeleteStoragePoolCommand;
import com.cloud.host.Host;
import com.cloud.storage.StoragePool;
import com.cloud.utils.exception.CloudRuntimeException;
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
import org.apache.cloudstack.engine.subsystem.api.storage.HypervisorHostListener;
import com.cloud.host.dao.HostDao;

import java.util.Map;

public class OntapHostListener implements HypervisorHostListener {
protected Logger logger = LogManager.getLogger(getClass());
Expand All @@ -63,26 +71,39 @@ public class OntapHostListener implements HypervisorHostListener {

@Override
public boolean hostConnect(long hostId, long poolId) {
logger.info("Connect to host " + hostId + " from pool " + poolId);
logger.info("hostConnect: Connecting host {} to pool {}", hostId, poolId);
Host host = _hostDao.findById(hostId);
if (host == null) {
logger.error("host was not found with id : {}", hostId);
logger.error("hostConnect: Host was not found with id: {}", hostId);
return false;
}
if (!host.getHypervisorType().equals(Hypervisor.HypervisorType.KVM)) {
logger.error("ONTAP plugin does not support {} type host currently ", host.getHypervisorType());
logger.error("hostConnect: ONTAP plugin does not support {} type host currently", host.getHypervisorType());
return false;
}

StoragePool pool = _storagePoolDao.findById(poolId);
if (pool == null) {
logger.error("Failed to connect host - storage pool not found with id: {}", poolId);
logger.error("hostConnect: Failed to connect host - storage pool not found with id: {}", poolId);
return false;
}
logger.info("Connecting host {} to ONTAP storage pool {}", host.getName(), pool.getName());
logger.info("hostConnect: Connecting host {} to ONTAP storage pool {}", host.getName(), pool.getName());
try {
// Load storage pool details from database to pass mount options and other config to agent
Map<String, String> detailsMap = _storagePoolDetailsDao.listDetailsKeyPairs(poolId);
if (detailsMap == null || detailsMap.isEmpty()) {
logger.error("hostConnect: Failed to load storage pool details for pool id: {}", poolId);
return false;
}

if (detailsMap.get(OntapStorageConstants.PROTOCOL) == null) {
logger.error("hostConnect: Storage pool details missing required protocol type for pool id: {}", poolId);
return false;
}

// Update NFS export policy for this connected host when the pool protocol is NFS3.
updateNfsExportPolicyForConnectedHostIfNeeded(poolId, hostId, host, detailsMap);
Comment thread
sandeeplocharla marked this conversation as resolved.

// Create the ModifyStoragePoolCommand to send to the agent
// Note: Always send command even if database entry exists, because agent may have restarted
// and lost in-memory pool registration. The command handler is idempotent.
Expand Down Expand Up @@ -118,19 +139,19 @@ public boolean hostConnect(long hostId, long poolId) {
}

String localPath = poolInfo.getLocalPath();
logger.info("Storage pool {} successfully mounted at: {}", pool.getName(), localPath);
logger.info("hostConnect: Storage pool {} successfully mounted at: {}", pool.getName(), localPath);

// Update or create the storage_pool_host_ref entry with the correct local_path
StoragePoolHostVO storagePoolHost = storagePoolHostDao.findByPoolHost(poolId, hostId);

if (storagePoolHost == null) {
storagePoolHost = new StoragePoolHostVO(poolId, hostId, localPath);
storagePoolHostDao.persist(storagePoolHost);
logger.info("Created storage_pool_host_ref entry for pool {} and host {}", pool.getName(), host.getName());
logger.info("hostConnect: Created storage_pool_host_ref entry for pool {} and host {}", pool.getName(), host.getName());
} else {
storagePoolHost.setLocalPath(localPath);
storagePoolHostDao.update(storagePoolHost.getId(), storagePoolHost);
logger.info("Updated storage_pool_host_ref entry with local_path: {}", localPath);
logger.info("hostConnect: Updated storage_pool_host_ref entry with local_path: {}", localPath);
}

// Update pool capacity/usage information
Expand All @@ -139,62 +160,127 @@ public boolean hostConnect(long hostId, long poolId) {
poolVO.setCapacityBytes(poolInfo.getCapacityBytes());
poolVO.setUsedBytes(poolInfo.getCapacityBytes() - poolInfo.getAvailableBytes());
_storagePoolDao.update(poolVO.getId(), poolVO);
logger.info("Updated storage pool capacity: {} GB, used: {} GB", poolInfo.getCapacityBytes() / (1024 * 1024 * 1024), (poolInfo.getCapacityBytes() - poolInfo.getAvailableBytes()) / (1024 * 1024 * 1024));
logger.info("hostConnect: Updated storage pool capacity: {} GB, used: {} GB", poolInfo.getCapacityBytes() / (1024 * 1024 * 1024), (poolInfo.getCapacityBytes() - poolInfo.getAvailableBytes()) / (1024 * 1024 * 1024));
}

} catch (Exception e) {
logger.error("Exception while connecting host {} to storage pool {}", host.getName(), pool.getName(), e);
logger.error("hostConnect: Exception while connecting host {} to storage pool {}", host.getName(), pool.getName(), e);
// CRITICAL: Don't throw exception - it crashes the agent and causes restart loops
// Return false to indicate failure without crashing
return false;
}
return true;
}

@Override
public boolean hostDisconnected(long hostId, long poolId) {
logger.info("Disconnect from host " + hostId + " from pool " + poolId);
private void updateNfsExportPolicyForConnectedHostIfNeeded(long poolId, long hostId, Host host, Map<String, String> detailsMap) {
Comment thread
sandeeplocharla marked this conversation as resolved.
if (!ProtocolType.NFS3.name().equalsIgnoreCase(detailsMap.get(OntapStorageConstants.PROTOCOL))) {
return;
}
Comment thread
sandeeplocharla marked this conversation as resolved.

Host hostToremove = _hostDao.findById(hostId);
if (hostToremove == null) {
logger.error("Failed to add host by HostListener as host was not found with id : {}", hostId);
return false;
if (host == null) {
throw new CloudRuntimeException("Host was not found with id: " + hostId);
}

StoragePool pool = _storagePoolDao.findById(poolId);
if (pool == null) {
logger.error("Failed to disconnect host - storage pool not found with id: {}", poolId);
if (!isNfs3EnabledOnHost(host)) {
throw new CloudRuntimeException("NFS protocol is not enabled on host with id: " + hostId);
}
Comment thread
sandeeplocharla marked this conversation as resolved.

AccessGroup accessGroup = new AccessGroup();
accessGroup.setStoragePoolId(poolId);
accessGroup.setHostsToConnect(List.of((HostVO) host));

StorageStrategy strategy = OntapStorageUtils.getStrategyByStoragePoolDetails(detailsMap);
strategy.updateAccessGroup(accessGroup);
logger.info("hostConnect: updateNfsExportPolicyForConnectedHostIfNeeded: Updated NFS export policy rules for host {} on storage pool {}", host.getName(), poolId);
}

private boolean isNfs3EnabledOnHost(Host host) {
if (host == null) {
return false;
}
logger.info("Disconnecting host {} from ONTAP storage pool {}", hostToremove.getName(), pool.getName());

try {
DeleteStoragePoolCommand cmd = new DeleteStoragePoolCommand(pool);
Answer answer = _agentMgr.easySend(hostId, cmd);
if (answer != null && answer.getResult()) {
logger.info("Successfully disconnected host {} from ONTAP storage pool {}", hostToremove.getName(), pool.getName());
return true;
} else {
String errMsg = (answer != null) ? answer.getDetails() : "Unknown error";
logger.warn("Failed to disconnect host {} from storage pool {}. Error: {}", hostToremove.getName(), pool.getName(), errMsg);
return false;
}
} catch (Exception e) {
logger.error("Exception while disconnecting host {} from storage pool {}", hostToremove.getName(), pool.getName(), e);
String storageIp = host.getStorageIpAddress() != null ? host.getStorageIpAddress().trim() : "";
if (storageIp.isEmpty() && StringUtils.isBlank(host.getPrivateIpAddress())) {
logger.warn("isNfs3EnabledOnHost: Host {} is not eligible for NFS3 protocol: both storage IP and private IP are empty",
host.getId());
return false;
}

return true;
}

@Override
public boolean hostAboutToBeRemoved(long hostId) {
public boolean hostDisconnected(long hostId, long poolId) {
logger.info("hostDisconnected: Disconnecting host {} from pool {}", hostId, poolId);
// Note: This is not currently being called for NetApp ONTAP storage plugin.
Comment thread
sandeeplocharla marked this conversation as resolved.
return false;
}
Comment thread
sandeeplocharla marked this conversation as resolved.
Comment thread
sandeeplocharla marked this conversation as resolved.
Comment thread
sandeeplocharla marked this conversation as resolved.
Comment thread
sandeeplocharla marked this conversation as resolved.
Comment thread
sandeeplocharla marked this conversation as resolved.
Comment thread
sandeeplocharla marked this conversation as resolved.
Comment thread
sandeeplocharla marked this conversation as resolved.

@Override
public boolean hostAboutToBeRemoved(long hostId) {

@rajiv-jain-netapp rajiv-jain-netapp Jul 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking aloud, do we not have any usecase where this method returns false?

@sandeeplocharla sandeeplocharla Jul 13, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If export policy rule hasn't been removed, we could catch the exception and return false

logger.info("hostAboutToBeRemoved: Host {} is about to be removed", hostId);

Host host = _hostDao.findById(hostId);
if (host == null) {
logger.warn("hostAboutToBeRemoved: Host not found with id: {}, considering it as no-op", hostId);
return true;
}

List<StoragePoolHostVO> poolHostRefs = storagePoolHostDao.listByHostId(hostId);
if (poolHostRefs == null || poolHostRefs.isEmpty()) {
logger.debug("hostAboutToBeRemoved: No storage pool associations found for host {}", hostId);
return true;
}

for (StoragePoolHostVO ref : poolHostRefs) {
StoragePoolVO pool = _storagePoolDao.findById(ref.getPoolId());
if (pool != null) {
removeHostFromOntapPoolIfNeeded(pool, host);
}
}

logger.info("hostAboutToBeRemoved: Cleaned up ONTAP export policies for host {} about to be removed", hostId);
return true;
}

@Override
public boolean hostRemoved(long hostId, long clusterId) {
return false;
}

private void removeHostFromOntapPoolIfNeeded(StoragePoolVO pool, Host host) {
try {
Map<String, String> detailsMap = _storagePoolDetailsDao.listDetailsKeyPairs(pool.getId());
if (detailsMap == null || detailsMap.isEmpty()) {
logger.debug("hostAboutToBeRemoved: removeHostFromOntapPoolIfNeeded: No pool details found for pool id: {}", pool.getId());
return;
}

// Skip non-NFS3 pools; Currently, for iSCSI type, iGroup rules are being handled as part of revokeAccess in OntapPrimaryDataStoreDriver, so no need to handle here.
if (!ProtocolType.NFS3.name().equalsIgnoreCase(detailsMap.get(OntapStorageConstants.PROTOCOL))) {
return;
}

logger.info("hostAboutToBeRemoved: removeHostFromOntapPoolIfNeeded: Removing export policy rule for host {} from storage pool {}", host.getName(), pool.getName());
if (!isNfs3EnabledOnHost(host)) {
logger.warn("hostAboutToBeRemoved: removeHostFromOntapPoolIfNeeded: Skipping NFS export policy removal for host {} on pool {} as host is not NFS-enabled",
host.getId(), pool.getId());
return;
}
AccessGroup accessGroup = new AccessGroup();
Comment thread
sandeeplocharla marked this conversation as resolved.
accessGroup.setStoragePoolId(pool.getId());
accessGroup.setHostsToConnect(List.of((HostVO) host));
accessGroup.setHostRuleAction(AccessGroup.HostRuleAction.REMOVE);

StorageStrategy strategy = OntapStorageUtils.getStrategyByStoragePoolDetails(detailsMap);
strategy.updateAccessGroup(accessGroup);
logger.info("hostAboutToBeRemoved: removeHostFromOntapPoolIfNeeded: Removed NFS export policy rules for removed host {} from storage pool {}", host.getName(), pool.getName());
} catch (Exception e) {
logger.warn("hostAboutToBeRemoved: removeHostFromOntapPoolIfNeeded: Failed to remove NFS export policy rule for host {} from pool {}: {}", host.getId(), pool.getName(), e.getMessage());
// Continue processing other pools even if one fails
}
}

@Override
public boolean hostEnabled(long hostId) {
return false;
Expand Down
18 changes: 10 additions & 8 deletions ...age/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,16 @@

package org.apache.cloudstack.storage.service;

import com.cloud.utils.exception.CloudRuntimeException;
import feign.FeignException;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;

import org.apache.cloudstack.storage.feign.FeignClientFactory;
import org.apache.cloudstack.storage.feign.client.AggregateFeignClient;
import org.apache.cloudstack.storage.feign.client.JobFeignClient;
import org.apache.cloudstack.storage.feign.client.NetworkFeignClient;
import org.apache.cloudstack.storage.feign.client.NASFeignClient;
import org.apache.cloudstack.storage.feign.client.NetworkFeignClient;
import org.apache.cloudstack.storage.feign.client.SANFeignClient;
import org.apache.cloudstack.storage.feign.client.SnapshotFeignClient;
import org.apache.cloudstack.storage.feign.client.SvmFeignClient;
Expand All @@ -48,10 +51,9 @@
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import com.cloud.utils.exception.CloudRuntimeException;

import feign.FeignException;

/**
* Storage Strategy represents the communication path for all the ONTAP storage options
Expand Down Expand Up @@ -615,7 +617,7 @@ public abstract JobResponse revertSnapshotForCloudStackVolume(String snapshotNam
* @param accessGroup the access group to update
* @return the updated AccessGroup object
*/
abstract AccessGroup updateAccessGroup(AccessGroup accessGroup);
public abstract AccessGroup updateAccessGroup(AccessGroup accessGroup);
Comment thread
sandeeplocharla marked this conversation as resolved.

/**
* Method encapsulates the behavior based on the opted protocol in subclasses
Expand Down
Loading
Loading