Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -18,26 +18,13 @@
*/
package org.apache.cloudstack.storage.driver;

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,36 @@
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.exception.StorageConflictException;
import com.cloud.exception.StorageUnavailableException;
import com.cloud.host.Host;
import com.cloud.host.HostVO;
import com.cloud.storage.ScopeType;
import com.cloud.storage.Storage;
import com.cloud.storage.StorageManager;
import com.cloud.storage.StoragePool;
import com.cloud.storage.StoragePoolHostVO;
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 All @@ -91,6 +97,7 @@ public class OntapPrimaryDatastoreDriver implements PrimaryDataStoreDriver {
@Inject private VolumeDao volumeDao;
@Inject private VolumeDetailsDao volumeDetailsDao;
@Inject private SnapshotDetailsDao snapshotDetailsDao;
@Inject private StorageManager storageManager;

@Override
public Map<String, String> getCapabilities() {
Expand Down Expand Up @@ -392,8 +399,9 @@ public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore
// Only retrieve LUN name for iSCSI volumes
grantAccessIscsi(host, volumeVO, details, svmName, storagePool);
} else if (ProtocolType.NFS3.name().equalsIgnoreCase(details.get(OntapStorageConstants.PROTOCOL))) {
// For NFS, no access grant needed - file is accessible via mount
logger.debug("grantAccess: NFS volume [{}], no igroup mapping required", volumeVO.getUuid());
// For NFS, ensure export policy has host rule and host is connected to pool.
ensureNfsHostAccess(host, storagePool, details);
logger.debug("grantAccess: NFS volume [{}], ensured host access to storage pool [{}]", volumeVO.getUuid(), storagePool.getId());
return true;
}
volumeVO.setPoolType(storagePool.getPoolType());
Expand All @@ -410,6 +418,43 @@ public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore
}
}

private void ensureNfsHostAccess(Host host, StoragePoolVO storagePool, Map<String, String> details) {
boolean hostConnectedToPool = false;
List<StoragePoolHostVO> connectedPools = storageManager.findStoragePoolsConnectedToHost(host.getId());
if (connectedPools != null) {
for (StoragePoolHostVO connectedPoolRef : connectedPools) {
if (connectedPoolRef.getPoolId() == storagePool.getId()) {
hostConnectedToPool = true;
break;
}
}
}

if (!hostConnectedToPool) {
try {
logger.info("grantAccess: Host [{}] is not connected to NFS pool [{}], reconnecting host to pool", host.getId(), storagePool.getId());
boolean connected = storageManager.connectHostToSharedPool(host, storagePool.getId());

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The shared pool term is picked from the CloudStack perspective, or from the way we are approaching storage pool design. Please add code comment, if possible

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.

Though I'm aware that this code is a bit of a stretch, I have added with nfs protocol being enabled on a host post its addition to the storage pool. I wasn't able to simulate the scenario and looks like a rare occurence. So, on discussing with the team in the scrum today, we came to consensus that at this point we can document the pre-requisite of enabling the protocol on host and remove this code. I wanted to take your opinion on the same. So, please let me know if you feel the same?

if (!connected) {
throw new CloudRuntimeException("Failed to connect host " + host.getId() + " to NFS pool " + storagePool.getId());
}
} catch (StorageUnavailableException | StorageConflictException e) {
throw new CloudRuntimeException("Unable to connect host " + host.getId() + " to NFS pool " + storagePool.getId(), e);
}
return;
}

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

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

StorageStrategy strategy = OntapStorageUtils.getStrategyByStoragePoolDetails(details);
strategy.updateAccessGroup(accessGroup);
}

private void grantAccessIscsi(Host host, VolumeVO volumeVO, Map<String, String> details, String svmName, StoragePoolVO storagePool) {
String cloudStackVolumeName = volumeDetailsDao.findDetail(volumeVO.getId(), OntapStorageConstants.LUN_DOT_NAME).getValue();
UnifiedSANStrategy sanStrategy = (UnifiedSANStrategy) OntapStorageUtils.getStrategyByStoragePoolDetails(details);
Expand Down
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
Loading
Loading