Conversation
🔴 Test Coverage Grade:
|
| Metric | Value |
|---|---|
| Line coverage | 24.74% |
| Branch coverage | 18.87% |
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
🔴 Test Coverage Grade:
|
| Metric | Value |
|---|---|
| Line coverage | 24.74% |
| Branch coverage | 18.87% |
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
🔴 Test Coverage Grade:
|
| Metric | Value |
|---|---|
| Line coverage | 24.73% |
| Branch coverage | 18.86% |
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
|
|
||
| @attr(tags=["iscsi_workflow"], required_hardware=True) | ||
| def test_01_create_primary_storage_pool(self): | ||
| def test_01_reject_create_when_flexvol_name_exists(self): |
There was a problem hiding this comment.
Relocate this test after the positive test case for pool creation. That way you wouldn't have to create a pool just for this.
| 06 Enter maintenance mode | ||
| 07 Cancel maintenance mode | ||
| 08 Enter maintenance mode and delete the storage pool | ||
| 09 Create a new pool and allocate a CloudStack data volume (LUN created) |
There was a problem hiding this comment.
Do think of negative cases that can occur on the kvm host too. For example, in case of iSCSI, you add a test case when either iscsi session closed or got logged out; similarly, for NFS3, you might have a case when either nfs service stopped, or the mount point got deleted.
| return "OntapISCSI%s_%d" % (suffix, random.randint(0, 99999)) | ||
|
|
||
|
|
||
| def _cleanup_throwaway_pool(self, pool, flexvol_name=None): |
There was a problem hiding this comment.
You might not need this once you change the test case as suggested in earlier comment.
| """(igroup name, initiator IQN) per cluster host that reports an IQN.""" | ||
| return self._iscsi_host_specs() | ||
|
|
||
| def _enter_maintenance(self, pool): |
There was a problem hiding this comment.
Try to make this a common helper method and use it across the tests wherever pool in maintenance is required.
| if not self._volume_exists_in_cs(vol.id): | ||
| self.__class__.volume2 = None | ||
|
|
||
| def _exit_maintenance(self, pool, label): |
There was a problem hiding this comment.
This can also be a common helper method
| label, pool.name, exc) | ||
| return False | ||
|
|
||
| def _enter_maintenance_quietly(self, pool, label): |
There was a problem hiding this comment.
Make sure the agent creates method only when modularity in code is required
|
|
||
| DESTROYED_VOLUME_STATES = ("destroy", "destroyed", "expunging", "expunged") | ||
|
|
||
| def _cs_volume_state(self, vol_id): |
There was a problem hiding this comment.
If this method including the above and below are needed frequently, consider putting them as a helper method to be used across tests in all suites
| state = self._cs_volume_state(vol_id) | ||
| return state is None or state.lower() in self.DESTROYED_VOLUME_STATES | ||
|
|
||
| def _remove_cs_volume(self, pool, vol, label): |
There was a problem hiding this comment.
I don't think this case affects pool deletion. Maybe we need to have a separate for Volume only operations, this can be a part of that.
| # ------------------------------------------------------------------ | ||
|
|
||
| @attr(tags=["iscsi_with_volumes"], required_hardware=True) | ||
| def test_11_cancel_maintenance_after_volume_deleted(self): |
There was a problem hiding this comment.
This is just performing Cancel Maintenance on a StoragePool. I don't think it adds any value
| Test order — 03-06 are a sequential workflow that must run in order; 01, 02, | ||
| 07 and 08 are isolated negative/recovery cases, each owning the pool it | ||
| creates, so they can be run on their own: | ||
| 01 Create rejected when a FlexVol of the same name already exists |
There was a problem hiding this comment.
ONTAP plugin doesn't behave differently between Cluster and Zone scoped pools. So, the same test cases are not needed here.
FYI, Zone scoped test cases are added to do just perform sanity check
2 participants
Description
This PR...
Negative test cases added for ONTAP storage pool create, delete, and maintenance (NFS3 and iSCSI).
nfs3/pool/test_pool_lifecycle.py (+4)
test_01_reject_create_when_flexvol_name_exists — create is rejected when a FlexVol with that name already exists on ONTAP
test_02_reject_create_when_no_aggregate_space — create is rejected when requested capacity exceeds every online SVM aggregate
test_11_delete_pool_with_flexvol_predeleted — empty pool can still be deleted after its FlexVol was removed on ONTAP
test_12_delete_pool_with_export_policy_predeleted — empty pool can still be deleted after its NFS export policy was removed on ONTAP
nfs3/pool/test_pool_with_volumes.py (+3)
test_08_delete_pool_with_volume_flexvol_missing — force-delete a pool that still has a CS volume after the FlexVol was removed on ONTAP
test_09_delete_pool_with_volume_export_policy_missing — force-delete a pool that still has a CS volume after the export policy was removed on ONTAP
test_10_cancel_maintenance_after_volume_deleted — cancel maintenance after the CS volume on the pool has already been deleted
nfs3/pool/test_zone_scoped_pool.py (+4):
test_01_create_zone_pool_rejected_when_flexvol_exists — zone-scoped create is rejected when a same-name FlexVol already exists
test_02_create_zone_pool_rejected_when_no_aggregate_space — zone-scoped create is rejected when no aggregate has enough free space
test_07_delete_zone_pool_with_flexvol_predeleted — empty zone pool can still be deleted after its FlexVol was removed on ONTAP
test_08_delete_zone_pool_with_export_policy_predeleted — empty zone pool can still be deleted after its export policy was removed on ONTAP
iscsi/pool/test_pool_lifecycle.py (+4):
test_01_reject_create_when_flexvol_name_exists — create is rejected when a FlexVol with that name already exists on ONTAP
test_02_reject_create_when_no_aggregate_space — create is rejected when requested capacity exceeds every online SVM aggregate
test_11_delete_pool_with_flexvol_predeleted — empty pool can still be deleted after its FlexVol was removed on ONTAP
test_12_delete_pool_with_igroups_predeleted — empty pool can still be deleted after host igroups were removed on ONTAP
iscsi/pool/test_pool_with_volumes.py (+4):
test_08_delete_pool_with_volume_flexvol_missing — force-delete a pool that still has a CS volume after the FlexVol was removed on ONTAP
test_09_delete_pool_with_volume_igroups_missing — force-delete a pool that still has a CS volume after host igroups were removed on ONTAP
test_10_enter_maintenance_lun_maps_predeleted — enter maintenance after LUN maps for the volume were removed on ONTAP
test_11_cancel_maintenance_after_volume_deleted — cancel maintenance after the CS volume on the pool has already been deleted
iscsi/pool/test_zone_scoped_pool.py (+4):
test_01_create_zone_pool_rejected_when_flexvol_exists — zone-scoped create is rejected when a same-name FlexVol already exists
test_02_create_zone_pool_rejected_when_no_aggregate_space — zone-scoped create is rejected when no aggregate has enough free space
test_07_delete_zone_pool_with_flexvol_predeleted — empty zone pool can still be deleted after its FlexVol was removed on ONTAP
test_08_delete_zone_pool_with_igroups_predeleted — empty zone pool can still be deleted after host igroups were removed on ONTAP
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
nfs3/pool/test_pool_lifecycle.py:
test_01_reject_create_when_flexvol_name_exists — Creates a FlexVol on ONTAP first, then calls createStoragePool with the same name. Create must raise CloudstackAPIException, CloudStack must not keep a pool of that name, and the original FlexVol must still be there (plugin must not adopt or delete it).
test_02_reject_create_when_no_aggregate_space — Reads live free space from the SVM’s online aggregates, requests 1 GiB more than the largest one, and expects No suitable aggregates. No CloudStack pool and no FlexVol may be left behind.
test_11_delete_pool_with_flexvol_predeleted — Creates an empty pool, unmounts NFS on the KVM hosts, deletes the FlexVol directly on ONTAP, then deletes the pool in CloudStack. Delete must succeed; the pool record and the NFS export policy must both be gone.
test_12_delete_pool_with_export_policy_predeleted — Same empty-pool path, but the NFS export policy is removed on ONTAP first. CloudStack delete must still succeed and must remove the FlexVol.
nfs3/pool/test_pool_with_volumes.py:
test_08_delete_pool_with_volume_flexvol_missing — Isolated pool + CS volume. After the FlexVol is deleted on ONTAP, deleteStoragePool(forced=True) is still rejected while the CS volume exists. After the volume is removed, forced delete must succeed and the leftover CS volume record must be cleanable.
test_09_delete_pool_with_volume_export_policy_missing — Isolated pool + CS volume. Export policy is deleted on ONTAP while the FlexVol remains. Forced delete (after the CS volume is gone) must tolerate the missing policy and still delete the FlexVol.
test_10_cancel_maintenance_after_volume_deleted — Isolated pool + volume. Volume is deleted while the pool is Up (NFS3 cannot delete a volume in Maintenance), then the pool is put in Maintenance and cancelled. Pool must return to Up; FlexVol stays online and the export policy stays intact.
nfs3/pool/test_zone_scoped_pool.py:
test_01_create_zone_pool_rejected_when_flexvol_exists — Same duplicate-FlexVol reject as cluster scope, but createStoragePool is zone-scoped (attachZone). No CS pool; pre-created FlexVol unchanged.
test_02_create_zone_pool_rejected_when_no_aggregate_space — Same over-capacity reject as cluster scope, for a zone-scoped pool. Error must contain No suitable aggregates; no pool and no FlexVol.
test_07_delete_zone_pool_with_flexvol_predeleted — Empty zone pool: FlexVol removed on ONTAP, then CloudStack delete. Pool gone; export policy cleaned up.
test_08_delete_zone_pool_with_export_policy_predeleted — Empty zone pool: export policy removed on ONTAP, then CloudStack delete. Pool gone; FlexVol deleted.
iscsi/pool/test_pool_lifecycle.py:
test_01_reject_create_when_flexvol_name_exists — Same duplicate-name create reject as NFS3, for an iSCSI pool.
test_02_reject_create_when_no_aggregate_space — Same aggregate-space create reject as NFS3, for an iSCSI pool.
test_11_delete_pool_with_flexvol_predeleted — Empty iSCSI pool: FlexVol deleted on ONTAP first, then CloudStack delete. Pool must disappear; FlexVol must not come back.
test_12_delete_pool_with_igroups_predeleted — Empty iSCSI pool. Host igroups (named from host UUID + SVM, shared on the SVM) are seeded then deleted on ONTAP. CloudStack delete must succeed and still remove the FlexVol. Skips if another ONTAP pool is using the same SVM.
iscsi/pool/test_pool_with_volumes.py:
test_08_delete_pool_with_volume_flexvol_missing — Isolated iSCSI pool + LUN-backed CS volume. FlexVol gone on ONTAP; forced delete after the CS volume is removed must succeed.
test_09_delete_pool_with_volume_igroups_missing — Isolated pool + volume. Host igroups deleted on ONTAP first. Forced delete after the CS volume is removed must succeed and still delete the FlexVol. Skips if the SVM is shared with another ONTAP pool.
test_10_enter_maintenance_lun_maps_predeleted — Isolated pool + volume. A LUN map is created then deleted on ONTAP. enableStorageMaintenance must still put the pool in Maintenance, the CS volume must remain, the LUN stays online, and maps must not reappear.
test_11_cancel_maintenance_after_volume_deleted — Isolated pool + volume. CS volume deleted, then cancel maintenance. Pool returns to Up; FlexVol/LUN remain; shared host igroups are unchanged from the suite baseline.
iscsi/pool/test_zone_scoped_pool.py:
test_01_create_zone_pool_rejected_when_flexvol_exists — Zone-scoped iSCSI create reject when the FlexVol name already exists.
test_02_create_zone_pool_rejected_when_no_aggregate_space — Zone-scoped iSCSI create reject when no aggregate has enough free space.
test_07_delete_zone_pool_with_flexvol_predeleted — Empty zone iSCSI pool deleted after the FlexVol was removed on ONTAP.
test_08_delete_zone_pool_with_igroups_predeleted — Empty zone iSCSI pool deleted after host igroups were removed on ONTAP; FlexVol must still be deleted.
How did you try to break this feature and the system with this change?