Merge "Add CHAP support to Dell EMC PowerStore driver"

This commit is contained in:
Zuul 2020-12-15 14:00:41 +00:00 committed by Gerrit Code Review
commit 108c554dc3
11 changed files with 228 additions and 64 deletions

View File

@ -20,9 +20,11 @@ from cinder.tests.unit.volume.drivers.dell_emc import powerstore
class TestBase(powerstore.TestPowerStoreDriver):
@mock.patch("cinder.volume.drivers.dell_emc.powerstore.client."
"PowerStoreClient.get_chap_config")
@mock.patch("cinder.volume.drivers.dell_emc.powerstore.client."
"PowerStoreClient.get_appliance_id_by_name")
def test_configuration(self, mock_appliance):
def test_configuration(self, mock_appliance, mock_chap):
mock_appliance.return_value = "A1"
self.driver.check_for_setup_error()
@ -50,11 +52,16 @@ class TestBase(powerstore.TestPowerStoreDriver):
self.driver.check_for_setup_error)
self.assertIn("Failed to query PowerStore appliances.", error.msg)
@mock.patch("cinder.volume.drivers.dell_emc.powerstore.client."
"PowerStoreClient.get_chap_config")
@mock.patch("cinder.volume.drivers.dell_emc.powerstore.client."
"PowerStoreClient.get_appliance_id_by_name")
@mock.patch("cinder.volume.drivers.dell_emc.powerstore.client."
"PowerStoreClient.get_appliance_metrics")
def test_update_volume_stats(self, mock_metrics, mock_appliance):
def test_update_volume_stats(self,
mock_metrics,
mock_appliance,
mock_chap):
mock_appliance.return_value = "A1"
mock_metrics.return_value = {
"physical_total": 2147483648,
@ -63,12 +70,15 @@ class TestBase(powerstore.TestPowerStoreDriver):
self.driver.check_for_setup_error()
self.driver._update_volume_stats()
@mock.patch("cinder.volume.drivers.dell_emc.powerstore.client."
"PowerStoreClient.get_chap_config")
@mock.patch("cinder.volume.drivers.dell_emc.powerstore.client."
"PowerStoreClient.get_appliance_id_by_name")
@mock.patch("requests.request")
def test_update_volume_stats_bad_status(self,
mock_metrics,
mock_appliance):
mock_appliance,
mock_chap):
mock_appliance.return_value = "A1"
mock_metrics.return_value = powerstore.MockResponse(rc=400)
self.driver.check_for_setup_error()

View File

@ -22,9 +22,11 @@ from cinder.tests.unit.volume.drivers.dell_emc import powerstore
class TestSnapshotCreateDelete(powerstore.TestPowerStoreDriver):
@mock.patch("cinder.volume.drivers.dell_emc.powerstore.client."
"PowerStoreClient.get_chap_config")
@mock.patch("cinder.volume.drivers.dell_emc.powerstore.client."
"PowerStoreClient.get_appliance_id_by_name")
def setUp(self, mock_appliance):
def setUp(self, mock_appliance, mock_chap):
super(TestSnapshotCreateDelete, self).setUp()
mock_appliance.return_value = "A1"
self.driver.check_for_setup_error()

View File

@ -24,11 +24,14 @@ from cinder.volume.drivers.dell_emc.powerstore import utils
class TestVolumeAttachDetach(powerstore.TestPowerStoreDriver):
@mock.patch("cinder.volume.drivers.dell_emc.powerstore.client."
"PowerStoreClient.get_chap_config")
@mock.patch("cinder.volume.drivers.dell_emc.powerstore.client."
"PowerStoreClient.get_appliance_id_by_name")
def setUp(self, mock_appliance):
def setUp(self, mock_appliance, mock_chap):
super(TestVolumeAttachDetach, self).setUp()
mock_appliance.return_value = "A1"
mock_chap.return_value = {"mode": "Single"}
self.iscsi_driver.check_for_setup_error()
self.fc_driver.check_for_setup_error()
self.volume = fake_volume.fake_volume_obj(
@ -50,7 +53,7 @@ class TestVolumeAttachDetach(powerstore.TestPowerStoreDriver):
attached_host=self.volume.host
)
]
self.fake_iscsi_targets_response = [
fake_iscsi_targets_response = [
{
"address": "1.2.3.4",
"ip_port": {
@ -66,7 +69,7 @@ class TestVolumeAttachDetach(powerstore.TestPowerStoreDriver):
},
},
]
self.fake_fc_wwns_response = [
fake_fc_wwns_response = [
{
"wwn": "58:cc:f0:98:49:21:07:02"
},
@ -79,18 +82,52 @@ class TestVolumeAttachDetach(powerstore.TestPowerStoreDriver):
"wwpns": ["58:cc:f0:98:49:21:07:02", "58:cc:f0:98:49:23:07:02"],
"initiator": "fake_initiator",
}
self.iscsi_targets_mock = self.mock_object(
self.iscsi_driver.adapter.client,
"get_ip_pool_address",
return_value=fake_iscsi_targets_response
)
self.fc_wwns_mock = self.mock_object(
self.fc_driver.adapter.client,
"get_fc_port",
return_value=fake_fc_wwns_response
)
@mock.patch("cinder.volume.drivers.dell_emc.powerstore.client."
"PowerStoreClient.get_fc_port")
def test_get_fc_targets(self, mock_get_ip_pool):
mock_get_ip_pool.return_value = self.fake_fc_wwns_response
def test_initialize_connection_chap_enabled(self):
self.iscsi_driver.adapter.use_chap_auth = True
with mock.patch.object(self.iscsi_driver.adapter,
"_create_host_and_attach",
return_value=(
utils.get_chap_credentials(),
1
)):
connection_properties = self.iscsi_driver.initialize_connection(
self.volume,
self.fake_connector
)
self.assertIn("auth_username", connection_properties["data"])
self.assertIn("auth_password", connection_properties["data"])
def test_initialize_connection_chap_disabled(self):
self.iscsi_driver.adapter.use_chap_auth = False
with mock.patch.object(self.iscsi_driver.adapter,
"_create_host_and_attach",
return_value=(
utils.get_chap_credentials(),
1
)):
connection_properties = self.iscsi_driver.initialize_connection(
self.volume,
self.fake_connector
)
self.assertNotIn("auth_username", connection_properties["data"])
self.assertNotIn("auth_password", connection_properties["data"])
def test_get_fc_targets(self):
wwns = self.fc_driver.adapter._get_fc_targets("A1")
self.assertEqual(2, len(wwns))
@mock.patch("cinder.volume.drivers.dell_emc.powerstore.client."
"PowerStoreClient.get_fc_port")
def test_get_fc_targets_filtered(self, mock_get_ip_pool):
mock_get_ip_pool.return_value = self.fake_fc_wwns_response
def test_get_fc_targets_filtered(self):
self.fc_driver.adapter.allowed_ports = ["58:cc:f0:98:49:23:07:02"]
wwns = self.fc_driver.adapter._get_fc_targets("A1")
self.assertEqual(1, len(wwns))
@ -98,10 +135,7 @@ class TestVolumeAttachDetach(powerstore.TestPowerStoreDriver):
utils.fc_wwn_to_string("58:cc:f0:98:49:21:07:02") in wwns
)
@mock.patch("cinder.volume.drivers.dell_emc.powerstore.client."
"PowerStoreClient.get_fc_port")
def test_get_fc_targets_filtered_no_matched_ports(self, mock_get_ip_pool):
mock_get_ip_pool.return_value = self.fake_fc_wwns_response
def test_get_fc_targets_filtered_no_matched_ports(self):
self.fc_driver.adapter.allowed_ports = ["fc_wwn_1", "fc_wwn_2"]
error = self.assertRaises(exception.VolumeBackendAPIException,
self.fc_driver.adapter._get_fc_targets,
@ -109,18 +143,12 @@ class TestVolumeAttachDetach(powerstore.TestPowerStoreDriver):
self.assertIn("There are no accessible Fibre Channel targets on the "
"system.", error.msg)
@mock.patch("cinder.volume.drivers.dell_emc.powerstore.client."
"PowerStoreClient.get_ip_pool_address")
def test_get_iscsi_targets(self, mock_get_ip_pool):
mock_get_ip_pool.return_value = self.fake_iscsi_targets_response
def test_get_iscsi_targets(self):
iqns, portals = self.iscsi_driver.adapter._get_iscsi_targets("A1")
self.assertTrue(len(iqns) == len(portals))
self.assertEqual(2, len(portals))
@mock.patch("cinder.volume.drivers.dell_emc.powerstore.client."
"PowerStoreClient.get_ip_pool_address")
def test_get_iscsi_targets_filtered(self, mock_get_ip_pool):
mock_get_ip_pool.return_value = self.fake_iscsi_targets_response
def test_get_iscsi_targets_filtered(self):
self.iscsi_driver.adapter.allowed_ports = ["1.2.3.4"]
iqns, portals = self.iscsi_driver.adapter._get_iscsi_targets("A1")
self.assertTrue(len(iqns) == len(portals))
@ -129,11 +157,7 @@ class TestVolumeAttachDetach(powerstore.TestPowerStoreDriver):
"iqn.2020-07.com.dell:dellemc-powerstore-test-iqn-2" in iqns
)
@mock.patch("cinder.volume.drivers.dell_emc.powerstore.client."
"PowerStoreClient.get_ip_pool_address")
def test_get_iscsi_targets_filtered_no_matched_ports(self,
mock_get_ip_pool):
mock_get_ip_pool.return_value = self.fake_iscsi_targets_response
def test_get_iscsi_targets_filtered_no_matched_ports(self):
self.iscsi_driver.adapter.allowed_ports = ["1.1.1.1", "2.2.2.2"]
error = self.assertRaises(exception.VolumeBackendAPIException,
self.iscsi_driver.adapter._get_iscsi_targets,

View File

@ -22,9 +22,11 @@ from cinder.volume.drivers.dell_emc.powerstore import client
class TestVolumeCreateDeleteExtend(powerstore.TestPowerStoreDriver):
@mock.patch("cinder.volume.drivers.dell_emc.powerstore.client."
"PowerStoreClient.get_chap_config")
@mock.patch("cinder.volume.drivers.dell_emc.powerstore.client."
"PowerStoreClient.get_appliance_id_by_name")
def setUp(self, mock_appliance):
def setUp(self, mock_appliance, mock_chap):
super(TestVolumeCreateDeleteExtend, self).setUp()
mock_appliance.return_value = "A1"
self.driver.check_for_setup_error()

View File

@ -22,9 +22,11 @@ from cinder.tests.unit.volume.drivers.dell_emc import powerstore
class TestVolumeCreateFromSource(powerstore.TestPowerStoreDriver):
@mock.patch("cinder.volume.drivers.dell_emc.powerstore.client."
"PowerStoreClient.get_chap_config")
@mock.patch("cinder.volume.drivers.dell_emc.powerstore.client."
"PowerStoreClient.get_appliance_id_by_name")
def setUp(self, mock_appliance):
def setUp(self, mock_appliance, mock_chap):
super(TestVolumeCreateFromSource, self).setUp()
mock_appliance.return_value = "A1"
self.driver.check_for_setup_error()

View File

@ -16,6 +16,7 @@
"""Adapter for Dell EMC PowerStore Cinder driver."""
from oslo_log import log as logging
from oslo_utils import strutils
from cinder import coordination
from cinder import exception
@ -30,6 +31,7 @@ from cinder.volume import volume_utils
LOG = logging.getLogger(__name__)
PROTOCOL_FC = "FC"
PROTOCOL_ISCSI = "iSCSI"
CHAP_MODE_SINGLE = "Single"
class CommonAdapter(object):
@ -41,6 +43,7 @@ class CommonAdapter(object):
self.configuration = configuration
self.storage_protocol = None
self.allowed_ports = None
self.use_chap_auth = None
@staticmethod
def initiators(connector):
@ -83,13 +86,20 @@ class CommonAdapter(object):
self.appliances_to_ids_map[appliance_name] = (
self.client.get_appliance_id_by_name(appliance_name)
)
self.use_chap_auth = False
if self.storage_protocol == PROTOCOL_ISCSI:
chap_config = self.client.get_chap_config()
if chap_config.get("mode") == CHAP_MODE_SINGLE:
self.use_chap_auth = True
LOG.debug("Successfully initialized PowerStore %(protocol)s adapter. "
"PowerStore appliances: %(appliances)s. "
"Allowed ports: %(allowed_ports)s.",
"Allowed ports: %(allowed_ports)s. "
"Use CHAP authentication: %(use_chap_auth)s.",
{
"protocol": self.storage_protocol,
"appliances": self.appliances,
"allowed_ports": self.allowed_ports,
"use_chap_auth": self.use_chap_auth,
})
def create_volume(self, volume):
@ -314,7 +324,9 @@ class CommonAdapter(object):
{
"volume_name": volume.name,
"volume_id": volume.id,
"connection_properties": connection_properties,
"connection_properties": strutils.mask_password(
connection_properties
),
})
return connection_properties
@ -429,13 +441,17 @@ class CommonAdapter(object):
"""Create PowerStore host if it does not exist.
:param connector: connection properties
:return: PowerStore host object
:return: PowerStore host object, iSCSI CHAP credentials
"""
initiators = self.initiators(connector)
host = self._filter_hosts_by_initiators(initiators)
if self.use_chap_auth:
chap_credentials = utils.get_chap_credentials()
else:
chap_credentials = {}
if host:
self._modify_host_initiators(host, initiators)
self._modify_host_initiators(host, chap_credentials, initiators)
else:
host_name = utils.powerstore_host_name(
connector,
@ -451,6 +467,7 @@ class CommonAdapter(object):
{
"port_name": initiator,
"port_type": self.storage_protocol,
**chap_credentials,
} for initiator in initiators
]
host = self.client.create_host(host_name, ports)
@ -463,12 +480,13 @@ class CommonAdapter(object):
"initiators": initiators,
"host_provider_id": host["id"],
})
return host
return host, chap_credentials
def _modify_host_initiators(self, host, initiators):
def _modify_host_initiators(self, host, chap_credentials, initiators):
"""Update PowerStore host initiators if needed.
:param host: PowerStore host object
:param chap_credentials: iSCSI CHAP credentials
:param initiators: list of initiators
:return: None
"""
@ -476,17 +494,22 @@ class CommonAdapter(object):
initiators_added = [
initiator["port_name"] for initiator in host["host_initiators"]
]
initiators_to_add = []
initiators_to_modify = []
initiators_to_remove = [
initiator for initiator in initiators_added
if initiator not in initiators
]
initiators_to_add = [
{
for initiator in initiators:
initiator_add_modify = {
"port_name": initiator,
"port_type": self.storage_protocol,
} for initiator in initiators
if initiator not in initiators_added
]
**chap_credentials,
}
if initiator not in initiators_added:
initiator_add_modify["port_type"] = self.storage_protocol
initiators_to_add.append(initiator_add_modify)
elif self.use_chap_auth:
initiators_to_modify.append(initiator_add_modify)
if initiators_to_remove:
LOG.debug("Remove initiators from PowerStore host %(host_name)s. "
"Initiators: %(initiators_to_remove)s. "
@ -514,7 +537,9 @@ class CommonAdapter(object):
"%(host_provider_id)s.",
{
"host_name": host["name"],
"initiators_to_add": initiators_to_add,
"initiators_to_add": strutils.mask_password(
initiators_to_add
),
"host_provider_id": host["id"],
})
self.client.modify_host_initiators(
@ -526,7 +551,34 @@ class CommonAdapter(object):
"PowerStore host id: %(host_provider_id)s.",
{
"host_name": host["name"],
"initiators_to_add": initiators_to_add,
"initiators_to_add": strutils.mask_password(
initiators_to_add
),
"host_provider_id": host["id"],
})
if initiators_to_modify:
LOG.debug("Modify initiators of PowerStore host %(host_name)s. "
"Initiators: %(initiators_to_modify)s. "
"PowerStore host id: %(host_provider_id)s.",
{
"host_name": host["name"],
"initiators_to_modify": strutils.mask_password(
initiators_to_modify
),
"host_provider_id": host["id"],
})
self.client.modify_host_initiators(
host["id"],
modify_initiators=initiators_to_modify
)
LOG.debug("Successfully modified initiators of PowerStore host "
"%(host_name)s. Initiators: %(initiators_to_modify)s. "
"PowerStore host id: %(host_provider_id)s.",
{
"host_name": host["name"],
"initiators_to_modify": strutils.mask_password(
initiators_to_modify
),
"host_provider_id": host["id"],
})
@ -572,11 +624,11 @@ class CommonAdapter(object):
:param connector: connection properties
:param volume: OpenStack volume object
:return: attached volume logical number
:return: iSCSI CHAP credentials, volume logical number
"""
host = self._create_host_if_not_exist(connector)
return self._attach_volume_to_host(host, volume)
host, chap_credentials = self._create_host_if_not_exist(connector)
return chap_credentials, self._attach_volume_to_host(host, volume)
def _connect_volume(self, volume, connector):
"""Attach PowerStore volume and return it's connection properties.
@ -588,12 +640,21 @@ class CommonAdapter(object):
appliance_name = volume_utils.extract_host(volume.host, "pool")
appliance_id = self.appliances_to_ids_map[appliance_name]
volume_lun = self._create_host_and_attach(
chap_credentials, volume_lun = self._create_host_and_attach(
connector,
volume
)
return self._get_connection_properties(appliance_id,
volume_lun)
connection_properties = self._get_connection_properties(appliance_id,
volume_lun)
if self.use_chap_auth:
connection_properties["data"]["auth_method"] = "CHAP"
connection_properties["data"]["auth_username"] = (
chap_credentials.get("chap_single_username")
)
connection_properties["data"]["auth_password"] = (
chap_credentials.get("chap_single_password")
)
return connection_properties
def _detach_volume_from_hosts(self, volume, hosts_to_detach=None):
"""Detach volume from PowerStore hosts.
@ -731,7 +792,7 @@ class FibreChannelAdapter(CommonAdapter):
return {
"driver_volume_type": self.driver_volume_type,
"data": {
"target_discovered": True,
"target_discovered": False,
"target_lun": volume_lun,
"target_wwn": target_wwns,
}
@ -782,7 +843,10 @@ class iSCSIAdapter(CommonAdapter):
return {
"driver_volume_type": self.driver_volume_type,
"data": {
"target_discovered": True,
"target_discovered": False,
"target_portal": portals[0],
"target_iqn": iqns[0],
"target_lun": volume_lun,
"target_portals": portals,
"target_iqns": iqns,
"target_luns": [volume_lun] * len(portals),

View File

@ -19,6 +19,7 @@ import functools
import json
from oslo_log import log as logging
from oslo_utils import strutils
import requests
from cinder import exception
@ -83,7 +84,12 @@ class PowerStoreClient(object):
"verify_cert": self._verify_cert,
})
def _send_request(self, method, url, payload=None, params=None):
def _send_request(self,
method,
url,
payload=None,
params=None,
log_response_data=True):
if not payload:
payload = {}
if not params:
@ -106,11 +112,12 @@ class PowerStoreClient(object):
"REST Request: %s %s with body %s",
r.request.method,
r.request.url,
r.request.body)
LOG.log(log_level,
"REST Response: %s with data %s",
r.status_code,
r.text)
strutils.mask_password(r.request.body))
if log_response_data or log_level == logging.ERROR:
msg = "REST Response: %s with data %s" % (r.status_code, r.text)
else:
msg = "REST Response: %s" % r.status_code
LOG.log(log_level, msg)
try:
response = r.json()
@ -123,6 +130,19 @@ class PowerStoreClient(object):
_send_patch_request = functools.partialmethod(_send_request, "PATCH")
_send_delete_request = functools.partialmethod(_send_request, "DELETE")
def get_chap_config(self):
r, response = self._send_get_request(
"/chap_config/0",
params={
"select": "mode"
}
)
if r.status_code not in self.ok_codes:
msg = _("Failed to query PowerStore CHAP configuration.")
LOG.error(msg)
raise exception.VolumeBackendAPIException(data=msg)
return response
def get_appliance_id_by_name(self, appliance_name):
r, response = self._send_get_request(
"/appliance",
@ -148,7 +168,8 @@ class PowerStoreClient(object):
payload={
"entity": "space_metrics_by_appliance",
"entity_id": appliance_id,
}
},
log_response_data=False
)
if r.status_code not in self.ok_codes:
msg = (_("Failed to query metrics for "

View File

@ -37,9 +37,10 @@ class PowerStoreDriver(driver.VolumeDriver):
Version history:
1.0.0 - Initial version
1.0.1 - Add CHAP support
"""
VERSION = "1.0.0"
VERSION = "1.0.1"
VENDOR = "Dell EMC"
# ThirdPartySystems wiki page

View File

@ -23,9 +23,12 @@ from oslo_utils import units
from cinder import exception
from cinder.i18n import _
from cinder.objects import fields
from cinder.volume import volume_utils
LOG = logging.getLogger(__name__)
CHAP_DEFAULT_USERNAME = "PowerStore_iSCSI_CHAP_Username"
CHAP_DEFAULT_SECRET_LENGTH = 60
def bytes_to_gib(size_in_bytes):
@ -134,3 +137,17 @@ def is_multiattached_to_host(volume_attachment, host_name):
attachment.attached_host == host_name)
]
return len(attachments) > 1
def get_chap_credentials():
"""Generate CHAP credentials.
:return: CHAP username and secret
"""
return {
"chap_single_username": CHAP_DEFAULT_USERNAME,
"chap_single_password": volume_utils.generate_password(
CHAP_DEFAULT_SECRET_LENGTH
)
}

View File

@ -77,3 +77,19 @@ Thin provisioning and compression
The driver creates thin provisioned compressed volumes by default.
Thick provisioning is not supported.
CHAP authentication support
~~~~~~~~~~~~~~~~~~~~~~~~~~~
The driver supports one-way (Single mode) CHAP authentication.
To use CHAP authentication CHAP Single mode has to be enabled on the storage
side.
.. note:: When enabling CHAP, any previously added hosts will need to be updated
with CHAP configuration since there will be I/O disruption for those hosts.
It is recommended that before adding hosts to the cluster,
decide what type of CHAP configuration is required, if any.
CHAP configuration is retrieved from the storage during driver initialization,
no additional configuration is needed.
Secrets are generated automatically.

View File

@ -0,0 +1,5 @@
---
fixes:
- |
`Bug #1900979 <https://bugs.launchpad.net/cinder/+bug/1900979>`_:
Fix bug with using PowerStore with enabled CHAP as a storage backend.