Repository navigation
Conversation
🔴 Test Coverage Grade:
|
| Metric | Value |
|---|---|
| Line coverage | 25.82% |
| Branch coverage | 20.11% |
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
f9b9e50 to
9a18590
Compare
|
|
||
| FileInfo file = new FileInfo(); | ||
| file.setPath(OntapStorageUtils.toFlexVolRelativePath(sourceVolumePath, flexVolName)); | ||
| String relativeSourcePath = OntapStorageUtils.toFlexVolRelativePath(sourceVolumePath, flexVolName); |
There was a problem hiding this comment.
add example as a comment for relative path and similarly for path in file.setPath()
There was a problem hiding this comment.
Added example comments for both paths.
| sourcePath, destinationPath, flexVolName, snapshotName); | ||
| try { | ||
| FileCloneRequest request = new FileCloneRequest(flexVolUuid, flexVolName, sourcePath, destinationPath, snapshotName); | ||
| FileCloneRequest request = new FileCloneRequest(flexVolUuid, flexVolName, sourcePath, destinationPath); |
There was a problem hiding this comment.
Since the snapshot name has been removed from the constructor, the corresponding constructor in the FileCloneRequest model is no longer required. Could you please remove that constructor as well as the snapshotName field, since it is no longer being used
There was a problem hiding this comment.
Done. Removed the five-argument constructor, the unused snapshot field, its getter/setter, and the related SnapshotRef model from FileCloneRequest.
There was a problem hiding this comment.
this should not be required now as it is already set as a path.
Similarly check other places for setter and getters for this snapshot name for file and remove that
There was a problem hiding this comment.
Done. Removed request.setSnapshotName(snapshotName) and removed the unused snapshotName field, getter, and setter from CloudStackVolume
| * Builds the request that clones a file out of a FlexVolume snapshot into a new file named after | ||
| * the volume uuid, inside the same FlexVolume. | ||
| */ | ||
| private CloudStackVolume createCloneFileFromSnapshotRequest(StoragePoolVO storagePool, Map<String, String> details, |
There was a problem hiding this comment.
check the file for comments related to snapshot.name and update it
There was a problem hiding this comment.
Done. Updated the comments in this file to describe the snapshot source through source_path using ./.snapshot//. Removed the outdated snapshot.name references.
9a18590 to
1a143ed
Compare
| * (create-volume-from-snapshot). Null for live clones (e.g. template cache). | ||
| */ | ||
| private String snapshotName; | ||
|
|
There was a problem hiding this comment.
is this not required even for LUN ?
There was a problem hiding this comment.
For iSCSI, cloneCloudStackVolumeFromSnapshot() reads ontap_snap_name from snapshot_details and passes it to createCloneLunFromSnapshotRequest(), which builds the snapshot LUN path:
/vol//.snapshot//
Description
Creating a NFS volume from an ONTAP snapshot sent POST /api/storage/file/clone with a separate snapshot field. That API has no snapshot property, so ONTAP returned 400, code 262179, Unexpected argument "snapshot".
The snapshot source now goes in source_path, which is what the file-clone API accepts:
./.snapshot/ < snapshot-name > / < source-file >
createCloneFileFromSnapshotRequest builds that path. cloneCloudStackVolume sends the 4-argument FileCloneRequest and no longer sets snapshot.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
Before the fix:

After the fix:

Tested on 9.15.1 ONTAP Filer:



How Has This Been Tested?
How did you try to break this feature and the system with this change?