CSTACKEX-306: support for create CS colume from CS snapshot on the lo… - #107
rajiv-jain-netapp wants to merge 1 commit into
Conversation
…cal primary storage pool
🔴 Test Coverage Grade:
|
| Metric | Value |
|---|---|
| Line coverage | 24.77% |
| Branch coverage | 18.90% |
Grade Scale
| Grade | Line Coverage | Meaning |
|---|---|---|
| 🟢 A | ≥ 80% | Excellent - this code sleeps well at night 😴 |
| 🟡 B | 60-79% | Good - almost there, don't stop now 😉 |
| 🟠 C | 40-59% | Acceptable - your code is wearing a seatbelt, but no airbags 😬 |
| 🔴 D | 20-39% | Marginal - boldly shipping where no test has gone before 🖖 |
| ⛔ F | < 20% | Failing - tests? what tests? 🔥 |
Branch coverage is shown as a secondary signal. Grade is determined by line coverage.
View full Actions run
| CloudStackVolume clonedCloudStackVolume = cloneOfTemplateId != null | ||
| ? cloneCloudStackVolumeFromTemplate(storagePool, volInfo, details, cloneOfTemplateId) | ||
| : createCloudStackVolume(storagePool, volInfo, details); | ||
| /* |
There was a problem hiding this comment.
Better have a README.md in driver also if such detailed comments are needed?
| * @return created CloudStackVolume with protocol-specific identity (LUN uuid or file path) | ||
| */ | ||
| abstract public CloudStackVolume cloneCloudStackVolumeFromSnapshot(StoragePoolVO storagePool, | ||
| Map<String, String> details, |
There was a problem hiding this comment.
For clarity, do mention that its poolDetails, else it could be interpreted as volumeDetails
| * for the duration of {@code createAsync} only (same pattern as {@link #getTemplateIdForCloning}).</p> | ||
| */ | ||
| private Long getSnapshotIdForCloning(long volumeId) { | ||
| VolumeDetailVO detail = volumeDetailsDao.findDetail(volumeId, OntapStorageConstants.CLONE_OF_SNAPSHOT); |
There was a problem hiding this comment.
Does cloudstack save this detail OntapStorageConstants.CLONE_OF_SNAPSHOT in volume details?
| sourcePath, destinationPath, snapshotName, flexVolName); | ||
| try { | ||
| FileCloneRequest request = new FileCloneRequest(flexVolUuid, flexVolName, sourcePath, destinationPath, snapshotName); | ||
| JobResponse jobResponse = nasFeignClient.cloneFile(getAuthHeader(), request); |
There was a problem hiding this comment.
Till the new adapter for nfs is brought in, we wanted to continue using kvm commands only right?
| // onto this CloudStack volume (and ONTAP QoS if applicable) after successful clone. | ||
| } else { | ||
| Long cloneOfTemplateId = getTemplateIdForCloning(volInfo.getId()); | ||
| clonedCloudStackVolume = cloneOfTemplateId != null |
There was a problem hiding this comment.
Instead would it be possible to have checks based on dataObject.getType()?
Like dataObject.getType() == DataObjectType.TEMPLATE or dataObject.getType() == DataObjectType.VOLUME, etc.,
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?