NFS3 and iSCSI automation tests; Fix for NFS Cancel Maintenance - #66
NFS3 and iSCSI automation tests; Fix for NFS Cancel Maintenance#66sandeeplocharla wants to merge 11 commits into
Conversation
19bcfc3 to
e656893
Compare
96b40d9 to
9bf2bc7
Compare
9bf2bc7 to
a2cd65e
Compare
There was a problem hiding this comment.
Pull request overview
This pull request adds a comprehensive Marvin-based integration test suite for the NetApp ONTAP primary storage plugin, covering NFS3 and iSCSI workflows end-to-end (pool lifecycle, volume lifecycle, and VM attach/detach), along with supporting documentation and a convenience runner script.
Changes:
- Added 10 ONTAP integration test suites (NFS3 + iSCSI) with a shared
OntapTestBase+ ONTAP REST verification client. - Added documentation (README + full test-case matrix) and a
run_tests.shwrapper to execute suites by tag. - Added an
ontap.cfgconfiguration file for test environments (needs scrubbing before merge).
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test/integration/plugins/ontap/TEST_CASES.md | Adds a detailed test-case reference matrix for all ONTAP suites. |
| test/integration/plugins/ontap/run_tests.sh | Adds a tag-driven runner to execute each suite file sequentially and summarize results. |
| test/integration/plugins/ontap/README.md | Documents test layout, prerequisites, configuration, and execution patterns. |
| test/integration/plugins/ontap/ontap.cfg | Provides a Marvin config for ONTAP tests (currently contains real-looking credentials/IPs). |
| test/integration/plugins/ontap/ontap_test_base.py | Introduces shared base class, ONTAP REST client, and common helpers/cleanup. |
| test/integration/plugins/ontap/init.py | Package initializer for the ONTAP test plugin directory. |
| test/integration/plugins/ontap/nfs3/init.py | Package initializer for NFS3 tests. |
| test/integration/plugins/ontap/nfs3/pool/init.py | Package initializer for NFS3 pool suites. |
| test/integration/plugins/ontap/nfs3/pool/test_pool_lifecycle.py | Adds NFS3 pool lifecycle workflow tests and ONTAP-side assertions. |
| test/integration/plugins/ontap/nfs3/pool/test_pool_with_volumes.py | Adds NFS3 pool lifecycle tests with a live volume present (incl. cancel-maintenance behavior checks). |
| test/integration/plugins/ontap/nfs3/pool/test_zone_scoped_pool.py | Adds zone-scoped NFS3 pool tests including host connectivity/export-policy assertions. |
| test/integration/plugins/ontap/nfs3/volume/init.py | Package initializer for NFS3 volume suites. |
| test/integration/plugins/ontap/nfs3/volume/test_volume_lifecycle.py | Adds NFS3 volume lifecycle tests (CS metadata semantics) and negative delete coverage. |
| test/integration/plugins/ontap/nfs3/instance/init.py | Package initializer for NFS3 instance suites. |
| test/integration/plugins/ontap/nfs3/instance/test_vm_volume_attach.py | Adds NFS3 VM + volume attach/detach lifecycle tests. |
| test/integration/plugins/ontap/iscsi/init.py | Package initializer for iSCSI tests. |
| test/integration/plugins/ontap/iscsi/pool/init.py | Package initializer for iSCSI pool suites. |
| test/integration/plugins/ontap/iscsi/pool/test_pool_lifecycle.py | Adds iSCSI pool lifecycle workflow tests and ONTAP-side assertions (igroups, LUNs). |
| test/integration/plugins/ontap/iscsi/pool/test_pool_with_volumes.py | Adds iSCSI pool lifecycle tests with a live volume (LUN) present. |
| test/integration/plugins/ontap/iscsi/pool/test_zone_scoped_pool.py | Adds zone-scoped iSCSI pool tests including igroup assertions. |
| test/integration/plugins/ontap/iscsi/volume/init.py | Package initializer for iSCSI volume suites. |
| test/integration/plugins/ontap/iscsi/volume/test_volume_lifecycle.py | Adds iSCSI volume lifecycle tests (LUN per CS volume) and negative delete coverage. |
| test/integration/plugins/ontap/iscsi/instance/init.py | Package initializer for iSCSI instance suites. |
| test/integration/plugins/ontap/iscsi/instance/test_vm_volume_attach.py | Adds iSCSI VM + volume attach/detach lifecycle tests with LUN-map assertions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
a2cd65e to
930ca9e
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 26 changed files in this pull request and generated 8 comments.
Comments suppressed due to low confidence (4)
test/integration/plugins/ontap/ontap_test_base.py:39
tearDownClassuses enableStorageMaintenance but it is not imported, so teardown will raise NameError and skip pool cleanup.
from marvin.cloudstackAPI import (
cancelStorageMaintenance,
createVolume as createVolumeAPI,
deleteStoragePool as deleteStoragePoolAPI,
deleteVolume as deleteVolumeAPI,
test/integration/plugins/ontap/ontap.cfg:29
- This config file includes what appears to be a real KVM host password. Please replace with placeholders/sample values so secrets are not committed.
"url": "http://10.193.56.65",
"username": "root",
"password": "netapp1!"
test/integration/plugins/ontap/ontap.cfg:54
- The management server credentials in this committed config should be placeholders/sample values (not real or default admin credentials).
"mgtSvrIp": "10.193.56.62",
"port": 8096,
"user": "admin",
"passwd": "password",
"hypervisor": "kvm"
test/integration/plugins/ontap/ontap.cfg:61
- The ONTAP credentials in this committed config should be placeholders/sample values so secrets are not committed.
"storageIP": "10.196.38.187",
"svmName": "vs0",
"username": "admin",
"password": "netapp1!"
930ca9e to
c0810cf
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 26 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (6)
test/integration/plugins/ontap/ontap_test_base.py:39
enableStorageMaintenanceis used intearDownClass()but is not imported, which will raise aNameErrorduring cleanup.
from marvin.cloudstackAPI import (
cancelStorageMaintenance,
createVolume as createVolumeAPI,
deleteStoragePool as deleteStoragePoolAPI,
deleteVolume as deleteVolumeAPI,
test/integration/plugins/ontap/ontap.cfg:30
- This config file contains what appear to be real KVM host SSH credentials. These should not be committed; use placeholder values (or a separate local-only file) instead.
"url": "http://10.193.56.65",
"username": "root",
"password": "netapp1!"
}
test/integration/plugins/ontap/ontap.cfg:54
- This config file contains what appear to be real CloudStack management server credentials. These should not be committed; use placeholder values (or a separate local-only file) instead.
"mgtSvrIp": "10.193.56.62",
"port": 8096,
"user": "admin",
"passwd": "password",
"hypervisor": "kvm"
test/integration/plugins/ontap/ontap.cfg:62
- This config file contains what appear to be real ONTAP credentials. These should not be committed; use placeholder values (or a separate local-only file) instead.
"storageIP": "10.196.38.187",
"svmName": "vs0",
"username": "admin",
"password": "netapp1!"
},
test/integration/plugins/ontap/OVERVIEW.html:1068
- This HTML doc hardcodes an environment-specific ONTAP IP address. To avoid leaking internal details and to keep the doc reusable, replace it with a placeholder value.
<div class="arch-box-sub">ONTAP REST API<br>SVM: vs0<br><em>10.196.38.187</em></div>
test/integration/plugins/ontap/OVERVIEW.html:1402
- This example hardcodes an ONTAP IP address in the code snippet. Use a placeholder (and HTML-escape it) so the documentation stays generic.
cmd.url = <span class="st">"nfs://10.196.38.187/ontap"</span>
c0810cf to
95909da
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated no new comments.
Suppressed comments (10)
test/integration/plugins/ontap/ontap_test_base.py:42
enableStorageMaintenanceis used later intearDownClassbut is not imported, which will raiseNameErrorduring cleanup when re-entering Maintenance.
from marvin.cloudstackAPI import (
cancelStorageMaintenance,
createVolume as createVolumeAPI,
deleteStoragePool as deleteStoragePoolAPI,
deleteVolume as deleteVolumeAPI,
listDiskOfferings as listDiskOfferingsAPI,
updateStoragePool as updateStoragePoolAPI,
)
test/integration/plugins/ontap/ontap.cfg:30
- This config file includes what appear to be real host credentials/IPs. Repo configs should not contain real secrets; keep only placeholders/sample values and require users to provide local overrides.
"url": "http://10.193.56.65",
"username": "root",
"password": "netapp1!"
}
test/integration/plugins/ontap/ontap.cfg:62
- The ONTAP endpoint and admin credentials here look like real values. Please replace with placeholders (and ideally provide an
ontap.cfg.example+ addontap.cfgto.gitignoreso real credentials stay local).
"ontap": {
"storageIP": "10.196.38.187",
"svmName": "vs0",
"username": "admin",
"password": "netapp1!"
},
test/integration/plugins/ontap/nfs3/pool/test_zone_scoped_pool.py:42
- The docstring run command points to
test/integration/plugins/ontap/test_ontap_zone_scoped_pool.py, but that file doesn't exist (the suite lives undernfs3/pool/). This makes the instructions incorrect for users.
Running:
nosetests --with-marvin \\
--marvin-config=test/integration/plugins/ontap/ontap.cfg \\
test/integration/plugins/ontap/test_ontap_zone_scoped_pool.py -v
test/integration/plugins/ontap/nfs3/pool/test_pool_with_volumes.py:42
- The docstring run command references
test/integration/plugins/ontap/test_ontap_nfs3_pool_with_volumes.py, but the actual test file isnfs3/pool/test_pool_with_volumes.py. Users following the docstring will fail to run the suite.
Running:
nosetests --with-marvin \\
--marvin-config=test/integration/plugins/ontap/ontap.cfg \\
test/integration/plugins/ontap/test_ontap_nfs3_pool_with_volumes.py -v
test/integration/plugins/ontap/iscsi/pool/test_pool_with_volumes.py:60
- The docstring run command points to
test/integration/plugins/ontap/test_ontap_iscsi_pool_with_volumes.py, but the suite is located atiscsi/pool/test_pool_with_volumes.py.
Running:
nosetests --with-marvin \\
--marvin-config=test/integration/plugins/ontap/ontap.cfg \\
test/integration/plugins/ontap/test_ontap_iscsi_pool_with_volumes.py -v
"""
test/integration/plugins/ontap/nfs3/instance/test_vm_volume_attach.py:44
- The docstring run command references
test/integration/plugins/ontap/test_ontap_vm_volume_attach.py, but the suite file is undernfs3/instance/.
Running:
nosetests --with-marvin \\
--marvin-config=test/integration/plugins/ontap/ontap.cfg \\
test/integration/plugins/ontap/test_ontap_vm_volume_attach.py -v
test/integration/plugins/ontap/iscsi/instance/test_vm_volume_attach.py:60
- The docstring run command points to
test/integration/plugins/ontap/test_ontap_vm_volume_attach_iscsi.py, which doesn't exist here; the suite file isiscsi/instance/test_vm_volume_attach.py.
Running:
nosetests --with-marvin \\
--marvin-config=test/integration/plugins/ontap/ontap.cfg \\
test/integration/plugins/ontap/test_ontap_vm_volume_attach_iscsi.py -v
test/integration/plugins/ontap/ontap.cfg:55
- This section hard-codes a management server IP and credentials. Even if these are defaults, committing concrete values encourages secret leakage; use placeholders and document that users must supply real values locally.
{
"mgtSvrIp": "10.193.56.62",
"port": 8096,
"user": "admin",
"passwd": "password",
"hypervisor": "kvm"
}
test/integration/plugins/ontap/run_tests.sh:55
- If the log-folder marker isn't found,
log_folderbecomes empty andos.path.realpath('')resolves to the current working directory, which can make the script look for./results.txtand report misleading pass/fail totals.
local log_folder
log_folder=$(echo "$out" | grep "Final results are now copied to" | sed 's/.*copied to: //; s/ ===.*//' | tr -d '[:space:]')
log_folder=$(python3 -c "import os; print(os.path.realpath('$log_folder'))" 2>/dev/null || echo "")
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated no new comments.
Suppressed comments (8)
test/integration/plugins/ontap/ontap_test_base.py:42
tearDownClasscallsenableStorageMaintenance.enableStorageMaintenanceCmd(), butenableStorageMaintenanceis not imported in this module. This will raise aNameErrorduring cleanup and can mask the original test failure (or leave resources behind).
from marvin.cloudstackAPI import (
cancelStorageMaintenance,
createVolume as createVolumeAPI,
deleteStoragePool as deleteStoragePoolAPI,
deleteVolume as deleteVolumeAPI,
listDiskOfferings as listDiskOfferingsAPI,
updateStoragePool as updateStoragePoolAPI,
)
test/integration/plugins/ontap/ontap.cfg:30
- This config file appears to include real environment credentials (KVM host password). Test configs committed to the repo should not contain real passwords/IPs; use placeholder values so the sample can be safely shared.
"url": "http://10.193.56.65",
"username": "root",
"password": "netapp1!"
}
test/integration/plugins/ontap/ontap.cfg:55
- This config file includes environment-specific management server credentials (
mgtSvr.user/mgtSvr.passwd). These should be placeholders (or omitted) to avoid leaking real access details and to keep the sample reusable.
"mgtSvrIp": "10.193.56.62",
"port": 8096,
"user": "admin",
"passwd": "password",
"hypervisor": "kvm"
}
test/integration/plugins/ontap/ontap.cfg:62
- This config file includes ONTAP admin credentials and a concrete storage IP. Even though this is test-only, committing real credentials is a security risk; replace with placeholders so downstream users can fill in their own values.
"storageIP": "10.196.38.187",
"svmName": "vs0",
"username": "admin",
"password": "netapp1!"
},
test/integration/plugins/ontap/ontap.cfg:44
- The sample config hardcodes a specific DB host/user. For an in-repo example file, prefer placeholders so the file is reusable and doesn't reveal environment details.
"dbSvr": {
"dbSvr": "10.193.56.62",
"passwd": "",
"db": "cloud",
"port": 3306,
"user": "root"
},
test/integration/plugins/ontap/nfs3/pool/test_pool_with_volumes.py:42
- The module docstring’s example
nosetestscommand referencestest/integration/plugins/ontap/test_ontap_nfs3_pool_with_volumes.py, but the actual test file is undernfs3/pool/test_pool_with_volumes.py. As written, the command is not runnable.
Running:
nosetests --with-marvin \\
--marvin-config=test/integration/plugins/ontap/ontap.cfg \\
test/integration/plugins/ontap/test_ontap_nfs3_pool_with_volumes.py -v
test/integration/plugins/ontap/iscsi/instance/test_vm_volume_attach.py:61
- The module docstring’s example
nosetestscommand referencestest/integration/plugins/ontap/test_ontap_vm_volume_attach_iscsi.py, but that file does not exist here; the test lives atiscsi/instance/test_vm_volume_attach.py. Update the example so users can run the suite as documented.
Running:
nosetests --with-marvin \\
--marvin-config=test/integration/plugins/ontap/ontap.cfg \\
test/integration/plugins/ontap/test_ontap_vm_volume_attach_iscsi.py -v
"""
test/integration/plugins/ontap/TEST_CASES.md:157
- Suite 7’s tag is listed as
iscsi_workflow, but the actual test suite and runner useiscsi_with_volumes(seerun_tests.shand the@attr(tags=["iscsi_with_volumes"])decorators). This mismatch makes the documentation misleading and can cause users to run the wrong suite by tag.
**File:** `iscsi/pool/test_pool_with_volumes.py`
**Class:** `TestOntapISCSIPoolWithVolumes`
**Tag:** `iscsi_workflow`
**Total:** 7 tests | **Scope:** cluster-scoped iSCSI pool with a live CloudStack volume (LUN) throughout
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated no new comments.
Suppressed comments (6)
test/integration/plugins/ontap/ontap_test_base.py:42
tearDownClasscallsenableStorageMaintenance.enableStorageMaintenanceCmd()butenableStorageMaintenanceis never imported in this module, which will raise aNameErrorduring cleanup and can leave test resources orphaned.
from marvin.cloudstackAPI import (
cancelStorageMaintenance,
createVolume as createVolumeAPI,
deleteStoragePool as deleteStoragePoolAPI,
deleteVolume as deleteVolumeAPI,
listDiskOfferings as listDiskOfferingsAPI,
updateStoragePool as updateStoragePoolAPI,
)
test/integration/plugins/ontap/ontap.cfg:29
- This config file appears to include a real KVM host IP and root password. Committing real credentials (even for test environments) is a security risk; replace these with placeholders (or move this to a
.samplefile) before merging.
"url": "http://10.193.56.65",
"username": "root",
"password": "netapp1!"
test/integration/plugins/ontap/ontap.cfg:54
- This config hardcodes a specific management server IP/credentials. Please replace with placeholders so environment-specific details and credentials are not committed to the repository.
"mgtSvrIp": "10.193.56.62",
"port": 8096,
"user": "admin",
"passwd": "password",
"hypervisor": "kvm"
test/integration/plugins/ontap/ontap.cfg:62
- This config hardcodes ONTAP management IP and credentials. Please replace with placeholders (and do not commit real passwords) before merging.
"storageIP": "10.196.38.187",
"svmName": "vs0",
"username": "admin",
"password": "netapp1!"
},
test/integration/plugins/ontap/README.md:251
- The README documents
list_files_in_volume(svm_name, vol_name), butOntapRestClient.list_files_in_volumeis defined aslist_files_in_volume(self, vol_name, path="/")(nosvm_nameparameter). Update the method signature in the table so readers call it correctly.
| `list_luns_in_volume(svm_name, vol_name)` | LUNs present in a FlexVol | iSCSI volume/instance suites |
| `list_lun_maps_for_volume(svm_name, vol_name)` | Active LUN-maps for a volume | iSCSI instance suite |
| `list_files_in_volume(svm_name, vol_name)` | Files inside a FlexVol | NFS3 instance suite |
test/integration/plugins/ontap/nfs3/pool/test_pool_with_volumes.py:507
- This suite docstring says the “KVM/NFS3 fix” is confirmed, but this PR’s changes (as shown) are entirely in the integration test tree and config/docs. Consider rewording this to indicate it’s a regression check (or ensure the actual production fix is included in the PR) so reviewers don’t assume the fix is implemented here.
Cancel maintenance mode while a CloudStack data volume exists on the pool:
- cancelStorageMaintenance succeeds (KVM/NFS3 fix confirmed)
- Pool returns to Up state
Description
This PR has the following:
Automation tests for NFS3 and iSCSI protocols
Run all 10 suites
Run a single suite by tag
Fix for Cancel maintenance not functioning properly in case of NFS3
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?