openstack/cinder
Code Issues Proposed changes

Create iSCSI lio portals with right IPs and port

Browse Source 
Currently lioadm helper creates portals with fixed IP addresses 0.0.0.0
and ::0 and also uses fixed standard port 3260 instead of using store
configuration (iscsi_ip_address, iscsi_secondary_ip_addresses,
iscsi_port) from the configuration as it should.

This could lead to problems if a different port was defined in the
configuration as the target would be available only on 3260 and other
services would look for it at configured port.

With this fix we now create the right portals with the right port.

Since now we pass list of ips and port to helpers cxt helper has also
been updated to use these parameters instead of going directly to the
configuration.

Change-Id: I77e8865b1f015a9fa155f2b4552a5d9c27f5e122
Closes-Bug: #1425875
This commit is contained in:
Gorka Eguileor
2015-03-04 14:24:25 +01:00
parent 7ed2476311
commit 3abc1d2f63
10 changed files with 333 additions and 63 deletions
Download Patch File Download Diff File Expand all files Collapse all files

89
cinder/cmd/rtstool.py
View File

@@ -34,7 +34,11 @@ class RtstoolImportError(RtstoolError):
def create(backing_device, name, userid, password, iser_enabled,
initiator_iqns=None):
initiator_iqns=None, portals_ips=None, portals_port=3260):
# List of IPS that will not raise an error when they fail binding.
# Originally we will fail on all binding errors.
ips_allow_fail = ()
try:
rtsroot = rtslib.root.RTSRoot()
except rtslib.utils.RTSLibError:
@@ -68,36 +72,33 @@ def create(backing_device, name, userid, password, iser_enabled,
tpg_new.enable = 1
try:
portal = rtslib.NetworkPortal(tpg_new, '0.0.0.0', 3260, mode='any')
except rtslib.utils.RTSLibError:
print(_('Error creating NetworkPortal: ensure port 3260 '
'is not in use by another service.'))
raise
try:
if iser_enabled == 'True':
portal.iser = True
except rtslib.utils.RTSLibError:
print(_('Error enabling iSER for NetworkPortal: please ensure that '
'RDMA is supported on your iSCSI port.'))
raise
portal = None
try:
portal = rtslib.NetworkPortal(tpg_new, '::0', 3260, mode='any')
except rtslib.utils.RTSLibError:
# If no ips are given we'll bind to all IPv4 and v6
if not portals_ips:
portals_ips = ('0.0.0.0', '::0')
# TODO(emh): Binding to IPv6 fails sometimes -- let pass for now.
pass
ips_allow_fail = ('::0',)
try:
if portal and iser_enabled == 'True':
portal.iser = True
except rtslib.utils.RTSLibError:
print (_('Error enabling iSER for IPv6 NetworkPortal: please '
'ensure that RDMA is supported on your iSCSI port.'))
raise
for ip in portals_ips:
try:
portal = rtslib.NetworkPortal(tpg_new, ip, portals_port,
mode='any')
except rtslib.utils.RTSLibError:
raise_exc = ip not in ips_allow_fail
msg_type = 'Error' if raise_exc else 'Warning'
print(_('%(msg_type)s: creating NetworkPortal: ensure port '
'%(port)d on ip %(ip)s is not in use by another service.')
% {'msg_type': msg_type, 'port': portals_port, 'ip': ip})
if raise_exc:
raise
else:
try:
if iser_enabled == 'True':
portal.iser = True
except rtslib.utils.RTSLibError:
print(_('Error enabling iSER for NetworkPortal: please ensure '
'that RDMA is supported on your iSCSI port %(port)d '
'on ip %(ip)s.') % {'port': portals_port, 'ip': ip})
raise
def _lookup_target(target_iqn, initiator_iqn):
@@ -174,7 +175,7 @@ def usage():
print("Usage:")
print(sys.argv[0] +
" create [device] [name] [userid] [password] [iser_enabled]" +
" <initiator_iqn,iqn2,iqn3,...>")
" <initiator_iqn,iqn2,iqn3,...> [-a<IP1,IP2,...>] [-pPORT]")
print(sys.argv[0] +
" add-initiator [target_iqn] [userid] [password] [initiator_iqn]")
print(sys.argv[0] +
@@ -197,6 +198,25 @@ def save_to_file(destination_file):
{'file_path': destination_file})
def parse_optional_create(argv):
optional_args = {}
for arg in argv:
if arg.startswith('-a'):
ips = filter(None, arg[2:].split(','))
if not ips:
usage()
optional_args['portals_ips'] = ips
elif arg.startswith('-p'):
try:
optional_args['portals_port'] = int(arg[2:])
except ValueError:
usage()
else:
optional_args['initiator_iqns'] = arg
return optional_args
def main(argv=None):
if argv is None:
argv = sys.argv
@@ -208,7 +228,7 @@ def main(argv=None):
if len(argv) < 7:
usage()
if len(argv) > 8:
if len(argv) > 10:
usage()
backing_device = argv[2]
@@ -216,13 +236,14 @@ def main(argv=None):
userid = argv[4]
password = argv[5]
iser_enabled = argv[6]
initiator_iqns = None
if len(argv) > 7:
initiator_iqns = argv[7]
optional_args = parse_optional_create(argv[7:])
else:
optional_args = {}
create(backing_device, name, userid, password, iser_enabled,
initiator_iqns)
**optional_args)
elif argv[1] == 'add-initiator':
if len(argv) < 6:

9
cinder/tests/unit/targets/targets_fixture.py
View File

@@ -29,6 +29,7 @@ class TargetDriverFixture(test.TestCase):
self.configuration.append_config_values = mock.Mock(return_value=0)
self.configuration.safe_get = mock.Mock(side_effect=self.fake_safe_get)
self.configuration.iscsi_ip_address = '10.9.8.7'
self.configuration.iscsi_port = 3260
self.fake_volumes_dir = tempfile.mkdtemp()
fileutils.ensure_tree(self.fake_volumes_dir)
@@ -65,9 +66,11 @@ class TargetDriverFixture(test.TestCase):
'size': 1,
'id': self.fake_volume_id,
'volume_type_id': None,
'provider_location': '10.9.8.7:3260 '
'iqn.2010-10.org.openstack:'
'volume-%s 2' % self.fake_volume_id,
'provider_location': ('%(ip)s:%(port)d%(iqn)svolume-%(vol)s 2' %
{'ip': self.configuration.iscsi_ip_address,
'port': self.configuration.iscsi_port,
'iqn': self.iscsi_target_prefix,
'vol': self.fake_volume_id}),
'provider_auth': 'CHAP stack-1-a60e2611875f40199931f2'
'c76370d66b 2FE0CQ8J196R',
'provider_geometry': '512 512',

56
cinder/tests/unit/targets/test_cxt_driver.py
View File

@@ -14,6 +14,7 @@
# under the License.
import contextlib
import os
import StringIO
import mock
@@ -97,11 +98,57 @@ class TestCxtAdmDriver(tf.TargetDriverFixture):
self.test_vol,
1,
0,
self.fake_volumes_dir))
self.fake_volumes_dir,
portals_ips=[self.configuration.iscsi_ip_address]))
self.assertTrue(mock_get.called)
self.assertTrue(mock_execute.called)
self.assertTrue(mock_get_targ.called)
@mock.patch('cinder.volume.targets.cxt.CxtAdm._get_target',
return_value=1)
@mock.patch('cinder.utils.execute', return_value=('fake out', 'fake err'))
def test_create_iscsi_target_port_ips(self, mock_execute, mock_get_targ):
ips = ['10.0.0.15', '127.0.0.1']
port = 3261
mock_execute.return_value = ('', '')
with mock.patch.object(self.target, '_get_volumes_dir') as mock_get:
mock_get.return_value = self.fake_volumes_dir
test_vol = 'iqn.2010-10.org.openstack:'\
'volume-83c2e877-feed-46be-8435-77884fe55b45'
self.assertEqual(
1,
self.target.create_iscsi_target(
test_vol,
1,
0,
self.fake_volumes_dir,
portals_port=port,
portals_ips=ips))
self.assertTrue(mock_get.called)
self.assertTrue(mock_execute.called)
self.assertTrue(mock_get_targ.called)
file_path = os.path.join(self.fake_volumes_dir,
test_vol.split(':')[1])
expected_cfg = {
'name': test_vol,
'device': self.fake_volumes_dir,
'ips': ','.join(map(lambda ip: '%s:%s' % (ip, port), ips)),
'spaces': ' ' * 14,
'spaces2': ' ' * 23}
expected_file = ('\n%(spaces)starget:'
'\n%(spaces2)sTargetName=%(name)s'
'\n%(spaces2)sTargetDevice=%(device)s'
'\n%(spaces2)sPortalGroup=1@%(ips)s'
'\n%(spaces)s ') % expected_cfg
with open(file_path, 'r') as cfg_file:
result = cfg_file.read()
self.assertEqual(expected_file, result)
@mock.patch('cinder.volume.targets.cxt.CxtAdm._get_target',
return_value=1)
@mock.patch('cinder.utils.execute', return_value=('fake out', 'fake err'))
@@ -115,7 +162,8 @@ class TestCxtAdmDriver(tf.TargetDriverFixture):
self.test_vol,
1,
0,
self.fake_volumes_dir))
self.fake_volumes_dir,
portals_ips=[self.configuration.iscsi_ip_address]))
self.assertTrue(mock_get.called)
self.assertTrue(mock_get_targ.called)
self.assertTrue(mock_execute.called)
@@ -158,4 +206,6 @@ class TestCxtAdmDriver(tf.TargetDriverFixture):
'iqn.2010-10.org.openstack:testvol',
1, 0, self.fake_volumes_dir, fake_creds,
check_exit_code=False,
old_name=None)
old_name=None,
portals_ips=[self.configuration.iscsi_ip_address],
portals_port=self.configuration.iscsi_port)

3
cinder/tests/unit/targets/test_iet_driver.py
View File

@@ -27,6 +27,7 @@ from cinder.volume.targets import iet
class TestIetAdmDriver(tf.TargetDriverFixture):
def setUp(self):
super(TestIetAdmDriver, self).setUp()
self.target = iet.IetAdm(root_helper=utils.get_root_helper(),
@@ -222,5 +223,7 @@ class TestIetAdmDriver(tf.TargetDriverFixture):
self.target.create_iscsi_target.assert_called_once_with(
'iqn.2010-10.org.openstack:testvol',
1, 0, self.fake_volumes_dir, None,
portals_ips=[self.configuration.iscsi_ip_address],
portals_port=int(self.configuration.iscsi_port),
check_exit_code=False,
old_name=None)

102
cinder/tests/unit/targets/test_lio_driver.py
View File

@@ -73,6 +73,73 @@ class TestLioAdmDriver(tf.TargetDriverFixture):
0,
self.fake_volumes_dir))
mpersist_cfg.assert_called_once_with(self.volume_name)
mexecute.assert_called_once_with(
'cinder-rtstool',
'create',
self.fake_volumes_dir,
self.test_vol,
'',
'',
self.target.iscsi_protocol == 'iser',
run_as_root=True)
@mock.patch.object(utils, 'execute')
@mock.patch.object(lio.LioAdm, '_get_target', return_value=1)
def test_create_iscsi_target_port_ip(self, mget_target, mexecute):
test_vol = 'iqn.2010-10.org.openstack:'\
'volume-83c2e877-feed-46be-8435-77884fe55b45'
ip = '10.0.0.15'
port = 3261
self.assertEqual(
1,
self.target.create_iscsi_target(
name=test_vol,
tid=1,
lun=0,
path=self.fake_volumes_dir,
**{'portals_port': port, 'portals_ips': [ip]}))
mexecute.assert_any_call(
'cinder-rtstool',
'create',
self.fake_volumes_dir,
test_vol,
'',
'',
self.target.iscsi_protocol == 'iser',
'-p%s' % port,
'-a' + ip,
run_as_root=True)
@mock.patch.object(utils, 'execute')
@mock.patch.object(lio.LioAdm, '_get_target', return_value=1)
def test_create_iscsi_target_port_ips(self, mget_target, mexecute):
test_vol = 'iqn.2010-10.org.openstack:'\
'volume-83c2e877-feed-46be-8435-77884fe55b45'
ips = ['10.0.0.15', '127.0.0.1']
port = 3261
self.assertEqual(
1,
self.target.create_iscsi_target(
name=test_vol,
tid=1,
lun=0,
path=self.fake_volumes_dir,
**{'portals_port': port, 'portals_ips': ips}))
mexecute.assert_any_call(
'cinder-rtstool',
'create',
self.fake_volumes_dir,
test_vol,
'',
'',
self.target.iscsi_protocol == 'iser',
'-p%s' % port,
'-a' + ','.join(ips),
run_as_root=True)
@mock.patch.object(lio.LioAdm, '_persist_configuration')
@mock.patch('cinder.utils.execute',
@@ -132,7 +199,9 @@ class TestLioAdmDriver(tf.TargetDriverFixture):
self.iscsi_target_prefix + 'testvol',
0, 0, self.fake_volumes_dir, ('foo', 'bar'),
check_exit_code=False,
old_name=None)
old_name=None,
portals_ips=[self.configuration.iscsi_ip_address],
portals_port=self.configuration.iscsi_port)
@mock.patch.object(lio.LioAdm, '_persist_configuration')
@mock.patch('cinder.utils.execute')
@@ -195,3 +264,34 @@ class TestLioAdmDriver(tf.TargetDriverFixture):
def test_iscsi_protocol(self):
self.assertEqual(self.target.iscsi_protocol, 'iscsi')
@mock.patch.object(lio.LioAdm, '_get_target_and_lun', return_value=(1, 2))
@mock.patch.object(lio.LioAdm, 'create_iscsi_target', return_value=3)
@mock.patch.object(lio.LioAdm, '_get_target_chap_auth',
return_value=(mock.sentinel.user, mock.sentinel.pwd))
def test_create_export(self, mock_chap, mock_create, mock_get_target):
ctxt = context.get_admin_context()
result = self.target.create_export(ctxt, self.testvol_2,
self.fake_volumes_dir)
loc = (u'%(ip)s:%(port)d,3 %(prefix)s%(name)s 2' %
{'ip': self.configuration.iscsi_ip_address,
'port': self.configuration.iscsi_port,
'prefix': self.iscsi_target_prefix,
'name': self.testvol_2['name']})
expected_result = {
'location': loc,
'auth': 'CHAP %s %s' % (mock.sentinel.user, mock.sentinel.pwd),
}
self.assertEqual(expected_result, result)
mock_create.assert_called_once_with(
self.iscsi_target_prefix + self.testvol_2['name'],
1,
2,
self.fake_volumes_dir,
(mock.sentinel.user, mock.sentinel.pwd),
portals_ips=[self.configuration.iscsi_ip_address],
portals_port=self.configuration.iscsi_port)

4
cinder/tests/unit/targets/test_tgt_driver.py
View File

@@ -405,4 +405,6 @@ class TestTgtAdmDriver(tf.TargetDriverFixture):
self.iscsi_target_prefix + self.testvol['name'],
0, 1, self.fake_volumes_dir, ('foo', 'bar'),
check_exit_code=False,
old_name=None)
old_name=None,
portals_ips=[self.configuration.iscsi_ip_address],
portals_port=self.configuration.iscsi_port)

75
cinder/tests/unit/test_cmd.py
View File

@@ -824,6 +824,38 @@ class TestCinderRtstoolCmd(test.TestCase):
def test_create_ipv6(self):
self._test_create('::0')
@mock.patch.object(cinder_rtstool, 'rtslib', autospec=True)
def test_create_ips_and_port(self, mock_rtslib):
port = 3261
ips = ['ip1', 'ip2', 'ip3']
mock_rtslib.BlockStorageObject.return_value = mock.sentinel.bso
mock_rtslib.Target.return_value = mock.sentinel.target_new
mock_rtslib.FabricModule.return_value = mock.sentinel.iscsi_fabric
tpg_new = mock_rtslib.TPG.return_value
cinder_rtstool.create(mock.sentinel.backing_device,
mock.sentinel.name,
mock.sentinel.userid,
mock.sentinel.password,
mock.sentinel.iser_enabled,
portals_ips=ips,
portals_port=port)
mock_rtslib.Target.assert_called_once_with(mock.sentinel.iscsi_fabric,
mock.sentinel.name,
'create')
mock_rtslib.TPG.assert_called_once_with(mock.sentinel.target_new,
mode='create')
mock_rtslib.LUN.assert_called_once_with(
tpg_new,
storage_object=mock.sentinel.bso)
mock_rtslib.NetworkPortal.assert_has_calls(
map(lambda ip: mock.call(tpg_new, ip, port, mode='any'), ips),
any_order=True
)
@mock.patch('rtslib.root.RTSRoot')
def test_add_initiator_rtslib_error(self, rtsroot):
rtsroot.side_effect = rtslib.utils.RTSLibError()
@@ -996,19 +1028,46 @@ class TestCinderRtstoolCmd(test.TestCase):
mock.sentinel.name,
mock.sentinel.userid,
mock.sentinel.password,
mock.sentinel.initiator_iqns,
mock.sentinel.iser_enabled]
mock.sentinel.iser_enabled,
str(mock.sentinel.initiator_iqns)]
rc = cinder_rtstool.main()
create.assert_called_once_with(mock.sentinel.backing_device,
mock.sentinel.name,
mock.sentinel.userid,
mock.sentinel.password,
mock.sentinel.initiator_iqns,
mock.sentinel.iser_enabled)
create.assert_called_once_with(
mock.sentinel.backing_device,
mock.sentinel.name,
mock.sentinel.userid,
mock.sentinel.password,
mock.sentinel.iser_enabled,
initiator_iqns=str(mock.sentinel.initiator_iqns))
self.assertEqual(0, rc)
@mock.patch('cinder.cmd.rtstool.create')
def test_main_create_ips_and_port(self, mock_create):
sys.argv = ['cinder-rtstool',
'create',
mock.sentinel.backing_device,
mock.sentinel.name,
mock.sentinel.userid,
mock.sentinel.password,
mock.sentinel.iser_enabled,
str(mock.sentinel.initiator_iqns),
'-p3261',
'-aip1,ip2,ip3']
rc = cinder_rtstool.main()
mock_create.assert_called_once_with(
mock.sentinel.backing_device,
mock.sentinel.name,
mock.sentinel.userid,
mock.sentinel.password,
mock.sentinel.iser_enabled,
initiator_iqns=str(mock.sentinel.initiator_iqns),
portals_ips=['ip1', 'ip2', 'ip3'],
portals_port=3261)
self.assertEqual(0, rc)
def test_main_add_initiator(self):
with mock.patch('cinder.cmd.rtstool.add_initiator') as add_initiator:
sys.argv = ['cinder-rtstool',

30
cinder/volume/targets/cxt.py
View File

@@ -37,7 +37,7 @@ class CxtAdm(iscsi.ISCSITarget):
"""
TARGET_FMT = """
target:
target:
TargetName=%s
TargetDevice=%s
PortalGroup=1@%s
@@ -114,6 +114,18 @@ class CxtAdm(iscsi.ISCSITarget):
LOG.debug('Failed to find CHAP auth from config for %s', vol_id)
return None
@staticmethod
def _get_portal(ip, port=None):
# ipv6 addresses use [ip]:port format, ipv4 use ip:port
portal_port = ':%d' % port if port else ''
if netutils.is_valid_ipv4(ip):
portal_ip = ip
else:
portal_ip = '[' + ip + ']'
return portal_ip + portal_port
def create_iscsi_target(self, name, tid, lun, path,
chap_auth=None, **kwargs):
@@ -127,19 +139,17 @@ class CxtAdm(iscsi.ISCSITarget):
vol_id = name.split(':')[1]
if netutils.is_valid_ipv4(self.configuration.iscsi_ip_address):
portal = "%s:%s" % (self.configuration.iscsi_ip_address,
self.configuration.iscsi_port)
else:
# ipv6 addresses use [ip]:port format, ipv4 use ip:port
portal = "[%s]:%s" % (self.configuration.iscsi_ip_address,
self.configuration.iscsi_port)
cfg_port = kwargs.get('portals_port')
cfg_ips = kwargs.get('portals_ips')
portals = ','.join(map(lambda ip: self._get_portal(ip, cfg_port),
cfg_ips))
if chap_auth is None:
volume_conf = self.TARGET_FMT % (name, path, portal)
volume_conf = self.TARGET_FMT % (name, path, portals)
else:
volume_conf = self.TARGET_FMT_WITH_CHAP % (name,
path, portal,
path, portals,
'"%s":"%s"' % chap_auth)
LOG.debug('Creating iscsi_target for: %s', vol_id)
volume_path = os.path.join(volumes_dir, vol_id)

19
cinder/volume/targets/iscsi.py
View File

@@ -185,6 +185,14 @@ class ISCSITarget(driver.Target):
return target
return None
def _get_portals_config(self):
# Prepare portals configuration
portals_ips = ([self.configuration.iscsi_ip_address]
+ self.configuration.iscsi_secondary_ip_addresses or [])
return {'portals_ips': portals_ips,
'portals_port': self.configuration.iscsi_port}
def create_export(self, context, volume, volume_path):
"""Creates an export for a logical volume."""
# 'iscsi_name': 'iqn.2010-10.org.openstack:volume-00000001'
@@ -199,13 +207,17 @@ class ISCSITarget(driver.Target):
chap_auth = (vutils.generate_username(),
vutils.generate_password())
# Get portals ips and port
portals_config = self._get_portals_config()
# NOTE(jdg): For TgtAdm case iscsi_name is the ONLY param we need
# should clean this all up at some point in the future
tid = self.create_iscsi_target(iscsi_name,
iscsi_target,
lun,
volume_path,
chap_auth)
chap_auth,
**portals_config)
data = {}
data['location'] = self._iscsi_location(
self.configuration.iscsi_ip_address, tid, iscsi_name, lun,
@@ -254,11 +266,14 @@ class ISCSITarget(driver.Target):
LOG.info(_LI("Skipping ensure_export. No iscsi_target "
"provision for volume: %s"), volume['id'])
# Get portals ips and port
portals_config = self._get_portals_config()
iscsi_target, lun = self._get_target_and_lun(context, volume)
self.create_iscsi_target(
iscsi_name, iscsi_target, lun, volume_path,
chap_auth, check_exit_code=False,
old_name=None)
old_name=None, **portals_config)
def initialize_connection(self, volume, connector):
"""Initializes the connection and returns connection info.

9
cinder/volume/targets/lio.py
View File

@@ -101,6 +101,13 @@ class LioAdm(iscsi.ISCSITarget):
if chap_auth is not None:
(chap_auth_userid, chap_auth_password) = chap_auth
optional_args = []
if 'portals_port' in kwargs:
optional_args.append('-p%s' % kwargs['portals_port'])
if 'portals_ips' in kwargs:
optional_args.append('-a' + ','.join(kwargs['portals_ips']))
try:
command_args = ['cinder-rtstool',
'create',
@@ -108,7 +115,7 @@ class LioAdm(iscsi.ISCSITarget):
name,
chap_auth_userid,
chap_auth_password,
self.iscsi_protocol == 'iser']
self.iscsi_protocol == 'iser'] + optional_args
utils.execute(*command_args, run_as_root=True)
except putils.ProcessExecutionError:
LOG.exception(_LE("Failed to create iscsi target for volume "