Merge pull request #1175 from rackerlabs/discover_new_svm

feat: Add task to looping calls to discover new SVMs every 5 minutes

Author: nidzrai

python/cinder-understack/cinder_understack/dynamic_netapp_driver.py

@@ -4,6 +4,7 @@
 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
@@ -19,6 +20,7 @@
 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
@@ -33,6 +35,13 @@
         "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
@@ -144,6 +153,8 @@ def __init__(self, *args, **kwargs):
         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
@@ -222,12 +233,76 @@ def do_setup(self, ctxt):
             svm_lib.do_setup(ctxt)
             self._libraries[svm_name] = svm_lib
 
+    def _remove_svm_lib(self, svm_name: str, svm_lib: NetAppMinimalLibrary):
+        """Remove resources for a given SVM library."""
+        # TODO: Need to free up resources here.
+        LOG.info("Removing resources for SVM library %s", svm_name)
+        # Stop any looping calls if they exist
+        if svm_lib._looping_call and hasattr(svm_lib, "loopingcalls"):
+            LOG.info("Stopping looping call for SVM library %s", svm_name)
+            svm_lib.loopingcalls.stop_tasks()
+            # Adding None coz it has a reference to the looping call
+            svm_lib.looping_call = None
+            # There are other attributes which are in svm_lib even after
+            # Stopping the looping.
+
+    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.info("Start refreshing SVM libraries")
+        # Print all current library keys
+        existing_libs = set(self._libraries.keys())
+        LOG.info("Existing library keys: %s", existing_libs)
+        # Get the current SVMs from cluster
+        current_svms = set(self._get_svms())
+        LOG.info("Current SVMs detected from cluster: %s", 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)
+            # TODO : stop looping calls, free resources.
+            svm_lib = self._libraries.get(svm_name)
+            self._remove_svm_lib(svm_name, 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)
+            try:
+                lib = self._create_svm_lib(svm_name)
+                # 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,
+                )
+        LOG.info("Final libraries loaded: %s", list(self._libraries.keys()))
+
     def check_for_setup_error(self):
         """Check for setup errors."""
         for svm_name, svm_lib in self._libraries.items():
             LOG.info("Checking NVMe library for errors for SVM %s", svm_name)
             svm_lib.check_for_setup_error()
 
+        # 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
+                )
+                # removed initial_delay the first call run after full interval .
+                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

python/cinder-understack/cinder_understack/tests/test_dynamic_netapp_driver.py

@@ -106,3 +106,103 @@ 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)
+
+    # Mocked check_for_setup_errort to avoid below error .
+    # 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": mock.Mock(vserver="os-old-svm"),
+            expected_svm: mock.Mock(vserver=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 = mock.create_autospec(
+            dynamic_netapp_driver.NetAppMinimalLibrary, instance=True
+        )
+
+        # Mock the new lib instance created
+        mock_lib_instance.vserver = "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."""
+        mock_get_svms.return_value = ["os-new-failing-svm"]
+        mock_create_svm_lib.side_effect = Exception("Simulated failure")
+
+        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)