CSTACKEX-261: enabling cloudstack volume resize (GROW) for iscsi and nfs - #100
sathvikaragi wants to merge 1 commit into
Conversation
🔴 Test Coverage Grade:
|
| Metric | Value |
|---|---|
| Line coverage | 24.71% |
| Branch coverage | 18.85% |
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
| private Long maxIops; | ||
|
|
||
| @Parameter(name = ApiConstants.SIZE, type = CommandType.LONG, required = false, description = "New volume size in GB") | ||
| @Parameter(name = ApiConstants.SIZE, type = CommandType.LONG, required = false, description = "New volume size in GB",validations = {ApiArgValidator.PositiveNumber}) |
There was a problem hiding this comment.
This can impact other vendors also, do we know whether keeping negative is expected for few existing usecases ?
There was a problem hiding this comment.
without this change we are getting this error in UI:
Unable to update on DB, due to: Data truncation: Out of range value for column 'capacity_bytes' at row 1
If I do sql query describe storage_pool I can see that capacity bytes is bigint unsigned (accepts non-negative only) so this validation is there from DB side
|
|
||
| logger.debug("getUsedBytes: FlexVolume [{}] backing pool [{}] reports {} bytes used", | ||
| flexVolUuid, storagePool.getId(), flexVol.getSpace().getUsed()); | ||
| return flexVol.getSpace().getUsed(); |
There was a problem hiding this comment.
is this method giving values in bytes or other unit, lets validate? we have to convert the values as per bytes if required.
There was a problem hiding this comment.
It is giving in bytes only checked the log that I am printing just above it :
DEBUG [o.a.c.s.d.OntapPrimaryDatastoreDriver] (Work-Job-Executor-2:[ctx-8df71e7e, job-387/job-388, ctx-adae6557]) (logid:9d9fc91a) getUsedBytes: FlexVolume [437425de-ab7b-11f1-9dad-005056bdaa40] backing pool [16] reports 1483042816 bytes used
| } | ||
|
|
||
| public Volume getStorageVolume(String uuid) { | ||
| logger.info("getStorageVolume: Fetching ONTAP volume by UUID: {}", uuid); |
There was a problem hiding this comment.
Better to have null check for UUID field
| ? (ResizeVolumePayload) volumeObject.getpayload() | ||
| : null; | ||
| String instanceName = resizePayload != null ? resizePayload.instanceName : "none"; | ||
| ResizeVolumeCommand cmd = new ResizeVolumeCommand(volume.getPath(), new StorageFilerTO(storagePool), |
There was a problem hiding this comment.
any reasons for not using control path for resize, instead using data path resize ? even in case of data path operations, why do we need this code to be under NAS strategy class?
There was a problem hiding this comment.
actually this method was implemented as a part of your cache the template local to the storage pool PR I had just changed the last parameter which is instance name. im just getting that from resizevolumepayload and passing, I haven't changed anything regards to path.
2 participants
Description
Cloudstack volume resize supports grow for both NFS and Iscsi which on ontap leads to increase in size for a LUN in case of iscsi and file in case of NFS.Also consists of UT's for the changes and implementation for getusedbytes which is called in this flow to check if storagepool has enough space.
This PR...
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?