Merge "Rewrite write_image.sh in Python"

3 files changed, 42 insertions, 78 deletions

diff --git a/ironic_python_agent/extensions/standby.py b/ironic_python_agent/extensions/standby.py
index ab1414df..74f244e0 100644
--- a/ironic_python_agent/extensions/standby.py
+++ b/ironic_python_agent/extensions/standby.py
@@ -23,6 +23,7 @@ from ironic_lib import exception
 from oslo_concurrency import processutils
 from oslo_config import cfg
 from oslo_log import log
+from oslo_utils import units
 import requests
 
 from ironic_python_agent import errors
@@ -46,16 +47,6 @@ def _image_location(image_info):
     return os.path.join(tempfile.gettempdir(), image_info['id'])
 
 
-def _path_to_script(script):
-    """Get the location of a script which ships with ironic-python-agent.
-
-    :param script: The script name as a string.
-    :returns: The relative path to the script.
-    """
-    cwd = os.path.dirname(os.path.realpath(__file__))
-    return os.path.join(cwd, '..', script)
-
-
 def _download_with_proxy(image_info, url, image_id):
     """Opens a download stream for the given URL.
 
@@ -203,14 +194,24 @@ def _write_whole_disk_image(image, image_info, device):
     :raises: ImageWriteError if the command to write the image encounters an
              error.
     """
-    script = _path_to_script('shell/write_image.sh')
-    command = ['/bin/bash', script, image, device]
+    # FIXME(dtantsur): pass the real node UUID for logging
+    disk_utils.destroy_disk_metadata(device, '')
+    disk_utils.udev_settle()
+
+    command = ['qemu-img', 'convert',
+               '-t', 'directsync', '-O', 'host_device', '-W',
+               image, device]
     LOG.info('Writing image with command: {}'.format(' '.join(command)))
     try:
-        stdout, stderr = utils.execute(*command)
+        # TODO(dtantsur): switch to disk_utils.convert_image when it supports
+        # -t and -W flags and defaults to 2 GiB memory limit.
+        limits = processutils.ProcessLimits(address_space=2048 * units.Mi)
+        utils.execute(*command, prlimit=limits)
     except processutils.ProcessExecutionError as e:
         raise errors.ImageWriteError(device, e.exit_code, e.stdout, e.stderr)
 
+    disk_utils.trigger_device_rescan(device)
+
 
 def _write_image(image_info, device):
     """Writes an image to the specified device.
diff --git a/ironic_python_agent/shell/write_image.sh b/ironic_python_agent/shell/write_image.sh
deleted file mode 100755
index 61e9d0b9..00000000
--- a/ironic_python_agent/shell/write_image.sh
+++ /dev/null
@@ -1,55 +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.
-
-set -e
-
-log() {
-    echo "`basename $0`: $@"
-}
-
-usage() {
-    [[ -z "$1" ]] || echo -e "USAGE ERROR: $@\n"
-    echo "`basename $0`: IMAGEFILE DEVICE"
-    echo "  - This script images DEVICE with IMAGEFILE"
-    exit 1
-}
-
-IMAGEFILE="$1"
-DEVICE="$2"
-
-[[ -f $IMAGEFILE ]] || usage "$IMAGEFILE (IMAGEFILE) is not a file"
-[[ -b $DEVICE ]] || usage "$DEVICE (DEVICE) is not a block device"
-
-# In production this will be replaced with secure erasing the drives
-# For now we need to ensure there aren't any old (GPT) partitions on the drive
-log "Erasing existing GPT and MBR data structures from ${DEVICE}"
-
-# NOTE(gfidente): GPT uses 33*512 sectors, this is an attempt to avoid bug:
-# https://bugs.launchpad.net/ironic-python-agent/+bug/1737556
-DEVICE_SECTORS_COUNT=`blockdev --getsz $DEVICE`
-dd bs=512 if=/dev/zero of=$DEVICE count=33
-dd bs=512 if=/dev/zero of=$DEVICE count=33 seek=$((${DEVICE_SECTORS_COUNT} - 33))
-sgdisk -Z $DEVICE
-udevadm settle
-
-log "Imaging $IMAGEFILE to $DEVICE"
-
-# limit the memory usage for qemu-img to 2 GiB
-ulimit -v 2097152
-qemu-img convert -t directsync -O host_device -W $IMAGEFILE $DEVICE
-sync
-
-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 1f985652..6f905a36 100644
--- a/ironic_python_agent/tests/unit/extensions/test_standby.py
+++ b/ironic_python_agent/tests/unit/extensions/test_standby.py
@@ -168,25 +168,45 @@ class TestStandbyExtension(base.IronicAgentTest):
         self.assertEqual(expected_loc, location)
 
     @mock.patch('ironic_lib.disk_utils.fix_gpt_partition', autospec=True)
-    @mock.patch('builtins.open', autospec=True)
+    @mock.patch('ironic_lib.disk_utils.trigger_device_rescan', autospec=True)
     @mock.patch('ironic_python_agent.utils.execute', autospec=True)
-    def test_write_image(self, execute_mock, open_mock, fix_gpt_mock):
+    @mock.patch('ironic_lib.disk_utils.udev_settle', autospec=True)
+    @mock.patch('ironic_lib.disk_utils.destroy_disk_metadata', autospec=True)
+    def test_write_image(self, wipe_mock, udev_mock, execute_mock,
+                         rescan_mock, fix_gpt_mock):
         image_info = _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]
-        execute_mock.return_value = ('', '')
+        command = ['qemu-img', 'convert', '-t', 'directsync',
+                   '-O', 'host_device', '-W', location, device]
 
         standby._write_image(image_info, device)
-        execute_mock.assert_called_once_with(*command)
+
+        execute_mock.assert_called_once_with(*command, prlimit=mock.ANY)
+        wipe_mock.assert_called_once_with(device, '')
+        udev_mock.assert_called_once_with()
+        rescan_mock.assert_called_once_with(device)
         fix_gpt_mock.assert_called_once_with(device, node_uuid=None)
 
+    @mock.patch('ironic_lib.disk_utils.fix_gpt_partition', autospec=True)
+    @mock.patch('ironic_lib.disk_utils.trigger_device_rescan', autospec=True)
+    @mock.patch('ironic_python_agent.utils.execute', autospec=True)
+    @mock.patch('ironic_lib.disk_utils.udev_settle', autospec=True)
+    @mock.patch('ironic_lib.disk_utils.destroy_disk_metadata', autospec=True)
+    def test_write_image_gpt_fails(self, wipe_mock, udev_mock, execute_mock,
+                                   rescan_mock, fix_gpt_mock):
+        image_info = _build_fake_image_info()
+        device = '/dev/sda'
+
         fix_gpt_mock.side_effect = exception.InstanceDeployFailure
         standby._write_image(image_info, device)
 
-        execute_mock.reset_mock()
-        execute_mock.return_value = ('', '')
+    @mock.patch('ironic_python_agent.utils.execute', autospec=True)
+    @mock.patch('ironic_lib.disk_utils.udev_settle', autospec=True)
+    @mock.patch('ironic_lib.disk_utils.destroy_disk_metadata', autospec=True)
+    def test_write_image_fails(self, wipe_mock, udev_mock, execute_mock):
+        image_info = _build_fake_image_info()
+        device = '/dev/sda'
         execute_mock.side_effect = processutils.ProcessExecutionError
 
         self.assertRaises(errors.ImageWriteError,
@@ -194,8 +214,6 @@ class TestStandbyExtension(base.IronicAgentTest):
                           image_info,
                           device)
 
-        execute_mock.assert_called_once_with(*command)
-
     @mock.patch.object(utils, 'get_node_boot_mode', lambda self: 'bios')
     @mock.patch.object(hardware, 'dispatch_to_managers', autospec=True)
     @mock.patch('builtins.open', autospec=True)