diff --git a/python/cinder-understack/cinder_understack/dynamic_netapp_driver.py b/python/cinder-understack/cinder_understack/dynamic_netapp_driver.py deleted file mode 100644 index 37a9e7611..000000000 --- a/python/cinder-understack/cinder_understack/dynamic_netapp_driver.py +++ /dev/null @@ -1,536 +0,0 @@ -"""NetApp NVMe driver with dynamic multi-SVM support.""" - -import uuid as _uuid -from collections.abc import Generator -from contextlib import contextmanager -from functools import cached_property - -from cinder import context -from cinder import exception -from cinder import interface -from cinder.volume import configuration -from cinder.volume import driver as volume_driver -from cinder.volume import volume_utils -from cinder.volume.drivers.netapp import options as na_opts -from cinder.volume.drivers.netapp import utils as na_utils -from cinder.volume.drivers.netapp.dataontap.client.client_cmode_rest import ( - RestClient as RestNaServer, -) -from cinder.volume.drivers.netapp.dataontap.nvme_library import NetAppNVMeStorageLibrary -from cinder.volume.drivers.netapp.dataontap.performance import perf_cmode -from cinder.volume.drivers.netapp.dataontap.utils import capabilities -from oslo_config import cfg -from oslo_log import log as logging -from oslo_service import loopingcall - -LOG = logging.getLogger(__name__) -CONF = cfg.CONF - -# Dynamic SVM options - our custom configuration group -netapp_dynamic_opts = [ - cfg.StrOpt( - "netapp_vserver_prefix", - default="os-", - help="Prefix to use when constructing SVM/vserver names from tenant IDs. " - "The SVM name will be formed as . This allows " - "the driver to dynamically select different SVMs based on the " - "volume's project/tenant ID instead of being confined to one SVM.", - ), - cfg.IntOpt( - "netapp_svm_discovery_interval", - default=300, - help="In seconds for SVM discovery. The driver will " - "periodically scan the NetApp cluster for new SVMs matching the " - "configured prefix.", - ), -] - -# Configuration options for dynamic NetApp driver -# Using cinder.volume.configuration approach for better abstraction -NETAPP_DYNAMIC_OPTS = [ - na_opts.netapp_connection_opts, - na_opts.netapp_basicauth_opts, - na_opts.netapp_transport_opts, - na_opts.netapp_provisioning_opts, - na_opts.netapp_support_opts, - na_opts.netapp_san_opts, - na_opts.netapp_cluster_opts, - netapp_dynamic_opts, -] - - -# We use a + because of the special meaning of # in -# cinder/volume/volume_utils.py extract_host() -_SVM_NAME_DELIM = "+" - - -class NetAppMinimalLibrary(NetAppNVMeStorageLibrary): - """Minimal overriding library. - - The purpose of this class is to take the existing upstream class - and patch it as necessary to allow for our multi-SVM approach. - This class is still intended to exist per SVM, its just fixes - for that approach. - """ - - def __init__(self, driver_name, driver_protocol, **kwargs): - super().__init__(driver_name, driver_protocol, **kwargs) - # the upstream library sets this field by parsing the host - # which is "pod@config_group" in syntax. The issue is that - # will point to our parent group. This backend_name is then - # used by the connection code which reloads the whole configuration - # but now it loaded the parent configuration and not the one targeting - # the SVM. By fixing this backend_name it should point to the - # right place. - self.backend_name = self.configuration.config_group - - def do_setup(self, ctxt): - """Override the upstream call. - - This is a copy and paste except for the self.client setup, - instead of calling the library function which makes a brand new - self.configuration object and then reads from that. This creates - the rest client from our existing self.configuration so that we - can supply our overridden one. If we had used the upstream one it - would check that the dynamic config group we made existed in the - parsed config, which it does not and it would then fail. - """ - na_utils.check_flags(self.REQUIRED_FLAGS_BASIC, self.configuration) - self.namespace_ostype = ( - self.configuration.netapp_namespace_ostype or self.DEFAULT_NAMESPACE_OS - ) - self.host_type = self.configuration.netapp_host_type or self.DEFAULT_HOST_TYPE - - na_utils.check_flags(self.REQUIRED_CMODE_FLAGS, self.configuration) - - # this is the change from upstream right here - self.client = RestNaServer( - transport_type=self.configuration.netapp_transport_type, - ssl_cert_path=self.configuration.netapp_ssl_cert_path, - username=self.configuration.netapp_login, - password=self.configuration.netapp_password, - hostname=self.configuration.netapp_server_hostname, - port=self.configuration.netapp_server_port, - vserver=self.configuration.netapp_vserver, - trace=volume_utils.TRACE_API, - api_trace_pattern=self.configuration.netapp_api_trace_pattern, - async_rest_timeout=self.configuration.netapp_async_rest_timeout, - private_key_file=None, - certificate_file=None, - ca_certificate_file=None, - certificate_host_validation=None, - ) - self.vserver = self.client.vserver - - # Storage service catalog. - self.ssc_library = capabilities.CapabilitiesLibrary( - self.driver_protocol, self.vserver, self.client, self.configuration - ) - - self.ssc_library.check_api_permissions() - - self.using_cluster_credentials = self.ssc_library.cluster_user_supported() - - # Performance monitoring library. - self.perf_library = perf_cmode.PerformanceCmodeLibrary(self.client) - - -@interface.volumedriver -class NetappCinderDynamicDriver(volume_driver.BaseVD): - """NetApp NVMe driver with dynamic multi-SVM support. - - This driver follows the standard Cinder pattern by inheriting from BaseVD - and delegating storage operations to the NetappDynamicLibrary. - """ - - VERSION = "1.0.0" - DRIVER_NAME = "NetApp_Dynamic_NVMe" - - def __init__(self, *args, **kwargs): - """Initialize the driver and create library instance.""" - super().__init__(*args, **kwargs) - self.configuration.append_config_values(self.__class__.get_driver_options()) - # save the arguments supplied - self._init_kwargs = kwargs - # but we don't need the configuration - del self._init_kwargs["configuration"] - # child libraries - self._libraries = {} - # aggregated stats - self._stats = self._empty_volume_stats() - # looping call placeholder - self._looping_call = None - - def _create_svm_lib(self, svm_name: str) -> NetAppMinimalLibrary: - # we create a configuration object per SVM library to - # provide the SVM name to the SVM library - child_grp = f"{self.configuration.config_group}_{svm_name}" - child_cfg = configuration.Configuration( - volume_driver.volume_opts, - config_group=child_grp, - ) - # register the options - child_cfg.append_config_values(self.__class__.get_driver_options()) - # we need to copy the configs so get the base group - for opt in self.__class__.get_driver_options(): - try: - val = getattr(self.configuration, opt.name) - CONF.set_override(opt.name, val, group=child_grp) - except cfg.NoSuchOptError: - # this exception occurs if the option isn't set at all - # which means we don't need to set an override - pass - - # now set the SVM name - CONF.set_override("netapp_vserver", svm_name, group=child_grp) - # now set the backend configuration name - CONF.set_override("volume_backend_name", child_grp, group=child_grp) - # return an instance of the library scoped to one SVM - # netapp_mode=proxy is necessary to quiet the driver from reporting that - # its not - return NetAppMinimalLibrary( - self.DRIVER_NAME, - "NVMe", - configuration=child_cfg, - netapp_mode="proxy", - **self._init_kwargs, - ) - - @staticmethod - def get_driver_options(): - """All options this driver supports.""" - return [item for sublist in NETAPP_DYNAMIC_OPTS for item in sublist] - - @cached_property - def cluster(self) -> RestNaServer: - return RestNaServer( - transport_type=self.configuration.netapp_transport_type, - ssl_cert_path=self.configuration.netapp_ssl_cert_path, - username=self.configuration.netapp_login, - password=self.configuration.netapp_password, - hostname=self.configuration.netapp_server_hostname, - port=self.configuration.netapp_server_port, - vserver=None, - trace=volume_utils.TRACE_API, - api_trace_pattern=self.configuration.netapp_api_trace_pattern, - async_rest_timeout=self.configuration.netapp_async_rest_timeout, - private_key_file=None, - certificate_file=None, - ca_certificate_file=None, - certificate_host_validation=None, - ) - - def _get_svms(self): - prefix = self.configuration.safe_get("netapp_vserver_prefix") - svm_filter = { - "state": "running", - "nvme.enabled": "true", - "name": f"{prefix}*", - "fields": "name,uuid", - } - ret = self.cluster.get_records( - "svm/svms", query=svm_filter, enable_tunneling=False - ) - return [rec["name"] for rec in ret["records"]] - - def do_setup(self, ctxt): - """Setup the driver. - - Connected to the NetApp with cluster credentials to find the SVMs. - """ - for svm_name in self._get_svms(): - if svm_name in self._libraries: - LOG.info("NVMe library already exists for SVM %s, skipping", svm_name) - continue - - LOG.info("Creating NVMe library instance for SVM %s", svm_name) - svm_lib = self._create_svm_lib(svm_name) - svm_lib.do_setup(ctxt) - self._libraries[svm_name] = svm_lib - - def _remove_svm_lib(self, svm_lib: NetAppMinimalLibrary): - """Remove resources for a given SVM library.""" - # TODO: Need to free up resources here. - for task in svm_lib.loopingcalls.tasks: - task.looping_call.stop() - svm_lib.loopingcalls.tasks = [] - - def _refresh_svm_libraries(self): - return self._actual_refresh_svm_libraries(context.get_admin_context()) - - def _actual_refresh_svm_libraries(self, ctxt): - """Refresh the SVM libraries.""" - LOG.debug("Start refreshing SVM libraries") - existing_libs = set(self._libraries.keys()) - current_svms = set(self._get_svms()) - LOG.debug( - "_refresh_svm_libraries: existing=%s current=%s", - existing_libs, - current_svms, - ) - # Remove libraries for SVMs that no longer exist - stale_svms = existing_libs - current_svms - for svm_name in stale_svms: - LOG.info("Removing stale NVMe library for SVM: %s", svm_name) - svm_lib = self._libraries[svm_name] - self._remove_svm_lib(svm_lib) - del self._libraries[svm_name] - - # Add new SVM libraries - new_svms = current_svms - existing_libs - for svm_name in new_svms: - LOG.info("Creating NVMe library for new SVM: %s", svm_name) - lib = self._create_svm_lib(svm_name) - try: - # Call do_setup to initialize the library - lib.do_setup(ctxt) - lib.check_for_setup_error() - LOG.info("Library creation success for SVM: %s", svm_name) - self._libraries[svm_name] = lib - except Exception: - LOG.exception( - "Failed to create library for SVM %s", - svm_name, - ) - self._remove_svm_lib(lib) - LOG.info("Final libraries loaded: %s", list(self._libraries.keys())) - - def check_for_setup_error(self): - """Check for setup errors.""" - svm_to_init = set(self._libraries.keys()) - LOG.debug( - "check_for_setup_error: verifying %d SVM(s): %s", - len(svm_to_init), - svm_to_init, - ) - for svm_name in svm_to_init: - LOG.info("Checking NVMe library for errors for SVM %s", svm_name) - svm_lib = self._libraries[svm_name] - try: - svm_lib.check_for_setup_error() - LOG.debug("SVM %s setup check passed", svm_name) - except Exception: - LOG.exception("Failed to initialize SVM %s, skipping", svm_name) - self._remove_svm_lib(svm_lib) - del self._libraries[svm_name] - LOG.debug( - "check_for_setup_error complete: active SVMs=%s", - list(self._libraries.keys()), - ) - - # looping call to refresh SVM libraries - if not self._looping_call: - interval = self.configuration.safe_get("netapp_svm_discovery_interval") - if interval and interval > 0: - self._looping_call = loopingcall.FixedIntervalLoopingCall( - self._refresh_svm_libraries - ) - self._looping_call.start(interval=interval) - else: - LOG.info("SVM discovery timer disabled (interval=%s)", interval) - - def _svmify_pool(self, pool: dict, svm_name: str, **kwargs) -> dict: - """Applies SVM info to a pool so we can target it and track it.""" - # We need to prefix our pool_name, which is 1:1 with the FlexVol - # name on the SVM, with the SVM name. This is because the name of - # a FlexVol is unique within 1 SVM. Two different SVMs can have - # the same FlexVol however so we need to prefix it. We avoid - # using # as our separator because it has special meaning to - # cinder. See the cinder/volume/volume_utils.py extract_host() - # function for details. - pool_name = pool["pool_name"] - pool["pool_name"] = f"{svm_name}{_SVM_NAME_DELIM}{pool_name}" - pool["netapp_vserver"] = svm_name - prefix = self.configuration.safe_get("netapp_vserver_prefix") - project_uuid = svm_name.removeprefix(prefix) - pool["netapp_project_id"] = project_uuid - - pool_regex = na_utils.get_pool_name_filter_regex(self.configuration) - match = pool_regex.match(pool_name) - netapp_volume_type_id = "" - if match: - raw_id = match.group(1) - try: - netapp_volume_type_id = str(_uuid.UUID(raw_id)) - except ValueError: - LOG.warning( - "_svmify_pool: pool=%s capture group %r is not a valid UUID," - " netapp_volume_type_id left empty", - pool_name, - raw_id, - ) - else: - LOG.warning( - "_svmify_pool: pool=%s did not match pattern," - " netapp_volume_type_id left empty", - pool_name, - ) - pool["netapp_volume_type_id"] = netapp_volume_type_id - pool.update(kwargs) - return pool - - @contextmanager - def _volume_to_library(self, volume) -> Generator[NetAppMinimalLibrary]: - """From a volume find the specific NVMe library to use.""" - # save this to restore it in the end - original_host = volume["host"] - - # Pool name format is "{svm_name}+{flexvol_name}" - qualified_pool = volume_utils.extract_host(original_host, level="pool") - if not qualified_pool: - raise exception.InvalidInput( - reason=f"pool name not found in {original_host}" - ) - - svm_name, _, flexvol_name = qualified_pool.partition(_SVM_NAME_DELIM) - if not flexvol_name: - raise exception.InvalidInput( - reason=f"pool {qualified_pool!r} missing delimiter {_SVM_NAME_DELIM!r}" - ) - - LOG.debug( - "_volume_to_library: host=%s svm=%s flexvol=%s", - original_host, - svm_name, - flexvol_name, - ) - - try: - lib = self._libraries[svm_name] - except KeyError: - LOG.error( - "_volume_to_library: SVM=%s not found in active libraries=%s", - svm_name, - list(self._libraries.keys()), - ) - raise exception.DriverNotInitialized() from None - - if lib.vserver != svm_name: - LOG.error( - "NVMe library vserver %s mismatch with volume.host SVM %s", - lib.vserver, - svm_name, - ) - raise exception.InvalidInput( - reason="NVMe library vserver mismatch with volume.host" - ) - - volume["host"] = original_host.replace(f"{svm_name}{_SVM_NAME_DELIM}", "") - yield lib - volume["host"] = original_host - - def create_volume(self, volume): - """Create a volume.""" - with self._volume_to_library(volume) as lib: - return lib.create_volume(volume) - - def delete_volume(self, volume): - """Delete a volume.""" - with self._volume_to_library(volume) as lib: - return lib.delete_volume(volume) - - def create_snapshot(self, snapshot): - """Create a snapshot.""" - with self._volume_to_library(snapshot.volume) as lib: - return lib.create_snapshot(snapshot) - - def delete_snapshot(self, snapshot): - """Delete a snapshot.""" - with self._volume_to_library(snapshot.volume) as lib: - return lib.delete_snapshot(snapshot) - - def create_volume_from_snapshot(self, volume, snapshot): - """Create a volume from a snapshot.""" - with self._volume_to_library(volume) as lib: - return lib.create_volume_from_snapshot(volume, snapshot) - - def create_cloned_volume(self, volume, src_vref): - """Create a cloned volume.""" - with self._volume_to_library(volume) as lib: - return lib.create_cloned_volume(volume, src_vref) - - def extend_volume(self, volume, new_size): - """Extend a volume.""" - with self._volume_to_library(volume) as lib: - return lib.extend_volume(volume, new_size) - - def initialize_connection(self, volume, connector): - """Initialize connection to volume.""" - # TODO: the nova ironic driver sends the field 'initiator' but the NetApp - # cinder driver expects the field to be 'nqn' so copy the field over - if "initiator" in connector and "nqn" not in connector: - connector["nqn"] = connector["initiator"] - with self._volume_to_library(volume) as lib: - return lib.initialize_connection(volume, connector) - - def terminate_connection(self, volume, connector, **kwargs): - """Terminate connection to volume.""" - with self._volume_to_library(volume) as lib: - return lib.terminate_connection(volume, connector, **kwargs) - - def get_filter_function(self): - """Prefixes any filter function with our SVM and volume type matching.""" - base_filter = super().get_filter_function() - svm_filter = "(capabilities.netapp_project_id == volume.project_id)" - vol_type_filter = ( - '(capabilities.netapp_volume_type_id == ""' - " or capabilities.netapp_volume_type_id == volume.volume_type_id)" - ) - combined = f"{svm_filter} and {vol_type_filter}" - if base_filter: - result = f"{combined} and {base_filter}" - else: - result = combined - return result - - def _empty_volume_stats(self): - data = {} - data["volume_backend_name"] = ( - self.configuration.safe_get("volume_backend_name") or self.DRIVER_NAME - ) - data["vendor_name"] = "NetApp" - data["driver_version"] = self.VERSION - data["storage_protocol"] = "NVMe" - data["sparse_copy_volume"] = True - data["replication_enabled"] = False - # each SVM is going to have different limits - data["total_capacity_gb"] = "unknown" - data["free_capacity_gb"] = "unknown" - # ensure we filter our pools by SVM - data["filter_function"] = self.get_filter_function() - data["goodness_function"] = self.get_goodness_function() - data["pools"] = [] - return data - - def get_volume_stats(self, refresh=False): - """Get volume stats.""" - if refresh: - data = self._empty_volume_stats() - for svm_name, svm_lib in self._libraries.items(): - LOG.info("Get Volume Stats for SVM %s", svm_name) - ret = svm_lib.get_volume_stats(refresh) - data["pools"].extend( - [ - self._svmify_pool( - pool, svm_name, filter_function=data["filter_function"] - ) - for pool in ret["pools"] - ] - ) - self._stats = data - return self._stats - - def create_export(self, context, volume, connector): - """Create export for volume.""" - with self._volume_to_library(volume) as lib: - return lib.create_export(context, volume) - - def ensure_export(self, context, volume): - """Ensure export for volume.""" - with self._volume_to_library(volume) as lib: - return lib.ensure_export(context, volume) - - def remove_export(self, context, volume): - """Remove export for volume.""" - with self._volume_to_library(volume) as lib: - return lib.remove_export(context, volume) diff --git a/python/cinder-understack/cinder_understack/netapp_nvme.py b/python/cinder-understack/cinder_understack/netapp_nvme.py new file mode 100644 index 000000000..a3bd4154a --- /dev/null +++ b/python/cinder-understack/cinder_understack/netapp_nvme.py @@ -0,0 +1,41 @@ +"""NetApp NVMe driver with Nova/Ironic connector compatibility. + +Thin wrapper around the native NetApp NVMe driver that translates +the 'initiator' field from Nova/Ironic to the 'nqn' field expected +by the NetApp driver. +""" + +from cinder.volume.drivers.netapp.dataontap.nvme_cmode import NetAppCmodeNVMeDriver + + +class NetAppNVMeDriver(NetAppCmodeNVMeDriver): + """NetApp NVMe driver with Nova/Ironic connector compatibility. + + This minimal wrapper only translates connector['initiator'] to + connector['nqn'] for compatibility with Nova/Ironic which send + 'initiator' instead of 'nqn'. + + All other functionality is provided by the native upstream driver. + """ + + def initialize_connection(self, volume, connector): + """Initialize connection with connector field translation. + + Nova/Ironic send 'initiator' but NetApp driver expects 'nqn'. + Translate if needed, then call upstream. + """ + if "initiator" in connector and "nqn" not in connector: + connector["nqn"] = connector["initiator"] + + return super().initialize_connection(volume, connector) + + def terminate_connection(self, volume, connector, **kwargs): + """Terminate connection with connector field translation. + + Nova/Ironic send 'initiator' but NetApp driver expects 'nqn'. + Translate if needed, then call upstream. + """ + if connector and "initiator" in connector and "nqn" not in connector: + connector["nqn"] = connector["initiator"] + + return super().terminate_connection(volume, connector, **kwargs) diff --git a/python/cinder-understack/cinder_understack/tests/test_dynamic_netapp_driver.py b/python/cinder-understack/cinder_understack/tests/test_dynamic_netapp_driver.py deleted file mode 100644 index 6e5db6f74..000000000 --- a/python/cinder-understack/cinder_understack/tests/test_dynamic_netapp_driver.py +++ /dev/null @@ -1,343 +0,0 @@ -"""Test NetApp dynamic driver implementation.""" - -import uuid -from unittest import mock - -from cinder import context -from cinder import db -from cinder.tests.unit import fake_volume -from cinder.tests.unit import test -from cinder.tests.unit import utils as test_utils -from cinder.tests.unit.volume.drivers.netapp import fakes as na_fakes -from cinder.volume.drivers.netapp.dataontap.nvme_library import NetAppNVMeStorageLibrary -from cinder.volume.drivers.netapp.dataontap.utils import loopingcalls - -from cinder_understack import dynamic_netapp_driver - - -def _create_mock_svm_lib(svm_name: str): - mock_lib = mock.create_autospec( - dynamic_netapp_driver.NetAppMinimalLibrary, instance=True - ) - mock_lib.vserver = svm_name - mock_lib.loopingcalls = loopingcalls.LoopingCalls() - return mock_lib - - -class NetappDynamicDriverTestCase(test.TestCase): - """Test case for NetappCinderDynamicDriver.""" - - def setUp(self): - """Set up test case.""" - super().setUp() - - self.user_id = str(uuid.uuid4()) - self.project_id = str(uuid.uuid4()) - self.svms = [f"os-{self.project_id}"] - - self.ctxt = context.RequestContext( - self.user_id, self.project_id, auth_token=True - ) - - kwargs = { - "configuration": self.get_config_base(), - "host": "openstack@netapp_dynamic", - } - self.driver = dynamic_netapp_driver.NetappCinderDynamicDriver(**kwargs) - self.override_config("netapp_pool_name_search_pattern", r"vol_(.+)") - self.driver._cluster = mock.Mock() - self.driver._get_svms = mock.Mock(return_value=self.svms) - - with mock.patch( - "cinder_understack.dynamic_netapp_driver.RestNaServer" - ) as mock_rest: - self._setup_rest_mock(mock_rest) - self.driver.do_setup(context.get_admin_context()) - for svm_name in self.svms: - self.driver._libraries[svm_name].vserver = svm_name - - def get_config_base(self): - """Get base configuration for testing.""" - cfg = na_fakes.create_configuration() - cfg.netapp_login = "fake_user" - cfg.netapp_password = "fake_pass" # noqa: S105 - cfg.netapp_server_hostname = "127.0.0.1" - return cfg - - def _setup_rest_mock(self, rest): - rest.get_ontap_version = mock.Mock(return_value=(9, 16, 0)) - return rest - - def _get_fake_volume(self, vol_type_id): - return fake_volume.fake_volume_obj( - self.ctxt, - name=na_fakes.VOLUME_NAME, - size=4, - id=na_fakes.VOLUME_ID, - host=f"fake_host@fake_backend#os-{self.project_id}+fake_pool", - volume_type_id=vol_type_id, - ) - - def test_driver_has_correct_attributes(self): - """Test that driver has expected attributes.""" - self.assertEqual("1.0.0", self.driver.VERSION) - self.assertEqual("NetApp_Dynamic_NVMe", self.driver.DRIVER_NAME) - - def test_library_inherits_from_netapp_library(self): - """Test that library inherits from NetApp NVMe library.""" - for svm_lib in self.driver._libraries.values(): - self.assertIsInstance(svm_lib, NetAppNVMeStorageLibrary) - - @mock.patch.object(NetAppNVMeStorageLibrary, "do_setup") - def test_do_setup_calls_library(self, old_do_setup): - """Test that do_setup delegates to library.""" - self.driver.do_setup(self.ctxt) - old_do_setup.assert_not_called() - self.assertEqual(self.svms, list(self.driver._libraries.keys())) - - @mock.patch.object(NetAppNVMeStorageLibrary, "create_volume") - def test_create_volume_calls_library(self, mock_create_volume): - """Test that create_volume delegates to library.""" - ctxt = context.get_admin_context() - self.driver.do_setup(ctxt) - vol_type = test_utils.create_volume_type( - ctxt, - self, - id=na_fakes.VOLUME.volume_type_id, - name="my_vol_type", - is_public=False, - ) - db.volume_type_access_add(ctxt, vol_type.id, self.project_id) - test_vol = self._get_fake_volume(vol_type.id) - self.driver.create_volume(test_vol) - mock_create_volume.assert_called_once_with(test_vol) - - @mock.patch.object(NetAppNVMeStorageLibrary, "get_volume_stats") - def test_get_volume_stats_calls_library(self, mock_get_volume_stats): - """Test that get_volume_stats delegates to library.""" - self.driver.get_volume_stats(refresh=True) - mock_get_volume_stats.assert_called_with(True) - - # Mock check_for_setup_error to avoid: - # NetAppDriverException: No pools are available for provisioning volumes. - @mock.patch( - "cinder_understack.dynamic_netapp_driver.loopingcall.FixedIntervalLoopingCall" - ) - @mock.patch.object(NetAppNVMeStorageLibrary, "check_for_setup_error") - def test_looping_call_starts_once(self, mock_check_setup, mock_looping_call_class): - """Test that looping call starts correctly and only once.""" - # To avoid config errors mocking the SVM lib setup check - mock_check_setup.return_value = None - - # Mock instance for FixedIntervalLoopingCall - mock_looping_instance = mock.Mock() - mock_looping_call_class.return_value = mock_looping_instance - - # Clear the looping call so that it can be start - self.driver._looping_call = None - - # Trigger setup error check ( to start the loop) - self.driver.check_for_setup_error() - - # Verify loop initialized and started - mock_looping_call_class.assert_called_once_with( - self.driver._refresh_svm_libraries - ) - # Todo: Use constants for interval and initial_delay - mock_looping_instance.start.assert_called_once_with(interval=300) - - # Test second call should not start loop again - mock_looping_instance.reset_mock() - self.driver.check_for_setup_error() - mock_looping_instance.start.assert_not_called() - - @mock.patch.object( - dynamic_netapp_driver.NetappCinderDynamicDriver, "_create_svm_lib" - ) - @mock.patch.object(dynamic_netapp_driver.NetappCinderDynamicDriver, "_get_svms") - def test_refresh_svm_libraries_adds_and_removes_svms( - self, mock_get_svms, mock_create_svm_lib - ): - """Test _refresh_svm_libraries add new SVMs and removes stale ones.""" - # Existing SVMs (before refresh called) - expected_svm = f"os-{self.project_id}" - - self.driver._libraries = { - "os-old-svm": _create_mock_svm_lib("os-old-svm"), - expected_svm: _create_mock_svm_lib(expected_svm), - } - - self.driver._get_svms = mock_get_svms - # Returned by _get_svms (after refresh) - mock_get_svms.return_value = [expected_svm, "os-new-svm"] - - # make the created lib look like the real thing - mock_lib_instance = _create_mock_svm_lib("os-new-svm") - mock_create_svm_lib.return_value = mock_lib_instance - - # Trigger refresh - self.driver._context = self.ctxt - - self.driver._actual_refresh_svm_libraries(self.ctxt) - - # Check stale SVM was removed - self.assertNotIn("os-old-svm", self.driver._libraries) - - # Check SVM was retained - self.assertIn(expected_svm, self.driver._libraries) - - # Check new SVM was added - self.assertIn("os-new-svm", self.driver._libraries) - - # New SVM lib should've been created and setup - mock_create_svm_lib.assert_called_once_with("os-new-svm") - mock_lib_instance.do_setup.assert_called_once_with(self.ctxt) - mock_lib_instance.check_for_setup_error.assert_called_once() - - @mock.patch.object( - dynamic_netapp_driver.NetappCinderDynamicDriver, "_create_svm_lib" - ) - @mock.patch.object(dynamic_netapp_driver.NetappCinderDynamicDriver, "_get_svms") - def test_refresh_svm_libraries_handles_lib_creation_failure( - self, mock_get_svms, mock_create_svm_lib - ): - """Ensure that failure in lib creation is caught and logged, not raised.""" - test_svm_name = "os-new-failing_svm" - mock_get_svms.return_value = [test_svm_name] - mock_svm_lib = _create_mock_svm_lib(test_svm_name) - mock_svm_lib.check_for_setup_error.side_effect = Exception("Simulated failure") - mock_create_svm_lib.return_value = mock_svm_lib - - self.driver._libraries = {} - - # Should not raise exception - self.driver._actual_refresh_svm_libraries(mock.Mock()) - - # The failing SVM should not be added to self._libraries - self.assertNotIn("os-new-failing-svm", self.driver._libraries) - - # --- _svmify_pool tests --- - - def _make_pool(self, pool_name): - return { - "pool_name": pool_name, - "free_capacity_gb": 100, - "total_capacity_gb": 200, - } - - def test_svmify_pool_vol_matching_pattern(self): - """FlexVol matching pool name pattern gets netapp_volume_type_id set.""" - svm_name = f"os-{self.project_id}" - vol_type_id = str(uuid.uuid4()) - pool_name = f"vol_{vol_type_id}" - pool = self._make_pool(pool_name) - - result = self.driver._svmify_pool(pool, svm_name) - - self.assertEqual(vol_type_id, result["netapp_volume_type_id"]) - self.assertEqual(self.project_id, result["netapp_project_id"]) - - def test_svmify_pool_multi_flexvol(self): - """Multi-FlexVol vol_{volume_type_uuid} gets netapp_volume_type_id set.""" - svm_name = f"os-{self.project_id}" - vol_type_id = str(uuid.uuid4()) - pool_name = f"vol_{vol_type_id.replace('-', '')}" - pool = self._make_pool(pool_name) - - result = self.driver._svmify_pool(pool, svm_name) - - self.assertEqual(vol_type_id, result["netapp_volume_type_id"]) - - def test_svmify_pool_invalid_uuid_suffix(self): - """FlexVol with vol_ prefix but non-UUID suffix gets empty volume_type_id.""" - svm_name = f"os-{self.project_id}" - pool = self._make_pool("vol_not_a_uuid") - - result = self.driver._svmify_pool(pool, svm_name) - - self.assertEqual("", result["netapp_volume_type_id"]) - - def test_svmify_pool_no_pattern_match(self): - """FlexVol not matching pool name pattern gets empty netapp_volume_type_id.""" - svm_name = f"os-{self.project_id}" - pool = self._make_pool("svm_root") - - result = self.driver._svmify_pool(pool, svm_name) - - self.assertEqual("", result["netapp_volume_type_id"]) - - def test_svmify_pool_sets_project_id_and_vserver(self): - """_svmify_pool sets netapp_project_id and netapp_vserver.""" - svm_name = f"os-{self.project_id}" - pool = self._make_pool("some_pool") - - result = self.driver._svmify_pool(pool, svm_name) - - self.assertEqual(self.project_id, result["netapp_project_id"]) - self.assertEqual(svm_name, result["netapp_vserver"]) - - def test_svmify_pool_prefixes_pool_name_with_svm(self): - """_svmify_pool prefixes pool_name with svm_name+delimiter.""" - svm_name = f"os-{self.project_id}" - pool = self._make_pool("some_pool") - - result = self.driver._svmify_pool(pool, svm_name) - - self.assertEqual( - f"{svm_name}{dynamic_netapp_driver._SVM_NAME_DELIM}some_pool", - result["pool_name"], - ) - - # --- get_filter_function tests --- - - def test_get_filter_function_contains_project_filter(self): - """Filter must include netapp_project_id == volume.project_id.""" - f = self.driver.get_filter_function() - self.assertIn("capabilities.netapp_project_id == volume.project_id", f) - - def test_get_filter_function_contains_volume_type_filter(self): - """Filter must include netapp_volume_type_id conditions.""" - f = self.driver.get_filter_function() - self.assertIn('capabilities.netapp_volume_type_id == ""', f) - self.assertIn("capabilities.netapp_volume_type_id == volume.volume_type_id", f) - - def test_get_filter_function_combines_with_base_filter(self): - """If a base filter exists it should be appended with AND.""" - with mock.patch.object( - self.driver.__class__.__bases__[0], - "get_filter_function", - return_value="capabilities.some_cap > 0", - ): - f = self.driver.get_filter_function() - self.assertIn("capabilities.some_cap > 0", f) - self.assertIn("capabilities.netapp_project_id == volume.project_id", f) - - def test_get_filter_function_no_base_filter(self): - """When no base filter exists the result contains only our filters.""" - with mock.patch.object( - self.driver.__class__.__bases__[0], - "get_filter_function", - return_value=None, - ): - f = self.driver.get_filter_function() - self.assertNotIn("and None", f) - self.assertIn("capabilities.netapp_project_id == volume.project_id", f) - - # --- delete_volume tests --- - - @mock.patch.object(NetAppNVMeStorageLibrary, "delete_volume") - def test_delete_volume_calls_library(self, mock_delete_volume): - """delete_volume delegates to the correct SVM library.""" - ctxt = context.get_admin_context() - self.driver.do_setup(ctxt) - vol_type = test_utils.create_volume_type( - ctxt, - self, - id=na_fakes.VOLUME.volume_type_id, - name="my_vol_type_del", - is_public=False, - ) - db.volume_type_access_add(ctxt, vol_type.id, self.project_id) - test_vol = self._get_fake_volume(vol_type.id) - self.driver.delete_volume(test_vol) - mock_delete_volume.assert_called_once_with(test_vol) diff --git a/python/cinder-understack/cinder_understack/tests/test_netapp_nvme.py b/python/cinder-understack/cinder_understack/tests/test_netapp_nvme.py new file mode 100644 index 000000000..766c1d8e7 --- /dev/null +++ b/python/cinder-understack/cinder_understack/tests/test_netapp_nvme.py @@ -0,0 +1,121 @@ +"""Tests for NetApp NVMe connector translation.""" + +from unittest import TestCase +from unittest import mock + +from cinder_understack import netapp_nvme + + +class TestNetAppNVMeDriver(TestCase): + """Tests for NetAppNVMeDriver connector field translation.""" + + def test_initialize_connection_translates_initiator_to_nqn(self): + """Test that initiator field is copied to nqn field.""" + # Mock the parent __init__ to avoid needing real config + with mock.patch.object( + netapp_nvme.NetAppCmodeNVMeDriver, "__init__", return_value=None + ): + driver = netapp_nvme.NetAppNVMeDriver("driver", "nvme") + + volume = {"id": "test-volume"} + connector = {"initiator": "nqn.2014-08.test:nvme:host01"} + + # Mock the parent initialize_connection + with mock.patch.object( + netapp_nvme.NetAppCmodeNVMeDriver, + "initialize_connection", + return_value={"driver_volume_type": "nvmeof"}, + ) as mock_parent: + result = driver.initialize_connection(volume, connector) + + # Verify nqn was set from initiator + assert connector["nqn"] == "nqn.2014-08.test:nvme:host01" + + # Verify parent was called + mock_parent.assert_called_once_with(volume, connector) + + # Verify result was returned + assert result == {"driver_volume_type": "nvmeof"} + + def test_initialize_connection_preserves_existing_nqn(self): + """Test that existing nqn field is not overwritten.""" + # Mock the parent __init__ to avoid needing real config + with mock.patch.object( + netapp_nvme.NetAppCmodeNVMeDriver, "__init__", return_value=None + ): + driver = netapp_nvme.NetAppNVMeDriver("driver", "nvme") + + volume = {"id": "test-volume"} + connector = { + "initiator": "nqn.2014-08.old:nvme:host01", + "nqn": "nqn.2014-08.new:nvme:host01", + } + + with mock.patch.object( + netapp_nvme.NetAppCmodeNVMeDriver, "initialize_connection", return_value={} + ): + driver.initialize_connection(volume, connector) + + # Verify existing nqn was preserved + assert connector["nqn"] == "nqn.2014-08.new:nvme:host01" + + def test_initialize_connection_no_initiator_field(self): + """Test handling when initiator field is missing.""" + # Mock the parent __init__ to avoid needing real config + with mock.patch.object( + netapp_nvme.NetAppCmodeNVMeDriver, "__init__", return_value=None + ): + driver = netapp_nvme.NetAppNVMeDriver("driver", "nvme") + + volume = {"id": "test-volume"} + connector = {"nqn": "nqn.2014-08.test:nvme:host01"} + + with mock.patch.object( + netapp_nvme.NetAppCmodeNVMeDriver, "initialize_connection", return_value={} + ): + driver.initialize_connection(volume, connector) + + # Verify nqn unchanged + assert connector["nqn"] == "nqn.2014-08.test:nvme:host01" + + def test_terminate_connection_translates_initiator_to_nqn(self): + """Test that initiator field is copied to nqn field on terminate.""" + # Mock the parent __init__ to avoid needing real config + with mock.patch.object( + netapp_nvme.NetAppCmodeNVMeDriver, "__init__", return_value=None + ): + driver = netapp_nvme.NetAppNVMeDriver("driver", "nvme") + + volume = {"id": "test-volume"} + connector = {"initiator": "nqn.2014-08.test:nvme:host01"} + + # Mock the parent terminate_connection + with mock.patch.object( + netapp_nvme.NetAppCmodeNVMeDriver, "terminate_connection", return_value=None + ) as mock_parent: + driver.terminate_connection(volume, connector) + + # Verify nqn was set from initiator + assert connector["nqn"] == "nqn.2014-08.test:nvme:host01" + + # Verify parent was called + mock_parent.assert_called_once_with(volume, connector) + + def test_terminate_connection_handles_none_connector(self): + """Test that None connector doesn't cause errors on terminate.""" + # Mock the parent __init__ to avoid needing real config + with mock.patch.object( + netapp_nvme.NetAppCmodeNVMeDriver, "__init__", return_value=None + ): + driver = netapp_nvme.NetAppNVMeDriver("driver", "nvme") + + volume = {"id": "test-volume"} + + # Mock the parent terminate_connection + with mock.patch.object( + netapp_nvme.NetAppCmodeNVMeDriver, "terminate_connection", return_value=None + ) as mock_parent: + driver.terminate_connection(volume, None) + + # Verify parent was called with None + mock_parent.assert_called_once_with(volume, None) diff --git a/python/cinder-understack/pyproject.toml b/python/cinder-understack/pyproject.toml index 9b7495b0e..71aa5ec4d 100644 --- a/python/cinder-understack/pyproject.toml +++ b/python/cinder-understack/pyproject.toml @@ -31,6 +31,9 @@ dependencies = [ Source = "https://github.com/rackerlabs/understack" [dependency-groups] +dev = [ + "pytest>=9.1.1", +] test = [ "ddt>=1.4.4", "fixtures>=3.0.0", diff --git a/python/cinder-understack/uv.lock b/python/cinder-understack/uv.lock index 7d5b0e43d..73c98ec4d 100644 --- a/python/cinder-understack/uv.lock +++ b/python/cinder-understack/uv.lock @@ -291,6 +291,9 @@ dependencies = [ ] [package.dev-dependencies] +dev = [ + { name = "pytest" }, +] test = [ { name = "ddt" }, { name = "fixtures" }, @@ -301,6 +304,7 @@ test = [ requires-dist = [{ name = "cinder", specifier = ">=27.0.0,<28" }] [package.metadata.requires-dev] +dev = [{ name = "pytest", specifier = ">=9.1.1" }] test = [ { name = "ddt", specifier = ">=1.4.4" }, { name = "fixtures", specifier = ">=3.0.0" }, @@ -339,6 +343,15 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/4c/83/0f65933b7daa436912173f3d63232d158b60686318fccc7cf458ff15bfe8/cmd2-2.7.0-py3-none-any.whl", hash = "sha256:c85faf603e8cfeb4302206f49c0530a83d63386b0d90ff6a957f2c816eb767d7", size = 154309, upload-time = "2025-06-30T16:54:25.039Z" }, ] +[[package]] +name = "colorama" +version = "0.4.6" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/d8/53/6f443c9a4a8358a93a6792e2acffb9d9d5cb0a5cfd8802644b7b1c9a02e4/colorama-0.4.6.tar.gz", hash = "sha256:08695f5cb7ed6e0531a20572697297273c47b8cae5a63ffc6d6ed5c201be6e44", size = 27697, upload-time = "2022-10-25T02:36:22.414Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/d1/d6/3965ed04c63042e047cb6a3e6ed1a63a35087b6a609aa3a15ed8ac56c221/colorama-0.4.6-py2.py3-none-any.whl", hash = "sha256:4f1d9991f5acc0ca119f9d443620b77f9d6b33703e51011c16baf57afb285fc6", size = 25335, upload-time = "2022-10-25T02:36:20.889Z" }, +] + [[package]] name = "cryptography" version = "46.0.3" @@ -623,6 +636,15 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/0e/61/66938bbb5fc52dbdf84594873d5b51fb1f7c7794e9c0f5bd885f30bc507b/idna-3.11-py3-none-any.whl", hash = "sha256:771a87f49d9defaf64091e6e6fe9c18d4833f140bd19464795bc32d966ca37ea", size = 71008, upload-time = "2025-10-12T14:55:18.883Z" }, ] +[[package]] +name = "iniconfig" +version = "2.3.0" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/72/34/14ca021ce8e5dfedc35312d08ba8bf51fdd999c576889fc2c24cb97f4f10/iniconfig-2.3.0.tar.gz", hash = "sha256:c76315c77db068650d49c5b56314774a7804df16fee4402c1f19d6d15d8c4730", size = 20503, upload-time = "2025-10-18T21:55:43.219Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/cb/b1/3846dd7f199d53cb17f49cba7e651e9ce294d8497c8c150530ed11865bb8/iniconfig-2.3.0-py3-none-any.whl", hash = "sha256:f631c04d2c48c52b84d0d0549c99ff3859c98df65b3101406327ecc7d53fbf12", size = 7484, upload-time = "2025-10-18T21:55:41.639Z" }, +] + [[package]] name = "invoke" version = "2.2.1" @@ -1380,6 +1402,15 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/c0/db/61efa0d08a99f897ef98256b03e563092d36cc38dc4ebe4a85020fe40b31/pbr-7.0.3-py2.py3-none-any.whl", hash = "sha256:ff223894eb1cd271a98076b13d3badff3bb36c424074d26334cd25aebeecea6b", size = 131898, upload-time = "2025-11-03T17:04:54.875Z" }, ] +[[package]] +name = "pluggy" +version = "1.6.0" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/f9/e2/3e91f31a7d2b083fe6ef3fa267035b518369d9511ffab804f839851d2779/pluggy-1.6.0.tar.gz", hash = "sha256:7dcc130b76258d33b90f61b658791dede3486c3e6bfb003ee5c9bfb396dd22f3", size = 69412, upload-time = "2025-05-15T12:30:07.975Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/54/20/4d324d65cc6d9205fabedc306948156824eb9f0ee1633355a8f7ec5c66bf/pluggy-1.6.0-py3-none-any.whl", hash = "sha256:e920276dd6813095e9377c0bc5566d94c932c33b27a3e3945d8389c374dd4746", size = 20538, upload-time = "2025-05-15T12:30:06.134Z" }, +] + [[package]] name = "prettytable" version = "3.17.0" @@ -1590,6 +1621,22 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/5a/dc/491b7661614ab97483abf2056be1deee4dc2490ecbf7bff9ab5cdbac86e1/pyreadline3-3.5.4-py3-none-any.whl", hash = "sha256:eaf8e6cc3c49bcccf145fc6067ba8643d1df34d604a1ec0eccbf7a18e6d3fae6", size = 83178, upload-time = "2024-09-19T02:40:08.598Z" }, ] +[[package]] +name = "pytest" +version = "9.1.1" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "colorama", marker = "sys_platform == 'win32'" }, + { name = "iniconfig" }, + { name = "packaging" }, + { name = "pluggy" }, + { name = "pygments" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/e4/47/b9efed96c114afcfa3c9d3fe98a76a1d14c74a9e266d397cf6eb64be5e01/pytest-9.1.1.tar.gz", hash = "sha256:1088fbde8f2b49d95a549a195707afa7a76a3ce9bcadc26b6d71f0ffda5fe313", size = 1636369, upload-time = "2026-06-19T10:58:32.857Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/24/25/1de2678b631f5a49215c6c96fff41ba892b0a34df68d6d80292b1b48aa7f/pytest-9.1.1-py3-none-any.whl", hash = "sha256:37a86b45efb9a47a61a36449063e8e18d0cab3161329fc099eb21783169c4f0c", size = 386536, upload-time = "2026-06-19T10:58:31.347Z" }, +] + [[package]] name = "python-barbicanclient" version = "7.2.0"