openstack
/
cinder
Code Issues Proposed changes

Merge "Move tgt targets to privsep"

This commit is contained in:
Zuul 2018-12-19 18:49:25 +00:00 committed by Gerrit Code Review
parent 47350119ec b4a7d50671
commit d69b047553
5 changed files with 100 additions and 69 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

0
cinder/privsep/targets/__init__.py Normal file
View File

60
cinder/privsep/targets/tgt.py Normal file
View File

@ -0,0 +1,60 @@
# Copyright 2018 Red Hat, Inc
# Copyright 2017 Rackspace Australia
# Copyright 2018 Michael Still and Aptira
#
# 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.
"""
Helpers for iscsi related routines.
"""
from oslo_concurrency import processutils
import cinder.privsep
@cinder.privsep.sys_admin_pctxt.entrypoint
def tgtadmin_show():
return processutils.execute('tgt-admin', '--show')
@cinder.privsep.sys_admin_pctxt.entrypoint
def tgtadmin_update(name, force=False):
cmd = ['tgt-admin']
cmd.extend(['--update', name])
if force:
cmd.extend(['-f'])
return processutils.execute(*cmd)
@cinder.privsep.sys_admin_pctxt.entrypoint
def tgtadmin_delete(iqn, force=False):
cmd = ['tgt-admin']
cmd.extend(['--delete', iqn])
if force:
cmd.extend(['-f'])
processutils.execute(*cmd)
@cinder.privsep.sys_admin_pctxt.entrypoint
def tgtadm_show():
cmd = ('tgtadm', '--lld', 'iscsi', '--op', 'show', '--mode', 'target')
return processutils.execute(*cmd)
@cinder.privsep.sys_admin_pctxt.entrypoint
def tgtadm_create(tid, path):
cmd = ('tgtadm', '--lld', 'iscsi', '--op', 'new', '--mode',
'logicalunit', '--tid', tid, '--lun', '1', '-b',
path)
return processutils.execute(*cmd)

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

@ -87,7 +87,7 @@ class TestTgtAdmDriver(tf.TargetDriverFixture):
self.assertEqual('iscsi', self.target.iscsi_protocol)
def test_get_target(self):
with mock.patch('cinder.utils.execute',
with mock.patch('cinder.privsep.targets.tgt.tgtadmin_show',
return_value=(self.fake_iscsi_scan, None)):
iqn = self.test_vol
self.assertEqual('1', self.target._get_target(iqn))
@ -95,35 +95,29 @@ class TestTgtAdmDriver(tf.TargetDriverFixture):
def test_verify_backing_lun(self):
iqn = self.test_vol
with mock.patch('cinder.utils.execute',
with mock.patch('cinder.privsep.targets.tgt.tgtadmin_show',
return_value=(self.fake_iscsi_scan, None)):
self.assertTrue(self.target._verify_backing_lun(iqn, '1'))
# Test the failure case
bad_scan = self.fake_iscsi_scan.replace('LUN: 1', 'LUN: 3')
with mock.patch('cinder.utils.execute',
with mock.patch('cinder.privsep.targets.tgt.tgtadmin_show',
return_value=(bad_scan, None)):
self.assertFalse(self.target._verify_backing_lun(iqn, '1'))
@mock.patch.object(time, 'sleep')
@mock.patch('cinder.utils.execute')
def test_recreate_backing_lun(self, mock_execute, mock_sleep):
mock_execute.return_value = ('out', 'err')
@mock.patch('cinder.privsep.targets.tgt.tgtadm_create')
def test_recreate_backing_lun(self, mock_privsep, mock_sleep):
mock_privsep.return_value = ('out', 'err')
self.target._recreate_backing_lun(self.test_vol, '1',
self.testvol['name'],
self.testvol_path)
expected_command = ('tgtadm', '--lld', 'iscsi', '--op', 'new',
'--mode', 'logicalunit', '--tid', '1',
'--lun', '1', '-b',
self.testvol_path)
mock_execute.assert_called_once_with(*expected_command,
run_as_root=True)
mock_privsep.assert_called_once_with('1', self.testvol_path)
# Test the failure case
mock_execute.side_effect = putils.ProcessExecutionError
mock_privsep.side_effect = putils.ProcessExecutionError
self.assertIsNone(
self.target._recreate_backing_lun(self.test_vol,
'1',
@ -147,9 +141,11 @@ class TestTgtAdmDriver(tf.TargetDriverFixture):
@test.testtools.skipIf(sys.platform == "darwin", "SKIP on OSX")
def test_create_iscsi_target(self):
with mock.patch('cinder.utils.execute', return_value=('', '')),\
with mock.patch('cinder.privsep.targets.tgt.tgtadm_show', return_value=('', '')),\
mock.patch.object(self.target, '_get_target',
side_effect=lambda x: 1),\
mock.patch('cinder.privsep.targets.tgt.tgtadmin_update',
return_value=('', '')), \
mock.patch.object(self.target, '_verify_backing_lun',
side_effect=lambda x, y: True):
self.assertEqual(
@ -167,11 +163,14 @@ class TestTgtAdmDriver(tf.TargetDriverFixture):
self.iscsi_write_cache = 'bar'
mock_open = mock.mock_open()
with mock.patch('cinder.utils.execute', return_value=('', '')),\
with mock.patch('cinder.privsep.targets.tgt.tgtadm_show',
return_value=('', '')),\
mock.patch.object(self.target, '_get_target',
side_effect=lambda x: 1),\
mock.patch.object(self.target, '_verify_backing_lun',
side_effect=lambda x, y: True),\
mock.patch('cinder.privsep.targets.tgt.tgtadmin_update',
return_value=('', '')), \
mock.patch('cinder.volume.targets.tgt.open',
mock_open, create=True):
self.assertEqual(
@ -199,7 +198,10 @@ class TestTgtAdmDriver(tf.TargetDriverFixture):
side_effect=lambda x: 1),\
mock.patch.object(self.target, '_verify_backing_lun',
side_effect=lambda x, y: True),\
mock.patch('cinder.utils.execute', _fake_execute):
mock.patch('cinder.privsep.targets.tgt.tgtadmin_update',
return_value=('', '')), \
mock.patch('cinder.privsep.targets.tgt.tgtadm_show',
_fake_execute):
self.assertEqual(
1,
self.target.create_iscsi_target(
@ -210,7 +212,7 @@ class TestTgtAdmDriver(tf.TargetDriverFixture):
@mock.patch('os.path.isfile', return_value=True)
@mock.patch('os.path.exists', return_value=True)
@mock.patch('cinder.utils.execute')
@mock.patch('cinder.privsep.targets.tgt.tgtadmin_delete')
@mock.patch('os.unlink', return_value=None)
def test_delete_target_not_found(self,
mock_unlink,
@ -250,7 +252,7 @@ class TestTgtAdmDriver(tf.TargetDriverFixture):
@mock.patch('os.path.isfile', return_value=True)
@mock.patch('os.path.exists', return_value=True)
@mock.patch('cinder.utils.execute')
@mock.patch('cinder.privsep.targets.tgt.tgtadmin_delete')
@mock.patch('os.unlink', return_value=None)
def test_delete_target_acl_not_found(self,
mock_unlink,
@ -306,7 +308,9 @@ class TestTgtAdmDriver(tf.TargetDriverFixture):
@mock.patch.object(os.path, 'exists')
@mock.patch.object(os.path, 'isfile')
@mock.patch.object(os, 'unlink')
@mock.patch('cinder.privsep.targets.tgt.tgtadmin_delete')
def test_remove_iscsi_target(self,
mock_delete,
mock_unlink,
mock_isfile,
mock_path_exists,
@ -328,14 +332,8 @@ class TestTgtAdmDriver(tf.TargetDriverFixture):
1,
self.testvol['id'],
self.testvol['name'])
calls = [mock.call('tgt-admin', '--force', '--delete',
self.iscsi_target_prefix + self.testvol['name'],
run_as_root=True),
mock.call('tgt-admin', '--delete',
self.iscsi_target_prefix + self.testvol['name'],
run_as_root=True)]
mock_execute.assert_has_calls(calls)
mock_delete.assert_called_with(
self.iscsi_target_prefix + self.testvol['name'])
@test.testtools.skipIf(sys.platform == "darwin", "SKIP on OSX")
def test_create_export(self):
@ -344,7 +342,7 @@ class TestTgtAdmDriver(tf.TargetDriverFixture):
self.testvol['name'] + ' 1',
'auth': 'CHAP QZJb P68e'}
with mock.patch('cinder.utils.execute', return_value=('', '')),\
with mock.patch('cinder.privsep.targets.tgt.tgtadm_show', return_value=('', '')),\
mock.patch.object(self.target, '_get_target',
side_effect=lambda x: 1),\
mock.patch.object(self.target, '_verify_backing_lun',
@ -353,6 +351,8 @@ class TestTgtAdmDriver(tf.TargetDriverFixture):
side_effect=lambda x, y: None) as m_chap,\
mock.patch.object(vutils, 'generate_username',
side_effect=lambda: 'QZJb'),\
mock.patch('cinder.privsep.targets.tgt.tgtadmin_update',
return_value=('', '')), \
mock.patch.object(vutils, 'generate_password',
side_effect=lambda: 'P68e'):
@ -390,9 +390,11 @@ class TestTgtAdmDriver(tf.TargetDriverFixture):
@test.testtools.skipIf(sys.platform == "darwin", "SKIP on OSX")
def test_create_iscsi_target_retry(self):
with mock.patch('cinder.utils.execute', return_value=('', '')),\
with mock.patch('cinder.privsep.targets.tgt.tgtadm_show', return_value=('', '')),\
mock.patch.object(self.target, '_get_target',
side_effect=[None, None, 1]) as get_target,\
mock.patch('cinder.privsep.targets.tgt.tgtadmin_update',
return_value=('', '')), \
mock.patch.object(self.target, '_verify_backing_lun',
side_effect=lambda x, y: True):
self.assertEqual(

47
cinder/volume/targets/tgt.py
View File

@ -19,6 +19,7 @@ from oslo_log import log as logging
from oslo_utils import fileutils
from cinder import exception
import cinder.privsep.targets.tgt
from cinder import utils
from cinder.volume.targets import iscsi
@ -45,7 +46,7 @@ class TgtAdm(iscsi.ISCSITarget):
""")
def _get_target(self, iqn):
(out, err) = utils.execute('tgt-admin', '--show', run_as_root=True)
(out, err) = cinder.privsep.targets.tgt.tgtadmin_show()
lines = out.split('\n')
for line in lines:
if iqn in line:
@ -60,7 +61,7 @@ class TgtAdm(iscsi.ISCSITarget):
capture = False
target_info = []
(out, err) = utils.execute('tgt-admin', '--show', run_as_root=True)
(out, err) = cinder.privsep.targets.tgt.tgtadmin_show()
lines = out.split('\n')
for line in lines:
@ -88,11 +89,7 @@ class TgtAdm(iscsi.ISCSITarget):
(out, err) = (None, None)
try:
(out, err) = utils.execute('tgtadm', '--lld', 'iscsi',
'--op', 'new', '--mode',
'logicalunit', '--tid',
tid, '--lun', '1', '-b',
path, run_as_root=True)
(out, err) = cinder.privsep.targets.tgt.tgtadm_create(tid, path)
except putils.ProcessExecutionError as e:
LOG.error("Failed recovery attempt to create "
"iscsi backing lun for Volume "
@ -112,12 +109,7 @@ class TgtAdm(iscsi.ISCSITarget):
@utils.retry(putils.ProcessExecutionError)
def _do_tgt_update(self, name, force=False):
args = ['tgt-admin', '--update', name]
if force:
# Note: force may fail if there is active IO, but retry decorator
# should allow it to succeed on second attempt
args.append('-f')
(out, err) = utils.execute(*args, run_as_root=True)
(out, err) = cinder.privsep.targets.tgt.tgtadmin_update(name, force)
LOG.debug("StdOut from tgt-admin --update: %s", out)
LOG.debug("StdErr from tgt-admin --update: %s", err)
@ -131,14 +123,7 @@ class TgtAdm(iscsi.ISCSITarget):
# NOTE(jdg): Remove this when we get to the bottom of bug: #1398078
# for now, since we intermittently hit target already exists we're
# adding some debug info to try and pinpoint what's going on
(out, err) = utils.execute('tgtadm',
'--lld',
'iscsi',
'--op',
'show',
'--mode',
'target',
run_as_root=True)
(out, err) = cinder.privsep.targets.tgt.tgtadm_show()
LOG.debug("Targets prior to update: %s", out)
fileutils.ensure_tree(self.volumes_dir)
@ -206,14 +191,7 @@ class TgtAdm(iscsi.ISCSITarget):
# Grab targets list for debug
# Consider adding a check for lun 0 and 1 for tgtadm
# before considering this as valid
(out, err) = utils.execute('tgtadm',
'--lld',
'iscsi',
'--op',
'show',
'--mode',
'target',
run_as_root=True)
cinder.privsep.targets.tgt.tgtadm_show()
LOG.debug("Targets after update: %s", out)
iqn = '%s%s' % (self.iscsi_target_prefix, vol_id)
@ -267,11 +245,7 @@ class TgtAdm(iscsi.ISCSITarget):
try:
# NOTE(vish): --force is a workaround for bug:
# https://bugs.launchpad.net/cinder/+bug/1159948
utils.execute('tgt-admin',
'--force',
'--delete',
iqn,
run_as_root=True)
cinder.privsep.targets.tgt.tgtadmin_delete(iqn, force=True)
except putils.ProcessExecutionError as e:
non_fatal_errors = ("can't find the target",
"access control rule does not exist")
@ -298,10 +272,7 @@ class TgtAdm(iscsi.ISCSITarget):
try:
LOG.warning('Silent failure of target removal '
'detected, retry....')
utils.execute('tgt-admin',
'--delete',
iqn,
run_as_root=True)
cinder.privsep.targets.tgt.tgtadmin_delete(iqn)
except putils.ProcessExecutionError as e:
LOG.error("Failed to remove iscsi target for Volume "
"ID: %(vol_id)s: %(e)s",

2
etc/cinder/rootwrap.d/volume.filters
View File

@ -4,9 +4,7 @@
[Filters]
# cinder/volume/iscsi.py: iscsi_helper '--op' ...
ietadm: CommandFilter, ietadm, root
tgtadm: CommandFilter, tgtadm, root
iscsictl: CommandFilter, iscsictl, root
tgt-admin: CommandFilter, tgt-admin, root
cinder-rtstool: CommandFilter, cinder-rtstool, root
scstadmin: CommandFilter, scstadmin, root