Skip to content

Refactor code to choose aggregate, network interface and creating storage volume; Also, the corresponding UT changes #89

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

Merged
sandeeplocharla merged 9 commits into from
Nov 2, 2026
Merged

Refactor code to choose aggregate, network interface and creating storage volume; Also, the corresponding UT changes #89

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
639d9e1
Refactor code to choose aggregate, network interface and creating sto…
sandeeplocharla Aug 10, 2026
1650341
Added missing UTs
sandeeplocharla Aug 10, 2026
4ff2956
Addressed review comments
sandeeplocharla Aug 12, 2026
2a6db30
Committed missed test files
sandeeplocharla Aug 12, 2026
6aacd02
Addressed review comments
sandeeplocharla Oct 7, 2026
2013a6d
Brought back the missing changes
sandeeplocharla Oct 8, 2026
364e96c
pre-check failure fix
sandeeplocharla Oct 8, 2026
ed7ac6b
Added a precautionary null check for network interface IP
sandeeplocharla Oct 15, 2026
10c3d31
Fixed a build issue
sandeeplocharla Oct 15, 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
39 changes: 24 additions & 15 deletions ...src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
import org.apache.cloudstack.storage.datastore.lifecycle.BasePrimaryDataStoreLifeCycleImpl;
import org.apache.cloudstack.storage.feign.model.Aggregate;
import org.apache.cloudstack.storage.feign.model.OntapStorage;
import org.apache.cloudstack.storage.feign.model.Volume;
import org.apache.cloudstack.storage.provider.StorageProviderFactory;
Expand All @@ -68,7 +69,6 @@
import com.cloud.storage.StorageManager;
import com.cloud.storage.StoragePool;
import com.cloud.storage.StoragePoolAutomation;
import com.cloud.utils.Pair;
import com.cloud.utils.exception.CloudRuntimeException;
import com.google.common.base.Preconditions;

Expand Down Expand Up @@ -147,10 +147,30 @@ public DataStore initialize(Map<String, Object> dsInfos) {
if (storageStrategy.getResolvedSvmUuid() != null && !storageStrategy.getResolvedSvmUuid().isEmpty()) {
details.put(OntapStorageConstants.SVM_UUID, storageStrategy.getResolvedSvmUuid());
}
Aggregate aggregate;
try {
aggregate = storageStrategy.chooseAggregate(storageStrategy.getAggregates(), capacityBytes);
} catch (Exception e) {
logger.error("Exception occurred while choosing aggregate for pool: " + storagePoolName, e);
throw new CloudRuntimeException("Failed to choose ONTAP aggregate for pool: " + storagePoolName
+ ". Error: " + e.getMessage(), e);
}

Map<String, String> lifResult;
try {
lifResult = storageStrategy.getNetworkInterface(aggregate);
} catch (Exception e) {
logger.error("Exception occurred while retrieving network interface for pool: " + storagePoolName, e);
throw new CloudRuntimeException("Failed to retrieve Data LIF from ONTAP: " + e.getMessage(), e);
}
String dataLif = lifResult.get(OntapStorageConstants.DATA_LIF);
String lifWarning = lifResult.get(OntapStorageConstants.LIF_WARNING);
processDataLifSelection(dataLif, lifWarning, details, storagePoolName, zoneId, podId);
Comment thread
sandeeplocharla marked this conversation as resolved.
Resolved

Comment thread
sandeeplocharla marked this conversation as resolved.
Resolved
logger.info("Creating ONTAP volume '" + storagePoolName + "' with size: " + capacityBytes + " bytes (" +
(capacityBytes / (1024 * 1024 * 1024)) + " GB)");
try {
Volume volume = storageStrategy.createStorageVolume(storagePoolName, capacityBytes);
Volume volume = storageStrategy.createStorageVolume(storagePoolName, capacityBytes, aggregate);

@rajiv-jain-netapp rajiv-jain-netapp Nov 2, 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.

One negative scenario I see here is when the volume creation itself fails. In that case, displaying a LIF warning to the user may not add much value. Can we consider showing the LIF warning only if the volume creation succeeds?

@sandeeplocharla sandeeplocharla Nov 2, 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.

Even Surya pointed this out. Don't you think if there's an issue with their ONTAP which is causing the LIF warning, displaying it would help the user fix it?
Anyways the LIF warning will be shown separately under 'Alerts' section in Infrastructure summary, while the reason for Volume creation failure would be an event notification. So, chances of confusion also is less, I feel.
What do you think?

if (volume == null) {
logger.error("createStorageVolume returned null for volume: " + storagePoolName);
throw new CloudRuntimeException("Failed to create ONTAP volume: " + storagePoolName);
Expand All @@ -162,15 +182,6 @@ public DataStore initialize(Map<String, Object> dsInfos) {
logger.error("Exception occurred while creating ONTAP volume: " + storagePoolName, e);
throw new CloudRuntimeException("Failed to create ONTAP volume: " + storagePoolName + ". Error: " + e.getMessage(), e);
}

Pair<String, String> lifResult;
try {
lifResult = storageStrategy.getNetworkInterface();
} catch (Exception e) {
logger.error("Exception occurred while retrieving network interface for pool: " + storagePoolName, e);
throw new CloudRuntimeException("Failed to retrieve Data LIF from ONTAP: " + e.getMessage(), e);
}
processDataLifSelection(lifResult, details, storagePoolName, zoneId, podId);
} else {
throw new CloudRuntimeException("ONTAP details validation failed, cannot create primary storage");
}
Expand Down Expand Up @@ -289,18 +300,16 @@ private void validateInitializeInputs(Long capacityBytes, Long capacityIops, Lon
}
}

private void processDataLifSelection(Pair<String, String> lifResult, Map<String, String> details,
private void processDataLifSelection(String dataLIF, String lifWarning, Map<String, String> details,
String storagePoolName, Long zoneId, Long podId) {
String dataLIF = lifResult.first();
if (dataLIF == null || dataLIF.isEmpty()) {
throw new CloudRuntimeException("Failed to retrieve Data LIF from ONTAP, cannot create primary storage");
}
logger.info("Using Data LIF for storage access: " + dataLIF);
details.put(OntapStorageConstants.DATA_LIF, dataLIF);

// Persist LIF warning as a pool detail and fire a storage alert so the user is informed
if (lifResult.second() != null) {
String lifWarning = lifResult.second();
if (lifWarning != null) {
details.put(OntapStorageConstants.LIF_WARNING, lifWarning);
logger.warn("LIF selection warning for pool '" + storagePoolName + "': " + lifWarning);
String alertSubject = "ONTAP Storage Pool '" + storagePoolName + "': "
Expand Down
Loading
Loading