Context manager to handle shared_targets

We introduced "shared_targets" and "service_uuid" fields in volumes to
allow volume consumers to protect themselves from unintended leftover
devices when handling iSCSI connections with shared targets.

The way they protect themselves from the automatic scans that happen on
detach/map race conditions is by locking and only allowing one attach or
one detach operation for each server to happen at a given time.

When using an up to date Open iSCSI initiator we don't need to use
locks, as it has the possibility to disable automatic LUN scans (which
are the real cause of the leftover devices), and OS-Brick already
supports this feature.

This is currently not the case, since Nova is blindly locking whenever
"shared_targets" is set to True.

Thanks to the context manager introduced in this patch we can improve
our separation of concerns (Nova doesn't have to care when we have to
lock), and we can fine tune the locking to only lock when the iSCSI
initiator doesn't support disabling automatic scans.

The context manager "guard_connection" lives in
"os_brick.initiator.utils" and needs a cinder volume.  The volume can be
passed as a dictionary or OVO instance.

Change-Id: I4970363301d5d1f4e7d0f07e09b34d15ee6884c3
Related-Bug: #1800515
This commit is contained in:
Gorka Eguileor 2018-10-29 18:51:39 +01:00
parent 633e3dd344
commit 71a1e22418
4 changed files with 121 additions and 0 deletions

View File

@ -32,6 +32,7 @@ from os_brick.i18n import _
from os_brick import initiator
from os_brick.initiator.connectors import base
from os_brick.initiator.connectors import base_iscsi
from os_brick.initiator import utils as initiator_utils
from os_brick import utils
synchronized = lockutils.synchronized_with_prefix('os-brick-')
@ -1030,6 +1031,11 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector):
'node.session.scan', 'manual',
check_exit_code=False)
manual_scan = not res[1]
# Update global indicator of manual scan support used for
# shared_targets locking so we support upgrading open iscsi to a
# version supporting the manual scan feature without restarting Nova
# or Cinder.
initiator_utils.ISCSI_SUPPORTS_MANUAL_SCAN = manual_scan
if connection_properties.get('auth_method'):
self._iscsiadm_update(connection_properties,

View File

@ -0,0 +1,46 @@
# Copyright 2018 Red Hat, Inc.
# All Rights Reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# 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 contextlib
import os
from oslo_concurrency import lockutils
from oslo_concurrency import processutils as putils
def check_manual_scan():
if os.name == 'nt':
return False
try:
putils.execute('grep', '-F', 'node.session.scan', '/sbin/iscsiadm')
except putils.ProcessExecutionError:
return False
return True
ISCSI_SUPPORTS_MANUAL_SCAN = check_manual_scan()
@contextlib.contextmanager
def guard_connection(device):
"""Context Manager handling locks for attach/detach operations."""
if ISCSI_SUPPORTS_MANUAL_SCAN or not device.get('shared_targets'):
yield
else:
# Cinder passes an OVO, but Nova passes a dictionary, so we use dict
# key access that works with both.
with lockutils.lock(device['service_uuid'], 'os-brick-'):
yield

View File

@ -21,6 +21,7 @@ from oslo_concurrency import processutils as putils
from os_brick import exception
from os_brick.initiator.connectors import iscsi
from os_brick.initiator import linuxscsi
from os_brick.initiator import utils
from os_brick.privileged import rootwrap as priv_rootwrap
from os_brick.tests.initiator import test_connector
@ -987,6 +988,7 @@ Setting up iSCSI targets: unused
[('tcp:', 'session1', 'ip1:port1', '1', 'tgt'),
('tcp:', session, 'ip1:port1', '-1', 'tgt1')]
]
utils.ISCSI_SUPPORTS_MANUAL_SCAN = None
with mock.patch.object(self.connector, '_execute') as exec_mock:
exec_mock.side_effect = [('', 'error'), ('', None),
('', None), ('', None),
@ -996,6 +998,7 @@ Setting up iSCSI targets: unused
# True refers to "manual scans", since the call to update
# node.session.scan didn't fail they are set to manual
self.assertEqual((session, True), res)
self.assertTrue(utils.ISCSI_SUPPORTS_MANUAL_SCAN)
prefix = 'iscsiadm -m node -T tgt1 -p ip1:port1'
expected_cmds = [
prefix,
@ -1018,6 +1021,7 @@ Setting up iSCSI targets: unused
[('tcp:', 'session1', 'Ip1:port1', '1', 'tgt'),
('tcp:', session, 'IP1:port1', '-1', 'tgt1')]
]
utils.ISCSI_SUPPORTS_MANUAL_SCAN = None
with mock.patch.object(self.connector, '_execute') as exec_mock:
exec_mock.side_effect = [('', 'error'), ('', None),
('', None), ('', None),
@ -1027,6 +1031,7 @@ Setting up iSCSI targets: unused
# True refers to "manual scans", since the call to update
# node.session.scan didn't fail they are set to manual
self.assertEqual((session, True), res)
self.assertTrue(utils.ISCSI_SUPPORTS_MANUAL_SCAN)
prefix = 'iscsiadm -m node -T tgt1 -p ip1:port1'
expected_cmds = [
prefix,
@ -1048,9 +1053,11 @@ Setting up iSCSI targets: unused
con_props = self.CON_PROPS.copy()
con_props.update(auth_method='CHAP', auth_username='user',
auth_password='pwd')
utils.ISCSI_SUPPORTS_MANUAL_SCAN = None
res = self.connector._connect_to_iscsi_portal(con_props)
# False refers to "manual scans", so we have manual iscsi scans
self.assertEqual((session, True), res)
self.assertTrue(utils.ISCSI_SUPPORTS_MANUAL_SCAN)
prefix = 'iscsiadm -m node -T tgt1 -p ip1:port1'
expected_cmds = [
prefix,

View File

@ -0,0 +1,62 @@
# Copyright 2018 Red Hat, Inc.
# All Rights Reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# 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 mock
from os_brick.initiator import utils
from os_brick.tests import base
class InitiatorUtilsTestCase(base.TestCase):
@mock.patch('os.name', 'nt')
def test_check_manual_scan_windows(self):
self.assertFalse(utils.check_manual_scan())
@mock.patch('os.name', 'posix')
@mock.patch('oslo_concurrency.processutils.execute')
def test_check_manual_scan_supported(self, mock_exec):
self.assertTrue(utils.check_manual_scan())
mock_exec.assert_called_once_with('grep', '-F', 'node.session.scan',
'/sbin/iscsiadm')
@mock.patch('os.name', 'posix')
@mock.patch('oslo_concurrency.processutils.execute',
side_effect=utils.putils.ProcessExecutionError)
def test_check_manual_scan_not_supported(self, mock_exec):
self.assertFalse(utils.check_manual_scan())
mock_exec.assert_called_once_with('grep', '-F', 'node.session.scan',
'/sbin/iscsiadm')
@mock.patch('oslo_concurrency.lockutils.lock')
def test_guard_connection_manual_scan_support(self, mock_lock):
utils.ISCSI_SUPPORTS_MANUAL_SCAN = True
# We confirm that shared_targets is ignored
with utils.guard_connection({'shared_targets': True}):
mock_lock.assert_not_called()
@mock.patch('oslo_concurrency.lockutils.lock')
def test_guard_connection_manual_scan_unsupported_not_shared(self,
mock_lock):
utils.ISCSI_SUPPORTS_MANUAL_SCAN = False
with utils.guard_connection({'shared_targets': False}):
mock_lock.assert_not_called()
@mock.patch('oslo_concurrency.lockutils.lock')
def test_guard_connection_manual_scan_unsupported_hared(self, mock_lock):
utils.ISCSI_SUPPORTS_MANUAL_SCAN = False
with utils.guard_connection({'service_uuid': mock.sentinel.uuid,
'shared_targets': True}):
mock_lock.assert_called_once_with(mock.sentinel.uuid, 'os-brick-')