From 89e1b0258a13d693d0ff28c28715fc906b626c70 Mon Sep 17 00:00:00 2001 From: Chuck Short Date: Thu, 18 Oct 2018 14:53:07 -0400 Subject: [PATCH] Switch ietadm to use olso.privsep 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 --- cinder/privsep/targets/iet.py | 85 ++++++++++++++++++++ cinder/tests/unit/targets/test_iet_driver.py | 55 +++++++------ cinder/volume/targets/iet.py | 74 +++-------------- etc/cinder/rootwrap.d/volume.filters | 2 +- 4 files changed, 126 insertions(+), 90 deletions(-) create mode 100644 cinder/privsep/targets/iet.py diff --git a/cinder/privsep/targets/iet.py b/cinder/privsep/targets/iet.py new file mode 100644 index 00000000000..afbbf41b0f9 --- /dev/null +++ b/cinder/privsep/targets/iet.py @@ -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)) diff --git a/cinder/tests/unit/targets/test_iet_driver.py b/cinder/tests/unit/targets/test_iet_driver.py index da324f6d3ba..ac613421c9c 100644 --- a/cinder/tests/unit/targets/test_iet_driver.py +++ b/cinder/tests/unit/targets/test_iet_driver.py @@ -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) diff --git a/cinder/volume/targets/iet.py b/cinder/volume/targets/iet.py index b98cffe64e1..5ab11148ac5 100644 --- a/cinder/volume/targets/iet.py +++ b/cinder/volume/targets/iet.py @@ -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) diff --git a/etc/cinder/rootwrap.d/volume.filters b/etc/cinder/rootwrap.d/volume.filters index 2cc1b068978..90e3fcd708b 100644 --- a/etc/cinder/rootwrap.d/volume.filters +++ b/etc/cinder/rootwrap.d/volume.filters @@ -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