openstack/cinder
Code Issues Proposed changes

Switch ietadm to use olso.privsep

Browse Source 
Switch ietadm to use oslo.privsep rather than olso.rootwrap.
This is mostly a straight copy of the the methods used to
create targets.

Change-Id: Ia4dc63d75960935e770a7fef352b51a7c75be4d6
Signed-off-by: Chuck Short <chucks@redhat.com>
This commit is contained in:
Chuck Short 2018-10-18 14:53:07 -04:00
parent 5b37d2aa33
commit 89e1b0258a
4 changed files with 126 additions and 90 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

85
cinder/privsep/targets/iet.py Normal file
View File

@ -0,0 +1,85 @@
# 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 ietadm related routines.
"""
from oslo_concurrency import processutils
import cinder.privsep
@cinder.privsep.sys_admin_pctxt.entrypoint
def new_target(name, tid):
"""Create new scsi target using specified parameters.
If the target already exists, ietadm returns
'Invalid argument' and error code '234'.
This should be ignored for ensure export case.
"""
processutils.execute('ietadm', '--op', 'new',
'--tid=%s' % tid,
'--params', 'Name=%s' % name,
check_exit_code=[0, 234])
@cinder.privsep.sys_admin_pctxt.entrypoint
def delete_target(tid):
processutils.execute('ietadm', '--op', 'delete',
'--tid=%s' % tid)
@cinder.privsep.sys_admin_pctxt.entrypoint
def force_delete_target(tid, sid, cid):
processutils.execute('ietadm', '--op', 'delete',
'--tid=%s' % tid,
'--sid=%s' % sid,
'--cid=%s' % cid)
@cinder.privsep.sys_admin_pctxt.entrypoint
def new_logicalunit(tid, lun, path, iotype):
"""Attach a new volume to scsi target as a logical unit.
If a logical unit exists on the specified target lun,
ietadm returns 'File exists' and error code '239'.
This should be ignored for ensure export case.
"""
processutils.execute('ietadm', '--op', 'new',
'--tid=%s' % tid,
'--lun=%d' % lun,
'--params',
'Path=%s,Type=%s' % (path, iotype),
check_exit_code=[0, 239])
@cinder.privsep.sys_admin_pctxt.entrypoint
def delete_logicalunit(tid, lun):
processutils.execute('ietadm', '--op', 'delete',
'--tid=%s' % tid,
'--lun=%d' % lun)
@cinder.privsep.sys_admin_pctxt.entrypoint
def new_auth(tid, type, username, password):
processutils.execute('ietadm', '--op', 'new',
'--tid=%s' % tid,
'--user',
'--params=%s=%s,Password=%s' % (type,
username,
password))

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

@ -53,13 +53,14 @@ class TestIetAdmDriver(tf.TargetDriverFixture):
@mock.patch('cinder.volume.targets.iet.IetAdm._get_target',
return_value=0)
@mock.patch('cinder.utils.execute')
@mock.patch('cinder.privsep.targets.iet.new_target')
@mock.patch('cinder.privsep.targets.iet.new_logicalunit')
@mock.patch('os.path.exists', return_value=True)
@mock.patch('cinder.utils.temporary_chown')
@mock.patch.object(iet, 'LOG')
def test_create_iscsi_target(self, mock_log, mock_chown, mock_exists,
mock_execute, mock_get_targ):
mock_execute.return_value = ('', '')
mock_new_logical_unit, mock_new_target,
mock_get_targ):
tmp_file = six.StringIO()
with mock.patch('six.moves.builtins.open') as mock_open:
mock_open.return_value = contextlib.closing(tmp_file)
@ -70,9 +71,10 @@ class TestIetAdmDriver(tf.TargetDriverFixture):
0,
0,
self.fake_volumes_dir))
self.assertTrue(mock_execute.called)
self.assertTrue(mock_new_target.called)
self.assertTrue(mock_open.called)
self.assertTrue(mock_get_targ.called)
self.assertTrue(mock_new_logical_unit.called)
# Test the failure case: Failed to chown the config file
mock_open.side_effect = putils.ProcessExecutionError
@ -84,7 +86,7 @@ class TestIetAdmDriver(tf.TargetDriverFixture):
self.fake_volumes_dir)
# Test the failure case: Failed to set new auth
mock_execute.side_effect = putils.ProcessExecutionError
mock_new_target.side_effect = putils.ProcessExecutionError
self.assertRaises(exception.ISCSITargetCreateFailed,
self.target.create_iscsi_target,
self.test_vol,
@ -107,10 +109,10 @@ class TestIetAdmDriver(tf.TargetDriverFixture):
@mock.patch('cinder.volume.targets.iet.IetAdm._get_target',
return_value=1)
@mock.patch('cinder.utils.execute')
def test_create_iscsi_target_already_exists(self, mock_execute,
mock_get_targ):
mock_execute.return_value = ('fake out', 'fake err')
@mock.patch('cinder.privsep.targets.iet.new_target')
@mock.patch('cinder.privsep.targets.iet.new_logicalunit')
def test_create_iscsi_target_already_exists(
self, mock_new_logical_unit, mock_new_target, mock_get_targ):
self.assertEqual(
1,
self.target.create_iscsi_target(
@ -119,27 +121,28 @@ class TestIetAdmDriver(tf.TargetDriverFixture):
0,
self.fake_volumes_dir))
self.assertTrue(mock_get_targ.called)
self.assertTrue(mock_execute.called)
self.assertTrue(mock_new_target.called)
self.assertTrue(mock_new_logical_unit.called)
@mock.patch('cinder.volume.targets.iet.IetAdm._find_sid_cid_for_target',
return_value=None)
@mock.patch('os.path.exists', return_value=False)
@mock.patch('cinder.utils.execute')
def test_remove_iscsi_target(self, mock_execute, mock_exists, mock_find):
@mock.patch('cinder.privsep.targets.iet.delete_logicalunit')
@mock.patch('cinder.privsep.targets.iet.delete_target')
def test_remove_iscsi_target(
self, mock_delete_target,
mock_delete_logicalunit, mock_exists, mock_find):
# Test the normal case
self.target.remove_iscsi_target(1,
0,
self.testvol['id'],
self.testvol['name'])
mock_execute.assert_any_call('ietadm',
'--op',
'delete',
'--tid=1',
run_as_root=True)
mock_delete_logicalunit.assert_called_once_with(1, 0)
mock_delete_target.assert_called_once_with(1)
# Test the failure case: putils.ProcessExecutionError
mock_execute.side_effect = putils.ProcessExecutionError
mock_delete_logicalunit.side_effect = putils.ProcessExecutionError
self.assertRaises(exception.ISCSITargetRemoveFailed,
self.target.remove_iscsi_target,
1,
@ -147,7 +150,8 @@ class TestIetAdmDriver(tf.TargetDriverFixture):
self.testvol['id'],
self.testvol['name'])
def test_find_sid_cid_for_target(self):
@mock.patch('cinder.privsep.targets.iet.delete_target')
def test_find_sid_cid_for_target(self, mock_delete_target):
tmp_file = six.StringIO()
tmp_file.write(
'tid:1 name:iqn.2010-10.org.openstack:volume-83c2e877-feed-46be-8435-77884fe55b45\n' # noqa
@ -165,11 +169,13 @@ class TestIetAdmDriver(tf.TargetDriverFixture):
@mock.patch('cinder.volume.targets.iet.IetAdm._get_target',
return_value=1)
@mock.patch('cinder.utils.execute')
@mock.patch('cinder.privsep.targets.iet.new_target')
@mock.patch('cinder.privsep.targets.iet.new_logicalunit')
@mock.patch('cinder.privsep.targets.iet.new_auth')
@mock.patch.object(iet.IetAdm, '_get_target_chap_auth')
def test_create_export(self, mock_get_chap, mock_execute,
mock_get_targ):
mock_execute.return_value = ('', '')
def test_create_export(
self, mock_get_chap, mock_new_auth, mock_new_logicalunit,
mock_new_target, mock_get_targ):
mock_get_chap.return_value = ('QZJbisGmn9AL954FNF4D',
'P68eE7u9eFqDGexd28DQ')
expected_result = {'location': '10.9.8.7:3260,1 '
@ -181,7 +187,8 @@ class TestIetAdmDriver(tf.TargetDriverFixture):
self.target.create_export(ctxt,
self.testvol,
self.fake_volumes_dir))
self.assertTrue(mock_execute.called)
self.assertTrue(mock_new_logicalunit.called)
self.assertTrue(mock_new_target.called)
@mock.patch('cinder.volume.targets.iet.IetAdm._get_target_chap_auth',
return_value=None)

74
cinder/volume/targets/iet.py
View File

@ -18,6 +18,7 @@ from oslo_concurrency import processutils as putils
from oslo_log import log as logging
from cinder import exception
import cinder.privsep.targets.iet
from cinder import utils
from cinder.volume.targets import iscsi
@ -91,14 +92,16 @@ class IetAdm(iscsi.ISCSITarget):
# Create a new iSCSI target. If a target already exists,
# the command returns 234, but we ignore it.
try:
self._new_target(name, tid)
cinder.privsep.targets.iet.new_target(name, tid)
tid = self._get_target(name)
self._new_logicalunit(tid, lun, path)
cinder.privsep.targets.iet.new_logicalunit(
tid, lun, path, self._iotype(path))
if chap_auth is not None:
(username, password) = chap_auth
config_auth = ' '.join((self.auth_type,) + chap_auth)
self._new_auth(tid, self.auth_type, username, password)
cinder.privsep.targets.iet.new_auth(
tid, self.auth_type, username, password)
except putils.ProcessExecutionError:
LOG.exception("Failed to create iscsi target for volume "
"id:%s", vol_id)
@ -147,13 +150,13 @@ class IetAdm(iscsi.ISCSITarget):
LOG.info("Removing iscsi_target for volume: %s", vol_id)
try:
self._delete_logicalunit(tid, lun)
cinder.privsep.targets.iet.delete_logicalunit(tid, lun)
session_info = self._find_sid_cid_for_target(tid, vol_name, vol_id)
if session_info:
sid, cid = session_info
self._force_delete_target(tid, sid, cid)
cinder.privsep.targets.iet.force_delete_target(tid, sid, cid)
self._delete_target(tid)
cinder.privsep.targets.iet.delete_target(tid)
except putils.ProcessExecutionError:
LOG.exception("Failed to remove iscsi target for volume "
"id:%s", vol_id)
@ -219,62 +222,3 @@ class IetAdm(iscsi.ISCSITarget):
return 'blockio' if self._is_block(path) else 'fileio'
else:
return self.iscsi_iotype
def _new_target(self, name, tid):
"""Create new scsi target using specified parameters.
If the target already exists, ietadm returns
'Invalid argument' and error code '234'.
This should be ignored for ensure export case.
"""
utils.execute('ietadm', '--op', 'new',
'--tid=%s' % tid,
'--params', 'Name=%s' % name,
run_as_root=True, check_exit_code=[0, 234])
def _delete_target(self, tid):
utils.execute('ietadm', '--op', 'delete',
'--tid=%s' % tid,
run_as_root=True)
def _force_delete_target(self, tid, sid, cid):
utils.execute('ietadm', '--op', 'delete',
'--tid=%s' % tid,
'--sid=%s' % sid,
'--cid=%s' % cid,
run_as_root=True)
def show_target(self, tid, iqn=None):
utils.execute('ietadm', '--op', 'show',
'--tid=%s' % tid,
run_as_root=True)
def _new_logicalunit(self, tid, lun, path):
"""Attach a new volume to scsi target as a logical unit.
If a logical unit exists on the specified target lun,
ietadm returns 'File exists' and error code '239'.
This should be ignored for ensure export case.
"""
utils.execute('ietadm', '--op', 'new',
'--tid=%s' % tid,
'--lun=%d' % lun,
'--params',
'Path=%s,Type=%s' % (path, self._iotype(path)),
run_as_root=True, check_exit_code=[0, 239])
def _delete_logicalunit(self, tid, lun):
utils.execute('ietadm', '--op', 'delete',
'--tid=%s' % tid,
'--lun=%d' % lun,
run_as_root=True)
def _new_auth(self, tid, type, username, password):
utils.execute('ietadm', '--op', 'new',
'--tid=%s' % tid,
'--user',
'--params=%s=%s,Password=%s' % (type,
username,
password),
run_as_root=True)

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

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