openstack/cinder
Code Issues Proposed changes

PowerFlex: Limit image volume cache clones

Browse Source 
Add a new powerflex_max_image_cache_vtree_size option that can be
tuned to adjust when a PowerFlex image volume cache entry has too
many vtree snaps to support another clone. When the threshold is
reached, the image volume cache entry is deleted and a new one is
seeded. The default value (0) preserves the current threshold,
which is the PowerFlex backend's vtree snapshot limit.

Anthropic's Claude Code v2.0.27 (Sonnet 4.5) assisted with changes
that introduced the new config option, the unit tests and the
release note. The code and text suggested by Claude was reworked
to meet the actual requirements.

Signed-off-by: Alan Bishop <abishop@redhat.com>
Assisted-by: Claude (AI Assistant) claude@anthropic.com
Change-Id: I881dffe82650ef2f3078fc1750ade4f57e1ebd1d
This commit is contained in:
Alan Bishop
2025-10-30 17:28:29 -07:00
parent c36f40684e
commit 3ac19bfea8
7 changed files with 314 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

60
cinder/tests/unit/test_volume_utils.py
View File

@@ -33,6 +33,7 @@ from cinder import db
from cinder.db.sqlalchemy import models
from cinder import exception
from cinder.objects import fields
from cinder.objects import volume as volume_object
from cinder.tests.unit.backup import fake_backup
from cinder.tests.unit import fake_constants as fake
from cinder.tests.unit import fake_group
@@ -1306,6 +1307,65 @@ class VolumeUtilsTestCase(test.TestCase):
volume_utils.require_driver_initialized,
driver)
@mock.patch('cinder.db.image_volume_cache_get_by_volume_id')
def test_is_image_cache_entry_true(self, mock_cache_get):
"""Test volume is an image cache entry when cache entry exists."""
mock_cache_get.return_value = {'id': 1, 'volume_id': fake.VOLUME_ID}
ctx = context.get_admin_context()
volume = fake_volume.fake_volume_obj(ctx)
result = volume_utils.is_image_cache_entry(volume)
self.assertTrue(result)
mock_cache_get.assert_called_once_with(ctx, volume.id)
@mock.patch('cinder.db.image_volume_cache_get_by_volume_id')
def test_is_image_cache_entry_false(self, mock_cache_get):
"""Test volume isn't an image cache entry when entry doesn't exist."""
mock_cache_get.return_value = None
ctx = context.get_admin_context()
volume = fake_volume.fake_volume_obj(ctx)
result = volume_utils.is_image_cache_entry(volume)
self.assertFalse(result)
mock_cache_get.assert_called_once_with(ctx, volume.id)
@mock.patch('cinder.db.image_volume_cache_get_by_volume_id')
def test_is_image_cache_entry_exception(self, mock_cache_get):
"""Test volume returns False when database error occurs."""
mock_cache_get.side_effect = Exception("Database error")
ctx = context.get_admin_context()
volume = fake_volume.fake_volume_obj(ctx)
result = volume_utils.is_image_cache_entry(volume)
self.assertFalse(result)
mock_cache_get.assert_called_once_with(ctx, volume.id)
def test_is_image_cache_entry_no_context(self):
"""Test volume returns False when no context is available."""
volume = volume_object.Volume()
result = volume_utils.is_image_cache_entry(volume)
self.assertFalse(result)
def test_is_image_cache_entry_not_volume_object(self):
"""Test returns False when input is not a Volume object."""
ctx = context.get_admin_context()
snapshot = fake_snapshot.fake_snapshot_obj(ctx)
result = volume_utils.is_image_cache_entry(snapshot)
self.assertFalse(result)
def test_is_image_cache_entry_none_input(self):
"""Test returns False when input is None."""
result = volume_utils.is_image_cache_entry(None)
self.assertFalse(result)
@ddt.ddt
class LogTracingTestCase(test.TestCase):

176
cinder/tests/unit/volume/drivers/dell_emc/powerflex/test_create_cloned_volume.py
View File

@@ -14,6 +14,7 @@
# under the License.
import json
from unittest import mock
import urllib.parse
from cinder import context
@@ -22,6 +23,8 @@ from cinder.tests.unit import fake_constants as fake
from cinder.tests.unit import fake_volume
from cinder.tests.unit.volume.drivers.dell_emc import powerflex
from cinder.tests.unit.volume.drivers.dell_emc.powerflex import mocks
from cinder.volume import configuration
from cinder.volume.drivers.dell_emc.powerflex import options
from cinder.volume.drivers.dell_emc.powerflex import utils as flex_utils
@@ -105,3 +108,176 @@ class TestCreateClonedVolume(powerflex.TestPowerFlexDriver):
self.set_https_response_mode(self.RESPONSE_MODE.Valid)
self.new_volume.size = 2
self.driver.create_cloned_volume(self.new_volume, self.src_volume)
@mock.patch('cinder.volume.drivers.dell_emc.powerflex.driver.'
'PowerFlexDriver._create_volume_from_source')
def test_create_cloned_volume_not_image_cache(self, mock_create):
"""Test cloning when source volume is not an image cache entry."""
self.set_https_response_mode(self.RESPONSE_MODE.Valid)
mock_create.return_value = {}
# Mock volume_utils to indicate source volume is not an image cache
# entry
with mock.patch('cinder.volume.volume_utils.is_image_cache_entry',
return_value=False):
self.driver.create_cloned_volume(self.new_volume, self.src_volume)
# Should proceed directly to _create_volume_from_source
mock_create.assert_called_once_with(self.new_volume, self.src_volume)
@mock.patch('cinder.volume.drivers.dell_emc.powerflex.driver.'
'PowerFlexDriver._create_volume_from_source')
def test_create_cloned_volume_image_cache_clone_limit_disabled(
self, mock_create):
"""Test cloning when clone limit is disabled (set to 0)."""
self.set_https_response_mode(self.RESPONSE_MODE.Valid)
mock_create.return_value = {}
# Set clone limit to 0 (disabled)
self.override_config(options.POWERFLEX_MAX_IMAGE_CACHE_VTREE_SIZE, 0,
configuration.SHARED_CONF_GROUP)
# Mock volume_utils to indicate source volume is an image cache entry
with mock.patch('cinder.volume.volume_utils.is_image_cache_entry',
return_value=True):
self.driver.create_cloned_volume(self.new_volume, self.src_volume)
# Should proceed directly to _create_volume_from_source
mock_create.assert_called_once_with(self.new_volume, self.src_volume)
@mock.patch('cinder.volume.drivers.dell_emc.powerflex.driver.'
'PowerFlexDriver._create_volume_from_source')
@mock.patch('cinder.volume.drivers.dell_emc.powerflex.driver.'
'PowerFlexDriver._get_client')
def test_create_cloned_volume_image_cache_within_limit(self, mock_client,
mock_create):
"""Test cloning image cache volume when within clone limit."""
self.set_https_response_mode(self.RESPONSE_MODE.Valid)
mock_create.return_value = {}
# Set clone limit to 20
self.override_config(options.POWERFLEX_MAX_IMAGE_CACHE_VTREE_SIZE, 20,
configuration.SHARED_CONF_GROUP)
# Mock REST client and vtree statistics response
mock_rest_client = mock.Mock()
mock_client.return_value = mock_rest_client
mock_rest_client.query_volume.return_value = {
'vtreeId': 'test_vtree_id'
}
mock_rest_client.query_vtree_statistics.return_value = {
'numOfVolumes': '10' # Within limit
}
# Mock volume_utils to indicate source volume is an image cache entry
with mock.patch('cinder.volume.volume_utils.is_image_cache_entry',
return_value=True):
self.driver.create_cloned_volume(self.new_volume, self.src_volume)
# Should query volume and vtree statistics and proceed to create
mock_rest_client.query_volume.assert_called_once_with(
self.src_volume.provider_id)
mock_rest_client.query_vtree_statistics.assert_called_once_with(
'test_vtree_id')
mock_create.assert_called_once_with(self.new_volume, self.src_volume)
@mock.patch('cinder.volume.drivers.dell_emc.powerflex.driver.'
'PowerFlexDriver._get_client')
def test_create_cloned_volume_image_cache_limit_reached(self, mock_client):
"""Test cloning image cache volume when clone limit is reached."""
self.set_https_response_mode(self.RESPONSE_MODE.Valid)
# Set clone limit to 20
self.override_config(options.POWERFLEX_MAX_IMAGE_CACHE_VTREE_SIZE, 20,
configuration.SHARED_CONF_GROUP)
# Mock REST client and vtree statistics response
mock_rest_client = mock.Mock()
mock_client.return_value = mock_rest_client
mock_rest_client.query_volume.return_value = {
'vtreeId': 'test_vtree_id'
}
mock_rest_client.query_vtree_statistics.return_value = {
'numOfVolumes': '20' # At limit
}
# Mock volume_utils to indicate source volume is an image cache entry
with mock.patch('cinder.volume.volume_utils.is_image_cache_entry',
return_value=True):
self.assertRaises(exception.SnapshotLimitReached,
self.driver.create_cloned_volume,
self.new_volume, self.src_volume)
# Should query volume and vtree statistics but fail before create
mock_rest_client.query_volume.assert_called_once_with(
self.src_volume.provider_id)
mock_rest_client.query_vtree_statistics.assert_called_once_with(
'test_vtree_id')
@mock.patch('cinder.volume.drivers.dell_emc.powerflex.driver.'
'PowerFlexDriver._create_volume_from_source')
@mock.patch('cinder.volume.drivers.dell_emc.powerflex.driver.'
'PowerFlexDriver._get_client')
@mock.patch('cinder.volume.drivers.dell_emc.powerflex.driver.LOG')
def test_create_cloned_volume_image_cache_stats_query_fails(
self, mock_log, mock_client, mock_create):
"""Test cloning when volume statistics query fails."""
self.set_https_response_mode(self.RESPONSE_MODE.Valid)
mock_create.return_value = {}
# Set clone limit to 20
self.override_config(options.POWERFLEX_MAX_IMAGE_CACHE_VTREE_SIZE, 20,
configuration.SHARED_CONF_GROUP)
# Mock REST client to raise exception on query_volume
mock_rest_client = mock.Mock()
mock_client.return_value = mock_rest_client
mock_rest_client.query_volume.side_effect = (
exception.VolumeBackendAPIException(data="Query failed"))
# Mock volume_utils to indicate source volume is an image cache entry
with mock.patch('cinder.volume.volume_utils.is_image_cache_entry',
return_value=True):
self.driver.create_cloned_volume(self.new_volume, self.src_volume)
# Should attempt query, log warning, and proceed to create
mock_rest_client.query_volume.assert_called_once_with(
self.src_volume.provider_id)
mock_log.warning.assert_called_once()
mock_create.assert_called_once_with(self.new_volume, self.src_volume)
@mock.patch('cinder.volume.drivers.dell_emc.powerflex.driver.'
'PowerFlexDriver._create_volume_from_source')
@mock.patch('cinder.volume.drivers.dell_emc.powerflex.driver.'
'PowerFlexDriver._get_client')
def test_create_cloned_volume_image_cache_missing_stats_field(
self, mock_client, mock_create):
"""Test cloning when statistics response missing expected field."""
self.set_https_response_mode(self.RESPONSE_MODE.Valid)
mock_create.return_value = {}
# Set clone limit to 20
self.override_config(options.POWERFLEX_MAX_IMAGE_CACHE_VTREE_SIZE, 20,
configuration.SHARED_CONF_GROUP)
# Mock REST client with response missing numOfVolumes
mock_rest_client = mock.Mock()
mock_client.return_value = mock_rest_client
mock_rest_client.query_volume.return_value = {
'vtreeId': 'test_vtree_id'
}
mock_rest_client.query_vtree_statistics.return_value = {
'otherField': 'value' # Missing numOfVolumes
}
# Mock volume_utils to indicate source volume is an image cache entry
with mock.patch('cinder.volume.volume_utils.is_image_cache_entry',
return_value=True):
self.driver.create_cloned_volume(self.new_volume, self.src_volume)
# Should query volume and vtree statistics, default to 0, proceed
mock_rest_client.query_volume.assert_called_once_with(
self.src_volume.provider_id)
mock_rest_client.query_vtree_statistics.assert_called_once_with(
'test_vtree_id')
mock_create.assert_called_once_with(self.new_volume, self.src_volume)

18
cinder/volume/drivers/dell_emc/powerflex/driver.py
View File

@@ -838,6 +838,24 @@ class PowerFlexDriver(driver.VolumeDriver):
LOG.info("Clone volume %(vol_id)s to %(target_vol_id)s.",
{"vol_id": src_vref.id, "target_vol_id": volume.id})
# Check if source volume is an image cache entry and under the max
# vTree size limit
max_size = self.configuration.powerflex_max_image_cache_vtree_size
if max_size > 0 and volume_utils.is_image_cache_entry(src_vref):
client = self._get_client()
try:
volume_info = client.query_volume(src_vref.provider_id)
vtree_id = volume_info["vtreeId"]
vtree_stats = client.query_vtree_statistics(vtree_id)
num_volumes = int(vtree_stats.get("numOfVolumes", "0"))
if num_volumes >= max_size:
raise exception.SnapshotLimitReached(set_limit=max_size)
except exception.VolumeBackendAPIException:
LOG.warning("Failed to query volume statistics for image "
"cache volume %(vol_id)s. Proceeding with clone.",
{"vol_id": src_vref.id})
return self._create_volume_from_source(volume, src_vref)
def delete_volume(self, volume):

11
cinder/volume/drivers/dell_emc/powerflex/options.py
View File

@@ -19,6 +19,7 @@ named Dell EMC VxFlex OS).
from oslo_config import cfg
from cinder.volume.drivers.dell_emc.powerflex import rest_client
from cinder.volume.drivers.dell_emc.powerflex import utils as flex_utils
# deprecated options
@@ -43,6 +44,7 @@ POWERFLEX_MAX_OVER_SUBSCRIPTION_RATIO = "powerflex_max_over_subscription_ratio"
POWERFLEX_ALLOW_NON_PADDED_VOLUMES = "powerflex_allow_non_padded_volumes"
POWERFLEX_ALLOW_MIGRATION_DURING_REBUILD = (
"powerflex_allow_migration_during_rebuild")
POWERFLEX_MAX_IMAGE_CACHE_VTREE_SIZE = "powerflex_max_image_cache_vtree_size"
deprecated_opts = [
cfg.PortOpt(VXFLEXOS_REST_SERVER_PORT,
@@ -156,5 +158,12 @@ actual_opts = [
cfg.IntOpt(flex_utils.POWERFLEX_REST_READ_TIMEOUT,
default=30, min=1,
help='Use this value to specify read '
'timeout value (in seconds) for rest call.')
'timeout value (in seconds) for rest call.'),
cfg.IntOpt(POWERFLEX_MAX_IMAGE_CACHE_VTREE_SIZE,
default=0, min=0, max=rest_client.MAX_SNAPS_IN_VTREE,
help='Maximum size of the vTree associated with an entry in '
'the image volume cache. When the size is exceeded, '
'the cache entry will be replaced with one created from '
'a new vTree. A value of 0 means the size is limited by '
'the PowerFlex vTree snapshot limit.')
]

12
cinder/volume/drivers/dell_emc/powerflex/rest_client.py
View File

@@ -707,6 +707,18 @@ class RestClient(object):
raise exception.VolumeBackendAPIException(msg)
return response
def query_vtree_statistics(self, vtree_id):
url = "/instances/VTree::%(vtree_id)s/relationships/Statistics"
r, response = self.execute_powerflex_get_request(url,
vtree_id=vtree_id)
if r.status_code != http_client.OK and "errorCode" in response:
msg = (_("Failed to query vtree statistics: %s.") %
response["message"])
LOG.error(msg)
raise exception.VolumeBackendAPIException(data=msg)
return response
def migrate_vtree(self, volume, params):
url = "/instances/Volume::%(vol_id)s/action/migrateVTree"

26
cinder/volume/volume_utils.py
View File

@@ -1643,3 +1643,29 @@ def log_unsupported_driver_warning(driver):
def is_all_zero(chunk: bytes) -> bool:
"""Return true if the chunk of bytes is all zeroes."""
return chunk == bytes(len(chunk))
def is_image_cache_entry(vol_or_snap) -> bool:
"""Check if this volume or snapshot is an image cache entry.
Returns True if the volume is used for image caching, False otherwise.
Image cache volumes are tracked in the image_volume_cache_entries
table.
:param vol_or_snap: volume or snapshot object to check
:return: True if it's an image cache entry, False otherwise
"""
# Only volumes can be image cache entries
if not isinstance(vol_or_snap, objects.Volume):
return False
# Need a context to query the database
if not hasattr(vol_or_snap, '_context') or not vol_or_snap._context:
return False
try:
cache_entry = db.image_volume_cache_get_by_volume_id(
vol_or_snap._context, vol_or_snap.id)
return cache_entry is not None
except Exception:
return False

12
releasenotes/notes/powerflex-image-cache-clone-limits-a3f8e9c12b8d6ea4.yaml Normal file
View File

@@ -0,0 +1,12 @@
---
features:
- |
Dell PowerFlex driver: Added support for limiting the number of clones
from image cache volumes. A new configuration option
``powerflex_max_image_cache_vtree_size`` controls the maximum number of
clones that can be created from a single image cache volume. This helps
prevent reaching PowerFlex vTree snapshot limits when multiple instances
are booted from the same cached image. When set to 0 (default is 0),
the maximum number of clones is limited only by the PowerFlex vTree
snapshot limit. When the vTree size limit is reached, the existing cache
entry is deleted and a fresh cache entry is created.