Skip to content

Commit

Permalink
Revert "CA-379329: check for missing iSCSI sessions and reconnect" (#65)
Browse files Browse the repository at this point in the history 
This reverts commit d28dcc1.

Prevent bad disconnection of ISCSi targets

In practice the commit that we revert is useful in the case where ISCSI connections are no longer valid,
but sometimes even without any config change or bad connections,
a disconnect/connect is launched preventing the correct use of the SR.

This change is a workaround, we don't have a better solution at this time.

Signed-off-by: Benjamin Reis <[email protected]>
  • Loading branch information
@benjamreis
benjamreis committed Aug 13, 2024
1 parent db0a6cb commit d2d42c1
Show file tree
Hide file tree
Showing 8 changed files with 227 additions and 563 deletions.
5 changes: 0 additions & 5 deletions Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ SM_LIBS += pluginutil
SM_LIBS += fcoelib
SM_LIBS += constants
SM_LIBS += cbtutil
SM_LIBS += sr_health_check

UDEV_RULES = 65-multipath 55-xs-mpath-scsidev 57-usb 58-xapi
MPATH_DAEMON = sm-multipath
Expand Down Expand Up @@ -184,10 +183,6 @@ install: precheck
$(SM_STAGING)/$(SYSTEMD_SERVICE_DIR)
install -m 644 systemd/storage-init.service \
$(SM_STAGING)/$(SYSTEMD_SERVICE_DIR)
install -m 644 systemd/sr_health_check.service \
$(SM_STAGING)/$(SYSTEMD_SERVICE_DIR)
install -m 644 systemd/sr_health_check.timer \
$(SM_STAGING)/$(SYSTEMD_SERVICE_DIR)
install -m 644 systemd/[email protected] \
$(SM_STAGING)/$(SYSTEMD_SERVICE_DIR)
install -m 644 systemd/linstor-monitor.service \
Expand Down
2 changes: 1 addition & 1 deletion drivers/BaseISCSI.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ def load(self, sr_uuid):
self.default_vdi_visibility = False

# Required parameters
if 'target' not in self.dconf or not self.dconf['target']:
if 'target' not in self.dconf or not self.dconf['target']:
raise xs_errors.XenError('ConfigTargetMissing')

# we are no longer putting hconf in the xml.
Expand Down
472 changes: 226 additions & 246 deletions drivers/LVHDoISCSISR.py
View file

Large diffs are not rendered by default.

4 changes: 0 additions & 4 deletions drivers/SR.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -357,10 +357,6 @@ def load(self, sr_uuid):
"""Post-init hook"""
pass

def check_sr(self, sr_uuid):
"""Hook to check SR health"""
pass

def vdi(self, uuid):
"""Return VDI object owned by this repository"""
if uuid not in self.vdis:
Expand Down
8 changes: 0 additions & 8 deletions systemd/sr_health_check.service
View file

This file was deleted.

13 changes: 0 additions & 13 deletions systemd/sr_health_check.timer
View file

This file was deleted.

193 changes: 0 additions & 193 deletions tests/test_LVHDoISCSISR.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
import unittest
import unittest.mock as mock

import traceback

from uuid import uuid4

import SR
Expand All @@ -18,8 +16,6 @@
from shared_iscsi_test_base import ISCSITestCase
from test_ISCSISR import NonInitingISCSISR

TEST_SR_UUID = 'test_uuid'


class RandomError(Exception):
pass
Expand Down Expand Up @@ -118,192 +114,3 @@ def test_1st_try_block_raise_RandomError(
str(cm.exception),
'General backend error [opterr=Raise RandomError]'
)


class TestLVHDoISCSISR(ISCSITestCase):

TEST_CLASS = 'LVHDoISCSISR'

def setUp(self):
util_patcher = mock.patch('LVHDoISCSISR.util', autospec=True)
self.mock_util = util_patcher.start()
# self.mock_util.SMlog.side_effect = print
self.mock_util.isVDICommand = util.isVDICommand
self.mock_util.sessions_less_than_targets = util.sessions_less_than_targets

self.base_srs = set()
baseiscsi_patcher = mock.patch('LVHDoISCSISR.BaseISCSI.BaseISCSISR',
autospec=True)
patched_baseiscsi = baseiscsi_patcher.start()
patched_baseiscsi.side_effect = self.baseiscsi
lvhdsr_patcher = mock.patch ('LVHDoISCSISR.LVHDSR')

self.mock_lvhdsr = lvhdsr_patcher.start()
self.mock_session = mock.MagicMock()
xenapi_patcher = mock.patch('SR.XenAPI')
mock_xenapi = xenapi_patcher.start()
mock_xenapi.xapi_local.return_value = self.mock_session

copy_patcher = mock.patch('LVHDoISCSISR.SR.copy.deepcopy')
self.mock_copy = copy_patcher.start()

def deepcopy(to_copy):
return to_copy

self.mock_copy.side_effect = deepcopy

lock_patcher = mock.patch('LVHDSR.Lock')
self.mock_lock = lock_patcher.start()
lvlock_patcher = mock.patch('LVHDSR.lvutil.Fairlock')
self.mock_lvlock = lvlock_patcher.start()

self.addCleanup(mock.patch.stopall)

super().setUp()

@property
def mock_baseiscsi(self):
assert len(self.base_srs) == 1
single_sr = None
for sr in self.base_srs:
single_sr = sr

return single_sr

def baseiscsi(self, srcmd, sr_uuid):
new_baseiscsi = mock.create_autospec(BaseISCSISR)
local_iqn = srcmd.dconf['localIQN']
target_iqn = srcmd.dconf['targetIQN']
target = srcmd.dconf['target']
new_baseiscsi.localIQN = local_iqn
new_baseiscsi.targetIQN = target_iqn
new_baseiscsi.target = target
new_baseiscsi.path = os.path.join('/dev/iscsi', target_iqn, target)
new_baseiscsi.port = 3260
new_baseiscsi.chapuser = srcmd.dconf.get('chapuser')
new_baseiscsi.chappassword = srcmd.dconf.get('chappassword')
new_baseiscsi.incoming_chapuser = srcmd.dconf.get('incoming_chapuser')
new_baseiscsi.incoming_chappassword = srcmd.dconf.get('incoming_chappassword')
self.base_srs.add(new_baseiscsi)

return new_baseiscsi

def create_test_sr(self, sr_cmd):
self.sr_uuid = str(uuid4())
self.subject = LVHDoISCSISR.LVHDoISCSISR(
sr_cmd, self.sr_uuid)

def test_check_sr_pbd_not_found(self):
# Arrange
self.mock_util.find_my_pbd.return_value = None
self.create_test_sr(self.create_sr_command())

# Act
self.subject.check_sr(TEST_SR_UUID)

# Assert
self.mock_util.find_my_pbd.assert_called_with(
self.mock_session, 'test_host', 'sr_ref')

def test_check_sr_correct_sessions_count(self):
# Arrange
self.mock_util.find_my_pbd.return_value = 'my_pbd'
self.mock_session.xenapi.PBD.get_other_config.return_value = {
'iscsi_sessions': 2
}
self.create_test_sr(self.create_sr_command())

# Act
self.subject.check_sr(TEST_SR_UUID)

# Assert
self.mock_session.xenapi.PBD.get_other_config.assert_called_with('my_pbd')

def test_check_sr_not_enough_sessions(self):
# Arrange
self.mock_util.find_my_pbd.return_value = 'my_pbd'
self.mock_session.xenapi.PBD.get_other_config.return_value = {
'iscsi_sessions': 1
}
self.create_test_sr(self.create_sr_command())

# Act
self.subject.check_sr(TEST_SR_UUID)

# Assert
self.mock_baseiscsi.attach.assert_called_with(
TEST_SR_UUID
)

def test_sr_attach_multi_session(self):
# Arrange
self.mock_util.find_my_pbd.return_value = 'my_pbd'
additional_dconf = {
'multiSession': '10.207.6.60,3260,iqn.2009-11.com.infinidat:storage:infinibox-sn-3393|'
'10.207.3.65,3260,iqn.2009-11.com.infinidat:storage:infinibox-sn-3394|'
'10.207.3.61,3260,iqn.2009-11.com.infinidat:storage:infinibox-sn-3393|'
'10.207.6.61,3260,iqn.2009-11.com.infinidat:storage:infinibox-sn-3393|'
'10.207.3.63,3260,iqn.2009-11.com.infinidat:storage:infinibox-sn-3394|'
'10.207.6.62,3260,iqn.2009-11.com.infinidat:storage:infinibox-sn-3393|'
'10.207.3.62,3260,iqn.2009-11.com.infinidat:storage:infinibox-sn-3393|'
'10.207.3.60,3260,iqn.2009-11.com.infinidat:storage:infinibox-sn-3393|'
'10.207.6.64,3260,iqn.2009-11.com.infinidat:storage:infinibox-sn-3394|'
'10.207.6.65,3260,iqn.2009-11.com.infinidat:storage:infinibox-sn-3394|'
'10.207.3.64,3260,iqn.2009-11.com.infinidat:storage:infinibox-sn-3394|'
'10.207.6.63,3260,iqn.2009-11.com.infinidat:storage:infinibox-sn-3394|'
}

tpg_data = [
[
('10.207.3.60:3260', 1, 'iqn.2009-11.com.infinidat:storage:infinibox-sn-3393'),
('10.207.3.61:3260', 1, 'iqn.2009-11.com.infinidat:storage:infinibox-sn-3393'),
('10.207.3.62:3260', 1, 'iqn.2009-11.com.infinidat:storage:infinibox-sn-3393')],
[
('10.207.3.63:3260', 1, 'iqn.2009-11.com.infinidat:storage:infinibox-sn-3394'),
('10.207.3.64:3260', 1, 'iqn.2009-11.com.infinidat:storage:infinibox-sn-3394'),
('10.207.3.65:3260', 1, 'iqn.2009-11.com.infinidat:storage:infinibox-sn-3394')],
[
('10.207.6.60:3260', 2, 'iqn.2009-11.com.infinidat:storage:infinibox-sn-3393'),
('10.207.6.61:3260', 2, 'iqn.2009-11.com.infinidat:storage:infinibox-sn-3393'),
('10.207.6.62:3260', 2, 'iqn.2009-11.com.infinidat:storage:infinibox-sn-3393')
],
[
('10.207.6.63:3260', 2, 'iqn.2009-11.com.infinidat:storage:infinibox-sn-3394'),
('10.207.6.64:3260', 2, 'iqn.2009-11.com.infinidat:storage:infinibox-sn-3394'),
('10.207.6.65:3260', 2, 'iqn.2009-11.com.infinidat:storage:infinibox-sn-3394')
]
]

self.discovery_data = {
'10.207.3.60': tpg_data[0],
'10.207.3.61': tpg_data[0],
'10.207.3.62': tpg_data[0],
'10.207.3.63': tpg_data[1],
'10.207.3.64': tpg_data[1],
'10.207.3.65': tpg_data[1],
'10.207.6.60': tpg_data[2],
'10.207.6.61': tpg_data[2],
'10.207.6.62': tpg_data[2],
'10.207.6.63': tpg_data[3],
'10.207.6.64': tpg_data[3],
'10.207.6.65': tpg_data[3]
}

# Create SR
self.create_test_sr(self.create_sr_command(
additional_dconf=additional_dconf,
cmd='sr_attach',
target_iqn='*'))

# Act
self.subject.attach(TEST_SR_UUID)

# Assert
# print(f"iscsilib calls {self.mock_iscsilib.mock_calls}")
attach_count = 0
for sr in self.base_srs:
attach_count += sr.attach.call_count

self.assertEqual(12, attach_count)
self.assertEqual(12, self.mock_iscsilib.discovery.call_count)
self.assertEqual(12, self.mock_iscsilib.login.call_count)
93 changes: 0 additions & 93 deletions tests/test_sr_health_check.py
View file

This file was deleted.

0 comments on commit d2d42c1

Please sign in to comment.