diff --git a/ironic_python_agent/errors.py b/ironic_python_agent/errors.py index 4f9fa8056..cb0b0afbb 100644 --- a/ironic_python_agent/errors.py +++ b/ironic_python_agent/errors.py @@ -173,29 +173,6 @@ class ImageWriteError(RESTError): super(ImageWriteError, self).__init__(details) -class ConfigDriveTooLargeError(RESTError): - """Error raised when a configdrive is larger than the partition.""" - - message = 'Configdrive is too large for intended partition' - - def __init__(self, filename, filesize): - details = ('Configdrive at {} has size {}, which is larger than ' - 'the intended partition.').format(filename, filesize) - super(ConfigDriveTooLargeError, self).__init__(details) - - -class ConfigDriveWriteError(RESTError): - """Error raised when a configdrive cannot be written to a device.""" - - message = 'Error writing configdrive to device' - - def __init__(self, device, exit_code, stdout, stderr): - details = ('Writing configdrive to device {} failed with exit code ' - '{}. stdout: {}. stderr: {}.') - details = details.format(device, exit_code, stdout, stderr) - super(ConfigDriveWriteError, self).__init__(details) - - class SystemRebootError(RESTError): """Error raised when a system cannot reboot.""" diff --git a/ironic_python_agent/extensions/standby.py b/ironic_python_agent/extensions/standby.py index f21f05a35..6db8e07a8 100644 --- a/ironic_python_agent/extensions/standby.py +++ b/ironic_python_agent/extensions/standby.py @@ -12,8 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -import base64 -import gzip import hashlib import os import requests @@ -144,88 +142,6 @@ def _write_image(image_info, device): return uuids -def _configdrive_is_url(configdrive): - """Determine if the configdrive location looks like an HTTP(S) URL. - - :param configdrive: Location of the configdrive as a string. - :returns: True if configdrive looks like an HTTP(S) URL, False otherwise. - """ - return (configdrive.startswith('http://') - or configdrive.startswith('https://')) - - -def _download_configdrive_to_file(configdrive, filename): - """Download the configdrive to a local file. - - :param configdrive: The URL of the configdrive. - :param filename: The filename of where to store the configdrive locally. - """ - content = requests.get(configdrive).content - _write_configdrive_to_file(content, filename) - - -def _write_configdrive_to_file(configdrive, filename): - """Writes the configdrive to a file. - - Note that the contents of the configdrive are expected to be gzipped and - base64 encoded. - - :param configdrive: Contents of the configdrive file. - :param filename: The filename of where to write the configdrive. - """ - LOG.debug('Writing configdrive to {}'.format(filename)) - # configdrive data is base64'd, decode it first - data = six.StringIO(base64.b64decode(configdrive)) - gunzipped = gzip.GzipFile('configdrive', 'rb', 9, data) - with open(filename, 'wb') as f: - f.write(gunzipped.read()) - gunzipped.close() - - -def _write_configdrive_to_partition(configdrive, device): - """Writes the configdrive to a partition on the given device. - - :param configdrive: A string containing the location of the config drive - as a URL OR the contents of the configdrive which - must be gzipped and base64 encoded. - :param device: The disk name, as a string, on which to store the image. - Example: '/dev/sda' - - :raises: ConfigDriveTooLargeError if the configdrive contents are too - large to store on the given device. - """ - filename = _configdrive_location() - if _configdrive_is_url(configdrive): - _download_configdrive_to_file(configdrive, filename) - else: - _write_configdrive_to_file(configdrive, filename) - - # check configdrive size before writing it - filesize = os.stat(filename).st_size - if filesize > (64 * 1024 * 1024): - raise errors.ConfigDriveTooLargeError(filename, filesize) - - starttime = time.time() - script = _path_to_script('shell/copy_configdrive_to_disk.sh') - command = ['/bin/bash', script, filename, device] - LOG.info('copying configdrive to disk with command {}'.format( - ' '.join(command))) - - try: - stdout, stderr = utils.execute(*command, check_exit_code=[0]) - except processutils.ProcessExecutionError as e: - raise errors.ConfigDriveWriteError(device, - e.exit_code, - e.stdout, - e.stderr) - - totaltime = time.time() - starttime - LOG.info('configdrive copied from {} to {} in {} seconds'.format( - filename, - device, - totaltime)) - - def _message_format(msg, image_info, device, partition_uuids): """Helper method to get and populate different messages.""" message = None @@ -527,7 +443,7 @@ class StandbyExtension(base.BaseAgentExtension): :raises: ImageChecksumError if the checksum of the local image does not match the checksum as reported by glance in image_info. :raises: ImageWriteError if writing the image fails. - :raises: ConfigDriveTooLargeError if the configdrive contents are too + :raises: InstanceDeployFailure if failed to create config drive. large to store on the given device. """ LOG.debug('Preparing image %s', image_info['id']) @@ -551,8 +467,18 @@ class StandbyExtension(base.BaseAgentExtension): # work_on_disk(). if image_info.get('image_type') != 'partition': if configdrive is not None: - _write_configdrive_to_partition(configdrive, device) - + # Will use dummy value of 'local' for 'node_uuid', + # if it is not available. This is to handle scenario + # wherein new IPA is being used with older version + # of Ironic that did not pass 'node_uuid' in 'image_info' + node_uuid = image_info.get('node_uuid', 'local') + starttime = time.time() + disk_utils.create_config_drive_partition(node_uuid, + device, + configdrive) + totaltime = time.time() - starttime + LOG.info('configdrive copied to {0} in {1} ' + 'seconds.'.format(device, totaltime)) msg = 'image ({}) written to device {} ' result_msg = _message_format(msg, image_info, device, self.partition_uuids) diff --git a/ironic_python_agent/shell/copy_configdrive_to_disk.sh b/ironic_python_agent/shell/copy_configdrive_to_disk.sh deleted file mode 100755 index b62ba6b9e..000000000 --- a/ironic_python_agent/shell/copy_configdrive_to_disk.sh +++ /dev/null @@ -1,118 +0,0 @@ -#!/bin/bash - -# Copyright 2013 Rackspace, Inc. -# -# 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. - -# This should work with almost any image that uses MBR partitioning and -# doesn't already have 3 or more partitions -- or else you'll no longer -# be able to create extended partitions on the disk. - -log() { - echo "`basename $0`: $@" -} - -fail() { - log "Error $@" - exit 1 -} - -usage() { - [[ -z "$1" ]] || echo -e "USAGE ERROR: $@\n" - echo "`basename $0`: CONFIGDRIVE DEVICE" - echo " - This script injects CONFIGDRIVE contents as an iso9660" - echo " filesystem on a partition at the end of DEVICE." - exit 1 -} - -MAX_DISK_PARTITIONS=128 -MAX_MBR_SIZE_MB=2097152 - -CONFIGDRIVE="$1" -DEVICE="$2" - -[[ -f $CONFIGDRIVE ]] || usage "$CONFIGDRIVE (CONFIGDRIVE) is not a regular file" -[[ -b $DEVICE ]] || usage "$DEVICE (DEVICE) is not a block device" - -# We need to run partx -u to ensure all partitions are visible so the -# following blkid command returns partitions just imaged to the device -partx -u $DEVICE || fail "running partx -u $DEVICE" - -# todo(jayf): partx -u doesn't work in all cases, but partprobe fails in -# devstack. We run both commands now as a temporary workaround for bug 1433812 -# long term, this should all be refactored into python and share code with -# the other partition-modifying code in the agent. -partprobe $DEVICE || true - -# Check for preexisting partition for configdrive -EXISTING_PARTITION=`/sbin/blkid -l -o device $DEVICE -t LABEL=config-2` -if [[ $? == 0 ]]; then - log "Existing configdrive found on ${DEVICE} at ${EXISTING_PARTITION}" - ISO_PARTITION=$EXISTING_PARTITION -else - - # Check if it is GPT partition and needs to be re-sized - parted $DEVICE print 2>&1 | grep "fix the GPT to use all of the space" - if [[ $? == 0 ]]; then - log "Fixing GPT to use all of the space on device $DEVICE" - sgdisk -e $DEVICE || fail "move backup GPT data structures to the end of ${DEVICE}" - - # Need to create new partition for config drive - # Not all images have partion numbers in a sequential numbers. There are holes. - # These holes get filled up when a new partition is created. - TEMP_DIR="$(mktemp -d)" - EXISTING_PARTITION_LIST=$TEMP_DIR/existing_partitions - UPDATED_PARTITION_LIST=$TEMP_DIR/updated_partitions - - # Sort partitions by second column, which is start sector - gdisk -l $DEVICE | grep -A$MAX_DISK_PARTITIONS "Number Start" | grep -v "Number Start" | sort -k 2 > $EXISTING_PARTITION_LIST - - # Create small partition at the end of the device - log "Adding configdrive partition to $DEVICE" - sgdisk -n 0:-64MB:0 $DEVICE || fail "creating configdrive on ${DEVICE}" - - gdisk -l $DEVICE | grep -A$MAX_DISK_PARTITIONS "Number Start" | grep -v "Number Start" | sort -k 2 > $UPDATED_PARTITION_LIST - - CONFIG_PARTITION_ID=`diff $EXISTING_PARTITION_LIST $UPDATED_PARTITION_LIST | tail -n1 |awk '{print $2}'` - ISO_PARTITION="${DEVICE}${CONFIG_PARTITION_ID}" - else - log "Working on MBR only device $DEVICE" - - # get total disk size, to detect if that exceeds 2TB msdos limit - disksize_bytes=$(blockdev --getsize64 $DEVICE) - disksize_mb=$(( ${disksize_bytes%% *} / 1024 / 1024)) - - startlimit=-64MiB - endlimit=-0 - if [ "$disksize_mb" -gt "$MAX_MBR_SIZE_MB" ]; then - # Create small partition at 2TB limit - startlimit=$(($MAX_MBR_SIZE_MB - 65)) - endlimit=$(($MAX_MBR_SIZE_MB - 1)) - fi - - log "Adding configdrive partition to $DEVICE" - parted -a optimal -s -- $DEVICE mkpart primary ext2 $startlimit $endlimit || fail "creating configdrive on ${DEVICE}" - - # Find partition we just created - # Dump all partitions, ignore empty ones, then get the last partition ID - ISO_PARTITION=`sfdisk --dump $DEVICE | grep -v ' 0,' | tail -n1 | awk -F ':' '{print $1}' | sed -e 's/\s*$//'` || fail "finding ISO partition created on ${DEVICE}" - fi - # Wait for udev to pick up the partition - udevadm settle --exit-if-exists=$ISO_PARTITION -fi - -# This writes the ISO image to the config drive. -log "Writing Configdrive contents in $CONFIGDRIVE to $ISO_PARTITION" -dd if=$CONFIGDRIVE of=$ISO_PARTITION bs=64K oflag=direct || fail "writing Configdrive to ${ISO_PARTITION}" - -log "${DEVICE} imaged successfully!" diff --git a/ironic_python_agent/tests/unit/extensions/test_standby.py b/ironic_python_agent/tests/unit/extensions/test_standby.py index 7ac9e6fed..ecdabf3e0 100644 --- a/ironic_python_agent/tests/unit/extensions/test_standby.py +++ b/ironic_python_agent/tests/unit/extensions/test_standby.py @@ -25,6 +25,7 @@ from ironic_python_agent.extensions import standby def _build_fake_image_info(): return { 'id': 'fake_id', + 'node_uuid': '1be26c0b-03f2-4d2e-ae87-c02d7f33c123', 'urls': [ 'http://example.org', ], @@ -282,84 +283,6 @@ class TestStandbyExtension(test_base.BaseTestCase): self.assertEqual(expected_uuid, work_on_disk_mock.return_value) - def test_configdrive_is_url(self): - self.assertTrue(standby._configdrive_is_url('http://some/url')) - self.assertTrue(standby._configdrive_is_url('https://some/url')) - self.assertFalse(standby._configdrive_is_url('ftp://some/url')) - self.assertFalse(standby._configdrive_is_url('binary-blob')) - - @mock.patch.object(standby, '_write_configdrive_to_file') - @mock.patch('requests.get', autospec=True) - def test_download_configdrive_to_file(self, get_mock, write_mock): - url = 'http://swift/configdrive' - get_mock.return_value.content = 'data' - standby._download_configdrive_to_file(url, 'filename') - get_mock.assert_called_once_with(url) - write_mock.assert_called_once_with('data', 'filename') - - @mock.patch('gzip.GzipFile', autospec=True) - @mock.patch('six.moves.builtins.open', autospec=True) - @mock.patch('base64.b64decode', autospec=True) - def test_write_configdrive_to_file(self, b64_mock, open_mock, gzip_mock): - open_mock.return_value.__enter__ = lambda s: s - open_mock.return_value.__exit__ = mock.Mock() - write_mock = open_mock.return_value.write - gzip_read_mock = gzip_mock.return_value.read - gzip_read_mock.return_value = 'ungzipped' - b64_mock.return_value = 'configdrive_data' - filename = standby._configdrive_location() - - standby._write_configdrive_to_file('b64data', filename) - open_mock.assert_called_once_with(filename, 'wb') - gzip_read_mock.assert_called_once_with() - write_mock.assert_called_once_with('ungzipped') - - @mock.patch('os.stat', autospec=True) - @mock.patch(('ironic_python_agent.extensions.standby.' - '_write_configdrive_to_file'), - autospec=True) - @mock.patch('six.moves.builtins.open', autospec=True) - @mock.patch('ironic_python_agent.utils.execute', autospec=True) - def test_write_configdrive_to_partition(self, execute_mock, open_mock, - configdrive_mock, stat_mock): - device = '/dev/sda' - configdrive = standby._configdrive_location() - script = standby._path_to_script('shell/copy_configdrive_to_disk.sh') - command = ['/bin/bash', script, configdrive, device] - execute_mock.return_value = ('', '') - stat_mock.return_value.st_size = 5 - - standby._write_configdrive_to_partition(configdrive, device) - execute_mock.assert_called_once_with(*command, check_exit_code=[0]) - - execute_mock.reset_mock() - execute_mock.return_value = ('', '') - execute_mock.side_effect = processutils.ProcessExecutionError - - self.assertRaises(errors.ConfigDriveWriteError, - standby._write_configdrive_to_partition, - configdrive, - device) - - execute_mock.assert_called_once_with(*command, check_exit_code=[0]) - - @mock.patch('os.stat', autospec=True) - @mock.patch(('ironic_python_agent.extensions.standby.' - '_write_configdrive_to_file'), - autospec=True) - @mock.patch('six.moves.builtins.open', autospec=True) - @mock.patch('ironic_python_agent.utils.execute', autospec=True) - def test_write_configdrive_too_large(self, execute_mock, open_mock, - configdrive_mock, stat_mock): - device = '/dev/sda' - configdrive = standby._configdrive_location() - stat_mock.return_value.st_size = 65 * 1024 * 1024 - - self.assertRaises(errors.ConfigDriveTooLargeError, - standby._write_configdrive_to_partition, - configdrive, - device) - @mock.patch('hashlib.md5') @mock.patch('six.moves.builtins.open') @mock.patch('requests.get') @@ -554,8 +477,7 @@ class TestStandbyExtension(test_base.BaseTestCase): '{} ').format(image_info['id'], 'manager') self.assertEqual(cmd_result, async_result.command_result['result']) - @mock.patch(('ironic_python_agent.extensions.standby.' - '_write_configdrive_to_partition'), + @mock.patch('ironic_lib.disk_utils.create_config_drive_partition', autospec=True) @mock.patch('ironic_python_agent.hardware.dispatch_to_managers', autospec=True) @@ -587,8 +509,9 @@ class TestStandbyExtension(test_base.BaseTestCase): download_mock.assert_called_once_with(image_info) write_mock.assert_called_once_with(image_info, 'manager') dispatch_mock.assert_called_once_with('get_os_install_device') - configdrive_copy_mock.assert_called_once_with('configdrive_data', - 'manager') + configdrive_copy_mock.assert_called_once_with(image_info['node_uuid'], + 'manager', + 'configdrive_data') self.assertEqual('SUCCEEDED', async_result.command_status) self.assertIn('result', async_result.command_result) @@ -596,29 +519,7 @@ class TestStandbyExtension(test_base.BaseTestCase): '{} ').format(image_info['id'], 'manager') self.assertEqual(cmd_result, async_result.command_result['result']) - download_mock.reset_mock() - write_mock.reset_mock() - configdrive_copy_mock.reset_mock() - # image is now cached, make sure download/write doesn't happen - async_result = self.agent_extension.prepare_image( - image_info=image_info, - configdrive='configdrive_data' - ) - async_result.join() - - self.assertEqual(0, download_mock.call_count) - self.assertEqual(0, write_mock.call_count) - configdrive_copy_mock.assert_called_once_with('configdrive_data', - 'manager') - - self.assertEqual('SUCCEEDED', async_result.command_status) - self.assertIn('result', async_result.command_result) - cmd_result = ('prepare_image: image ({}) written to device ' - '{} ').format(image_info['id'], 'manager') - self.assertEqual(cmd_result, async_result.command_result['result']) - - @mock.patch(('ironic_python_agent.extensions.standby.' - '_write_configdrive_to_partition'), + @mock.patch('ironic_lib.disk_utils.create_config_drive_partition', autospec=True) @mock.patch('ironic_python_agent.hardware.dispatch_to_managers', autospec=True) @@ -680,8 +581,7 @@ class TestStandbyExtension(test_base.BaseTestCase): image_info['id'], 'manager', 'root_uuid') self.assertEqual(cmd_result, async_result.command_result['result']) - @mock.patch(('ironic_python_agent.extensions.standby.' - '_write_configdrive_to_partition'), + @mock.patch('ironic_lib.disk_utils.create_config_drive_partition', autospec=True) @mock.patch('ironic_python_agent.hardware.dispatch_to_managers', autospec=True) @@ -717,8 +617,7 @@ class TestStandbyExtension(test_base.BaseTestCase): '{} ').format(image_info['id'], 'manager') self.assertEqual(cmd_result, async_result.command_result['result']) - @mock.patch(('ironic_python_agent.extensions.standby.' - '_write_configdrive_to_partition'), + @mock.patch('ironic_lib.disk_utils.create_config_drive_partition', autospec=True) @mock.patch('ironic_python_agent.hardware.dispatch_to_managers', autospec=True) diff --git a/ironic_python_agent/tests/unit/test_errors.py b/ironic_python_agent/tests/unit/test_errors.py index a904edb7f..40aa93996 100644 --- a/ironic_python_agent/tests/unit/test_errors.py +++ b/ironic_python_agent/tests/unit/test_errors.py @@ -111,11 +111,6 @@ class TestErrors(test_base.BaseTestCase): (errors.ImageWriteError('device', 'exit_code', 'stdout', 'stderr'), DIFF_CL_DETAILS), - (errors.ConfigDriveTooLargeError('filename', 'filesize'), - DIFF_CL_DETAILS), - (errors.ConfigDriveWriteError('device', 'exit_code', 'stdout', - 'stderr'), - DIFF_CL_DETAILS), (errors.SystemRebootError('exit_code', 'stdout', 'stderr'), DIFF_CL_DETAILS), (errors.BlockDeviceEraseError(DETAILS), SAME_DETAILS),