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

4 files changed, 113 insertions, 111 deletions

diff --git a/ironic_python_agent/cmd/agent.py b/ironic_python_agent/cmd/agent.py
index 31390378..770d6f17 100644
--- a/ironic_python_agent/cmd/agent.py
+++ b/ironic_python_agent/cmd/agent.py
@@ -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():
diff --git a/ironic_python_agent/extensions/standby.py b/ironic_python_agent/extensions/standby.py
index c2c0b8ab..17c031c7 100644
--- a/ironic_python_agent/extensions/standby.py
+++ b/ironic_python_agent/extensions/standby.py
@@ -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)
diff --git a/ironic_python_agent/shell/copy_configdrive_to_disk.sh b/ironic_python_agent/shell/copy_configdrive_to_disk.sh
index e3f08d40..768a37da 100755
--- a/ironic_python_agent/shell/copy_configdrive_to_disk.sh
+++ b/ironic_python_agent/shell/copy_configdrive_to_disk.sh
@@ -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!"
diff --git a/ironic_python_agent/tests/extensions/standby.py b/ironic_python_agent/tests/extensions/standby.py
index e65ac2a9..a1449d0d 100644
--- a/ironic_python_agent/tests/extensions/standby.py
+++ b/ironic_python_agent/tests/extensions/standby.py
@@ -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)