From a093755c231c96419139324a7de21b8a72b703ad Mon Sep 17 00:00:00 2001 From: Volodymyr Boiko Date: Fri, 28 Mar 2025 10:50:36 +0200 Subject: [PATCH 1/2] added API token based auth Change-Id: I2ea749bcbcf816cdbdd6b9df7d463ac3a758d080 --- .../drivers/vastdata_driver.rst | 1 + .../configuration/tables/manila-vastdata.inc | 2 + manila/share/drivers/vastdata/driver.py | 32 +++++++++++++--- manila/share/drivers/vastdata/rest.py | 37 ++++++++++++++++--- .../share/drivers/vastdata/test_driver.py | 12 +++++- .../tests/share/drivers/vastdata/test_rest.py | 25 +++++++++++-- ...api-token-based-auth-f6ee3fdce1ba6450.yaml | 6 +++ 7 files changed, 99 insertions(+), 16 deletions(-) create mode 100644 releasenotes/notes/vastdata-add-api-token-based-auth-f6ee3fdce1ba6450.yaml diff --git a/doc/source/configuration/shared-file-systems/drivers/vastdata_driver.rst b/doc/source/configuration/shared-file-systems/drivers/vastdata_driver.rst index 5aec382d26..95fffdcb44 100644 --- a/doc/source/configuration/shared-file-systems/drivers/vastdata_driver.rst +++ b/doc/source/configuration/shared-file-systems/drivers/vastdata_driver.rst @@ -69,6 +69,7 @@ other parameters that are not specific to VAST Share Driver. vast_mgmt_port = {vms_port} vast_mgmt_user = {mgmt_user} vast_mgmt_password = {mgmt_password} + vast_api_token = {vast_api_token} vast_vippool_name = {vip_pool} vast_root_export = {root_export} diff --git a/doc/source/configuration/tables/manila-vastdata.inc b/doc/source/configuration/tables/manila-vastdata.inc index 9ac4503717..02ffa56be9 100644 --- a/doc/source/configuration/tables/manila-vastdata.inc +++ b/doc/source/configuration/tables/manila-vastdata.inc @@ -30,3 +30,5 @@ - (String) Username for VAST management API. * - ``vast_mgmt_password`` = - (String) Password for VAST management API. + * - ``vast_api_token`` = + - (String) API token for accessing VAST mgmt. If provided, it will be used instead of 'vast_mgmt_user' and 'vast_mgmt_password'. diff --git a/manila/share/drivers/vastdata/driver.py b/manila/share/drivers/vastdata/driver.py index e115c2e456..99ab911116 100644 --- a/manila/share/drivers/vastdata/driver.py +++ b/manila/share/drivers/vastdata/driver.py @@ -79,6 +79,16 @@ help="Password for VAST management", secret=True ), + cfg.StrOpt( + "vast_api_token", + default="", + secret=True, + help=( + "API token for accessing VAST mgmt. " + "If provided, it will be used instead " + "of 'san_login' and 'san_password'." + ) + ), ] CONF = cfg.CONF @@ -116,19 +126,29 @@ def do_setup(self, context): username = self.configuration.safe_get("vast_mgmt_user") password = self.configuration.safe_get("vast_mgmt_password") + api_token = self.configuration.safe_get("vast_api_token") host = self.configuration.safe_get("vast_mgmt_host") port = self.configuration.safe_get("vast_mgmt_port") - if not all((username, password, port)): + if not host: + raise exception.VastDriverException( + reason="`vast_mgmt_host` must be set in manila.conf." + ) + # Require either (username & password) OR (API token) + if not ((username and password) or api_token): raise exception.VastDriverException( - reason="Not all required parameters are present." - " Make sure you specified `vast_mgmt_host`," - " `vast_mgmt_port`, and `vast_mgmt_user` " - "in manila.conf." + reason="Authentication failed: You must specify either " + "`vast_mgmt_user` and `vast_mgmt_password`, " + "or provide `vast_api_token` in manila.conf." ) if port: host = f"{host}:{port}" self.rest = vast_rest.RestApi( - host, username, password, False, self.VERSION + host=host, + username=username, + password=password, + api_token=api_token, + ssl_verify=False, + plugin_version=self.VERSION, ) LOG.debug("VAST Data driver setup is complete.") diff --git a/manila/share/drivers/vastdata/rest.py b/manila/share/drivers/vastdata/rest.py index 0b46f170a4..f70db98e0b 100644 --- a/manila/share/drivers/vastdata/rest.py +++ b/manila/share/drivers/vastdata/rest.py @@ -32,20 +32,37 @@ class Session(requests.Session): - def __init__(self, host, username, password, ssl_verify, plugin_version): + def __init__( + self, + host, + username, + password, + api_token, + ssl_verify, + plugin_version, + ): super().__init__() self.base_url = f"https://{host.strip('/')}/api" self.ssl_verify = ssl_verify self.username = username self.password = password + self.token = api_token self.headers["Accept"] = "application/json" self.headers["Content-Type"] = "application/json" self.headers["User-Agent"] = ( f"manila/v{plugin_version}" f" ({requests.utils.default_user_agent()})" ) - # will be updated on first request - self.headers["authorization"] = "Bearer" + if self.token: + LOG.info("VMS session is using API token authentication.") + self.headers["authorization"] = f"Api-Token {self.token}" + else: + # Will be updated on the first request + LOG.info( + "VMS session is using username/password authentication" + " (Bearer token will be acquired)." + ) + self.headers["authorization"] = "Bearer" if not ssl_verify: import urllib3 @@ -95,7 +112,8 @@ def request( ret = super().request( verb, url, verify=self.ssl_verify, params=params, **kwargs ) - if ret.status_code == 403 and "Token is invalid" in ret.text: + # No refresh for token based auth. Token should be long-lived. + if ret.status_code == 403 and not self.token: self.refresh_auth_token() raise exception.VastApiRetry(reason="Token is invalid or expired.") @@ -307,11 +325,20 @@ def delete(self, path): class RestApi: - def __init__(self, host, username, password, ssl_verify, plugin_version): + def __init__( + self, + host, + username, + password, + api_token, + ssl_verify, + plugin_version, + ): self.session = Session( host=host, username=username, password=password, + api_token=api_token, ssl_verify=ssl_verify, plugin_version=plugin_version, ) diff --git a/manila/tests/share/drivers/vastdata/test_driver.py b/manila/tests/share/drivers/vastdata/test_driver.py index b406da125b..e923a0f52a 100644 --- a/manila/tests/share/drivers/vastdata/test_driver.py +++ b/manila/tests/share/drivers/vastdata/test_driver.py @@ -94,7 +94,7 @@ def test_do_setup(self): self.assertFalse(session.ssl_verify) self.assertEqual(session.base_url, "https://test:443/api") - @ddt.data("vast_mgmt_user", "vast_vippool_name") + @ddt.data("vast_mgmt_user", "vast_vippool_name", "vast_mgmt_host") def test_do_setup_missing_required_fields(self, missing_field): self.fake_conf.set_default(missing_field, None) _driver = driver.VASTShareDriver( @@ -103,6 +103,16 @@ def test_do_setup_missing_required_fields(self, missing_field): with self.assertRaises(exception.VastDriverException): _driver.do_setup(self._context) + def test_do_setup_with_api_token(self): + self.fake_conf.set_default("vast_mgmt_user", None) + self.fake_conf.set_default("vast_mgmt_password", None) + self.fake_conf.set_default("vast_api_token", "test_token") + _driver = driver.VASTShareDriver( + execute=mock.MagicMock(), configuration=self.fake_conf + ) + _driver.do_setup(self._context) + self.assertEqual(_driver.rest.session.token, "test_token") + @mock.patch( "manila.share.drivers.vastdata.rest.Session.get", mock.MagicMock(return_value=fake_metrics), diff --git a/manila/tests/share/drivers/vastdata/test_rest.py b/manila/tests/share/drivers/vastdata/test_rest.py index f76d3db9c0..5235023966 100644 --- a/manila/tests/share/drivers/vastdata/test_rest.py +++ b/manila/tests/share/drivers/vastdata/test_rest.py @@ -101,8 +101,12 @@ class TestSession(unittest.TestCase): def setUp(self): self.session = vast_rest.Session( - "host", "username", - "password", False, "1.0" + "host", + "username", + "password", + "", + False, + "1.0", ) @mock.patch("requests.Session.request") @@ -274,6 +278,7 @@ def test_view_create(self): "host", "username", "password", + "", True, "1.0" ) @@ -324,6 +329,7 @@ def test_capacity_metrics(self): "host", "username", "password", + "", True, "1.0" ) @@ -344,7 +350,12 @@ class TestFolders(unittest.TestCase): ) def setUp(self): self.rest_api = vast_rest.RestApi( - "host", "username", "password", True, "1.0" + "host", + "username", + "password", + "", + True, + "1.0", ) @ddt.data( @@ -446,6 +457,7 @@ def setUp(self): "host", "username", "password", + "", True, "1.0" ) @@ -506,7 +518,12 @@ def test_get_sw_version(self, mock_session): mock.MagicMock(sys_version="1.0") ] rest_api = vast_rest.RestApi( - "host", "username", "password", True, "1.0" + "host", + "username", + "password", + "", + True, + "1.0", ) version = rest_api.get_sw_version() self.assertEqual(version, "1.0") diff --git a/releasenotes/notes/vastdata-add-api-token-based-auth-f6ee3fdce1ba6450.yaml b/releasenotes/notes/vastdata-add-api-token-based-auth-f6ee3fdce1ba6450.yaml new file mode 100644 index 0000000000..ff8c90278d --- /dev/null +++ b/releasenotes/notes/vastdata-add-api-token-based-auth-f6ee3fdce1ba6450.yaml @@ -0,0 +1,6 @@ +--- +features: + - Added support for authentication using an API token in the VAST Manila driver. + Introduced the `vast_api_token` configuration option, allowing users to + authenticate with a pre-generated API token instead of using `vast_mgmt_user` + and `vast_mgmt_password`. From e0e56064650eaa6bea37515568a6c22c817a6948 Mon Sep 17 00:00:00 2001 From: Volodymyr Boiko Date: Thu, 9 Oct 2025 11:45:54 +0300 Subject: [PATCH 2/2] VAST driver: Add multitenancy support via share types This patch adds multitenancy support to the VAST Manila driver by allowing administrators to specify different VIP pools per share type using the namespaced extra spec 'vast:vippool_name'. This enables network isolation between different tenants or projects while sharing the same Manila backend. Change-Id: If01c9fd888b4338a7ac5408f6a589e9bbeae8903 Signed-off-by: Volodymyr Boiko --- .../drivers/vastdata_driver.rst | 86 ++++- manila/share/drivers/vastdata/driver.py | 84 +++- manila/share/drivers/vastdata/driver_util.py | 71 ++++ manila/share/drivers/vastdata/rest.py | 111 ++++-- .../share/drivers/vastdata/test_driver.py | 361 +++++++++++++++++- .../tests/share/drivers/vastdata/test_rest.py | 235 +++++++++++- ...astdata-multitenancy-6936c95f94213548.yaml | 11 + 7 files changed, 889 insertions(+), 70 deletions(-) create mode 100644 releasenotes/notes/vastdata-multitenancy-6936c95f94213548.yaml diff --git a/doc/source/configuration/shared-file-systems/drivers/vastdata_driver.rst b/doc/source/configuration/shared-file-systems/drivers/vastdata_driver.rst index 95fffdcb44..cd14027b62 100644 --- a/doc/source/configuration/shared-file-systems/drivers/vastdata_driver.rst +++ b/doc/source/configuration/shared-file-systems/drivers/vastdata_driver.rst @@ -1,6 +1,6 @@ -==================================== +===================== Vastdata Share Driver -==================================== +===================== VAST Share Driver integrates OpenStack with `VAST Data `__'s Storage System. @@ -46,7 +46,7 @@ share driver. VAST Share Driver configuration example -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ The following example shows parameters in the ``manila.conf`` file that are used to configure VAST Share Driver. @@ -54,6 +54,13 @@ They include two options under ``[DEFAULT]`` and parameters under ``[vast]``. Note that a real ``manila.conf`` file would also include other parameters that are not specific to VAST Share Driver. +.. note:: + + The ``vast_vippool_name`` parameter can be omitted from ``manila.conf`` + if you plan to specify different VIP pools per share type using the + ``vast:vippool_name`` extra spec. See the Multitenancy support section + for more details. + .. code-block:: ini [DEFAULT] @@ -79,7 +86,7 @@ changes to take effect. Pre-configurations for share support --------------------------------------------------- +------------------------------------ To create a file share, you need to: @@ -96,8 +103,75 @@ Create an NFS share: openstack share create NFS ${size} --name ${share_name} --share-type ${share_type_name} +Multitenancy support via Share Types +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +The VAST Share Driver supports multitenancy by allowing different share types +to use different VIP pools. This enables tenant isolation and provides +flexibility in network configuration. + +VIP Pool Configuration +---------------------- + +The VIP pool can be specified in two ways: + +1. **Global default** in ``manila.conf`` using ``vast_vippool_name`` +2. **Per share type** using the ``vast:vippool_name`` extra spec + +When both are specified, the share type extra spec takes precedence over +the configuration file setting. + +Creating Share Types with Different VIP Pools +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +You can create multiple share types, each using a different VIP pool for +multitenancy: + +.. code-block:: console + + # Tenant A with dedicated VIP pool + openstack share type create vast-tenant-a false \ + --extra-specs share_backend_name=vast \ + --extra-specs vast:vippool_name=vippool-tenant-a + + # Tenant B with dedicated VIP pool + openstack share type create vast-tenant-b false \ + --extra-specs share_backend_name=vast \ + --extra-specs vast:vippool_name=vippool-tenant-b + +Creating Shares with Specific VIP Pools +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +When creating a share, specify the share type to determine which VIP pool +will be used: + +.. code-block:: console + + # Create a share for Tenant A (uses vippool-tenant-a) + openstack share create NFS 100 \ + --name tenant-a-share \ + --share-type vast-tenant-a + + # Create a share for Tenant B (uses vippool-tenant-b) + openstack share create NFS 50 \ + --name tenant-b-share \ + --share-type vast-tenant-b + +This approach enables: + +- **Network isolation** between different tenants or projects +- **Service differentiation** with different network configurations per tenant or environment +- **Flexible deployment** without modifying ``manila.conf`` + +.. note:: + + If ``vast_vippool_name`` is not specified in ``manila.conf`` and a share + is created without a share type that specifies ``vast:vippool_name``, + the share creation will fail with an error indicating that a VIP pool + must be specified. + Pre-Configurations for Snapshot support --------------------------------------------------- +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ The share type must have the following parameter specified: @@ -119,7 +193,7 @@ Or you can add it to an existing share type: To snapshot a share and create share from the snapshot ------------------------------------------------------- +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Create a share using a share type with snapshot_support=True. Then, create a snapshot of the share using the command: diff --git a/manila/share/drivers/vastdata/driver.py b/manila/share/drivers/vastdata/driver.py index 99ab911116..e3b2c2cefa 100644 --- a/manila/share/drivers/vastdata/driver.py +++ b/manila/share/drivers/vastdata/driver.py @@ -63,7 +63,8 @@ ), cfg.StrOpt( "vast_vippool_name", - help="Name of Virtual IP pool" + help="Default name of Virtual IP pool. Can be overridden per share " + "using share type extra spec 'vast:vippool_name'." ), cfg.StrOpt( "vast_root_export", @@ -104,9 +105,21 @@ driver_util.verbose_driver_trace ) class VASTShareDriver(driver.ShareDriver): - """Driver for the VastData Filesystem.""" + """Driver for the VastData Filesystem. + + Version history:: + + 1.0 - Initial version. + - Support for NFS shares. + - Support for snapshots (create, delete, revert). + - Support for share creation from snapshots. + - Support for share access rules (IP-based). + - Support for share resize (extend/shrink). + 1.1 - Added multi-tenancy support. + - Support for per-share VIP pool selection via extra specs. + """ - VERSION = "1.0" # driver version + VERSION = "1.1" def __init__(self, *args, **kwargs): super().__init__(False, *args, config_opts=[OPTS], **kwargs) @@ -116,10 +129,7 @@ def do_setup(self, context): backend_name = self.configuration.safe_get("share_backend_name") root_export = self.configuration.vast_root_export vip_pool_name = self.configuration.safe_get("vast_vippool_name") - if not vip_pool_name: - raise exception.VastDriverException( - reason="vast_vippool_name must be set" - ) + self._backend_name = backend_name or self.__class__.__name__ self._vippool_name = vip_pool_name self._root_export = "/" + root_export.strip("/") @@ -276,9 +286,14 @@ def shrink_share(self, share, new_size, share_server=None): self._resize_share(share, new_size) def create_snapshot(self, context, snapshot, share_server=None): - """Is called to create snapshot.""" + """Is called to create a snapshot.""" path = self._to_volume_path(snapshot["share_instance_id"]) - self.rest.snapshots.create(path=path, name=snapshot["name"]) + tenant_id = self._get_tenant_id_for_share(snapshot["share"]) + self.rest.snapshots.create( + name=snapshot["name"], + path=path, + tenant_id=tenant_id, + ) def delete_snapshot(self, context, snapshot, share_server=None): """Is called to remove share.""" @@ -316,6 +331,33 @@ def _resize_share(self, share, new_size): share_id=share['id']) self.rest.quotas.update(quota.id, hard_limit=requested_capacity) + def _get_vip_pool_for_share(self, share): + # Get extra specs from share type + extra_specs = driver_util.get_share_extra_specs_params(share) + # Use vippool_name from extra specs if provided, otherwise use config + vippool_name = extra_specs.get('vippool_name') or self._vippool_name + + if not vippool_name: + raise exception.VastDriverException( + reason="VIP pool name must be specified either in manila.conf " + "(vast_vippool_name) or in share type extra specs " + "(vast:vippool_name)" + ) + + LOG.debug( + "Using VIP pool '%s' for share %s (from %s)", + vippool_name, share["id"], + "extra_specs" if extra_specs.get('vippool_name') else "config" + ) + + return self.rest.vip_pools.one( + name=vippool_name, + fail_if_missing=True, + ) + + def _get_tenant_id_for_share(self, share): + return self._get_vip_pool_for_share(share).tenant_id + def _ensure_share(self, share): share_proto = share["share_proto"] if share_proto != "NFS": @@ -325,15 +367,26 @@ def _ensure_share(self, share): ) ) - vips = self.rest.vip_pools.vips(pool_name=self._vippool_name) + vippool = self._get_vip_pool_for_share(share) + vips = driver_util.generate_ip_range(vippool.ip_ranges) + if not vips: + raise exception.VastDriverException( + reason=f"Pool {vippool.name} has no available vips" + ) share_id = share["id"] requested_capacity = share["size"] * units.Gi path = self._to_volume_path(share_id) - policy = self.rest.view_policies.ensure(name=share_id) + policy = self.rest.view_policies.ensure( + name=share_id, + tenant_id=vippool.tenant_id, + ) quota = self.rest.quotas.ensure( - name=share_id, path=path, - create_dir=True, hard_limit=requested_capacity + name=share_id, + path=path, + create_dir=True, + hard_limit=requested_capacity, + tenant_id=vippool.tenant_id, ) if quota.hard_limit != requested_capacity: raise exception.VastDriverException( @@ -341,7 +394,10 @@ def _ensure_share(self, share): f" (requested={requested_capacity}, exists={quota.hard_limit})" ) view = self.rest.views.ensure( - name=share_id, path=path, policy_id=policy.id + name=share_id, + path=path, + policy_id=policy.id, + tenant_id=vippool.tenant_id, ) if view.policy != share_id: self.rest.views.update(view.id, policy_id=policy.id) diff --git a/manila/share/drivers/vastdata/driver_util.py b/manila/share/drivers/vastdata/driver_util.py index 9b34e78fed..4618968651 100644 --- a/manila/share/drivers/vastdata/driver_util.py +++ b/manila/share/drivers/vastdata/driver_util.py @@ -12,6 +12,7 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. +import copy import ipaddress import types @@ -19,10 +20,80 @@ from oslo_log import log from oslo_utils import timeutils +from manila.share import share_types + CONF = cfg.CONF LOG = log.getLogger(__name__) +# VAST driver namespace for extra specs +VAST_EXTRA_SPEC_NAMESPACE = 'vast' + +# Supported VAST extra specs options +VAST_EXTRA_SPECS_OPTS = { + 'vippool_name': None, +} + + +def get_share_extra_specs_params(share): + """Return the VAST-specific parameters from share extra specs. + + Args: + share: The share object containing share_type_id + + Returns: + dict: Dictionary of VAST-specific options extracted from extra specs + """ + specs = share_types.get_extra_specs_from_share(share) + return get_opts_from_specs(specs) + + +def get_opts_from_specs(specs): + """Parse extra specs and extract VAST-specific options. + + This function extracts options with the 'vast:' namespace prefix. + For example: vast:vippool_name=pool-1 + + Args: + specs: Dictionary of extra specs from a share type + + Returns: + dict: Dictionary of parsed VAST options + """ + opts = copy.deepcopy(VAST_EXTRA_SPECS_OPTS) + + for key, value in specs.items(): + # Get the scope (namespace), if using scope format + scope = None + key_split = key.split(':') + + # Skip invalid format (more than 2 parts) + if len(key_split) not in (1, 2): + continue + + if len(key_split) == 1: + # No namespace, skip for VAST-specific options + continue + else: + scope = key_split[0] + option_key = key_split[1] + + # Normalize to lowercase for comparison + if scope: + scope = scope.lower() + if option_key: + option_key = option_key.lower() + + # Extract options with 'vast:' namespace + if scope == VAST_EXTRA_SPEC_NAMESPACE and option_key in opts: + opts[option_key] = value + LOG.debug( + "Found VAST extra spec: %s:%s = %s", + scope, option_key, value + ) + + return opts + class Bunch(dict): # from https://github.com/real-easypy/easypy diff --git a/manila/share/drivers/vastdata/rest.py b/manila/share/drivers/vastdata/rest.py index f70db98e0b..fdc779a8c6 100644 --- a/manila/share/drivers/vastdata/rest.py +++ b/manila/share/drivers/vastdata/rest.py @@ -22,6 +22,7 @@ from oslo_utils import versionutils from packaging import version as packaging_version import requests +from requests import cookies from manila import exception from manila.share.drivers.vastdata import driver_util @@ -30,6 +31,14 @@ LOG = logging.getLogger(__name__) +class NoCookiesJar(cookies.RequestsCookieJar): + def set(self, name, value, **kwargs): + return None + + def set_cookie(self, cookie, *args, **kwargs): + return + + class Session(requests.Session): def __init__( @@ -47,6 +56,7 @@ def __init__( self.username = username self.password = password self.token = api_token + self.cookies = NoCookiesJar() self.headers["Accept"] = "application/json" self.headers["Content-Type"] = "application/json" self.headers["User-Agent"] = ( @@ -75,7 +85,7 @@ def refresh_auth_token(self): "POST", f"{self.base_url}/token/", verify=self.ssl_verify, - timeout=5, + timeout=30, json={"username": self.username, "password": self.password}, ) resp.raise_for_status() @@ -126,6 +136,14 @@ def request( raise exception.VastApiException(reason=str(exc)) ret = ret.json() if ret.content else {} + + # Handle pagination envelope (dict with 'results' and 'count') + # This provides idempotent handling for both paginated and + # non-paginated responses from the API + if isinstance(ret, dict) and "results" in ret and "count" in ret: + # This is a paginated response, extract the results array + ret = ret["results"] + if ret and log_result: formatted_response = textwrap.indent( pprint.pformat(ret), prefix="| " @@ -191,11 +209,11 @@ def __init__(self, rest): self.session = rest.session def list(self, **params): - """Get list of entries with optional filtering params""" + """Get a list of entries with optional filtering params""" return self.session.get(self.resource_name, params=params) def create(self, **params): - """Create new entry with provided params""" + """Create a new entry with provided params""" return self.session.post(self.resource_name, data=params) def update(self, entry_id, **params): @@ -204,51 +222,76 @@ def update(self, entry_id, **params): f"{self.resource_name}/{entry_id}", data=params ) - def delete(self, name): - """Delete entry by name. Skip if entry not found.""" - entry = self.one(name) + def delete(self, **params): + """Delete entry by provided params. Skip if entry not found.""" + entry = self.one(**params) if not entry: resource = self.__class__.__name__.lower() + serialized_params = json.dumps(params, separators=(",", ":")) LOG.warning( - f"{resource} {name} not found on VAST, skipping delete" + "%r not found for params %s, skipping delete", + resource, serialized_params ) return - return self.session.delete(f"{self.resource_name}/{entry.id}") + return self.delete_by_id(entry["id"]) - def one(self, name): - """Get single entry by name. + def delete_by_id(self, entry_id, **params): + """Delete entry by id""" + return self.session.delete( + f"{self.resource_name}/{entry_id}", + **params, + ) + + def one(self, *, fail_if_missing=False, **params): + """Retrieve a single entry by provided filter parameters. - Raise exception if multiple entries found. - """ - entries = self.list(name=name) + Args: + fail_if_missing: If True, raise exception when entry not found + **params: Filter parameters (keyword-only) + + Raises exception If no entry is found and `fail_if_missing` is True, + or if multiple entries are found. + """ + entries = self.list(**params) + resource = self.__class__.__name__.lower() if not entries: + if fail_if_missing: + serialized_params = json.dumps(params, separators=(",", ":")) + raise exception.VastDriverException( + reason=f"No {resource!r} " + f"found for params {serialized_params}" + ) return if len(entries) > 1: - resource = self.__class__.__name__.lower() + "s" + serialized_params = json.dumps(params, separators=(",", ":")) raise exception.VastDriverException( - reason=f"Too many {resource} found with name {name}" + reason=f"Too many '{resource}s' " + f"found for params {serialized_params}: {entries}" ) return entries[0] def ensure(self, name, **params): - entry = self.one(name) + entry = self.one(name=name) if not entry: entry = self.create(name=name, **params) return entry + def get(self, entry_id, **params): + """Get a single entry by id""" + return self.session.get( + f"{self.resource_name}/{entry_id}", + params=params, + ) + class View(VastResource): resource_name = "views" - def create(self, name, path, policy_id): - data = dict( - name=name, - path=path, - policy_id=policy_id, - create_dir=True, - protocols=["NFS"], - ) - return super().create(**data) + def create(self, **params): + # Set defaults for view creation + params.setdefault("create_dir", True) + params.setdefault("protocols", ["NFS"]) + return super().create(**params) class ViewPolicy(VastResource): @@ -280,6 +323,13 @@ class Quota(VastResource): class VipPool(VastResource): resource_name = "vippools" + @cachetools.cached(cache=cachetools.TTLCache(ttl=60 * 30, maxsize=128)) + def one(self, **params): + # Cache results to avoid repeated REST API calls for the same pool. + # The vippool is frequently accessed (e.g., to resolve tenant IDs + # for shares and snapshots), so heavy usage of this method is expected. + return super().one(**params) + def vips(self, pool_name): """Get list of ip addresses from vip pool""" vippool = self.one(name=pool_name) @@ -298,6 +348,12 @@ def vips(self, pool_name): class Snapshots(VastResource): resource_name = "snapshots" + def create(self, name, path, tenant_id=None): + data = dict(name=name, path=path) + if tenant_id: + data["tenant_id"] = tenant_id + return super().create(**data) + class Folders(VastResource): resource_name = "folders" @@ -350,8 +406,9 @@ def __init__( self.snapshots = Snapshots(self) self.folders = Folders(self) - # Refresh auth token to avoid initial "forbidden" status error. - self.session.refresh_auth_token() + if not api_token: + # Refresh auth token to avoid initial "forbidden" status error. + self.session.refresh_auth_token() @cachetools.cached(cache=cachetools.TTLCache(ttl=60 * 60, maxsize=1)) def get_sw_version(self): diff --git a/manila/tests/share/drivers/vastdata/test_driver.py b/manila/tests/share/drivers/vastdata/test_driver.py index e923a0f52a..4049d39893 100644 --- a/manila/tests/share/drivers/vastdata/test_driver.py +++ b/manila/tests/share/drivers/vastdata/test_driver.py @@ -84,7 +84,10 @@ def setUp(self, m_auth_token): execute=mock.MagicMock(), configuration=self.fake_conf ) self._driver.do_setup(self._context) - m_auth_token.assert_called_once() + if self.fake_conf.vast_api_token: + m_auth_token.assert_not_called() + else: + m_auth_token.assert_called_once() def test_do_setup(self): session = self._driver.rest.session @@ -94,7 +97,7 @@ def test_do_setup(self): self.assertFalse(session.ssl_verify) self.assertEqual(session.base_url, "https://test:443/api") - @ddt.data("vast_mgmt_user", "vast_vippool_name", "vast_mgmt_host") + @ddt.data("vast_mgmt_user", "vast_mgmt_host") def test_do_setup_missing_required_fields(self, missing_field): self.fake_conf.set_default(missing_field, None) _driver = driver.VASTShareDriver( @@ -123,7 +126,7 @@ def test_update_share_stats(self): self.assertEqual(result["share_backend_name"], "vast") self.assertEqual(result["driver_handles_share_servers"], False) self.assertEqual(result["vendor_name"], "VAST STORAGE") - self.assertEqual(result["driver_version"], "1.0") + self.assertEqual(result["driver_version"], "1.1") self.assertEqual(result["storage_protocol"], "NFS") self.assertEqual(result["total_capacity_gb"], 471.1061706542969) self.assertEqual(result["free_capacity_gb"], 450.2256333641708) @@ -158,7 +161,15 @@ def test_update_share_stats(self): ) ) @ddt.unpack - def test_create_shares(self, capacity, proto, policy): + @mock.patch('manila.share.share_types.get_extra_specs_from_share') + def test_create_shares( + self, + capacity, + proto, + policy, + mock_get_extra_specs, + ): + mock_get_extra_specs.return_value = {} share = fake_share.fake_share(share_proto=proto) mock_rest = self._create_mocked_rest_api() mock_rest.view_policies.ensure.return_value = driver_util.Bunch(id=1) @@ -168,7 +179,11 @@ def test_create_shares(self, capacity, proto, policy): mock_rest.views.ensure.return_value = driver_util.Bunch( id=3, policy=policy ) - mock_rest.vip_pools.vips.return_value = ["1.1.1.0", "1.1.1.1"] + # Mock vip_pools.one() to return pool with ip_ranges and tenant_id + mock_rest.vip_pools.one.return_value = driver_util.Bunch( + ip_ranges=[["1.1.1.0", "1.1.1.1"]], + tenant_id=123 + ) with mock.patch.object(self._driver, "rest", mock_rest): if proto != "NFS": with self.assertRaises(exception.InvalidShare) as exc: @@ -187,20 +202,26 @@ def test_create_shares(self, capacity, proto, policy): else: location = self._driver.create_share(self._context, share) - mock_rest.vip_pools.vips.assert_called_once_with( - pool_name="vippool" + mock_rest.vip_pools.one.assert_called_once_with( + name="vippool", + fail_if_missing=True, ) mock_rest.view_policies.ensure.assert_called_once_with( - name="fakeid" + name="fakeid", + tenant_id=123, ) mock_rest.quotas.ensure.assert_called_once_with( name="fakeid", path="/fake/manila-fakeid", create_dir=True, hard_limit=capacity, + tenant_id=123, ) mock_rest.views.ensure.assert_called_once_with( - name="fakeid", path="/fake/manila-fakeid", policy_id=1 + name="fakeid", + path="/fake/manila-fakeid", + policy_id=1, + tenant_id=123, ) self.assertListEqual( location, @@ -592,15 +613,47 @@ def test_resize_share_exceeded_hard_limit(self): self._driver.shrink_share(share, 9.7) self._driver.shrink_share(share, 10) - def test_create_snapshot(self): + @mock.patch('manila.share.share_types.get_extra_specs_from_share') + def test_create_snapshot(self, mock_get_extra_specs): + mock_get_extra_specs.return_value = {} + share = fake_share.fake_share(share_proto="NFS") snapshot = driver_util.Bunch( - name="fakesnap", share_instance_id="fakeid" + name="fakesnap", share_instance_id="fakeid", share=share ) mock_rest = self._create_mocked_rest_api() + mock_rest.vip_pools.one.return_value = driver_util.Bunch( + tenant_id=123 + ) with mock.patch.object(self._driver, "rest", mock_rest): self._driver.create_snapshot(self._context, snapshot, None) mock_rest.snapshots.create.assert_called_once_with( - path="/fake/manila-fakeid", name="fakesnap" + path="/fake/manila-fakeid", name="fakesnap", tenant_id=123 + ) + + @mock.patch('manila.share.share_types.get_extra_specs_from_share') + def test_create_snapshot_with_custom_vippool(self, mock_get_extra_specs): + """Test snapshot creation with custom vippool from extra specs.""" + mock_get_extra_specs.return_value = { + 'vast:vippool_name': 'custom-vippool' + } + share = fake_share.fake_share(share_proto="NFS") + snapshot = driver_util.Bunch( + name="fakesnap", share_instance_id="fakeid", share=share + ) + mock_rest = self._create_mocked_rest_api() + mock_rest.vip_pools.one.return_value = driver_util.Bunch( + tenant_id=456 + ) + with mock.patch.object(self._driver, "rest", mock_rest): + self._driver.create_snapshot(self._context, snapshot, None) + + # Verify that custom-vippool was used + mock_rest.vip_pools.one.assert_called_once_with( + name="custom-vippool", + fail_if_missing=True, + ) + mock_rest.snapshots.create.assert_called_once_with( + path="/fake/manila-fakeid", name="fakesnap", tenant_id=456 ) def test_delete_snapshot(self): @@ -616,7 +669,9 @@ def test_network_allocation_number(self): self.assertEqual(self._driver.get_network_allocations_number(), 0) @ddt.data([], ['fake/path/1', 'fake/path']) - def test_ensure_shares(self, fake_export_locations): + @mock.patch('manila.share.share_types.get_extra_specs_from_share') + def test_ensure_shares(self, fake_export_locations, mock_get_extra_specs): + mock_get_extra_specs.return_value = {} mock_rest = self._create_mocked_rest_api() mock_rest.view_policies.ensure.return_value = driver_util.Bunch(id=1) mock_rest.quotas.ensure.return_value = driver_util.Bunch( @@ -634,7 +689,10 @@ def test_ensure_shares(self, fake_export_locations): ) for _id, share_id in enumerate(["123", "456", "789"], 1) ] - mock_rest.vip_pools.vips.return_value = ["1.1.1.0", "1.1.1.1"] + mock_rest.vip_pools.one.return_value = driver_util.Bunch( + ip_ranges=[["1.1.1.0", "1.1.1.1"]], + tenant_id=123 + ) with mock.patch.object(self._driver, "rest", mock_rest): locations = self._driver.ensure_shares(self._context, shares) @@ -670,6 +728,281 @@ def test_backend_info(self): {'vast_vippool_name': 'vippool', 'vast_mgmt_host': 'test'} ) + @mock.patch('manila.share.share_types.get_extra_specs_from_share') + def test_create_share_with_extra_specs_vippool(self, mock_get_specs): + """Test that vippool_name from extra specs overrides config.""" + share = fake_share.fake_share( + share_proto="NFS", + share_type_id="fake-type-id", + ) + mock_get_specs.return_value = { + 'vast:vippool_name': 'custom-vippool' + } + + mock_rest = self._create_mocked_rest_api() + mock_rest.view_policies.ensure.return_value = driver_util.Bunch(id=1) + mock_rest.quotas.ensure.return_value = driver_util.Bunch( + id=2, hard_limit=1073741824 + ) + mock_rest.views.ensure.return_value = driver_util.Bunch( + id=3, policy="fakeid" + ) + mock_rest.vip_pools.one.return_value = driver_util.Bunch( + ip_ranges=[["2.2.2.0", "2.2.2.1"]], + tenant_id=456 + ) + + with mock.patch.object(self._driver, "rest", mock_rest): + location = self._driver.create_share(self._context, share) + + # Verify that custom-vippool was used instead of config's vippool + mock_rest.vip_pools.one.assert_called_once_with( + name="custom-vippool", + fail_if_missing=True, + ) + self.assertEqual(len(location), 2) + self.assertEqual( + location[0]['path'], '2.2.2.0:/fake/manila-fakeid' + ) + self.assertEqual( + location[1]['path'], '2.2.2.1:/fake/manila-fakeid' + ) + + @mock.patch('manila.share.share_types.get_extra_specs_from_share') + def test_create_share_without_extra_specs_uses_config( + self, mock_get_specs + ): + """Test that config vippool_name is used when no extra specs.""" + share = fake_share.fake_share( + share_proto="NFS", + share_type_id="fake-type-id", + ) + mock_get_specs.return_value = {} + + mock_rest = self._create_mocked_rest_api() + mock_rest.view_policies.ensure.return_value = driver_util.Bunch(id=1) + mock_rest.quotas.ensure.return_value = driver_util.Bunch( + id=2, hard_limit=1073741824 + ) + mock_rest.views.ensure.return_value = driver_util.Bunch( + id=3, policy="fakeid" + ) + mock_rest.vip_pools.one.return_value = driver_util.Bunch( + ip_ranges=[["1.1.1.0", "1.1.1.1"]], + tenant_id=123 + ) + + with mock.patch.object(self._driver, "rest", mock_rest): + self._driver.create_share(self._context, share) + + # Verify that config's vippool was used + mock_rest.vip_pools.one.assert_called_once_with( + name="vippool", + fail_if_missing=True, + ) + + @mock.patch('manila.share.share_types.get_extra_specs_from_share') + def test_create_share_without_vippool_raises_error(self, mock_get_specs): + """Test that error is raised when vippool_name is not available.""" + mock_get_specs.return_value = {} + # Create driver without vippool_name in config + self.fake_conf.set_default("vast_vippool_name", None) + _driver = driver.VASTShareDriver( + execute=mock.MagicMock(), configuration=self.fake_conf + ) + _driver.do_setup(self._context) + + # Create share without vippool in config or extra specs + share = fake_share.fake_share( + share_proto="NFS", + ) + + with self.assertRaises(exception.VastDriverException) as exc: + _driver._ensure_share(share) + + self.assertIn( + "VIP pool name must be specified", str(exc.exception) + ) + + @mock.patch('manila.share.share_types.get_extra_specs_from_share') + def test_create_share_with_empty_vip_pool_raises_error( + self, + mock_get_specs, + ): + """Test that error is raised when vip pool has no available IPs.""" + mock_get_specs.return_value = {} + share = fake_share.fake_share(share_proto="NFS") + + mock_rest = self._create_mocked_rest_api() + # Mock vip_pools.one() to return pool with empty ip_ranges + mock_rest.vip_pools.one.return_value = driver_util.Bunch( + name="vippool", + ip_ranges=[], # No IP ranges + tenant_id=123 + ) + + with mock.patch.object(self._driver, "rest", mock_rest): + with self.assertRaises(exception.VastDriverException) as exc: + self._driver.create_share(self._context, share) + + self.assertIn("has no available vips", str(exc.exception)) + + @mock.patch('manila.share.share_types.get_extra_specs_from_share') + def test_get_vip_pool_for_share_from_extra_specs(self, mock_get_specs): + """Test _get_vip_pool_for_share using extra specs.""" + mock_get_specs.return_value = {'vast:vippool_name': 'custom-pool'} + share = fake_share.fake_share(share_proto="NFS") + + mock_rest = self._create_mocked_rest_api() + mock_vippool = driver_util.Bunch( + name="custom-pool", + ip_ranges=[["1.1.1.1", "1.1.1.2"]], + tenant_id=456 + ) + mock_rest.vip_pools.one.return_value = mock_vippool + + with mock.patch.object(self._driver, "rest", mock_rest): + result = self._driver._get_vip_pool_for_share(share) + + self.assertEqual(result, mock_vippool) + mock_rest.vip_pools.one.assert_called_once_with( + name="custom-pool", + fail_if_missing=True, + ) + + @mock.patch('manila.share.share_types.get_extra_specs_from_share') + def test_get_vip_pool_for_share_from_config(self, mock_get_specs): + """Test _get_vip_pool_for_share using config default.""" + mock_get_specs.return_value = {} + share = fake_share.fake_share(share_proto="NFS") + + mock_rest = self._create_mocked_rest_api() + mock_vippool = driver_util.Bunch( + name="vippool", + ip_ranges=[["2.2.2.1", "2.2.2.2"]], + tenant_id=789 + ) + mock_rest.vip_pools.one.return_value = mock_vippool + + with mock.patch.object(self._driver, "rest", mock_rest): + result = self._driver._get_vip_pool_for_share(share) + + self.assertEqual(result, mock_vippool) + mock_rest.vip_pools.one.assert_called_once_with( + name="vippool", + fail_if_missing=True, + ) + + @mock.patch('manila.share.share_types.get_extra_specs_from_share') + def test_get_tenant_id_for_share(self, mock_get_specs): + """Test _get_tenant_id_for_share returns correct tenant_id.""" + mock_get_specs.return_value = {} + share = fake_share.fake_share(share_proto="NFS") + + mock_rest = self._create_mocked_rest_api() + mock_rest.vip_pools.one.return_value = driver_util.Bunch( + name="vippool", + tenant_id=999 + ) + + with mock.patch.object(self._driver, "rest", mock_rest): + tenant_id = self._driver._get_tenant_id_for_share(share) + + self.assertEqual(tenant_id, 999) + + +class TestExtraSpecsParsing(unittest.TestCase): + """Test extra specs parsing functions.""" + + def test_get_opts_from_specs_with_vast_namespace(self): + """Test parsing extra specs with vast: namespace.""" + specs = { + 'vast:vippool_name': 'pool-1', + 'capabilities:dedupe': 'true', + 'other:setting': 'value', + } + opts = driver_util.get_opts_from_specs(specs) + self.assertEqual(opts['vippool_name'], 'pool-1') + + def test_get_opts_from_specs_case_insensitive(self): + """Test that namespace and key are case insensitive.""" + specs = { + 'VAST:VipPool_Name': 'pool-1', + 'Vast:VIPPOOL_NAME': 'pool-2', # Should be ignored, first one wins + } + opts = driver_util.get_opts_from_specs(specs) + # First matching key should be used + self.assertIn(opts['vippool_name'], ['pool-1', 'pool-2']) + + def test_get_opts_from_specs_without_namespace(self): + """Test that options without namespace are ignored.""" + specs = { + 'vippool_name': 'pool-1', + 'other_setting': 'value', + } + opts = driver_util.get_opts_from_specs(specs) + self.assertIsNone(opts['vippool_name']) + + def test_get_opts_from_specs_with_invalid_format(self): + """Test that invalid format keys are skipped.""" + specs = { + 'vast:vippool_name:extra': 'pool-1', # Too many colons + 'vast:vippool_name': 'pool-2', + } + opts = driver_util.get_opts_from_specs(specs) + self.assertEqual(opts['vippool_name'], 'pool-2') + + def test_get_opts_from_specs_empty(self): + """Test parsing empty specs returns defaults.""" + specs = {} + opts = driver_util.get_opts_from_specs(specs) + self.assertIsNone(opts['vippool_name']) + + def test_get_opts_from_specs_with_empty_scope(self): + """Test that keys with empty scope are handled correctly.""" + specs = { + ':vippool_name': 'pool-1', # Empty scope + 'vast:vippool_name': 'pool-2', + } + opts = driver_util.get_opts_from_specs(specs) + # Empty scope should be skipped, so pool-2 should be used + self.assertEqual(opts['vippool_name'], 'pool-2') + + def test_get_opts_from_specs_with_empty_option_key(self): + """Test that keys with empty option_key are handled correctly.""" + specs = { + 'vast:': 'some-value', # Empty option_key + 'vast:vippool_name': 'pool-1', + } + opts = driver_util.get_opts_from_specs(specs) + # Empty option_key should be skipped, so pool-1 should be used + self.assertEqual(opts['vippool_name'], 'pool-1') + + @mock.patch('manila.share.share_types.get_extra_specs_from_share') + def test_get_share_extra_specs_params(self, mock_get_specs): + """Test get_share_extra_specs_params function.""" + mock_get_specs.return_value = { + 'vast:vippool_name': 'pool-1', + } + + share = {'share_type_id': 'fake-type-id'} + opts = driver_util.get_share_extra_specs_params(share) + + self.assertEqual(opts['vippool_name'], 'pool-1') + mock_get_specs.assert_called_once_with(share) + + @mock.patch('manila.share.share_types.get_extra_specs_from_share') + def test_get_share_extra_specs_params_no_extra_specs(self, mock_get_specs): + """Test get_share_extra_specs_params with no extra specs.""" + mock_get_specs.return_value = {} + + share = {'share_type_id': 'fake-type-id'} + opts = driver_util.get_share_extra_specs_params(share) + + # Should return default VAST extra specs when no vast: specs provided + self.assertEqual(opts, {'vippool_name': None}) + mock_get_specs.assert_called_once_with(share) + class TestPolicyPayloadFromRules(unittest.TestCase): def test_policy_payload_from_rules_update(self): diff --git a/manila/tests/share/drivers/vastdata/test_rest.py b/manila/tests/share/drivers/vastdata/test_rest.py index 5235023966..02f5848b80 100644 --- a/manila/tests/share/drivers/vastdata/test_rest.py +++ b/manila/tests/share/drivers/vastdata/test_rest.py @@ -194,6 +194,81 @@ def test_getattr_without_underscore(self, mock_request): self.session.__getattr__(attr)(**params) mock_request.assert_called_once_with("get", attr, params=params) + @mock.patch("requests.Session.request") + def test_request_with_pagination_envelope(self, mock_request): + """Test that paginated responses are unwrapped.""" + # Mock a paginated response + paginated_response = { + "count": 2, + "results": [ + {"id": 1, "name": "Item 1"}, + {"id": 2, "name": "Item 2"}, + ], + "next": None, + "previous": None, + } + + mock_response = mock.MagicMock() + mock_response.status_code = 200 + mock_response.content = b"response content" + mock_response.json.return_value = paginated_response + mock_response.raise_for_status.return_value = None + mock_request.return_value = mock_response + + result = self.session.request("GET", "test_method", log_result=False) + + # Result should be a list of Bunch objects (unwrapped from envelope) + self.assertIsInstance(result, list) + self.assertEqual(len(result), 2) + self.assertIsInstance(result[0], driver_util.Bunch) + self.assertEqual(result[0]["id"], 1) + self.assertEqual(result[1]["id"], 2) + + @mock.patch("requests.Session.request") + def test_request_without_pagination_envelope(self, mock_request): + """Test that non-paginated responses work as before.""" + # Mock a non-paginated response (flat list) + non_paginated_response = [ + {"id": 1, "name": "Item 1"}, + {"id": 2, "name": "Item 2"}, + ] + + mock_response = mock.MagicMock() + mock_response.status_code = 200 + mock_response.content = b"response content" + mock_response.json.return_value = non_paginated_response + mock_response.raise_for_status.return_value = None + mock_request.return_value = mock_response + + result = self.session.request("GET", "test_method", log_result=False) + + # Non-paginated responses should work as before - list of Bunches + self.assertIsInstance(result, list) + self.assertEqual(len(result), 2) + self.assertIsInstance(result[0], driver_util.Bunch) + self.assertEqual(result[0]["id"], 1) + self.assertEqual(result[1]["id"], 2) + + @mock.patch("requests.Session.request") + def test_request_with_single_dict(self, mock_request): + """Test that single dict responses work correctly.""" + # Mock a single dict response (not pagination envelope) + dict_response = {"id": 1, "name": "Single Item"} + + mock_response = mock.MagicMock() + mock_response.status_code = 200 + mock_response.content = b"response content" + mock_response.json.return_value = dict_response + mock_response.raise_for_status.return_value = None + mock_request.return_value = mock_response + + result = self.session.request("GET", "test_method", log_result=False) + + # Should return a Bunch with the dict data + self.assertIsInstance(result, driver_util.Bunch) + self.assertEqual(result["id"], 1) + self.assertEqual(result["name"], "Single Item") + class TestVastResource(unittest.TestCase): def setUp(self): @@ -221,21 +296,20 @@ def test_update_with_provided_params(self): def test_delete_when_entry_not_found(self): self.vast_resource.one = mock.MagicMock(return_value=None) - self.vast_resource.delete("test") + self.vast_resource.delete(name="test") self.mock_rest.session.delete.assert_not_called() def test_delete_when_entry_found(self): - mock_entry = mock.MagicMock() - mock_entry.id = "1" + mock_entry = {"id": "1", "name": "test"} self.vast_resource.one = mock.MagicMock(return_value=mock_entry) - self.vast_resource.delete("test") + self.vast_resource.delete(name="test") self.mock_rest.session.delete.assert_called_with( - f"{self.vast_resource.resource_name}/{mock_entry.id}" + f"{self.vast_resource.resource_name}/1" ) def test_one_when_no_entries_found(self): self.vast_resource.list = mock.MagicMock(return_value=[]) - result = self.vast_resource.one("test") + result = self.vast_resource.one(name="test") self.assertIsNone(result) def test_one_when_multiple_entries_found(self): @@ -243,14 +317,19 @@ def test_one_when_multiple_entries_found(self): return_value=[mock.MagicMock(), mock.MagicMock()] ) with self.assertRaises(manila_exception.VastDriverException): - self.vast_resource.one("test") + self.vast_resource.one(name="test") def test_one_when_single_entry_found(self): mock_entry = mock.MagicMock() self.vast_resource.list = mock.MagicMock(return_value=[mock_entry]) - result = self.vast_resource.one("test") + result = self.vast_resource.one(name="test") self.assertEqual(result, mock_entry) + def test_one_not_found_with_fail_if_missing(self): + self.vast_resource.list = mock.MagicMock(return_value=[]) + with self.assertRaises(manila_exception.VastDriverException): + self.vast_resource.one(name="test", fail_if_missing=True) + def test_ensure_when_entry_not_found(self): self.vast_resource.one = mock.MagicMock(return_value=None) mock_entry = mock.MagicMock() @@ -264,6 +343,15 @@ def test_ensure_when_entry_found(self): result = self.vast_resource.ensure("test", size=10) self.assertEqual(result, mock_entry) + def test_get_entry_by_id(self): + """Test getting a single entry by id.""" + entry_id = "123" + self.vast_resource.get(entry_id, param1="value1") + self.mock_rest.session.get.assert_called_with( + f"{self.vast_resource.resource_name}/{entry_id}", + params={"param1": "value1"} + ) + class ViewTest(unittest.TestCase): @mock.patch( @@ -282,7 +370,44 @@ def test_view_create(self): True, "1.0" ) - rest_api.views.create("test-view", "/test", 1) + rest_api.views.create(name="test-view", path="/test", policy_id=1) + + self.assertEqual(("views",), mock_session.call_args.args) + self.assertDictEqual( + { + "data": { + "name": "test-view", + "path": "/test", + "policy_id": 1, + "create_dir": True, + "protocols": ["NFS"], + } + }, + mock_session.call_args.kwargs, + ) + + @mock.patch( + "manila.share.drivers.vastdata.rest.Session.refresh_auth_token", + mock.MagicMock() + ) + def test_view_create_with_tenant_id(self): + with mock.patch( + "manila.share.drivers.vastdata.rest.Session.post" + ) as mock_session: + rest_api = vast_rest.RestApi( + "host", + "username", + "password", + "", + True, + "1.0" + ) + rest_api.views.create( + name="test-view", + path="/test", + policy_id=1, + tenant_id=123, + ) self.assertEqual(("views",), mock_session.call_args.args) self.assertDictEqual( @@ -293,6 +418,7 @@ def test_view_create(self): "policy_id": 1, "create_dir": True, "protocols": ["NFS"], + "tenant_id": 123, } }, mock_session.call_args.kwargs, @@ -509,6 +635,83 @@ def test_vips_ok(self): vips = self.rest_api.vip_pools.vips("test-vip") self.assertListEqual(vips, expected) + def test_one_with_fail_if_missing(self): + """Test that one() raises exception when fail_if_missing=True.""" + with mock.patch( + "manila.share.drivers.vastdata.rest.Session.get", + return_value=[] + ): + with self.assertRaises( + manila_exception.VastDriverException + ) as exc: + self.rest_api.vip_pools.one( + name="nonexistent-pool", + fail_if_missing=True + ) + self.assertIn("No 'vippool' found", str(exc.exception)) + + def test_vippool_one_caching(self): + """Test that VipPool.one() caches results.""" + vippool = driver_util.Bunch( + name="test-pool", + tenant_id=123, + ip_ranges=[["1.1.1.1", "1.1.1.2"]] + ) + + # Clear any existing cache + if hasattr(self.rest_api.vip_pools.one, 'cache'): + self.rest_api.vip_pools.one.cache.clear() + + with mock.patch( + "manila.share.drivers.vastdata.rest.Session.get", + return_value=[vippool] + ) as mock_get: + # First call - should hit the API + result1 = self.rest_api.vip_pools.one(name="test-pool") + self.assertEqual(result1.name, "test-pool") + self.assertEqual(result1.tenant_id, 123) + self.assertEqual(mock_get.call_count, 1) + + # Second call with same params - should use cache + result2 = self.rest_api.vip_pools.one(name="test-pool") + self.assertEqual(result2.name, "test-pool") + self.assertEqual(result2.tenant_id, 123) + # Call count should still be 1 (cached) + self.assertEqual(mock_get.call_count, 1) + + # Call with different params - should hit the API again + self.rest_api.vip_pools.one(name="other-pool") + self.assertEqual(mock_get.call_count, 2) + + def test_vippool_one_cache_ttl(self): + """Test that VipPool.one() cache expires after TTL.""" + + vippool1 = driver_util.Bunch( + name="test-pool", + tenant_id=123 + ) + driver_util.Bunch( + name="test-pool", + tenant_id=456 # Different tenant_id + ) + + # Clear any existing cache + if hasattr(self.rest_api.vip_pools.one, 'cache'): + self.rest_api.vip_pools.one.cache.clear() + + with mock.patch( + "manila.share.drivers.vastdata.rest.Session.get" + ) as mock_get: + # First call returns tenant_id 123 + mock_get.return_value = [vippool1] + result1 = self.rest_api.vip_pools.one(name="test-pool") + self.assertEqual(result1.tenant_id, 123) + + # Immediate second call should use cache (tenant_id still 123) + result2 = self.rest_api.vip_pools.one(name="test-pool") + self.assertEqual(result2.tenant_id, 123) + self.assertEqual(mock_get.call_count, 1) + class TestRestApi(unittest.TestCase): @@ -527,3 +730,17 @@ def test_get_sw_version(self, mock_session): ) version = rest_api.get_sw_version() self.assertEqual(version, "1.0") + + def test_api_token_initialization(self): + rest_api = vast_rest.RestApi( + "host", + "", + "", + "xxxxxxxxx", + True, + "1.0", + ) + self.assertEqual( + rest_api.session.headers["authorization"], + "Api-Token xxxxxxxxx", + ) diff --git a/releasenotes/notes/vastdata-multitenancy-6936c95f94213548.yaml b/releasenotes/notes/vastdata-multitenancy-6936c95f94213548.yaml new file mode 100644 index 0000000000..4a30c8c6f9 --- /dev/null +++ b/releasenotes/notes/vastdata-multitenancy-6936c95f94213548.yaml @@ -0,0 +1,11 @@ +--- +features: + - | + The VAST Manila driver enhances multitenancy via share types using + namespaced extra specs. Administrators can specify different VIP pools + per share type using the ``vast:vippool_name`` extra spec, enabling + network isolation between different tenants or projects. This allows + multiple share types to use different VIP pools while sharing the same + Manila backend. The configuration parameter ``vast_vippool_name`` in + ``manila.conf`` now serves as a default fallback and is no longer + strictly required when VIP pools are specified via share type extra specs.