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 3 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
3 commits
Select commit Hold shift + click to select a range
259a88f
Multi-Host support for NFS3 and iSCSI
sandeeplocharla Jun 18, 2026
6c24f0e
Addressed comments
sandeeplocharla Jun 29, 2026
1f0e3c1
Reverted few unnecessary changes
sandeeplocharla Jun 30, 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
54 changes: 28 additions & 26 deletions ...ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,26 +18,13 @@
*/
package org.apache.cloudstack.storage.driver;

@rajiv-jain-netapp rajiv-jain-netapp Jul 1, 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.

this clas does not have any change, Shall we revert this file?


import org.apache.cloudstack.storage.utils.OntapStorageConstants;
import com.cloud.agent.api.Answer;
import com.cloud.agent.api.to.DataObjectType;
import com.cloud.agent.api.to.DataStoreTO;
import com.cloud.agent.api.to.DataTO;
import com.cloud.exception.InvalidParameterValueException;
import com.cloud.host.Host;
import com.cloud.host.HostVO;
import com.cloud.storage.Storage;
import com.cloud.storage.StoragePool;
import com.cloud.storage.Volume;
import com.cloud.storage.VolumeDetailVO;
import com.cloud.storage.VolumeVO;
import com.cloud.storage.ScopeType;
import com.cloud.storage.dao.SnapshotDetailsDao;
import com.cloud.storage.dao.SnapshotDetailsVO;
import com.cloud.storage.dao.VolumeDao;
import com.cloud.storage.dao.VolumeDetailsDao;
import com.cloud.utils.Pair;
import com.cloud.utils.exception.CloudRuntimeException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import javax.inject.Inject;

import org.apache.cloudstack.engine.subsystem.api.storage.ChapInfo;
import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult;
import org.apache.cloudstack.engine.subsystem.api.storage.CreateCmdResult;
Expand Down Expand Up @@ -66,17 +53,32 @@
import org.apache.cloudstack.storage.service.model.CloudStackVolume;
import org.apache.cloudstack.storage.service.model.ProtocolType;
import org.apache.cloudstack.storage.to.SnapshotObjectTO;
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 org.jetbrains.annotations.Nullable;

import javax.inject.Inject;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import com.cloud.agent.api.Answer;
import com.cloud.agent.api.to.DataObjectType;
import com.cloud.agent.api.to.DataStoreTO;
import com.cloud.agent.api.to.DataTO;
import com.cloud.exception.InvalidParameterValueException;
import com.cloud.host.Host;
import com.cloud.host.HostVO;
import com.cloud.storage.ScopeType;
import com.cloud.storage.Storage;
import com.cloud.storage.StoragePool;
import com.cloud.storage.Volume;
import com.cloud.storage.VolumeDetailVO;
import com.cloud.storage.VolumeVO;
import com.cloud.storage.dao.SnapshotDetailsDao;
import com.cloud.storage.dao.SnapshotDetailsVO;
import com.cloud.storage.dao.VolumeDao;
import com.cloud.storage.dao.VolumeDetailsDao;
import com.cloud.utils.Pair;
import com.cloud.utils.exception.CloudRuntimeException;

/**
* Primary datastore driver for NetApp ONTAP storage systems.
Expand Down Expand Up @@ -1042,4 +1044,4 @@ private void updateSnapshotDetails(long csSnapshotId, long csVolumeId, String fl
}
}

}
}
14 changes: 11 additions & 3 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,13 +67,17 @@ public String toString() {
return String.valueOf(value);
}

@JsonCreator
public static ProtocolsEnum fromValue(String text) {
if (text == null) {
return null;
}
for (ProtocolsEnum b : ProtocolsEnum.values()) {
if (String.valueOf(b.value).equals(text)) {
if (String.valueOf(b.value).equalsIgnoreCase(text) || b.name().equalsIgnoreCase(text)) {
return b;
}
}
return null;
throw new IllegalArgumentException("Unexpected protocol value '" + text + "'");
}
}

Expand Down
180 changes: 141 additions & 39 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 Down Expand Up @@ -83,6 +91,19 @@ public boolean hostConnect(long hostId, long poolId) {
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("Failed to load storage pool details for pool id: {}", poolId);
return false;
}

if (detailsMap.get(OntapStorageConstants.PROTOCOL) == null) {
logger.error("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);

// 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 @@ -151,50 +172,131 @@ public boolean hostConnect(long hostId, long poolId) {
return true;
}

private void updateNfsExportPolicyForConnectedHostIfNeeded(long poolId, long hostId, Host host, Map<String, String> detailsMap) {
if (!ProtocolType.NFS3.name().equalsIgnoreCase(detailsMap.get(OntapStorageConstants.PROTOCOL))) {
return;
}

if (host == null) {
throw new CloudRuntimeException("Host was not found with id: " + hostId);
}

if (!(host instanceof HostVO)) {
throw new CloudRuntimeException("Host object is not of type HostVO for hostId: " + hostId);
}

HostVO hostVO = (HostVO) host;
if (!isNfs3EnabledOnHost(hostVO)) {
throw new CloudRuntimeException("NFS protocol is not enabled on host with id: " + hostId);
}

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

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

private boolean isNfs3EnabledOnHost(HostVO host) {
if (host == null) {
return false;
}

String storageIp = host.getStorageIpAddress() != null ? host.getStorageIpAddress().trim() : "";
if (storageIp.isEmpty() && StringUtils.isBlank(host.getPrivateIpAddress())) {
logger.warn("Host {} is not eligible for NFS3 protocol: both storage IP and private IP are empty",
host.getId());
return false;
}

return true;
}

@Override
public boolean hostDisconnected(long hostId, long poolId) {
logger.info("Disconnect from host " + hostId + " from pool " + poolId);
// Note: This is not currently being called for NetApp ONTAP storage plugin.
return false;
}

@Override
public boolean hostAboutToBeRemoved(long hostId) {
logger.info("Host {} is about to be removed", hostId);

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

StoragePool pool = _storagePoolDao.findById(poolId);
if (pool == null) {
logger.error("Failed to disconnect host - storage pool not found with id: {}", poolId);
if (!host.getHypervisorType().equals(Hypervisor.HypervisorType.KVM)) {

@rajiv-jain-netapp rajiv-jain-netapp Jul 1, 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.

If this logic is executed during host removal, and the host is not a KVM host, do we really need to return false? Would it make more sense to return true instead?

Returning false could potentially block the host removal workflow, which may not be desirable and does not appear to provide any benefit to our plugin. Could you please confirm the expected behavior here and whether returning false is intentionally required for non-KVM hosts?

logger.debug("ONTAP plugin does not support {} type host, skipping cleanup", host.getHypervisorType());
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;
List<StoragePoolHostVO> poolHostRefs = storagePoolHostDao.listByHostId(hostId);
if (poolHostRefs == null || poolHostRefs.isEmpty()) {
logger.debug("No storage pool associations found for host {}", hostId);
return true;
}

for (StoragePoolHostVO ref : poolHostRefs) {
StoragePoolVO pool = _storagePoolDao.findById(ref.getPoolId());
if (pool != null) {
removeHostFromOntapPoolIfNeeded(hostId, pool, host);
}
} catch (Exception e) {
logger.error("Exception while disconnecting host {} from storage pool {}", hostToremove.getName(), pool.getName(), e);
return false;
}
}

@Override
public boolean hostAboutToBeRemoved(long hostId) {
return false;
logger.info("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(long hostId, StoragePoolVO pool, Host host) {
try {
Map<String, String> detailsMap = _storagePoolDetailsDao.listDetailsKeyPairs(pool.getId());
if (detailsMap == null || detailsMap.isEmpty()) {
logger.debug("No pool details found for pool id: {}", pool.getId());
return;
}

// Skip non-NFS3 pools
if (!ProtocolType.NFS3.name().equalsIgnoreCase(detailsMap.get(OntapStorageConstants.PROTOCOL))) {
return;
}

if (!(host instanceof HostVO)) {
logger.warn("Host object is not of type HostVO for hostId: {}", hostId);
return;
}

logger.info("Removing export policy rule for host {} from storage pool {}", host.getName(), pool.getName());
HostVO hostVO = (HostVO) host;
if (!isNfs3EnabledOnHost(hostVO)) {
logger.warn("Skipping NFS export policy removal for host {} on pool {} as host is not NFS-enabled",
hostId, pool.getId());
return;
}
AccessGroup accessGroup = new AccessGroup();
accessGroup.setStoragePoolId(pool.getId());
accessGroup.setHostsToConnect(List.of(hostVO));
accessGroup.setHostRuleAction(AccessGroup.HostRuleAction.REMOVE);

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

@Override
public boolean hostEnabled(long hostId) {
return false;
Expand Down
20 changes: 11 additions & 9 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,17 @@

package org.apache.cloudstack.storage.service;

import com.cloud.utils.exception.CloudRuntimeException;
import feign.FeignException;
import java.util.ArrayList;
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,11 +52,9 @@
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

import java.util.HashMap;
import java.util.ArrayList;
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 @@ -590,7 +592,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);

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