Accept new parameters for prepare_image
The parameters sent to `prepare_image` changed in https://review.openstack.org/#/c/86490/ This patch brings `prepare_image` up to date with that change. It also changes the way configdrive is written to disk, to match that Ironic is now allowing Nova to build an ISO partition and send the raw image to the agent. This patch also swaps out subprocess.call for processutils.execute in the standby module, since the commands were being changed anyway. Lastly, this patch changes the expected `hashes` dict to be a string parameter called `checksum`, to match what glance returns. Change-Id: Id8af9be920ba51e7e1ce60f4ffd1477e413582c9
This commit is contained in:
Jim Rollenhagen
parent
ab46681e33
commit
620f05eb33
ironic_python_agent
@ -18,7 +18,8 @@ from ironic_python_agent import agent
|
||||
from ironic_python_agent.openstack.common import log
|
||||
|
||||
|
||||
LOG = log.getLogger()
|
||||
log.setup('ironic-python-agent')
|
||||
LOG = log.getLogger(__name__)
|
||||
|
||||
|
||||
def _get_kernel_params():
|
||||
|
||||
@ -12,17 +12,20 @@
|
||||
# See the License for the specific language governing permissions and
|
||||
# limitations under the License.
|
||||
|
||||
import base64
|
||||
import gzip
|
||||
import hashlib
|
||||
import os
|
||||
import requests
|
||||
import subprocess
|
||||
import six
|
||||
import StringIO
|
||||
import time
|
||||
|
||||
from ironic_python_agent import configdrive
|
||||
from ironic_python_agent import errors
|
||||
from ironic_python_agent.extensions import base
|
||||
from ironic_python_agent import hardware
|
||||
from ironic_python_agent.openstack.common import log
|
||||
from ironic_python_agent import utils
|
||||
|
||||
LOG = log.getLogger(__name__)
|
||||
|
||||
@ -47,7 +50,7 @@ def _write_image(image_info, device):
|
||||
script = _path_to_script('shell/write_image.sh')
|
||||
command = ['/bin/bash', script, image, device]
|
||||
LOG.info('Writing image with command: {0}'.format(' '.join(command)))
|
||||
exit_code = subprocess.call(command)
|
||||
exit_code = utils.execute(*command)
|
||||
if exit_code != 0:
|
||||
raise errors.ImageWriteError(exit_code, device)
|
||||
totaltime = time.time() - starttime
|
||||
@ -55,20 +58,33 @@ def _write_image(image_info, device):
|
||||
image, device, totaltime))
|
||||
|
||||
|
||||
def _copy_configdrive_to_disk(configdrive_dir, device):
|
||||
def _write_configdrive_to_file(configdrive, filename):
|
||||
LOG.debug('Writing configdrive to {0}'.format(filename))
|
||||
# configdrive data is base64'd, decode it first
|
||||
data = StringIO.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):
|
||||
filename = _configdrive_location()
|
||||
_write_configdrive_to_file(configdrive, filename)
|
||||
|
||||
starttime = time.time()
|
||||
script = _path_to_script('shell/copy_configdrive_to_disk.sh')
|
||||
command = ['/bin/bash', script, configdrive_dir, device]
|
||||
command = ['/bin/bash', script, filename, device]
|
||||
LOG.info('copying configdrive to disk with command {0}'.format(
|
||||
' '.join(command)))
|
||||
exit_code = subprocess.call(command)
|
||||
exit_code = utils.execute(*command)
|
||||
|
||||
if exit_code != 0:
|
||||
raise errors.ConfigDriveWriteError(exit_code, device)
|
||||
|
||||
totaltime = time.time() - starttime
|
||||
LOG.info('configdrive copied from {0} to {1} in {2} seconds'.format(
|
||||
configdrive_dir,
|
||||
configdrive,
|
||||
device,
|
||||
totaltime))
|
||||
|
||||
@ -114,28 +130,22 @@ def _download_image(image_info):
|
||||
|
||||
|
||||
def _verify_image(image_info, image_location):
|
||||
hashes = image_info['hashes']
|
||||
for k, v in hashes.items():
|
||||
algo = getattr(hashlib, k, None)
|
||||
if algo is None:
|
||||
continue
|
||||
log_msg = 'Verifying image at {0} with algorithm {1} against hash {2}'
|
||||
LOG.debug(log_msg.format(image_location, k, v))
|
||||
hash_ = algo(open(image_location).read()).hexdigest()
|
||||
if hash_ == v:
|
||||
return True
|
||||
else:
|
||||
log_msg = ('Image verification failed. Location: {0};'
|
||||
'algorithm: {1}; image hash: {2};'
|
||||
'verification hash: {3}')
|
||||
LOG.warning(log_msg.format(image_location, k, hash_, v))
|
||||
checksum = image_info['checksum']
|
||||
log_msg = 'Verifying image at {0} against MD5 checksum {1}'
|
||||
LOG.debug(log_msg.format(image_location, checksum))
|
||||
hash_ = hashlib.md5(open(image_location).read()).hexdigest()
|
||||
if hash_ == checksum:
|
||||
return True
|
||||
log_msg = ('Image verification failed. Location: {0};'
|
||||
'image hash: {1}; verification hash: {2}')
|
||||
LOG.warning(log_msg.format(image_location, checksum, hash_))
|
||||
return False
|
||||
|
||||
|
||||
def _validate_image_info(ext, image_info=None, **kwargs):
|
||||
image_info = image_info or {}
|
||||
|
||||
for field in ['id', 'urls', 'hashes']:
|
||||
for field in ['id', 'urls', 'checksum']:
|
||||
if field not in image_info:
|
||||
msg = 'Image is missing \'{0}\' field.'.format(field)
|
||||
raise errors.InvalidCommandParamsError(msg)
|
||||
@ -144,10 +154,10 @@ def _validate_image_info(ext, image_info=None, **kwargs):
|
||||
raise errors.InvalidCommandParamsError(
|
||||
'Image \'urls\' must be a list with at least one element.')
|
||||
|
||||
if type(image_info['hashes']) != dict or not image_info['hashes']:
|
||||
if (not isinstance(image_info['checksum'], six.string_types)
|
||||
or not image_info['checksum']):
|
||||
raise errors.InvalidCommandParamsError(
|
||||
'Image \'hashes\' must be a dictionary with at least one '
|
||||
'element.')
|
||||
'Image \'checksum\' must be a non-empty string.')
|
||||
|
||||
|
||||
class StandbyExtension(base.BaseAgentExtension):
|
||||
@ -171,9 +181,7 @@ class StandbyExtension(base.BaseAgentExtension):
|
||||
@base.async_command(_validate_image_info)
|
||||
def prepare_image(self,
|
||||
image_info=None,
|
||||
metadata=None,
|
||||
files=None):
|
||||
location = _configdrive_location()
|
||||
configdrive=None):
|
||||
device = hardware.get_manager().get_os_install_device()
|
||||
|
||||
# don't write image again if already cached
|
||||
@ -182,9 +190,7 @@ class StandbyExtension(base.BaseAgentExtension):
|
||||
_write_image(image_info, device)
|
||||
self.cached_image_id = image_info['id']
|
||||
|
||||
LOG.debug('Writing configdrive to {0}'.format(location))
|
||||
configdrive.write_configdrive(location, metadata, files)
|
||||
_copy_configdrive_to_disk(location, device)
|
||||
_write_configdrive_to_partition(configdrive, device)
|
||||
|
||||
@base.async_command()
|
||||
def run_image(self):
|
||||
@ -192,6 +198,6 @@ class StandbyExtension(base.BaseAgentExtension):
|
||||
LOG.info('Rebooting system')
|
||||
command = ['/bin/bash', script]
|
||||
# this should never return if successful
|
||||
exit_code = subprocess.call(command)
|
||||
exit_code = utils.execute(*command)
|
||||
if exit_code != 0:
|
||||
raise errors.SystemRebootError(exit_code)
|
||||
|
||||
@ -22,39 +22,28 @@ log() {
|
||||
|
||||
usage() {
|
||||
[[ -z "$1" ]] || echo -e "USAGE ERROR: $@\n"
|
||||
echo "`basename $0`: CONFIGDRIVE_DIR DEVICE"
|
||||
echo " - This script injects CONFIGDRIVE_DIR contents as an iso9660"
|
||||
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
|
||||
}
|
||||
|
||||
CONFIGDRIVE_DIR="$1"
|
||||
CONFIGDRIVE="$1"
|
||||
DEVICE="$2"
|
||||
|
||||
[[ -d $CONFIGDRIVE_DIR ]] || usage "$CONFIGDRIVE_DIR (CONFIGDRIVE_DIR) is not a directory"
|
||||
[[ -f $CONFIGDRIVE ]] || usage "$CONFIGDRIVE (CONFIGDRIVE) is not a regular file"
|
||||
[[ -b $DEVICE ]] || usage "$DEVICE (DEVICE) is not a block device"
|
||||
|
||||
# Create small partition at the end of the device
|
||||
log "Adding configdrive partition to $DEVICE"
|
||||
parted -a optimal -s -- $DEVICE mkpart primary ext2 -16MiB -0
|
||||
parted -a optimal -s -- $DEVICE mkpart primary ext2 -64MiB -0
|
||||
|
||||
# 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 '{print $1}'`
|
||||
|
||||
# This generates the ISO image of the config drive.
|
||||
log "Writing Configdrive contents in $CONFIGDRIVE_DIR to $ISO_PARTITION"
|
||||
genisoimage \
|
||||
-o ${ISO_PARTITION} \
|
||||
-ldots \
|
||||
-input-charset 'utf-8' \
|
||||
-allow-lowercase \
|
||||
-allow-multidot \
|
||||
-l \
|
||||
-publisher "ironic" \
|
||||
-J \
|
||||
-r \
|
||||
-V 'config-2' \
|
||||
${CONFIGDRIVE_DIR}
|
||||
log "Writing Configdrive contents in $CONFIGDRIVE to $ISO_PARTITION"
|
||||
dd if=$CONFIGDRIVE of=$ISO_PARTITION bs=64K oflag=direct
|
||||
|
||||
log "${DEVICE} imaged successfully!"
|
||||
|
||||
@ -36,16 +36,14 @@ class TestStandbyExtension(test_base.BaseTestCase):
|
||||
'urls': [
|
||||
'http://example.org',
|
||||
],
|
||||
'hashes': {
|
||||
'md5': 'abc123',
|
||||
},
|
||||
'checksum': 'abc123'
|
||||
}
|
||||
|
||||
def test_validate_image_info_success(self):
|
||||
standby._validate_image_info(None, self._build_fake_image_info())
|
||||
|
||||
def test_validate_image_info_missing_field(self):
|
||||
for field in ['id', 'urls', 'hashes']:
|
||||
for field in ['id', 'urls', 'checksum']:
|
||||
invalid_info = self._build_fake_image_info()
|
||||
del invalid_info[field]
|
||||
|
||||
@ -69,17 +67,17 @@ class TestStandbyExtension(test_base.BaseTestCase):
|
||||
standby._validate_image_info,
|
||||
invalid_info)
|
||||
|
||||
def test_validate_image_info_invalid_hashes(self):
|
||||
def test_validate_image_info_invalid_checksum(self):
|
||||
invalid_info = self._build_fake_image_info()
|
||||
invalid_info['hashes'] = 'this_is_not_a_dict'
|
||||
invalid_info['checksum'] = {'not': 'a string'}
|
||||
|
||||
self.assertRaises(errors.InvalidCommandParamsError,
|
||||
standby._validate_image_info,
|
||||
invalid_info)
|
||||
|
||||
def test_validate_image_info_empty_hashes(self):
|
||||
def test_validate_image_info_empty_checksum(self):
|
||||
invalid_info = self._build_fake_image_info()
|
||||
invalid_info['hashes'] = {}
|
||||
invalid_info['checksum'] = ''
|
||||
|
||||
self.assertRaises(errors.InvalidCommandParamsError,
|
||||
standby._validate_image_info,
|
||||
@ -103,49 +101,70 @@ class TestStandbyExtension(test_base.BaseTestCase):
|
||||
self.assertEqual(location, '/tmp/fake_id')
|
||||
|
||||
@mock.patch(OPEN_FUNCTION_NAME, autospec=True)
|
||||
@mock.patch('subprocess.call', autospec=True)
|
||||
def test_write_image(self, call_mock, open_mock):
|
||||
@mock.patch('ironic_python_agent.utils.execute', autospec=True)
|
||||
def test_write_image(self, execute_mock, open_mock):
|
||||
image_info = self._build_fake_image_info()
|
||||
device = '/dev/sda'
|
||||
location = standby._image_location(image_info)
|
||||
script = standby._path_to_script('shell/write_image.sh')
|
||||
command = ['/bin/bash', script, location, device]
|
||||
call_mock.return_value = 0
|
||||
execute_mock.return_value = 0
|
||||
|
||||
standby._write_image(image_info, device)
|
||||
call_mock.assert_called_once_with(command)
|
||||
execute_mock.assert_called_once_with(*command)
|
||||
|
||||
call_mock.reset_mock()
|
||||
call_mock.return_value = 1
|
||||
execute_mock.reset_mock()
|
||||
execute_mock.return_value = 1
|
||||
|
||||
self.assertRaises(errors.ImageWriteError,
|
||||
standby._write_image,
|
||||
image_info,
|
||||
device)
|
||||
|
||||
call_mock.assert_called_once_with(command)
|
||||
execute_mock.assert_called_once_with(*command)
|
||||
|
||||
@mock.patch('gzip.GzipFile', autospec=True)
|
||||
@mock.patch(OPEN_FUNCTION_NAME, autospec=True)
|
||||
@mock.patch('subprocess.call', autospec=True)
|
||||
def test_copy_configdrive_to_disk(self, call_mock, open_mock):
|
||||
@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(('ironic_python_agent.extensions.standby.'
|
||||
'_write_configdrive_to_file'),
|
||||
autospec=True)
|
||||
@mock.patch(OPEN_FUNCTION_NAME, autospec=True)
|
||||
@mock.patch('ironic_python_agent.utils.execute', autospec=True)
|
||||
def test_write_configdrive_to_partition(self, execute_mock, open_mock,
|
||||
configdrive_mock):
|
||||
device = '/dev/sda'
|
||||
configdrive = 'configdrive'
|
||||
configdrive = standby._configdrive_location()
|
||||
script = standby._path_to_script('shell/copy_configdrive_to_disk.sh')
|
||||
command = ['/bin/bash', script, configdrive, device]
|
||||
call_mock.return_value = 0
|
||||
execute_mock.return_value = 0
|
||||
|
||||
standby._copy_configdrive_to_disk(configdrive, device)
|
||||
call_mock.assert_called_once_with(command)
|
||||
standby._write_configdrive_to_partition(configdrive, device)
|
||||
execute_mock.assert_called_once_with(*command)
|
||||
|
||||
call_mock.reset_mock()
|
||||
call_mock.return_value = 1
|
||||
execute_mock.reset_mock()
|
||||
execute_mock.return_value = 1
|
||||
|
||||
self.assertRaises(errors.ConfigDriveWriteError,
|
||||
standby._copy_configdrive_to_disk,
|
||||
standby._write_configdrive_to_partition,
|
||||
configdrive,
|
||||
device)
|
||||
|
||||
call_mock.assert_called_once_with(command)
|
||||
execute_mock.assert_called_once_with(*command)
|
||||
|
||||
@mock.patch('hashlib.md5', autospec=True)
|
||||
@mock.patch(OPEN_FUNCTION_NAME, autospec=True)
|
||||
@ -160,7 +179,7 @@ class TestStandbyExtension(test_base.BaseTestCase):
|
||||
read_mock = open_mock.return_value.read
|
||||
read_mock.return_value = 'content'
|
||||
hexdigest_mock = md5_mock.return_value.hexdigest
|
||||
hexdigest_mock.return_value = list(image_info['hashes'].values())[0]
|
||||
hexdigest_mock.return_value = image_info['checksum']
|
||||
|
||||
standby._download_image(image_info)
|
||||
requests_mock.assert_called_once_with(image_info['urls'][0],
|
||||
@ -194,21 +213,16 @@ class TestStandbyExtension(test_base.BaseTestCase):
|
||||
image_info)
|
||||
|
||||
@mock.patch(OPEN_FUNCTION_NAME, autospec=True)
|
||||
@mock.patch('hashlib.sha1', autospec=True)
|
||||
@mock.patch('hashlib.md5', autospec=True)
|
||||
def test_verify_image_success(self, md5_mock, sha1_mock, open_mock):
|
||||
def test_verify_image_success(self, md5_mock, open_mock):
|
||||
image_info = self._build_fake_image_info()
|
||||
image_info['hashes']['sha1'] = image_info['hashes']['md5']
|
||||
hexdigest_mock = md5_mock.return_value.hexdigest
|
||||
hexdigest_mock.return_value = image_info['hashes']['md5']
|
||||
hexdigest_mock = sha1_mock.return_value.hexdigest
|
||||
hexdigest_mock.return_value = image_info['hashes']['sha1']
|
||||
hexdigest_mock.return_value = image_info['checksum']
|
||||
image_location = '/foo/bar'
|
||||
|
||||
verified = standby._verify_image(image_info, image_location)
|
||||
self.assertTrue(verified)
|
||||
# make sure we only check one hash, even though both are valid
|
||||
self.assertEqual(md5_mock.call_count + sha1_mock.call_count, 1)
|
||||
self.assertEqual(1, md5_mock.call_count)
|
||||
|
||||
@mock.patch(OPEN_FUNCTION_NAME, autospec=True)
|
||||
@mock.patch('hashlib.md5', autospec=True)
|
||||
@ -243,10 +257,7 @@ class TestStandbyExtension(test_base.BaseTestCase):
|
||||
self.assertEqual(None, async_result.command_result)
|
||||
|
||||
@mock.patch(('ironic_python_agent.extensions.standby.'
|
||||
'_copy_configdrive_to_disk'),
|
||||
autospec=True)
|
||||
@mock.patch(('ironic_python_agent.extensions.standby.configdrive.'
|
||||
'write_configdrive'),
|
||||
'_write_configdrive_to_partition'),
|
||||
autospec=True)
|
||||
@mock.patch('ironic_python_agent.hardware.get_manager', autospec=True)
|
||||
@mock.patch('ironic_python_agent.extensions.standby._write_image',
|
||||
@ -260,65 +271,60 @@ class TestStandbyExtension(test_base.BaseTestCase):
|
||||
download_mock,
|
||||
write_mock,
|
||||
hardware_mock,
|
||||
configdrive_mock,
|
||||
configdrive_copy_mock):
|
||||
image_info = self._build_fake_image_info()
|
||||
location_mock.return_value = 'THE CLOUD'
|
||||
location_mock.return_value = '/tmp/configdrive'
|
||||
download_mock.return_value = None
|
||||
write_mock.return_value = None
|
||||
manager_mock = hardware_mock.return_value
|
||||
manager_mock.get_os_install_device.return_value = 'manager'
|
||||
configdrive_mock.return_value = None
|
||||
configdrive_copy_mock.return_value = None
|
||||
|
||||
async_result = self.agent_extension.prepare_image('prepare_image',
|
||||
image_info=image_info,
|
||||
metadata={},
|
||||
files=[])
|
||||
image_info=image_info,
|
||||
configdrive='configdrive_data')
|
||||
async_result.join()
|
||||
|
||||
download_mock.assert_called_once_with(image_info)
|
||||
write_mock.assert_called_once_with(image_info, 'manager')
|
||||
configdrive_mock.assert_called_once_with('THE CLOUD', {}, [])
|
||||
configdrive_copy_mock.assert_called_once_with('THE CLOUD', 'manager')
|
||||
configdrive_copy_mock.assert_called_once_with('configdrive_data',
|
||||
'manager')
|
||||
|
||||
self.assertEqual('SUCCEEDED', async_result.command_status)
|
||||
self.assertEqual(None, async_result.command_result)
|
||||
|
||||
download_mock.reset_mock()
|
||||
write_mock.reset_mock()
|
||||
configdrive_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('prepare_image',
|
||||
image_info=image_info,
|
||||
metadata={},
|
||||
files=[])
|
||||
image_info=image_info,
|
||||
configdrive='configdrive_data')
|
||||
async_result.join()
|
||||
|
||||
self.assertEqual(download_mock.call_count, 0)
|
||||
self.assertEqual(write_mock.call_count, 0)
|
||||
configdrive_mock.assert_called_once_with('THE CLOUD', {}, [])
|
||||
configdrive_copy_mock.assert_called_once_with('THE CLOUD', 'manager')
|
||||
configdrive_copy_mock.assert_called_once_with('configdrive_data',
|
||||
'manager')
|
||||
|
||||
self.assertEqual('SUCCEEDED', async_result.command_status)
|
||||
self.assertEqual(None, async_result.command_result)
|
||||
|
||||
@mock.patch('subprocess.call', autospec=True)
|
||||
def test_run_image(self, call_mock):
|
||||
@mock.patch('ironic_python_agent.utils.execute', autospec=True)
|
||||
def test_run_image(self, execute_mock):
|
||||
script = standby._path_to_script('shell/reboot.sh')
|
||||
command = ['/bin/bash', script]
|
||||
call_mock.return_value = 0
|
||||
execute_mock.return_value = 0
|
||||
|
||||
success_result = self.agent_extension.run_image('run_image')
|
||||
success_result.join()
|
||||
call_mock.assert_called_once_with(command)
|
||||
execute_mock.assert_called_once_with(*command)
|
||||
|
||||
call_mock.reset_mock()
|
||||
call_mock.return_value = 1
|
||||
execute_mock.reset_mock()
|
||||
execute_mock.return_value = 1
|
||||
|
||||
failed_result = self.agent_extension.run_image('run_image')
|
||||
failed_result.join()
|
||||
|
||||
call_mock.assert_called_once_with(command)
|
||||
execute_mock.assert_called_once_with(*command)
|
||||
self.assertEqual('FAILED', failed_result.command_status)
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user