From f7fafe3c1946f1721720f941e8891b791b3c349a Mon Sep 17 00:00:00 2001 From: sandeeplocharla Date: Wed, 11 Nov 2026 01:06:11 +0530 Subject: [PATCH 1/2] CSTACKEX-380: add KVM host checks and fix igroup assertions in ONTAP iSCSI tests Verify KVM host, libvirt domain/XML and iSCSI device state in the Marvin tests, and make the igroup checks real instead of silently skipped. - Use cs__ igroup names, with host IQNs read over SSH (listHosts returns no storageurl for KVM, so the old checks never ran). - Host igroups are created at first volume access and removed by ONTAP (delete_on_unmap) when the last LUN is unmapped: assert pool operations leave them unchanged, and that an igroup exists exactly while a LUN is mapped to it. - test_19: check the volume's own LUN is gone instead of expecting an empty FlexVol (template cache LUN and router ROOT share it). - Pool lifecycle VM deploy uses a compute offering tagged with the ONTAP pool tag so the ROOT disk cannot land on another shared pool; fail early when another enabled pool with the same tag exists. --- test/integration/plugins/ontap/README.md | 7 + .../plugins/ontap/helpers/kvm_host.py | 172 ++++++++ .../template_cache_negative_workflow.py | 26 ++ .../iscsi/instance/test_vm_volume_attach.py | 111 ++++- .../ontap/iscsi/pool/test_pool_lifecycle.py | 231 +++++----- .../iscsi/volume/test_volume_lifecycle.py | 21 +- .../plugins/ontap/ontap_test_base.py | 393 +++++++++++++++++- 7 files changed, 797 insertions(+), 164 deletions(-) diff --git a/test/integration/plugins/ontap/README.md b/test/integration/plugins/ontap/README.md index 6a40a1402c2e..a8b8ab0806c7 100644 --- a/test/integration/plugins/ontap/README.md +++ b/test/integration/plugins/ontap/README.md @@ -44,6 +44,9 @@ test/integration/plugins/ontap/ ├── ontap.cfg # Environment config (IPs, credentials, zone info) ├── ontap_test_base.py # Shared base class and ONTAP REST client ├── guest_ssh.py # Guest SSH (paramiko) for the VM workflow suites +├── helpers/ +│ ├── kvm_host.py # KVM host checks: services, libvirt domain XML, by-id devices, iSCSI sessions +│ └── template_cache_*.py # Template-cache setup, assertions and negative workflow ├── setup_env.sh # venv: Marvin plus requirements.txt ├── requirements.txt # setuptools, pyvmomi, paramiko pins ├── TEST_CASES.md # Full test case reference table (151 tests) @@ -356,6 +359,10 @@ another ONTAP pool is present. | `_create_volume(pool_id)` | Creates a CloudStack data volume on the given pool | | `_delete_pool(pool_id, forced)` | Enters Maintenance then calls `deleteStoragePool` | | `_parse_pool_details(pool)` | Extracts key→value pairs from the pool's `details` list | +| `_assert_kvm_hosts_ready()` | Called from `_setup_cloudstack_resources()`: libvirtd, cloudstack-agent (and iscsid for iSCSI) active, `virsh` responsive, CloudStack host `Up`/`Enabled`; fails the class early otherwise | +| `_vm_placement(vm_id)` | Live host IP, libvirt instance name and VM object (deploy-time values go stale after stop/start) | +| `_assert_vm_disks_on_host(vm_id, pool, label)` | iSCSI: libvirt domain `running`; XML identity/sizing; one `block`/`raw`/`virtio` disk per CloudStack volume sourced from `/dev/disk/by-id/wwn-0x`; volume path is `//` and the WWID matches the ONTAP LUN serial; LUN mapped to the host's igroup; block device present; iSCSI session to an SVM data LIF | +| `_assert_disk_detached_on_host(...)`, `_assert_devices_gone(...)`, `_assert_domain_absent(...)` | After detach / stop / destroy: the disk is out of the domain XML, the by-id device and the libvirt domain are gone | | `OntapRestClient` | Thin HTTPS client for ONTAP REST API calls | ### `OntapRestClient` methods at a glance diff --git a/test/integration/plugins/ontap/helpers/kvm_host.py b/test/integration/plugins/ontap/helpers/kvm_host.py index de3b90dcc11e..d13a2a89b103 100644 --- a/test/integration/plugins/ontap/helpers/kvm_host.py +++ b/test/integration/plugins/ontap/helpers/kvm_host.py @@ -23,12 +23,22 @@ import json import logging +import re import shlex +import time +import xml.etree.ElementTree as ET from marvin.sshClient import SshClient logger = logging.getLogger("kvm_host") +# '6' (NAA IEEE Registered Extended) + NetApp OUI '00a098' + pad nibble '0'. +# Mirrors OntapStorageConstants.NETAPP_NAA_PREFIX in the ONTAP plugin. It is +# deliberately re-implemented here (not read from the plugin) so a mistake in +# the plugin's serial -> WWID mapping is caught by the tests. +NETAPP_NAA_PREFIX = "600a0980" +WWID_RE = re.compile(r"^%s[0-9a-f]{24}$" % NETAPP_NAA_PREFIX) + def cleanup_kvm_storage_pool_mounts(kvm_hosts_ssh_creds, pool_uuid): """Unmount and undefine the libvirt pool for ``pool_uuid`` on each host. @@ -135,3 +145,165 @@ def get_vm_data_disk_capacity(kvm_hosts_ssh_creds, instance_name): "Could not read VM data-disk capacity from any KVM host: %s" % "; ".join(errors) ) + + +# --------------------------------------------------------------------------- +# Host-side verification helpers (services, libvirt domain, iSCSI devices) +# +# Marvin's SshClient.execute() ignores exit codes and returns stderr only when +# stdout is empty, so every command below merges stderr (2>&1) or prints an +# explicit sentinel and the callers look for that text. +# --------------------------------------------------------------------------- + +def naa_from_serial(serial): + """Return the SCSI WWID the plugin derives from an ONTAP LUN serial number. + + The WWID is NETAPP_NAA_PREFIX followed by the hex of the 12 ASCII serial + bytes, e.g. serial ``x0M-8?/kEsJT`` -> + ``600a098078304d2d383f2f6b45734a54``. + """ + if not serial: + raise ValueError("ONTAP reported no LUN serial number") + return NETAPP_NAA_PREFIX + "".join( + "%02x" % b for b in serial.encode("ascii")) + + +def by_id_paths(wwid): + """The two /dev/disk/by-id aliases udev creates for a NetApp LUN WWID.""" + return ["/dev/disk/by-id/wwn-0x%s" % wwid, + "/dev/disk/by-id/scsi-3%s" % wwid] + + +def _creds_for(kvm_hosts_ssh_creds, host_ip): + for creds in kvm_hosts_ssh_creds: + if creds.get("host") == host_ip: + return creds + raise RuntimeError("No SSH credentials for KVM host %s" % host_ip) + + +def run_on_host(kvm_hosts_ssh_creds, host_ip, command, timeout=20.0): + """Run one command on a KVM host and return its output lines.""" + creds = _creds_for(kvm_hosts_ssh_creds, host_ip) + ssh = SshClient( + host_ip, 22, creds["user"], creds["password"], + retries=3, delay=3, timeout=timeout, + ) + return ssh.execute(command) or [] + + +def service_state(kvm_hosts_ssh_creds, host_ip, unit): + """``systemctl is-active`` for a unit ('active', 'inactive', 'failed').""" + out = run_on_host( + kvm_hosts_ssh_creds, host_ip, + "systemctl is-active %s 2>&1; true" % shlex.quote(unit)) + return out[0].strip() if out else "unknown" + + +def virsh_responsive(kvm_hosts_ssh_creds, host_ip): + """True when libvirt answers a plain ``virsh list`` within 15 seconds.""" + out = run_on_host( + kvm_hosts_ssh_creds, host_ip, + "timeout 15 virsh list >/dev/null 2>&1 " + "&& echo VIRSH_OK || echo VIRSH_FAIL") + return "VIRSH_OK" in "".join(out) + + +def domain_state(kvm_hosts_ssh_creds, host_ip, instance_name): + """libvirt state of a domain: 'running', 'shut off', ... or 'absent'. + + CloudStack defines its guests as transient domains, so a stopped or + destroyed VM is 'absent' rather than 'shut off'. + """ + out = run_on_host( + kvm_hosts_ssh_creds, host_ip, + "timeout 15 virsh domstate %s 2>&1; true" + % shlex.quote(str(instance_name))) + text = " ".join(line.strip() for line in out).strip() + lowered = text.lower() + if "failed to get domain" in lowered or "domain not found" in lowered: + return "absent" + return text + + +def domain_xml(kvm_hosts_ssh_creds, host_ip, instance_name): + """Return the parsed ``virsh dumpxml`` root element, or None if absent.""" + out = run_on_host( + kvm_hosts_ssh_creds, host_ip, + "timeout 15 virsh dumpxml %s 2>&1; true" + % shlex.quote(str(instance_name))) + text = "\n".join(out) + start = text.find("`` of a libvirt domain XML (any device type).""" + disks = [] + for disk in domain_root.findall("./devices/disk"): + driver = disk.find("driver") + source = disk.find("source") + target = disk.find("target") + disks.append({ + "device": disk.get("device"), + "type": disk.get("type"), + "driver_name": driver.get("name") if driver is not None else None, + "driver_type": driver.get("type") if driver is not None else None, + "cache": driver.get("cache") if driver is not None else None, + "source_dev": source.get("dev") if source is not None else None, + "source_file": source.get("file") if source is not None else None, + "target_dev": target.get("dev") if target is not None else None, + "target_bus": target.get("bus") if target is not None else None, + "serial": disk.findtext("serial"), + }) + return disks + + +def device_present(kvm_hosts_ssh_creds, host_ip, wwid): + """True when a block device for the WWID exists under /dev/disk/by-id.""" + wwn, scsi = by_id_paths(wwid) + out = run_on_host( + kvm_hosts_ssh_creds, host_ip, + "test -b %s -o -b %s && echo DEV_YES || echo DEV_NO" + % (shlex.quote(wwn), shlex.quote(scsi))) + return "DEV_YES" in "".join(out) + + +def wait_device(kvm_hosts_ssh_creds, host_ip, wwid, present, + timeout=90, interval=3): + """Poll until the WWID's device appears/disappears; return the final state. + + udev and the plugin's SCSI rescan/removal are asynchronous, so a single + immediate check would be flaky. + """ + deadline = time.time() + timeout + state = device_present(kvm_hosts_ssh_creds, host_ip, wwid) + while state != present and time.time() < deadline: + time.sleep(interval) + state = device_present(kvm_hosts_ssh_creds, host_ip, wwid) + return state + + +_SESSION_RE = re.compile( + r"^\w+:\s*\[(\d+)\]\s+([^,\s]+):(\d+),(\d+)\s+(\S+)") + + +def iscsi_sessions(kvm_hosts_ssh_creds, host_ip): + """Parse ``iscsiadm -m session`` into dicts (sid, portal_ip, port, target). + + Returns [] when there are no sessions. + """ + out = run_on_host( + kvm_hosts_ssh_creds, host_ip, "iscsiadm -m session 2>&1; true") + sessions = [] + for line in out: + match = _SESSION_RE.match(line.strip()) + if match: + sessions.append({ + "sid": match.group(1), + "portal_ip": match.group(2), + "port": match.group(3), + "target": match.group(5), + }) + return sessions diff --git a/test/integration/plugins/ontap/helpers/template_cache_negative_workflow.py b/test/integration/plugins/ontap/helpers/template_cache_negative_workflow.py index e54c32541d99..f81d442795b5 100644 --- a/test/integration/plugins/ontap/helpers/template_cache_negative_workflow.py +++ b/test/integration/plugins/ontap/helpers/template_cache_negative_workflow.py @@ -617,8 +617,34 @@ def step_03_deleted_cache_blocks_reuse(self): ) self._assert_cache_on_ontap(pool, spool) + # KVM host / libvirt: the seed VM runs from a by-id block disk + # (the ROOT LUN, a clone of the cache LUN) with the right XML, + # igroup and session. + seed_host = self._assert_vm_disks_on_host( + vm1.id, pool, "seed VM") + # ONTAP: the cache LUN is only mapped while the template is copied + # into it; once the VM runs it must not stay mapped to any host. + cache_lun = "/vol/%s/cs_tmpl_%s" % ( + pool.name, self.__class__.template_db_id) + stray_maps = [ + m for m in self.ontap.list_lun_maps_for_volume( + self.svm_name, pool.name) + if m.get("lun", {}).get("name") == cache_lun] + self.assertEqual( + stray_maps, [], + "Template cache LUN %s is still mapped after the seed VM " + "started: %s" % (cache_lun, stray_maps)) + self._destroy_vm_static(vm1.id) vm1 = None + # KVM host: the seed VM's domain and ROOT device are released + if seed_host: + self._assert_domain_absent( + seed_host["host_ip"], seed_host["instance"], + "seed VM destroyed") + self._assert_devices_gone( + seed_host["host_ip"], list(seed_host["wwids"].values()), + "seed VM destroyed") # Allow clone/ROOT LUN cleanup so the cache LUN is no longer a # FlexClone parent before out-of-band delete. time.sleep(30) diff --git a/test/integration/plugins/ontap/iscsi/instance/test_vm_volume_attach.py b/test/integration/plugins/ontap/iscsi/instance/test_vm_volume_attach.py index ab46b96e6a3b..eaf6d915e25d 100644 --- a/test/integration/plugins/ontap/iscsi/instance/test_vm_volume_attach.py +++ b/test/integration/plugins/ontap/iscsi/instance/test_vm_volume_attach.py @@ -104,7 +104,6 @@ import base64 import logging import random -import re import threading import time import unittest @@ -792,11 +791,17 @@ def _vm_state(self, vm_id): def _stop_vm(self, vm_id): if (self._vm_state(vm_id) or "").lower() == "stopped": return + # Remember where the VM runs so the host can be checked after the stop. + placement = self._vm_placement(vm_id) cmd = stopVirtualMachineAPI.stopVirtualMachineCmd() cmd.id = vm_id cmd.forced = True self.apiClient.stopVirtualMachine(cmd) self._poll_vm_state(vm_id, "Stopped", timeout=600) + # Host: the libvirt domain is gone and libvirtd/agent are still up. + if placement["host_ip"] and placement["instance"]: + self._assert_domain_absent( + placement["host_ip"], placement["instance"], "after stop") def _start_vm(self, vm_id): if (self._vm_state(vm_id) or "").lower() == "running": @@ -805,6 +810,9 @@ def _start_vm(self, vm_id): cmd.id = vm_id self.apiClient.startVirtualMachine(cmd) self._poll_vm_state(vm_id, "Running", timeout=900) + # Host: domain running, XML and by-id devices match every volume. + self._assert_vm_disks_on_host( + vm_id, self.__class__.pool, "after start") def _root_volume(self, vm_id): cmd = listVolumesAPI.listVolumesCmd() @@ -1057,6 +1065,10 @@ def test_03_deploy_vm(self): spool = self._wait_for_ready_spool_ref(self.__class__.pool_db_id) self._assert_cache_on_ontap(pool, spool) + # KVM host: libvirt domain running, XML (block/raw/virtio, by-id + # source of the ROOT LUN), block device, iSCSI session, igroup/LUN-map + self._assert_vm_disks_on_host(vm.id, pool, "deploy VM") + # Data volume LUN-map must not exist yet (volume not yet attached) lun_maps = self._lun_maps() self.assertEqual( @@ -1107,6 +1119,11 @@ def test_03a_deploy_second_vm_reuses_template_cache(self): "Expected one additional non-cache LUN for the VM-2 ROOT clone", ) + # KVM host: VM-2 has its own domain/ROOT device, and VM-1 is untouched + self._assert_vm_disks_on_host(vm2.id, pool, "deploy VM-2") + self._assert_vm_disks_on_host( + self.__class__.vm.id, pool, "VM-1 unaffected by VM-2 deploy") + # ------------------------------------------------------------------ # Step 03b — Expunge the second VM — template cache LUN survives # ------------------------------------------------------------------ @@ -1126,6 +1143,11 @@ def test_03b_expunge_second_vm_template_cache_survives(self): "Second VM absent — test_03a must pass first") pool = self.__class__.pool + # Remember where VM-2 and its LUNs live so the host can be checked + # once the VM is gone. + vm2_host = self._assert_vm_disks_on_host( + self.__class__.vm2.id, pool, "VM-2 before expunge") + self._stop_and_expunge_vm(self.__class__.vm2.id) self.__class__.vm2 = None @@ -1134,6 +1156,16 @@ def test_03b_expunge_second_vm_template_cache_survives(self): spool = self._assert_single_ready_spool_ref(self.__class__.pool_db_id) self._assert_cache_on_ontap(pool, spool) + # KVM host: VM-2's domain and its ROOT device are gone; VM-1 is fine + if vm2_host: + self._assert_domain_absent( + vm2_host["host_ip"], vm2_host["instance"], "VM-2 expunged") + self._assert_devices_gone( + vm2_host["host_ip"], list(vm2_host["wwids"].values()), + "VM-2 expunged") + self._assert_vm_disks_on_host( + self.__class__.vm.id, pool, "VM-1 unaffected by VM-2 expunge") + # ------------------------------------------------------------------ # Step 04 — Attach iSCSI volume to running VM (TDS SN 27 iSCSI) # ------------------------------------------------------------------ @@ -1186,6 +1218,19 @@ def test_04_attach_volume_to_vm(self): self._data_lun_size(), initial_size, "Initial ONTAP LUN size should match the CloudStack volume size") + # KVM host / libvirt: the VM now has ROOT + data disk, both block/raw/ + # virtio disks sourced from the LUN by-id paths; the data LUN's WWID + # (from the CloudStack path) matches the ONTAP serial; it is mapped to + # this host's igroup; the device exists; libvirt sees the full size. + host_info = self._assert_vm_disks_on_host( + self.__class__.vm.id, self.__class__.pool, "after attach") + if host_info: + self.assertEqual( + self._get_vm_data_disk_capacity(host_info["instance"]), + initial_size, + "libvirt capacity of the attached data disk should equal the " + "CloudStack volume size") + # ------------------------------------------------------------------ # Step 05 — Stop VM — LUN-maps should be removed @@ -1204,6 +1249,10 @@ def test_05_stop_vm_lun_unmapped(self): if self.__class__.vm is None: self.skipTest("VM not deployed — test_03 was skipped") + # KVM host: record where the VM runs and which LUN devices it holds + host_info = self._assert_vm_disks_on_host( + self.__class__.vm.id, self.__class__.pool, "before stop") + cmd = stopVirtualMachineAPI.stopVirtualMachineCmd() cmd.id = self.__class__.vm.id self.apiClient.stopVirtualMachine(cmd) @@ -1215,6 +1264,15 @@ def test_05_stop_vm_lun_unmapped(self): "VM should be 'Stopped', got '%s'" % result.state ) + # KVM host: the libvirt domain is gone and the LUN block devices are + # released (the agent disconnects them when the VM stops). + if host_info: + self._assert_domain_absent( + host_info["host_ip"], host_info["instance"], "after VM stop") + self._assert_devices_gone( + host_info["host_ip"], list(host_info["wwids"].values()), + "after VM stop") + # ONTAP: LUN-map must be removed once VM is stopped lun_maps = self._lun_maps() self.assertEqual( @@ -1354,6 +1412,12 @@ def test_08_start_vm_lun_remapped(self): "Expected ≥1 LUN-map after VM start (re-map), found 0" ) + # KVM host / libvirt: the domain runs again with both disks, the same + # by-id sources (WWID is stable across stop/start), devices are back, + # and the host has its iSCSI session to the SVM. + self._assert_vm_disks_on_host( + self.__class__.vm.id, self.__class__.pool, "after VM start") + # ------------------------------------------------------------------ # Step 09 — Guest SSH access for snapshot checks # ------------------------------------------------------------------ @@ -1782,6 +1846,10 @@ def test_24_detach_volume_from_vm(self): self.assertIsNotNone(self.__class__.volume, "Volume absent — test_02 must pass first") + # KVM host: record the data LUN's WWID while it is still attached + host_info = self._assert_vm_disks_on_host( + self.__class__.vm.id, self.__class__.pool, "before detach") + cmd = detachVolumeAPI.detachVolumeCmd() cmd.id = self.__class__.volume.id @@ -1827,6 +1895,18 @@ def test_24_detach_volume_from_vm(self): "Data volume LUN should still exist in ONTAP FlexVol after detach" ) + # KVM host / libvirt: the data disk is no longer in the running + # domain's XML and its block device is gone, while the VM keeps + # running on its ROOT disk (still a by-id block disk with a session). + if host_info: + self._assert_disk_detached_on_host( + host_info["host_ip"], host_info["instance"], + host_info["wwids"][self.__class__.volume.id], + "after detach") + self._assert_vm_disks_on_host( + self.__class__.vm.id, self.__class__.pool, + "after detach (ROOT only)") + # ------------------------------------------------------------------ # ------------------------------------------------------------------ @@ -1913,6 +1993,12 @@ def test_27_destroy_vm_and_cleanup(self): vms = self.apiClient.listVirtualMachines( _list_vms_cmd(self.__class__.vm.id)) current_state = vms[0].state if vms else "unknown" + # KVM host: record the domain and its LUN devices while the VM is + # still running, to prove they are released by the destroy. + host_info = None + if current_state.lower() == "running" and pool is not None: + host_info = self._assert_vm_disks_on_host( + self.__class__.vm.id, pool, "before destroy") if current_state.lower() not in ("stopped", "destroyed"): stop_cmd = stopVirtualMachineAPI.stopVirtualMachineCmd() stop_cmd.id = self.__class__.vm.id @@ -1932,6 +2018,16 @@ def test_27_destroy_vm_and_cleanup(self): self.__class__.pool_db_id) self._assert_cache_on_ontap(pool, spool) + # KVM host: the domain is gone, its LUN devices are released and + # libvirtd/agent are still healthy at the end of the suite. + if host_info: + self._assert_domain_absent( + host_info["host_ip"], host_info["instance"], + "after VM destroy") + self._assert_devices_gone( + host_info["host_ip"], list(host_info["wwids"].values()), + "after VM destroy") + if vol is not None and pool is not None: pool_name = pool.name @@ -1977,16 +2073,11 @@ def test_27_destroy_vm_and_cleanup(self): % pool_name ) - # ONTAP: per-host igroups must be deleted by the pool force-delete + # ONTAP: per-host igroups must be deleted by the pool force-delete. + # The driver names them cs__ (OntapStorageUtils + # .getIgroupName); build the same name so this check can fail. for host in self.cluster_hosts: - iqn = getattr(host, "storageurl", None) - if not iqn or not iqn.startswith("iqn."): - continue - short = host.name.split(".")[0] - igroup_name = "cs_%s_%s" % ( - self.svm_name, - re.sub(r"[^a-zA-Z0-9_-]", "_", short), - ) + igroup_name = self._igroup_name(host.id) igroup = self.ontap.get_igroup(self.svm_name, igroup_name) self.assertIsNone( igroup, diff --git a/test/integration/plugins/ontap/iscsi/pool/test_pool_lifecycle.py b/test/integration/plugins/ontap/iscsi/pool/test_pool_lifecycle.py index 85663d6370d0..aec8c36071f0 100644 --- a/test/integration/plugins/ontap/iscsi/pool/test_pool_lifecycle.py +++ b/test/integration/plugins/ontap/iscsi/pool/test_pool_lifecycle.py @@ -68,7 +68,7 @@ import base64 import logging import random -import re +import time import unittest from nose.plugins.attrib import attr @@ -160,17 +160,6 @@ def __init__(self, storage_ip, svm_name, username, password, } -# --------------------------------------------------------------------------- -# iSCSI path helpers -# --------------------------------------------------------------------------- - -def _igroup_name(svm_name, host_name): - """Mirror OntapStorageUtils.getIgroupName: cs_{svmName}_{sanitizedHostName}""" - short = host_name.split(".")[0] - sanitized = re.sub(r"[^a-zA-Z0-9_-]", "_", short) - return "cs_%s_%s" % (svm_name, sanitized) - - # --------------------------------------------------------------------------- # Sequential workflow test class # --------------------------------------------------------------------------- @@ -222,7 +211,11 @@ def setUpClass(cls): cls._setup_cloudstack_resources(config, cls.testdata[TestData.account]) - cls._discover_vm_deploy_resources() + cls._discover_vm_deploy_resources(storage_tags=tags) + + # Host igroups are shared across pools and the driver creates them + # lazily (grantAccess), so pool-only steps must leave them untouched. + cls._capture_igroup_baseline() # No per-test tearDown — state intentionally persists between steps. @@ -558,7 +551,7 @@ def test_03_create_primary_storage_pool(self): Create an iSCSI primary storage pool and verify: - CloudStack state is Up, type is OntapiSCSI - ONTAP: FlexVol exists and is online - - ONTAP: one igroup per cluster host exists with the correct IQN initiator + - ONTAP: host igroups are unchanged (they are created at first volume access) """ pool = self._create_pool() self.__class__.pool = pool @@ -583,25 +576,10 @@ def test_03_create_primary_storage_pool(self): "ONTAP FlexVol should be 'online', got '%s'" % ontap_vol.get("state") ) - # ONTAP: igroup must exist for each cluster host that has an IQN - for host in self.cluster_hosts: - iqn = getattr(host, "storageurl", None) or getattr(host, "StorageUrl", None) - if not iqn or not iqn.startswith("iqn."): - continue # host not iSCSI-enabled; skip igroup check for it - igroup_name = _igroup_name(self.svm_name, host.name) - igroup = self.ontap.get_igroup(self.svm_name, igroup_name) - self.assertIsNotNone( - igroup, - "ONTAP igroup '%s' not found for host '%s'" % (igroup_name, host.name) - ) - initiator_names = [ - i.get("name", "") for i in igroup.get("initiators", []) - ] - self.assertIn( - iqn, initiator_names, - "Host IQN '%s' not in igroup '%s' initiators: %s" - % (iqn, igroup_name, initiator_names) - ) + # ONTAP: the driver creates a host's igroup lazily at first volume + # access (grantAccess), not at pool creation, so creating the pool must + # leave the shared host igroups exactly as they were. + self._assert_igroup_baseline_unchanged("after pool creation") # Capacity reporting self._assert_pool_capacity(pool, "pool-created") @@ -702,17 +680,7 @@ def test_05_grow_storage_pool(self): ) self.assertEqual(resized_ontap_vol.get("state"), "online") - for host in self.cluster_hosts: - iqn = ( - getattr(host, "storageurl", None) - or getattr(host, "StorageUrl", None) - ) - if iqn and iqn.startswith("iqn."): - igroup_name = _igroup_name(self.svm_name, host.name) - self.assertIsNotNone( - self.ontap.get_igroup(self.svm_name, igroup_name), - "ONTAP igroup '%s' disappeared after increase" % igroup_name, - ) + self._assert_igroup_baseline_unchanged("after growing the pool") # ------------------------------------------------------------------ @@ -781,18 +749,7 @@ def test_06_reject_grow_above_max_size(self): "Pool should stay 'Up' after a rejected resize, got '%s'" % after[0].state ) - for host in self.cluster_hosts: - iqn = ( - getattr(host, "storageurl", None) - or getattr(host, "StorageUrl", None) - ) - if iqn and iqn.startswith("iqn."): - igroup_name = _igroup_name(self.svm_name, host.name) - self.assertIsNotNone( - self.ontap.get_igroup(self.svm_name, igroup_name), - "ONTAP igroup '%s' disappeared after a rejected " - "over-max grow" % igroup_name, - ) + self._assert_igroup_baseline_unchanged("after a rejected over-max grow") # ------------------------------------------------------------------ @@ -825,18 +782,7 @@ def test_07_shrink_storage_pool(self): ) self.assertEqual(shrunk_ontap_vol.get("state"), "online") - for host in self.cluster_hosts: - iqn = ( - getattr(host, "storageurl", None) - or getattr(host, "StorageUrl", None) - ) - if iqn and iqn.startswith("iqn."): - igroup_name = _igroup_name(self.svm_name, host.name) - self.assertIsNotNone( - self.ontap.get_igroup(self.svm_name, igroup_name), - "ONTAP igroup '%s' disappeared after safe shrink" - % igroup_name, - ) + self._assert_igroup_baseline_unchanged("after shrinking the pool") # ------------------------------------------------------------------ @@ -928,18 +874,7 @@ def test_09_grow_and_shrink_storage_pool_while_disabled(self): ) self.assertEqual(resized_ontap.get("state"), "online") - for host in self.cluster_hosts: - iqn = ( - getattr(host, "storageurl", None) - or getattr(host, "StorageUrl", None) - ) - if iqn and iqn.startswith("iqn."): - igroup_name = _igroup_name(self.svm_name, host.name) - self.assertIsNotNone( - self.ontap.get_igroup(self.svm_name, igroup_name), - "ONTAP igroup '%s' disappeared during disabled resize" - % igroup_name, - ) + self._assert_igroup_baseline_unchanged("during resize of the disabled pool") # ------------------------------------------------------------------ @@ -1059,18 +994,7 @@ def test_12_grow_and_shrink_storage_pool_in_maintenance(self): ) self.assertEqual(resized_ontap.get("state"), "online") - for host in self.cluster_hosts: - iqn = ( - getattr(host, "storageurl", None) - or getattr(host, "StorageUrl", None) - ) - if iqn and iqn.startswith("iqn."): - igroup_name = _igroup_name(self.svm_name, host.name) - self.assertIsNotNone( - self.ontap.get_igroup(self.svm_name, igroup_name), - "ONTAP igroup '%s' disappeared during maintenance resize" - % igroup_name, - ) + self._assert_igroup_baseline_unchanged("during resize in maintenance") # ------------------------------------------------------------------ @@ -1133,17 +1057,9 @@ def test_14_enter_maintenance_and_delete_pool(self): "ONTAP FlexVol '%s' still exists after pool deletion" % pool_name ) - # ONTAP: igroups for each cluster host must be deleted - for host in self.cluster_hosts: - iqn = getattr(host, "storageurl", None) or getattr(host, "StorageUrl", None) - if not iqn or not iqn.startswith("iqn."): - continue - igroup_name = _igroup_name(self.svm_name, host.name) - igroup = self.ontap.get_igroup(self.svm_name, igroup_name) - self.assertIsNone( - igroup, - "ONTAP igroup '%s' still exists after pool deletion" % igroup_name - ) + # ONTAP: host igroups are shared per host and SVM (cs__) + # and live only while a LUN is mapped to them. + self._assert_host_igroups_follow_lun_maps("after pool deletion") # ------------------------------------------------------------------ @@ -1248,6 +1164,23 @@ def test_16_create_vm_and_attach_volume(self): pool = self.__class__.pool vol = self.__class__.volume + # The VM's ROOT disk is placed by CloudStack on any enabled pool that + # matches the offering's storage tag. A pool left behind by an aborted + # run would capture it and fail the placement checks later with an + # unhelpful id mismatch, so name the problem here. + competing = [ + sp.name for sp in (list_storage_pools(self.apiClient) or []) + if sp.id != pool.id and sp.type == pool.type + and getattr(sp, "tags", None) == getattr(pool, "tags", None) + and sp.state == "Up" + ] + self.assertFalse( + competing, + "Other enabled pool(s) with tag '%s' exist: %s. The VM ROOT disk " + "could be placed on them instead of '%s'. Delete the leftovers " + "from an aborted run and re-run." + % (getattr(pool, "tags", None), competing, pool.name)) + log_progress( logger, "info", "Deploying VM for iSCSI pool '%s' (template=%s, offering=%s)", @@ -1320,18 +1253,10 @@ def test_16_create_vm_and_attach_volume(self): "No LUN present in FlexVol '%s' after attach" % pool.name ) - for host in self.cluster_hosts: - iqn = ( - getattr(host, "storageurl", None) - or getattr(host, "StorageUrl", None) - ) - if iqn and iqn.startswith("iqn."): - igroup_name = _igroup_name(self.svm_name, host.name) - self.assertIsNotNone( - self.ontap.get_igroup(self.svm_name, igroup_name), - "ONTAP igroup '%s' disappeared after volume attach" - % igroup_name, - ) + # KVM host / libvirt + ONTAP igroup of the host the VM runs on. The + # plugin creates a host's igroup on its first LUN mapping, so this is + # the point where it must exist (with the host IQN) and hold the LUNs. + self._assert_vm_disks_on_host(vm.id, pool, "VM with attached volume") # ------------------------------------------------------------------ @@ -1488,6 +1413,15 @@ def test_18_grow_and_shrink_pool_with_vm_attached(self): ) self.assertEqual(grown_vol.get("state"), "online") self._assert_vm_running_with_volume(vm.id, vol.id, "grow-with-vm") + # KVM host: the resize of the pool must not disturb the guest - + # same disks, device present, and libvirt still reports the LUN's size. + host_info = self._assert_vm_disks_on_host(vm.id, pool, "grow-with-vm") + if host_info: + self.assertEqual( + self._get_vm_data_disk_capacity(host_info["instance"]), + int(self._get_cs_volume(vol.id).size), + "libvirt capacity of the data disk changed after the pool " + "resize (grow-with-vm)") self.assertTrue( self.ontap.list_luns_in_volume(self.svm_name, pool.name), "LUN disappeared from FlexVol '%s' during the in-use resize" @@ -1515,6 +1449,15 @@ def test_18_grow_and_shrink_pool_with_vm_attached(self): ) self.assertEqual(shrunk_vol.get("state"), "online") self._assert_vm_running_with_volume(vm.id, vol.id, "shrink-with-vm") + # KVM host: the resize of the pool must not disturb the guest - + # same disks, device present, and libvirt still reports the LUN's size. + host_info = self._assert_vm_disks_on_host(vm.id, pool, "shrink-with-vm") + if host_info: + self.assertEqual( + self._get_vm_data_disk_capacity(host_info["instance"]), + int(self._get_cs_volume(vol.id).size), + "libvirt capacity of the data disk changed after the pool " + "resize (shrink-with-vm)") self.assertTrue( self.ontap.list_luns_in_volume(self.svm_name, pool.name), "LUN disappeared from FlexVol '%s' during the in-use resize" @@ -1536,16 +1479,32 @@ def test_19_delete_volume_and_pool(self): - Pool transitions to Maintenance - Pool is removed from CloudStack after force deletion - ONTAP: FlexVol deleted - - ONTAP: igroups for all cluster hosts deleted + - ONTAP: the host igroup is removed once its last LUN is unmapped """ self.assertIsNotNone(self.__class__.pool, "Pool absent - test_15 must pass first") self.assertIsNotNone(self.__class__.volume, "Volume absent - test_15 must pass first") # A volume still attached to a VM cannot be deleted, so unwind test_16 # first. Both steps are no-ops when test_16 skipped. + # KVM host: remember the domain and LUN devices while the VM runs + host_info = None + if self.__class__.vm is not None: + host_info = self._assert_vm_disks_on_host( + self.__class__.vm.id, self.__class__.pool, + "before detach/destroy") + self._detach_volume_if_attached(self.__class__.volume.id) self.__class__._destroy_vm_if_present() + # KVM host: the domain is gone and every LUN device is released + if host_info: + self._assert_domain_absent( + host_info["host_ip"], host_info["instance"], + "after VM destroy") + self._assert_devices_gone( + host_info["host_ip"], list(host_info["wwids"].values()), + "after VM destroy") + pool = self.__class__.pool pool_name = pool.name vol = self.__class__.volume @@ -1556,13 +1515,27 @@ def test_19_delete_volume_and_pool(self): self.apiClient.deleteVolume(cmd) self.__class__.volume = None - # ONTAP: LUN must be gone from the FlexVol - luns = self.ontap.list_luns_in_volume(self.svm_name, pool_name) - self.assertEqual( - len(luns), 0, - "Expected 0 LUNs in FlexVol '%s' after volume deletion, found %d" - % (pool_name, len(luns)) + # ONTAP: this volume's LUN must be gone from the FlexVol. The FlexVol + # also holds LUNs that are not this test's (the template cache LUN and + # the guest network router's ROOT disk), so "no LUNs at all" is not the + # right check; look for this volume's own LUN. + data_lun = "/vol/%s/%s" % (pool_name, vol.name) + deadline = time.time() + 60 + while True: + lun_names = [ + l.get("name", "") + for l in self.ontap.list_luns_in_volume(self.svm_name, pool_name) + ] + if data_lun not in lun_names or time.time() >= deadline: + break + time.sleep(3) + self.assertNotIn( + data_lun, lun_names, + "LUN '%s' still exists in FlexVol '%s' after volume deletion " + "(other LUNs present: %s)" % (data_lun, pool_name, lun_names) ) + logger.info("LUNs left in FlexVol '%s' after volume deletion: %s", + pool_name, lun_names) # ONTAP: FlexVol must still be online (pool not yet deleted) ontap_vol = self.ontap.get_volume(pool_name) @@ -1598,17 +1571,9 @@ def test_19_delete_volume_and_pool(self): "ONTAP FlexVol '%s' still exists after pool deletion" % pool_name ) - # ONTAP: igroups for each cluster host must be deleted - for host in self.cluster_hosts: - iqn = getattr(host, "storageurl", None) or getattr(host, "StorageUrl", None) - if not iqn or not iqn.startswith("iqn."): - continue - igroup_name = _igroup_name(self.svm_name, host.name) - igroup = self.ontap.get_igroup(self.svm_name, igroup_name) - self.assertIsNone( - igroup, - "ONTAP igroup '%s' still exists after pool deletion" % igroup_name - ) + # ONTAP: the host igroup is removed when its last LUN is unmapped, and + # kept while other LUNs (other pools, system VM disks) are still mapped. + self._assert_host_igroups_follow_lun_maps("after volume and pool deletion") # ------------------------------------------------------------------ # Step 20 - delete pool with flexvol predeleted diff --git a/test/integration/plugins/ontap/iscsi/volume/test_volume_lifecycle.py b/test/integration/plugins/ontap/iscsi/volume/test_volume_lifecycle.py index 3056f36c422c..d13503d2a26f 100644 --- a/test/integration/plugins/ontap/iscsi/volume/test_volume_lifecycle.py +++ b/test/integration/plugins/ontap/iscsi/volume/test_volume_lifecycle.py @@ -51,7 +51,6 @@ import base64 import logging import random -import re import unittest from nose.plugins.attrib import attr @@ -130,17 +129,6 @@ def __init__(self, storage_ip, svm_name, username, password, } -# --------------------------------------------------------------------------- -# iSCSI path helpers -# --------------------------------------------------------------------------- - -def _igroup_name(svm_name, host_name): - """Mirror OntapStorageUtils.getIgroupName: cs_{svmName}_{sanitizedHostName}""" - short = host_name.split(".")[0] - sanitized = re.sub(r"[^a-zA-Z0-9_-]", "_", short) - return "cs_%s_%s" % (svm_name, sanitized) - - # --------------------------------------------------------------------------- # Sequential workflow test class # --------------------------------------------------------------------------- @@ -404,12 +392,11 @@ def test_05_delete_volume_and_force_delete_pool(self): "ONTAP FlexVol '%s' still exists after force deletion" % pool_name ) - # ONTAP: igroups for each cluster host must be deleted + # ONTAP: igroups for each cluster host must be deleted. The driver + # names them cs__ (OntapStorageUtils.getIgroupName); + # use the same name so this check can actually fail. for host in self.cluster_hosts: - iqn = getattr(host, "storageurl", None) or getattr(host, "StorageUrl", None) - if not iqn or not iqn.startswith("iqn."): - continue - igroup_name = _igroup_name(self.svm_name, host.name) + igroup_name = self._igroup_name(host.id) igroup = self.ontap.get_igroup(self.svm_name, igroup_name) self.assertIsNone( igroup, diff --git a/test/integration/plugins/ontap/ontap_test_base.py b/test/integration/plugins/ontap/ontap_test_base.py index e528aba2b25f..e270bd6c605f 100644 --- a/test/integration/plugins/ontap/ontap_test_base.py +++ b/test/integration/plugins/ontap/ontap_test_base.py @@ -65,7 +65,7 @@ from marvin.cloudstackException import CloudstackAPIException from marvin.cloudstackTestCase import cloudstackTestCase from marvin.jsonHelper import jsonDump -from marvin.lib.base import Account, DiskOffering +from marvin.lib.base import Account, DiskOffering, ServiceOffering from marvin.sshClient import SshClient from marvin.lib.common import ( get_domain, get_zone, list_clusters, list_storage_pools, @@ -537,7 +537,7 @@ def get_lun(self, svm_name, lun_path): """Return the ONTAP LUN record for the given full path, or None.""" data = self._get("/storage/luns", params={"svm.name": svm_name, "name": lun_path, - "fields": "name,uuid,enabled,status,space.size"}) + "fields": "name,uuid,enabled,status,space.size,serial_number"}) records = data.get("records", []) return records[0] if records else None @@ -546,7 +546,7 @@ def list_luns_in_volume(self, svm_name, vol_name): prefix = "/vol/%s/" % vol_name data = self._get("/storage/luns", params={"svm.name": svm_name, - "fields": "name,uuid,enabled,status,space.size"}) + "fields": "name,uuid,enabled,status,space.size,serial_number"}) return [r for r in data.get("records", []) if r.get("name", "").startswith(prefix)] @@ -560,6 +560,15 @@ def list_lun_maps_for_volume(self, svm_name, vol_name): return [r for r in data.get("records", []) if r.get("lun", {}).get("name", "").startswith(prefix)] + def list_lun_maps_for_igroup(self, svm_name, igroup_name): + """Return the names of all LUNs currently mapped to the igroup.""" + data = self._get("/protocols/san/lun-maps", + params={"svm.name": svm_name, + "igroup.name": igroup_name, + "fields": "lun.name"}) + return [r.get("lun", {}).get("name", "") + for r in data.get("records", [])] + # -- NFS file helpers ---------------------------------------------------- def list_files_in_volume(self, vol_name, path="/"): @@ -868,6 +877,7 @@ class OntapTestBase(cloudstackTestCase): vm = None template_id = None service_offering_id = None + _tagged_offering = None network_id = None _created_network_id = None # Prefix for a guest network this suite creates on Advanced zones. @@ -994,6 +1004,10 @@ def _setup_cloudstack_resources(cls, config, account_testdata): % parse_ex ) + # Fail the class early when a KVM host cannot be trusted to verify + # results (libvirtd/agent/iscsid down, host not Up in CloudStack). + cls._assert_kvm_hosts_ready() + # ---- KVM storage cleanup helper ------------------------------------ @classmethod @@ -1020,9 +1034,14 @@ def _all_tracked_volumes(cls): # ---- VM deploy/attach helpers -------------------------------------- @classmethod - def _discover_vm_deploy_resources(cls): + def _discover_vm_deploy_resources(cls, storage_tags=None): """Resolve the template, service offering, and network for VM deploys. + With ``storage_tags`` a compute offering carrying those storage tags is + created (and deleted in tearDownClass) so the VM ROOT disk can only be + placed on the ONTAP pool; without it the smallest existing offering is + used and CloudStack may place the disk on any shared pool. + A missing template is not fatal: ``template_id`` is left as None so callers can skip the VM step while the rest of the suite still runs. On Advanced zones an existing account network is reused when present, @@ -1051,6 +1070,14 @@ def _discover_vm_deploy_resources(cls): if offerings: offerings.sort(key=lambda s: getattr(s, "memory", 9999)) cls.service_offering_id = offerings[0].id + if storage_tags: + cls._tagged_offering = ServiceOffering.create(cls.apiClient, { + "name": "OntapLifecycleSO_%d" % random.randint(0, 99999), + "displaytext": "ONTAP lifecycle SO (tagged)", + "cpunumber": 1, "cpuspeed": 500, "memory": 512, + "storagetype": "shared", "tags": storage_tags, + }) + cls.service_offering_id = cls._tagged_offering.id cls.network_id = None if getattr(cls.zone, "networktype", "Basic").lower() != "advanced": @@ -1198,6 +1225,13 @@ def tearDownClass(cls): """Best-effort cleanup of any resources left behind by a failed run.""" cls._destroy_vm_if_present() cls._delete_created_network() + if cls._tagged_offering is not None: + try: + cls._tagged_offering.delete(cls.apiClient) + except Exception as e: + logger.warning( + "tearDownClass: could not delete service offering: %s" % e) + cls._tagged_offering = None if (getattr(cls, "ontap", None) is not None and getattr(cls, "filler_filename", None) and getattr(cls, "filler_flexvol", None)): @@ -1455,6 +1489,333 @@ def _kvm_run(self, host_ip, command): ) return ssh.execute(command) or [] + # ---- KVM host / libvirt verification --------------------------------- + # + # A test only passes when CloudStack, ONTAP *and* the KVM host agree. These + # helpers add the host side: services, the libvirt domain and its XML, the + # by-id block device that belongs to each LUN, and the iSCSI session. + + @classmethod + def _assert_kvm_hosts_ready(cls): + """Fail the class early when a KVM host cannot be trusted. + + libvirtd and cloudstack-agent (plus iscsid for iSCSI) must be active, + virsh must answer, and CloudStack must see every cluster host as + Up/Enabled. Without this, a sick host shows up as obscure failures + deep inside a test. + """ + if not cls.kvm_hosts_ssh_creds: + logger.warning( + "_assert_kvm_hosts_ready: no KVM SSH credentials configured; " + "host-side checks are skipped for this class") + return + units = ["libvirtd", "cloudstack-agent"] + if str(cls.PROTOCOL).upper() == "ISCSI": + units.append("iscsid") + problems = [] + for creds in cls.kvm_hosts_ssh_creds: + host_ip = creds["host"] + try: + for unit in units: + state = kvm_host.service_state( + cls.kvm_hosts_ssh_creds, host_ip, unit) + if state != "active": + problems.append("%s: %s is '%s' (expected 'active')" + % (host_ip, unit, state)) + if not kvm_host.virsh_responsive( + cls.kvm_hosts_ssh_creds, host_ip): + problems.append("%s: virsh does not respond" % host_ip) + except Exception as ex: + problems.append("%s: could not verify host (%s)" + % (host_ip, ex)) + for host in (getattr(cls, "cluster_hosts", None) or []): + if (getattr(host, "state", None) != "Up" + or getattr(host, "resourcestate", None) != "Enabled"): + problems.append( + "CloudStack host %s is state=%s resourcestate=%s " + "(expected Up/Enabled)" + % (getattr(host, "name", "?"), + getattr(host, "state", None), + getattr(host, "resourcestate", None))) + if problems: + raise AssertionError( + "KVM host(s) are not ready: " + "; ".join(problems)) + logger.info("KVM hosts ready: %s", + ", ".join(c["host"] for c in cls.kvm_hosts_ssh_creds)) + + def _vm_placement(self, vm_id): + """Live placement of a VM: host ip, libvirt instance name, VM object. + + Re-read from the API each time, because the deploy-time host and + instance fields go stale after stop/start/revert. + """ + vms = self.apiClient.listVirtualMachines(_list_vms_cmd(vm_id)) or [] + self.assertTrue(vms, "VM %s not found" % vm_id) + vm = vms[0] + host_id = getattr(vm, "hostid", None) + host_ip = None + if host_id: + for host in (self.cluster_hosts or []): + if host.id == host_id: + host_ip = getattr(host, "ipaddress", None) + break + if host_ip is None: + cmd = listHostsAPI.listHostsCmd() + cmd.id = host_id + hosts = self.apiClient.listHosts(cmd) or [] + host_ip = hosts[0].ipaddress if hosts else None + return {"vm": vm, "instance": getattr(vm, "instancename", None), + "host_id": host_id, "host_ip": host_ip} + + def _kvm_host_reachable(self, host_ip): + """True when SSH credentials exist for the host (else log and skip).""" + if host_ip and any(c["host"] == host_ip + for c in self.kvm_hosts_ssh_creds): + return True + logger.warning("No KVM SSH credentials for host %s; host-side checks " + "skipped", host_ip) + return False + + def _assert_kvm_services(self, host_ip, label): + """libvirtd and the CloudStack agent are still active on the host.""" + for unit in ("libvirtd", "cloudstack-agent"): + state = kvm_host.service_state( + self.kvm_hosts_ssh_creds, host_ip, unit) + self.assertEqual( + state, "active", + "[%s] %s on KVM host %s is '%s', expected 'active'" + % (label, unit, host_ip, state)) + + def _cs_volume_wwid(self, vol, label): + """Check a CloudStack volume path is //. + + Returns the WWID. This is the PR #114 contract: the path carries the + LUN WWID rather than a logical unit number. + """ + path = getattr(vol, "path", None) + match = re.match(r"^/([^/]+)/([0-9a-f]{32})$", path or "") + self.assertIsNotNone( + match, + "[%s] volume %s path %r is not '//'" + % (label, vol.name, path)) + target_iqn = self.ontap.get_iscsi_target_iqn(self.svm_name) + self.assertEqual( + match.group(1), target_iqn, + "[%s] volume %s path IQN %r != ONTAP SVM target %r" + % (label, vol.name, match.group(1), target_iqn)) + wwid = match.group(2) + self.assertTrue( + kvm_host.WWID_RE.match(wwid), + "[%s] volume %s WWID %r is not a NetApp NAA identifier" + % (label, vol.name, wwid)) + return wwid + + def _assert_wwid_matches_ontap(self, flexvol_name, wwid, label): + """Some LUN in the FlexVol has a serial number that maps to the WWID.""" + luns = self.ontap.list_luns_in_volume(self.svm_name, flexvol_name) + derived = {} + for lun in luns: + serial = lun.get("serial_number") + if serial: + derived[kvm_host.naa_from_serial(serial)] = lun.get("name") + self.assertIn( + wwid, derived, + "[%s] no LUN in FlexVol '%s' has a serial that maps to WWID %s " + "(LUN WWIDs: %s)" % (label, flexvol_name, wwid, sorted(derived))) + return derived[wwid] + + def _assert_iscsi_session_to_svm(self, host_ip, label): + """The host holds an iSCSI session to the SVM target on a data LIF.""" + target_iqn = self.ontap.get_iscsi_target_iqn(self.svm_name) + lifs = set(self.ontap.get_iscsi_data_lifs(self.svm_name)) + sessions = kvm_host.iscsi_sessions(self.kvm_hosts_ssh_creds, host_ip) + match = [s for s in sessions + if s["target"] == target_iqn and s["portal_ip"] in lifs] + self.assertTrue( + match, + "[%s] KVM host %s has no iSCSI session to target %s on a data LIF " + "%s (sessions: %s)" % (label, host_ip, target_iqn, sorted(lifs), + sessions)) + + def _assert_vm_disks_on_host(self, vm_id, pool, label): + """Verify a running VM on the KVM host, end to end. + + Checks the live libvirt domain (state, identity, sizing) and its XML + (one block disk per CloudStack volume, sourced from the LUN's by-id + path, raw/virtio), that each LUN's block device exists on the host, and + that the host has an iSCSI session to the SVM. Returns a dict with the + host, instance and {volume id: WWID} for later cleanup checks, or None + when the host cannot be reached over SSH. + """ + placement = self._vm_placement(vm_id) + vm, host_ip, instance = ( + placement["vm"], placement["host_ip"], placement["instance"]) + self.assertEqual( + vm.state, "Running", + "[%s] VM %s is %s, expected Running" % (label, vm_id, vm.state)) + self.assertTrue( + host_ip and instance, + "[%s] VM %s has no host/instance (host=%s instance=%s)" + % (label, vm_id, host_ip, instance)) + if not self._kvm_host_reachable(host_ip): + return None + + self._assert_kvm_services(host_ip, label) + state = kvm_host.domain_state( + self.kvm_hosts_ssh_creds, host_ip, instance) + self.assertEqual( + state, "running", + "[%s] libvirt domain %s on %s is '%s', expected 'running'" + % (label, instance, host_ip, state)) + + root = kvm_host.domain_xml( + self.kvm_hosts_ssh_creds, host_ip, instance) + self.assertIsNotNone( + root, "[%s] no libvirt XML for domain %s on %s" + % (label, instance, host_ip)) + + # identity and sizing + self.assertEqual(root.get("type"), "kvm", + "[%s] domain type is not kvm" % label) + self.assertEqual(root.findtext("name"), instance, + "[%s] domain name != instance name" % label) + self.assertEqual(root.findtext("uuid"), vm.id, + "[%s] domain uuid != VM id" % label) + self.assertEqual( + int(root.findtext("vcpu")), int(vm.cpunumber), + "[%s] domain vcpu != service offering cpunumber" % label) + self.assertEqual( + int(root.findtext("memory")), int(vm.memory) * 1024, + "[%s] domain memory (KiB) != service offering memory" % label) + + # ONTAP igroup of the host the VM runs on: it exists once a LUN is + # mapped (the plugin creates it on first grantAccess, not at pool + # creation) and holds this host's initiator IQN. + igroup_name = self._igroup_name(placement["host_id"]) + igroup = self.ontap.get_igroup(self.svm_name, igroup_name) + self.assertIsNotNone( + igroup, "[%s] ONTAP igroup '%s' for host %s not found" + % (label, igroup_name, host_ip)) + iqn_lines = kvm_host.run_on_host( + self.kvm_hosts_ssh_creds, host_ip, + "awk -F= '/^InitiatorName=/{print $2}' " + "/etc/iscsi/initiatorname.iscsi 2>/dev/null") + host_iqn = next((line.strip() for line in iqn_lines + if line.strip().startswith("iqn.")), None) + self.assertIsNotNone( + host_iqn, "[%s] no initiator IQN found on KVM host %s" + % (label, host_ip)) + initiators = [i.get("name") for i in igroup.get("initiators", [])] + self.assertIn( + host_iqn, initiators, + "[%s] host IQN %s is not in igroup %s (initiators: %s)" + % (label, host_iqn, igroup_name, initiators)) + lun_maps = self.ontap.list_lun_maps_for_volume( + self.svm_name, pool.name) + + # disks + vols = self.apiClient.listVolumes(_list_vols_for_vm(vm_id)) or [] + disks = [d for d in kvm_host.parse_domain_disks(root) + if d["device"] == "disk"] + self.assertEqual( + len(disks), len(vols), + "[%s] libvirt has %d disk(s) for %s but CloudStack lists %d " + "volume(s): %s" % (label, len(disks), instance, len(vols), + [d["source_dev"] for d in disks])) + wwids = {} + for vol in vols: + self.assertEqual( + vol.storageid, pool.id, + "[%s] volume %s is on pool '%s' (%s), not on the ONTAP pool " + "'%s' (%s)" % (label, vol.name, getattr(vol, "storage", "?"), + vol.storageid, pool.name, pool.id)) + wwid = self._cs_volume_wwid(vol, label) + lun_name = self._assert_wwid_matches_ontap(pool.name, wwid, label) + wwids[vol.id] = wwid + self.assertTrue( + [m for m in lun_maps + if m.get("lun", {}).get("name") == lun_name + and m.get("igroup", {}).get("name") == igroup_name], + "[%s] LUN %s (volume %s) is not mapped to igroup %s" + % (label, lun_name, vol.name, igroup_name)) + found = [d for d in disks + if d["source_dev"] in kvm_host.by_id_paths(wwid)] + self.assertEqual( + len(found), 1, + "[%s] expected exactly one libvirt disk sourced from the " + "by-id path of %s (volume %s), found %d; disks: %s" + % (label, wwid, vol.name, len(found), + [d["source_dev"] for d in disks])) + disk = found[0] + self.assertEqual(disk["type"], "block", + "[%s] %s disk type" % (label, vol.name)) + self.assertIsNone(disk["source_file"], + "[%s] %s has a file source" % (label, vol.name)) + self.assertEqual(disk["driver_name"], "qemu", + "[%s] %s driver name" % (label, vol.name)) + self.assertEqual(disk["driver_type"], "raw", + "[%s] %s driver type" % (label, vol.name)) + self.assertEqual(disk["target_bus"], "virtio", + "[%s] %s target bus" % (label, vol.name)) + self.assertEqual( + disk["serial"], vol.id.replace("-", "")[:20], + "[%s] %s disk serial != volume uuid" % (label, vol.name)) + if str(vol.type).upper() == "ROOT": + self.assertEqual(disk["target_dev"], "vda", + "[%s] ROOT disk is not vda" % label) + self.assertTrue( + kvm_host.wait_device( + self.kvm_hosts_ssh_creds, host_ip, wwid, True), + "[%s] block device for %s (volume %s) missing on %s" + % (label, wwid, vol.name, host_ip)) + self._assert_iscsi_session_to_svm(host_ip, label) + logger.info("[%s] host check ok: %s on %s, %d disk(s) %s", + label, instance, host_ip, len(disks), sorted(wwids.values())) + return {"host_ip": host_ip, "instance": instance, "wwids": wwids} + + def _assert_disk_detached_on_host(self, host_ip, instance, wwid, label): + """A LUN is no longer a disk of a running domain nor a host device.""" + if not self._kvm_host_reachable(host_ip): + return + root = kvm_host.domain_xml( + self.kvm_hosts_ssh_creds, host_ip, instance) + self.assertIsNotNone( + root, "[%s] no libvirt XML for domain %s" % (label, instance)) + sources = [d["source_dev"] for d in kvm_host.parse_domain_disks(root)] + self.assertFalse( + [s for s in sources if s in kvm_host.by_id_paths(wwid)], + "[%s] domain %s still has a disk for %s: %s" + % (label, instance, wwid, sources)) + self._assert_devices_gone(host_ip, [wwid], label) + + def _assert_devices_gone(self, host_ip, wwids, label): + """The block devices for these WWIDs are gone from the KVM host.""" + if not self._kvm_host_reachable(host_ip): + return + for wwid in wwids: + self.assertFalse( + kvm_host.wait_device( + self.kvm_hosts_ssh_creds, host_ip, wwid, False), + "[%s] block device for %s still present on %s" + % (label, wwid, host_ip)) + + def _assert_domain_absent(self, host_ip, instance, label, timeout=60): + """The libvirt domain is gone (stopped/destroyed VMs are transient).""" + if not self._kvm_host_reachable(host_ip): + return + deadline = time.time() + timeout + state = kvm_host.domain_state( + self.kvm_hosts_ssh_creds, host_ip, instance) + while state != "absent" and time.time() < deadline: + time.sleep(3) + state = kvm_host.domain_state( + self.kvm_hosts_ssh_creds, host_ip, instance) + self.assertEqual( + state, "absent", + "[%s] libvirt domain %s on %s is '%s', expected it to be gone" + % (label, instance, host_ip, state)) + self._assert_kvm_services(host_ip, label) + @classmethod def _igroup_name(cls, host_uuid): """Return the igroup name used by OntapStorageUtils.""" @@ -1497,6 +1858,30 @@ def _capture_igroup_baseline(cls): cls.svm_name, cls.igroup_baseline, ) + def _assert_host_igroups_follow_lun_maps(self, context): + """A host igroup exists exactly while it still has a LUN mapped. + + The igroup is created at first volume access and removed when its last + LUN is unmapped, so after a teardown it must be gone when nothing else + (other pools, system VM disks) is mapped to it, and still be present + when something is. + """ + for igroup_name, _ in self._iscsi_host_specs(): + mapped = self.ontap.list_lun_maps_for_igroup( + self.svm_name, igroup_name) + igroup = self.ontap.get_igroup(self.svm_name, igroup_name) + if mapped: + self.assertIsNotNone( + igroup, + "ONTAP igroup '%s' is missing %s although LUNs %s are " + "still mapped to it" % (igroup_name, context, mapped)) + else: + self.assertIsNone( + igroup, + "ONTAP igroup '%s' still exists %s with no LUN mapped; " + "it should be removed when its last LUN is unmapped" + % (igroup_name, context)) + def _assert_igroup_baseline_unchanged(self, context): """Assert an operation did not change pre-existing shared igroups.""" for igroup_name, expected_initiators in self.igroup_baseline.items(): From de13cdce37b358335c76db384ac8c40775e8e0d7 Mon Sep 17 00:00:00 2001 From: sandeeplocharla Date: Wed, 11 Nov 2026 11:49:12 +0530 Subject: [PATCH 2/2] CSTACKEX-380: fix NFS3 template-cache host check and over-max resize precondition - Template-cache negative workflow: the seed-VM host check (_assert_vm_disks_on_host: igroup, LUN devices, iSCSI session) and the cache-LUN mapping check are iSCSI-only. Guard them with the PROTOCOL check so the NFS3 suite no longer asserts an igroup for an NFS pool. - Pool lifecycle test_06_reject_grow_above_max_size (iSCSI and NFS3): use the ONTAP FlexVol size (space.size) as the "at the 300 TiB maximum" precondition instead of CloudStack's reported capacity. NFS reports the host-measured usable space (95% of the FlexVol with the default snapshot reserve) and refreshes it periodically, so the reported value can differ from 300 TiB. The over-max request and the unchanged-size checks are unchanged. --- .../template_cache_negative_workflow.py | 38 ++++++++++--------- .../ontap/iscsi/pool/test_pool_lifecycle.py | 13 ++++++- .../ontap/nfs3/pool/test_pool_lifecycle.py | 13 ++++++- 3 files changed, 43 insertions(+), 21 deletions(-) diff --git a/test/integration/plugins/ontap/helpers/template_cache_negative_workflow.py b/test/integration/plugins/ontap/helpers/template_cache_negative_workflow.py index f81d442795b5..2bccd3a25d60 100644 --- a/test/integration/plugins/ontap/helpers/template_cache_negative_workflow.py +++ b/test/integration/plugins/ontap/helpers/template_cache_negative_workflow.py @@ -617,23 +617,27 @@ def step_03_deleted_cache_blocks_reuse(self): ) self._assert_cache_on_ontap(pool, spool) - # KVM host / libvirt: the seed VM runs from a by-id block disk - # (the ROOT LUN, a clone of the cache LUN) with the right XML, - # igroup and session. - seed_host = self._assert_vm_disks_on_host( - vm1.id, pool, "seed VM") - # ONTAP: the cache LUN is only mapped while the template is copied - # into it; once the VM runs it must not stay mapped to any host. - cache_lun = "/vol/%s/cs_tmpl_%s" % ( - pool.name, self.__class__.template_db_id) - stray_maps = [ - m for m in self.ontap.list_lun_maps_for_volume( - self.svm_name, pool.name) - if m.get("lun", {}).get("name") == cache_lun] - self.assertEqual( - stray_maps, [], - "Template cache LUN %s is still mapped after the seed VM " - "started: %s" % (cache_lun, stray_maps)) + # iSCSI only (NFS has no LUNs, igroups or block devices). + seed_host = None + if self.PROTOCOL.upper() == "ISCSI": + # KVM host / libvirt: the seed VM runs from a by-id block disk + # (the ROOT LUN, a clone of the cache LUN) with the right XML, + # igroup and session. + seed_host = self._assert_vm_disks_on_host( + vm1.id, pool, "seed VM") + # ONTAP: the cache LUN is only mapped while the template is + # copied into it; once the VM runs it must not stay mapped to + # any host. + cache_lun = "/vol/%s/cs_tmpl_%s" % ( + pool.name, self.__class__.template_db_id) + stray_maps = [ + m for m in self.ontap.list_lun_maps_for_volume( + self.svm_name, pool.name) + if m.get("lun", {}).get("name") == cache_lun] + self.assertEqual( + stray_maps, [], + "Template cache LUN %s is still mapped after the seed VM " + "started: %s" % (cache_lun, stray_maps)) self._destroy_vm_static(vm1.id) vm1 = None diff --git a/test/integration/plugins/ontap/iscsi/pool/test_pool_lifecycle.py b/test/integration/plugins/ontap/iscsi/pool/test_pool_lifecycle.py index aec8c36071f0..656c21b9c290 100644 --- a/test/integration/plugins/ontap/iscsi/pool/test_pool_lifecycle.py +++ b/test/integration/plugins/ontap/iscsi/pool/test_pool_lifecycle.py @@ -709,10 +709,19 @@ def test_06_reject_grow_above_max_size(self): listed = list_storage_pools(self.apiClient, id=pool.id) self.assertTrue(listed, "Pool missing before over-max resize") + # The precondition is the FlexVol size on ONTAP, which is what the + # over-max request is judged against and is the same for both + # protocols. CloudStack's own capacity figure is not used: iSCSI + # reports the stored value while NFS reports the host-measured usable + # space (95% of the FlexVol with the default snapshot reserve) and + # refreshes it periodically, so it can legitimately differ from 300 TiB. + ontap_vol = self.ontap.get_volume(pool.name) + self.assertIsNotNone( + ontap_vol, "ONTAP FlexVol '%s' not found" % pool.name) self.assertEqual( - int(getattr(listed[0], "capacitybytes", 0) or 0), + int(ontap_vol.get("space", {}).get("size", 0) or 0), TestData.ONTAP_MAX_VOLUME_SIZE, - "Pool should sit at the 300 TiB maximum - test_05 must pass first" + "FlexVol should sit at the 300 TiB maximum - test_05 must pass first" ) # Overshoot by only 1 GiB: at 0.0003% of the limit this probes the diff --git a/test/integration/plugins/ontap/nfs3/pool/test_pool_lifecycle.py b/test/integration/plugins/ontap/nfs3/pool/test_pool_lifecycle.py index 84e8681796bb..a2e9dd672134 100644 --- a/test/integration/plugins/ontap/nfs3/pool/test_pool_lifecycle.py +++ b/test/integration/plugins/ontap/nfs3/pool/test_pool_lifecycle.py @@ -891,10 +891,19 @@ def test_06_reject_grow_above_max_size(self): listed = list_storage_pools(self.apiClient, id=pool.id) self.assertTrue(listed, "Pool missing before over-max resize") + # The precondition is the FlexVol size on ONTAP, which is what the + # over-max request is judged against and is the same for both + # protocols. CloudStack's own capacity figure is not used: iSCSI + # reports the stored value while NFS reports the host-measured usable + # space (95% of the FlexVol with the default snapshot reserve) and + # refreshes it periodically, so it can legitimately differ from 300 TiB. + ontap_vol = self.ontap.get_volume(pool.name) + self.assertIsNotNone( + ontap_vol, "ONTAP FlexVol '%s' not found" % pool.name) self.assertEqual( - int(getattr(listed[0], "capacitybytes", 0) or 0), + int(ontap_vol.get("space", {}).get("size", 0) or 0), TestData.ONTAP_MAX_VOLUME_SIZE, - "Pool should sit at the 300 TiB maximum - test_05 must pass first" + "FlexVol should sit at the 300 TiB maximum - test_05 must pass first" ) # Overshoot by only 1 GiB: at 0.0003% of the limit this probes the