openstack/ironic-python-agent
Code Issues Proposed changes

Drop usage of run_as_root

Browse Source 
IPA can only be run as root and does not use rootwrap. We need to
eventually remove support for rootwrap from ironic-lib.

Change-Id: Iffd5cae5e3dc8637bc6dd10b3bcc9fe33932b8cf
This commit is contained in:
Dmitry Tantsur
2024-01-23 14:19:21 +01:00
parent 1e107bd625
commit 9f849472ca
4 changed files with 20 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
ironic_python_agent/extensions/standby.py
View File

@@ -753,7 +753,7 @@ def _validate_partitioning(device):
try: try:
# Ensure we re-read the partition table before we try to list # Ensure we re-read the partition table before we try to list
# partitions # partitions
utils.execute('partprobe', device, run_as_root=True, utils.execute('partprobe', device,
attempts=CONF.disk_utils.partprobe_attempts) attempts=CONF.disk_utils.partprobe_attempts)
except (processutils.UnknownArgumentError, except (processutils.UnknownArgumentError,
processutils.ProcessExecutionError, OSError) as e: processutils.ProcessExecutionError, OSError) as e:

10
ironic_python_agent/partition_utils.py
View File

@@ -156,7 +156,7 @@ def get_labelled_partition(device_path, label, node_uuid):
try: try:
output, err = utils.execute('lsblk', '-Po', 'name,label', device_path, output, err = utils.execute('lsblk', '-Po', 'name,label', device_path,
check_exit_code=[0, 1], check_exit_code=[0, 1],
use_standard_locale=True, run_as_root=True) use_standard_locale=True)
except (processutils.UnknownArgumentError, except (processutils.UnknownArgumentError,
processutils.ProcessExecutionError, OSError) as e: processutils.ProcessExecutionError, OSError) as e:
@@ -382,8 +382,7 @@ def create_config_drive_partition(node_uuid, device, configdrive):
create_option = '0:-%dMB:0' % MAX_CONFIG_DRIVE_SIZE_MB create_option = '0:-%dMB:0' % MAX_CONFIG_DRIVE_SIZE_MB
uuid_option = '0:%s' % part_uuid uuid_option = '0:%s' % part_uuid
utils.execute('sgdisk', '-n', create_option, utils.execute('sgdisk', '-n', create_option,
'-u', uuid_option, device, '-u', uuid_option, device)
run_as_root=True)
else: else:
cur_parts = set(part['number'] cur_parts = set(part['number']
for part in disk_utils.list_partitions(device)) for part in disk_utils.list_partitions(device))
@@ -425,7 +424,7 @@ def create_config_drive_partition(node_uuid, device, configdrive):
utils.execute('parted', '-a', 'optimal', '-s', '--', device, utils.execute('parted', '-a', 'optimal', '-s', '--', device,
'mkpart', 'primary', 'fat32', startlimit, 'mkpart', 'primary', 'fat32', startlimit,
endlimit, run_as_root=True) endlimit)
# Trigger device rescan # Trigger device rescan
disk_utils.trigger_device_rescan(device) disk_utils.trigger_device_rescan(device)
@@ -588,8 +587,7 @@ def _is_disk_larger_than_max_size(device, node_uuid):
try: try:
disksize_bytes, err = utils.execute('blockdev', '--getsize64', disksize_bytes, err = utils.execute('blockdev', '--getsize64',
device, device,
use_standard_locale=True, use_standard_locale=True)
run_as_root=True)
except (processutils.UnknownArgumentError, except (processutils.UnknownArgumentError,
processutils.ProcessExecutionError, OSError) as e: processutils.ProcessExecutionError, OSError) as e:
msg = ('Failed to get size of disk %(disk)s for node %(node)s. ' msg = ('Failed to get size of disk %(disk)s for node %(node)s. '

3
ironic_python_agent/tests/unit/extensions/test_standby.py
View File

@@ -887,7 +887,6 @@ class TestStandbyExtension(base.IronicAgentTest):
self.assertEqual(cmd_result, async_result.command_result['result']) self.assertEqual(cmd_result, async_result.command_result['result'])
list_part_mock.assert_called_with('manager') list_part_mock.assert_called_with('manager')
execute_mock.assert_called_with('partprobe', 'manager', execute_mock.assert_called_with('partprobe', 'manager',
run_as_root=True,
attempts=mock.ANY) attempts=mock.ANY)
self.assertEqual({'root uuid': 'ROOT'}, self.assertEqual({'root uuid': 'ROOT'},
self.agent_extension.partition_uuids) self.agent_extension.partition_uuids)
@@ -959,7 +958,6 @@ class TestStandbyExtension(base.IronicAgentTest):
self.assertEqual(cmd_result, async_result.command_result['result']) self.assertEqual(cmd_result, async_result.command_result['result'])
list_part_mock.assert_called_with('manager') list_part_mock.assert_called_with('manager')
execute_mock.assert_called_with('partprobe', 'manager', execute_mock.assert_called_with('partprobe', 'manager',
run_as_root=True,
attempts=mock.ANY) attempts=mock.ANY)
self.assertEqual({'root uuid': 'root_uuid'}, self.assertEqual({'root uuid': 'root_uuid'},
self.agent_extension.partition_uuids) self.agent_extension.partition_uuids)
@@ -1107,7 +1105,6 @@ class TestStandbyExtension(base.IronicAgentTest):
self.assertEqual(cmd_result, async_result.command_result['result']) self.assertEqual(cmd_result, async_result.command_result['result'])
list_part_mock.assert_called_with('manager') list_part_mock.assert_called_with('manager')
execute_mock.assert_called_with('partprobe', 'manager', execute_mock.assert_called_with('partprobe', 'manager',
run_as_root=True,
attempts=mock.ANY) attempts=mock.ANY)
self.assertEqual({}, self.agent_extension.partition_uuids) self.assertEqual({}, self.agent_extension.partition_uuids)

32
ironic_python_agent/tests/unit/test_partition_utils.py
View File

@@ -170,7 +170,7 @@ class GetLabelledPartitionTestCases(base.IronicAgentTest):
mock.call('partprobe', self.dev, run_as_root=True, attempts=10), mock.call('partprobe', self.dev, run_as_root=True, attempts=10),
mock.call('lsblk', '-Po', 'name,label', self.dev, mock.call('lsblk', '-Po', 'name,label', self.dev,
check_exit_code=[0, 1], check_exit_code=[0, 1],
use_standard_locale=True, run_as_root=True) use_standard_locale=True)
] ]
mock_execute.assert_has_calls(execute_calls) mock_execute.assert_has_calls(execute_calls)
@@ -186,7 +186,7 @@ class GetLabelledPartitionTestCases(base.IronicAgentTest):
mock.call('partprobe', self.dev, run_as_root=True, attempts=10), mock.call('partprobe', self.dev, run_as_root=True, attempts=10),
mock.call('lsblk', '-Po', 'name,label', self.dev, mock.call('lsblk', '-Po', 'name,label', self.dev,
check_exit_code=[0, 1], check_exit_code=[0, 1],
use_standard_locale=True, run_as_root=True) use_standard_locale=True)
] ]
mock_execute.assert_has_calls(execute_calls) mock_execute.assert_has_calls(execute_calls)
@@ -201,7 +201,7 @@ class GetLabelledPartitionTestCases(base.IronicAgentTest):
mock.call('partprobe', self.dev, run_as_root=True, attempts=10), mock.call('partprobe', self.dev, run_as_root=True, attempts=10),
mock.call('lsblk', '-Po', 'name,label', self.dev, mock.call('lsblk', '-Po', 'name,label', self.dev,
check_exit_code=[0, 1], check_exit_code=[0, 1],
use_standard_locale=True, run_as_root=True) use_standard_locale=True)
] ]
mock_execute.assert_has_calls(execute_calls) mock_execute.assert_has_calls(execute_calls)
@@ -220,7 +220,7 @@ class GetLabelledPartitionTestCases(base.IronicAgentTest):
mock.call('partprobe', self.dev, run_as_root=True, attempts=10), mock.call('partprobe', self.dev, run_as_root=True, attempts=10),
mock.call('lsblk', '-Po', 'name,label', self.dev, mock.call('lsblk', '-Po', 'name,label', self.dev,
check_exit_code=[0, 1], check_exit_code=[0, 1],
use_standard_locale=True, run_as_root=True) use_standard_locale=True)
] ]
mock_execute.assert_has_calls(execute_calls) mock_execute.assert_has_calls(execute_calls)
@@ -236,7 +236,7 @@ class GetLabelledPartitionTestCases(base.IronicAgentTest):
mock.call('partprobe', self.dev, run_as_root=True, attempts=10), mock.call('partprobe', self.dev, run_as_root=True, attempts=10),
mock.call('lsblk', '-Po', 'name,label', self.dev, mock.call('lsblk', '-Po', 'name,label', self.dev,
check_exit_code=[0, 1], check_exit_code=[0, 1],
use_standard_locale=True, run_as_root=True) use_standard_locale=True)
] ]
mock_execute.assert_has_calls(execute_calls) mock_execute.assert_has_calls(execute_calls)
self.assertEqual(1, mock_log.call_count) self.assertEqual(1, mock_log.call_count)
@@ -253,7 +253,7 @@ class IsDiskLargerThanMaxSizeTestCases(base.IronicAgentTest):
result = partition_utils._is_disk_larger_than_max_size(self.dev, result = partition_utils._is_disk_larger_than_max_size(self.dev,
self.node_uuid) self.node_uuid)
mock_execute.assert_called_once_with('blockdev', '--getsize64', mock_execute.assert_called_once_with('blockdev', '--getsize64',
'/dev/fake', run_as_root=True, '/dev/fake',
use_standard_locale=True) use_standard_locale=True)
return result return result
@@ -277,7 +277,7 @@ class IsDiskLargerThanMaxSizeTestCases(base.IronicAgentTest):
partition_utils._is_disk_larger_than_max_size, partition_utils._is_disk_larger_than_max_size,
self.dev, self.node_uuid) self.dev, self.node_uuid)
mock_execute.assert_called_once_with('blockdev', '--getsize64', mock_execute.assert_called_once_with('blockdev', '--getsize64',
'/dev/fake', run_as_root=True, '/dev/fake',
use_standard_locale=True) use_standard_locale=True)
self.assertEqual(1, mock_log.call_count) self.assertEqual(1, mock_log.call_count)
@@ -697,7 +697,7 @@ class CreateConfigDriveTestCases(base.IronicAgentTest):
config_url) config_url)
mock_execute.assert_has_calls([ mock_execute.assert_has_calls([
mock.call('sgdisk', '-n', '0:-64MB:0', '-u', '0:fake-uuid', mock.call('sgdisk', '-n', '0:-64MB:0', '-u', '0:fake-uuid',
self.dev, run_as_root=True), self.dev),
mock.call('sync'), mock.call('sync'),
mock.call('udevadm', 'settle'), mock.call('udevadm', 'settle'),
mock.call('partprobe', self.dev, attempts=10, run_as_root=True), mock.call('partprobe', self.dev, attempts=10, run_as_root=True),
@@ -758,7 +758,7 @@ class CreateConfigDriveTestCases(base.IronicAgentTest):
config_url) config_url)
mock_execute.assert_has_calls([ mock_execute.assert_has_calls([
mock.call('sgdisk', '-n', '0:-64MB:0', '-u', '0:fake-uuid', mock.call('sgdisk', '-n', '0:-64MB:0', '-u', '0:fake-uuid',
self.dev, run_as_root=True), self.dev),
mock.call('sync'), mock.call('sync'),
mock.call('udevadm', 'settle'), mock.call('udevadm', 'settle'),
mock.call('partprobe', self.dev, attempts=10, run_as_root=True), mock.call('partprobe', self.dev, attempts=10, run_as_root=True),
@@ -823,7 +823,7 @@ class CreateConfigDriveTestCases(base.IronicAgentTest):
config_url) config_url)
mock_execute.assert_has_calls([ mock_execute.assert_has_calls([
mock.call('sgdisk', '-n', '0:-64MB:0', '-u', '0:fake-uuid', mock.call('sgdisk', '-n', '0:-64MB:0', '-u', '0:fake-uuid',
self.dev, run_as_root=True), self.dev),
mock.call('sync'), mock.call('sync'),
mock.call('udevadm', 'settle'), mock.call('udevadm', 'settle'),
mock.call('partprobe', self.dev, attempts=10, run_as_root=True), mock.call('partprobe', self.dev, attempts=10, run_as_root=True),
@@ -918,13 +918,13 @@ class CreateConfigDriveTestCases(base.IronicAgentTest):
parted_call = mock.call('parted', '-a', 'optimal', '-s', parted_call = mock.call('parted', '-a', 'optimal', '-s',
'--', self.dev, 'mkpart', '--', self.dev, 'mkpart',
'primary', 'fat32', 2097087, 'primary', 'fat32', 2097087,
2097151, run_as_root=True) 2097151)
else: else:
self.assertEqual(0, mock_log.call_count) self.assertEqual(0, mock_log.call_count)
parted_call = mock.call('parted', '-a', 'optimal', '-s', parted_call = mock.call('parted', '-a', 'optimal', '-s',
'--', self.dev, 'mkpart', '--', self.dev, 'mkpart',
'primary', 'fat32', '-64MiB', 'primary', 'fat32', '-64MiB',
'-0', run_as_root=True) '-0')
mock_execute.assert_has_calls([ mock_execute.assert_has_calls([
parted_call, parted_call,
mock.call('sync'), mock.call('sync'),
@@ -1025,8 +1025,7 @@ class CreateConfigDriveTestCases(base.IronicAgentTest):
mock_execute.assert_has_calls([ mock_execute.assert_has_calls([
mock.call('parted', '-a', 'optimal', '-s', '--', mock.call('parted', '-a', 'optimal', '-s', '--',
self.dev, 'mkpart', 'primary', self.dev, 'mkpart', 'primary',
'fat32', '-64MiB', '-0', 'fat32', '-64MiB', '-0'),
run_as_root=True),
mock.call('sync'), mock.call('sync'),
mock.call('udevadm', 'settle'), mock.call('udevadm', 'settle'),
mock.call('partprobe', self.dev, attempts=10, run_as_root=True), mock.call('partprobe', self.dev, attempts=10, run_as_root=True),
@@ -1096,8 +1095,7 @@ class CreateConfigDriveTestCases(base.IronicAgentTest):
mock_get_configdrive.assert_called_with(config_url, self.node_uuid) mock_get_configdrive.assert_called_with(config_url, self.node_uuid)
mock_execute.assert_called_with('parted', '-a', 'optimal', '-s', '--', mock_execute.assert_called_with('parted', '-a', 'optimal', '-s', '--',
self.dev, 'mkpart', 'primary', self.dev, 'mkpart', 'primary',
'fat32', '-64MiB', '-0', 'fat32', '-64MiB', '-0')
run_as_root=True)
self.assertEqual(1, mock_list_partitions.call_count) self.assertEqual(1, mock_list_partitions.call_count)
mock_fix_gpt_partition.assert_called_with(self.dev, self.node_uuid) mock_fix_gpt_partition.assert_called_with(self.dev, self.node_uuid)
mock_table_type.assert_called_with(self.dev) mock_table_type.assert_called_with(self.dev)
@@ -1263,7 +1261,7 @@ class RealFilePartitioningTestCase(base.IronicAgentTest):
real_execute = utils.execute real_execute = utils.execute
def fake_execute(*cmd, **kwargs): def fake_execute(*cmd, **kwargs):
kwargs['run_as_root'] = False kwargs.pop('run_as_root', None)
return real_execute(*cmd, **kwargs) return real_execute(*cmd, **kwargs)
with mock.patch.object(utils, 'execute', fake_execute): with mock.patch.object(utils, 'execute', fake_execute):