diff options
35 files changed, 1288 insertions, 113 deletions
diff --git a/bindep.txt b/bindep.txt index 863787763..16adeecda 100644 --- a/bindep.txt +++ b/bindep.txt @@ -86,9 +86,13 @@ apparmor [platform:dpkg imagebuild] gnupg [imagebuild] squashfs-tools [platform:dpkg platform:redhat imagebuild] squashfs [platform:suse imagebuild] +# For custom partition images +kpartx [devstack] libguestfs0 [platform:dpkg imagebuild] -libguestfs [platform:rpm imagebuild] +libguestfs [platform:rpm imagebuild devstack] +libguestfs-tools [platform:dpkg devstack] python-guestfs [platform:dpkg imagebuild] +qemu-img [platform:rpm devstack] # for TinyIPA build wget [imagebuild] python-pip [imagebuild] diff --git a/devstack/files/bindep.txt b/devstack/files/bindep.txt index 8c386349a..820f9b8b0 100644 --- a/devstack/files/bindep.txt +++ b/devstack/files/bindep.txt @@ -87,9 +87,13 @@ apparmor [platform:dpkg imagebuild] gnupg [imagebuild] squashfs-tools [platform:dpkg platform:redhat imagebuild] squashfs [platform:suse imagebuild] +# For custom partition images +kpartx libguestfs0 [platform:dpkg imagebuild] libguestfs [platform:rpm imagebuild] +libguestfs-tools [platform:dpkg] python-guestfs [platform:dpkg imagebuild] +qemu-img [platform:rpm] # for TinyIPA build wget [imagebuild] python-pip [imagebuild] diff --git a/devstack/lib/ironic b/devstack/lib/ironic index 228229d2a..366eb03b8 100644 --- a/devstack/lib/ironic +++ b/devstack/lib/ironic @@ -2852,6 +2852,46 @@ function build_ipa_dib_ramdisk { rm -rf $tempdir } +function upload_image_if_needed { + if [[ "$IRONIC_PARTITIONED_IMAGE_NAME" =~ cirros ]] && is_service_enabled glance; then + echo Building a Cirros image suitable for local boot + + local dest + IRONIC_PARTITIONED_IMAGE_NAME=cirros-${CIRROS_VERSION}-x86_64-partition + dest="$IRONIC_DATA_DIR/$IRONIC_PARTITIONED_IMAGE_NAME.img" + + # Export some variables that the script is using. + CIRROS_ARCH=$CIRROS_ARCH CIRROS_VERSION=$CIRROS_VERSION \ + IRONIC_TTY_DEV=$IRONIC_TTY_DEV VERBOSE=$VERBOSE \ + $IRONIC_SCRIPTS_DIR/cirros-partition.sh "$dest" + + # TODO(dtantsur): stop uploading kernel/ramdisk when image_type support + # lands. + local kernel_id + kernel_id=$(openstack image list -f value -c ID -c Name \ + | awk '/cirros.*kernel/ { print $1; exit 0; }') + die_if_not_set $LINENO kernel_id "Cannot find cirros kernel" + + local ramdisk_id + ramdisk_id=$(openstack image list -f value -c ID -c Name \ + | awk '/cirros.*ramdisk/ { print $1; exit 0; }') + die_if_not_set $LINENO ramdisk_id "Cannot find cirros ramdisk" + + openstack image create $IRONIC_PARTITIONED_IMAGE_NAME \ + --public --disk-format raw --container-format bare \ + --property kernel_id=$kernel_id --property ramdisk_id=$ramdisk_id \ + --file "$dest" + + # Change the default image only if the provided settings prevent the + # default cirros image from working. + if [[ "$IRONIC_TEMPEST_WHOLE_DISK_IMAGE" != True \ + && "$IRONIC_DEFAULT_BOOT_OPTION" == local ]]; then + IRONIC_IMAGE_NAME=$IRONIC_PARTITIONED_IMAGE_NAME + DEFAULT_IMAGE_NAME=$IRONIC_IMAGE_NAME + fi + fi +} + # download EFI boot loader image and upload it to glance # this function sets ``IRONIC_EFIBOOT_ID`` function upload_baremetal_ironic_efiboot { @@ -3030,6 +3070,8 @@ function prepare_baremetal_basic_ops { upload_baremetal_ironic_efiboot fi + upload_image_if_needed + configure_tftpd configure_iptables } @@ -3152,6 +3194,13 @@ function ironic_configure_tempest { iniset $TEMPEST_CONFIG baremetal partition_image_ref $image_uuid fi + # Our cirros images cannot do local boot in legacy mode. + if [[ "${IRONIC_PARTITIONED_IMAGE_NAME}" =~ cirros && "${IRONIC_BOOT_MODE}" == "bios" ]]; then + iniset $TEMPEST_CONFIG baremetal partition_netboot True + else + iniset $TEMPEST_CONFIG baremetal partition_netboot False + fi + if [[ "$IRONIC_IP_VERSION" == "6" ]]; then iniset $TEMPEST_CONFIG baremetal whole_disk_image_url "http://$IRONIC_HOST_IPV6:$IRONIC_HTTP_PORT/${IRONIC_WHOLEDISK_IMAGE_NAME}.img" else @@ -3174,6 +3223,8 @@ function ironic_configure_tempest { # Driver for API tests iniset $TEMPEST_CONFIG baremetal driver fake-hardware + iniset $TEMPEST_CONFIG baremetal default_boot_option $IRONIC_DEFAULT_BOOT_OPTION + local adjusted_root_disk_size_gb if [[ "$IRONIC_IS_HARDWARE" == "False" ]]; then adjusted_root_disk_size_gb=$(( ${IRONIC_VM_SPECS_DISK} - ${IRONIC_VM_EPHEMERAL_DISK} )) diff --git a/devstack/tools/ironic/scripts/cirros-partition.sh b/devstack/tools/ironic/scripts/cirros-partition.sh new file mode 100755 index 000000000..40c87b19e --- /dev/null +++ b/devstack/tools/ironic/scripts/cirros-partition.sh @@ -0,0 +1,69 @@ +#!/bin/bash + +set -eu -o pipefail + +VERBOSE=${VERBOSE:-True} +if [[ "$VERBOSE" == True ]]; then + set -x + guestfish_args="--verbose" +fi + +CIRROS_VERSION=${CIRROS_VERSION:-0.5.2} +CIRROS_ARCH=${CIRROS_ARCH:-x86_64} +# TODO(dtantsur): use the image cached on infra images in the CI +DISK_URL=http://download.cirros-cloud.net/${CIRROS_VERSION}/cirros-${CIRROS_VERSION}-${CIRROS_ARCH}-disk.img +OUT=$(realpath ${1:-rootfs.img}) + +IRONIC_TTY_DEV=${IRONIC_TTY_DEV:-ttyS0,115200} +# rdroot : boot from the ramdisk present on the root partition instead of +# mounting the root partition. +# dslist : disable Nova metadata support, it takes a long time on boot. +KARGS=${KARGS:-nofb nomodeset vga=normal console=${IRONIC_TTY_DEV} rdroot dslist=configdrive} + +workdir=$(mktemp -d) +root_mp=$workdir/root +efi_mp=$workdir/efi +dest=$workdir/dest + +cd $workdir + +curl -Lf -o disk.qcow2 $DISK_URL +qemu-img convert -O raw disk.qcow2 disk.img +rm disk.qcow2 + +# kpartx automatically allocates loop devices for all partitions in the image +device=$(sudo kpartx -av disk.img | grep -oE 'loop[0-9]+p' | head -1) + +function clean_up { + set +e + sudo umount $efi_mp + sudo umount $root_mp + sudo kpartx -d $workdir/disk.img + sudo rm -rf $workdir +} +trap clean_up EXIT + +# TODO(dtantsur): some logic instead of hardcoding numbers 1 and 15? +rootdev=/dev/mapper/${device}1 +efidev=/dev/mapper/${device}15 + +mkdir -p $root_mp $efi_mp $dest/boot/efi +sudo mount $rootdev $root_mp +sudo mount $efidev $efi_mp + +sudo cp -aR $root_mp/* $dest/ +sudo cp -aR $efi_mp/EFI $dest/boot/efi/ + +# These locations are required by IPA even when it does not really run +# grub-install. +sudo mkdir -p $dest/{dev,proc,run,sys} + +# The default arguments don't work for us, update grub configuration. +sudo sed -i "/^ *linux /s/\$/ $KARGS/" $dest/boot/efi/EFI/ubuntu/grub.cfg + +LIBGUESTFS_BACKEND=direct sudo -E \ + virt-make-fs --size +50M --type ext3 --label cirros-rootfs \ + ${guestfish_args:-} "$dest" "$OUT" + +sudo chown $USER "$OUT" +qemu-img info "$OUT" diff --git a/doc/source/admin/adoption.rst b/doc/source/admin/adoption.rst index ba404fd0b..570b36072 100644 --- a/doc/source/admin/adoption.rst +++ b/doc/source/admin/adoption.rst @@ -51,10 +51,7 @@ The adoption process makes no changes to the physical node, with the exception of operator supplied configurations where virtual media is used to boot the node under normal circumstances. An operator should ensure that any supplied configuration defining the node is sufficient -for the continued operation of the node moving forward. Such as, if the -node is configured to network boot via instance_info/boot_option="netboot", -then appropriate driver specific node configuration should be set to -support this capability. +for the continued operation of the node moving forward. Possible Risk ============= diff --git a/doc/source/admin/drivers/idrac.rst b/doc/source/admin/drivers/idrac.rst index 494d151a7..df0ee0c6e 100644 --- a/doc/source/admin/drivers/idrac.rst +++ b/doc/source/admin/drivers/idrac.rst @@ -925,4 +925,13 @@ selected if default plug-in type has been used and never changed. Systems that have plug-in type changed will keep selected plug-in type after iDRAC firmware upgrade. +Firmware update from Swift fails +-------------------------------- + +When using Swift to stage firmware update files in Management interface +``firmware_update`` clean step of ``redfish`` or ``idrac`` hardware type, the +cleaning fails with error "An internal error occurred. Unable to complete the +specified operation." in iDRAC job. Until this is fixed, use HTTP service to +stage firmware files for iDRAC. + .. _SCP_Reference_Guide: http://downloads.dell.com/manuals/common/dellemc-server-config-profile-refguide.pdf diff --git a/doc/source/admin/drivers/ilo.rst b/doc/source/admin/drivers/ilo.rst index 40bb06735..4ffa8bcfb 100644 --- a/doc/source/admin/drivers/ilo.rst +++ b/doc/source/admin/drivers/ilo.rst @@ -1084,6 +1084,11 @@ intermediate images on conductor as described in Deploy Process ============== +.. note:: + Network boot is deprecated and will be removed in the Zed release. + +.. TODO(dtantsur): review these diagrams to exclude netboot. + Netboot with glance and swift ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/doc/source/admin/drivers/redfish.rst b/doc/source/admin/drivers/redfish.rst index 0878b08bf..d2d93d9ff 100644 --- a/doc/source/admin/drivers/redfish.rst +++ b/doc/source/admin/drivers/redfish.rst @@ -385,6 +385,8 @@ The ``update_firmware`` cleaning step accepts JSON in the following format:: "firmware_images":[ { "url": "<url_to_firmware_image1>", + "checksum": "<checksum for image, uses SHA1>", + "source": "<optional override source setting for image>", "wait": <number_of_seconds_to_wait> }, { @@ -410,16 +412,21 @@ Each firmware image dictionary, is of the form:: { "url": "<URL of firmware image file>", + "checksum": "<checksum for image, uses SHA1>", + "source": "<Optional override source setting for image>", "wait": <Optional time in seconds to wait after applying update> } -The ``url`` argument in the firmware image dictionary is mandatory, while the -``wait`` argument is optional. +The ``url``and ``checksum`` arguments in the firmware image dictionary are +mandatory, while the ``source`` and ``wait`` arguments are optional. +For ``url`` currently ``http``, ``https``, ``swift`` and ``file`` schemes are +supported. + +``source`` corresponds to ``[redfish]firmware_source`` and by setting it here, +it is possible to override global setting per firmware image in clean step +arguments. -.. note:: - Only ``http`` and ``https`` URLs are currently supported in the ``url`` - argument. .. note:: At the present time, targets for the firmware update cannot be specified. @@ -427,19 +434,20 @@ The ``url`` argument in the firmware image dictionary is mandatory, while the node. It is assumed that the BMC knows what components a given firmware image is applicable to. -To perform a firmware update, first download the firmware to a web server that -the BMC has network access to. This could be the ironic conductor web server -or another web server on the BMC network. Using a web browser, curl, or similar -tool on a server that has network access to the BMC, try downloading -the firmware to verify that the URLs are correct and that the web server is -configured properly. +To perform a firmware update, first download the firmware to a web server, +Swift or filesystem that the Ironic conductor or BMC has network access to. +This could be the ironic conductor web server or another web server on the BMC +network. Using a web browser, curl, or similar tool on a server that has +network access to the BMC or Ironic conductor, try downloading the firmware to +verify that the URLs are correct and that the web server is configured +properly. Next, construct the JSON for the firmware update cleaning step to be executed. When launching the firmware update, the JSON may be specified on the command -line directly or in a file. The following -example shows one cleaning step that installs two firmware updates. The first -updates the BMC firmware followed by a five minute wait to allow the BMC time -to start back up. The second updates the firmware on all applicable NICs.:: +line directly or in a file. The following example shows one cleaning step that +installs four firmware updates. All except 3rd entry that has explicit +``source`` added, uses setting from ``[redfish]firmware_source`` to determine +if and where to stage the files:: [{ "interface": "management", @@ -448,10 +456,21 @@ to start back up. The second updates the firmware on all applicable NICs.:: "firmware_images":[ { "url": "http://192.0.2.10/BMC_4_22_00_00.EXE", + "checksum": "<sha1-checksum-of-the-file>", "wait": 300 }, { - "url": "https://192.0.2.10/NIC_19.0.12_A00.EXE" + "url": "https://192.0.2.10/NIC_19.0.12_A00.EXE", + "checksum": "<sha1-checksum-of-the-file>" + }, + { + "url": "file:///firmware_images/idrac/9/PERC_WN64_6.65.65.65_A00.EXE", + "checksum": "<sha1-checksum-of-the-file>", + "source": "http" + }, + { + "url": "swift://firmware_container/BIOS_W8Y0W_WN64_2.1.7.EXE", + "checksum": "<sha1-checksum-of-the-file>" } ] } diff --git a/doc/source/admin/drivers/snmp.rst b/doc/source/admin/drivers/snmp.rst index 7a91bc126..1c402ab9b 100644 --- a/doc/source/admin/drivers/snmp.rst +++ b/doc/source/admin/drivers/snmp.rst @@ -77,30 +77,20 @@ Enabling the SNMP Hardware Type enabled_management_interfaces = noop enabled_power_interfaces = snmp -#. To set the default boot option, update ``default_boot_option`` in +#. To enable the network boot fallback, update ``enable_netboot_fallback`` in ``ironic.conf``: .. code-block:: ini - [DEFAULT] - default_boot_option = netboot - - .. note:: - Currently the default value of ``default_boot_option`` is ``netboot`` - but it will be changed to ``local`` in the future. It is recommended - to set an explicit value for this option. + [pxe] + enable_netboot_fallback = True .. note:: - It is important to set ``boot_option`` to ``netboot`` as SNMP hardware - type does not support setting of boot devices. One can also configure - a node to boot using ``netboot`` by setting its ``capabilities`` and - updating Nova flavor as described below: - - .. code-block:: console - - baremetal node set --property capabilities="boot_option:netboot" <node> - openstack flavor set --property "capabilities:boot_option"="netboot" ironic-flavor - + It is important to enable the fallback as SNMP hardware type does not + support setting of boot devices. When booting in legacy (BIOS) mode, + the generated network booting artifact will force booting from local + disk. In UEFI mode, Ironic will configure the boot order using UEFI + variables. #. Restart the Ironic conductor service. @@ -165,5 +155,4 @@ type: --driver snmp --driver-info snmp_driver=<pdu_manufacturer> \ --driver-info snmp_address=<ip_address> \ --driver-info snmp_outlet=<outlet_index> \ - --driver-info snmp_community=<community_string> \ - --properties capabilities=boot_option:netboot + --driver-info snmp_community=<community_string> diff --git a/doc/source/admin/report.txt b/doc/source/admin/report.txt index e098e5aef..1f1fc4d8e 100644 --- a/doc/source/admin/report.txt +++ b/doc/source/admin/report.txt @@ -321,7 +321,7 @@ default: deploy: continue_if_disk_secure_erase_fails = False - default_boot_option = netboot + default_boot_option = local erase_devices_metadata_priority = None erase_devices_priority = 0 http_root = /opt/stack/data/ironic/httpboot diff --git a/doc/source/admin/troubleshooting.rst b/doc/source/admin/troubleshooting.rst index 5cd2ec751..8cf49392f 100644 --- a/doc/source/admin/troubleshooting.rst +++ b/doc/source/admin/troubleshooting.rst @@ -469,7 +469,8 @@ the conductor is actively working on something related to the node. Often, this means there is an internal lock or ``reservation`` set on the node and the conductor is downloading, uploading, or attempting to perform some -sort of Input/Output operation. +sort of Input/Output operation - see `Why does API return "Node is locked by +host"?`_ for details. In the case the conductor gets stuck, these operations should timeout, but there are cases in operating systems where operations are blocked until @@ -677,12 +678,16 @@ How do I resolve this? Generally, you need to identify the port with the offending MAC address. Example: - openstack port list --mac-address 52:54:00:7c:c4:56 +.. code-block:: console + + $ openstack port list --mac-address 52:54:00:7c:c4:56 From the command's output, you should be able to identify the ``id`` field. Using that, you can delete the port. Example: - openstack port delete <id> +.. code-block:: console + + $ openstack port delete <id> .. warning:: Before deleting a port, you should always verify that it is no longer in @@ -810,7 +815,9 @@ Example failure A node in this state, when the ``network_interface`` was saved as ``neutron``, yet the ``neutron`` interface is no longer enabled will fail basic state -transition requests.: +transition requests: + +.. code-block:: console $ baremetal node manage 7164efca-37ab-1213-1112-b731cf795a5a Could not find the following interface in the 'ironic.hardware.interfaces.network' entrypoint: neutron. Valid interfaces are ['flat']. (HTTP 400) @@ -826,7 +833,9 @@ order of interfaces in the for the ``enabled_*_interfaces`` options. Once the conductor has been restarted with the updated configuration, you should now be able to update the interface using the ``baremetal node set`` command. In this example we use the ``network_interface`` as this is most -commonly where it is encountered.: +commonly where it is encountered: + +.. code-block:: console $ baremetal node set $NAME_OR_UUID --network-interface flat @@ -869,14 +878,98 @@ How do I resolve this? This can be addressed a few different ways: - * Use raw images, however these images can be substantially larger - and require more data to be transmitted "over the wire". - * Add more physical memory. - * Add swap space. - * Reduce concurrency, possibly via another conductor or changing the - nova-compute.conf ``max_concurrent_builds`` parameter. - * Or finally, adjust the ``[DEFAULT]minimum_required_memory`` parameter - in your ironic.conf file. The default should be considered a "default - of last resort" and you may need to reserve additional memory. You may - also wish to adjust the ``[DEFAULT]minimum_memory_wait_retries`` and - ``[DEFAULT]minimum_memory_wait_time`` parameters. +* Use raw images, however these images can be substantially larger + and require more data to be transmitted "over the wire". +* Add more physical memory. +* Add swap space. +* Reduce concurrency, possibly via another conductor or changing the + nova-compute.conf ``max_concurrent_builds`` parameter. +* Or finally, adjust the ``[DEFAULT]minimum_required_memory`` parameter + in your ironic.conf file. The default should be considered a "default + of last resort" and you may need to reserve additional memory. You may + also wish to adjust the ``[DEFAULT]minimum_memory_wait_retries`` and + ``[DEFAULT]minimum_memory_wait_time`` parameters. + +Why does API return "Node is locked by host"? +============================================= + +This error usually manifests as HTTP error 409 on the client side: + + Node d7e2aed8-50a9-4427-baaa-f8f595e2ceb3 is locked by host 192.168.122.1, + please retry after the current operation is completed. + +It happens, because an operation that modifies a node is requested, while +another such operation is running. The conflicting operation may be user +requested (e.g. a provisioning action) or related to the internal processes +(e.g. changing power state during :doc:`power-sync`). The reported host name +corresponds to the conductor instance that holds the lock. + +Normally, these errors are transient and safe to retry after a few seconds. If +the lock is held for significant time, these are the steps you can take. + +First of all, check the current ``provision_state`` of the node: + +``verifying`` + means that the conductor is trying to access the node's BMC. + If it happens for minutes, it means that the BMC is either unreachable or + misbehaving. Double-check the information in ``driver_info``, especially + the BMC address and credentials. + + If the access details seem correct, try resetting the BMC using, for + example, its web UI. + +``deploying``/``inspecting``/``cleaning`` + means that the conductor is doing some active work. It may include + downloading or converting images, executing synchronous out-of-band deploy + or clean steps, etc. A node can stay in this state for minutes, depending + on various factors. Consult the conductor logs. + +``available``/``manageable``/``wait call-back``/``clean wait`` + means that some background process is holding the lock. Most commonly it's + the power synchronization loop. Similarly to the ``verifying`` state, + it may mean that the BMC access is broken or too slow. The conductor logs + will provide you insights on what is happening. + +To trace the process using conductor logs: + +#. Isolate the relevant log parts. Lock messages come from the + ``ironic.conductor.task_manager`` module. You can also check the + ``ironic.common.states`` module for any state transitions: + + .. code-block:: console + + $ grep -E '(ironic.conductor.task_manager|ironic.common.states|NodeLocked)' \ + conductor.log > state.log + +#. Find the first instance of ``NodeLocked``. It may look like this (stripping + timestamps and request IDs here and below for readability):: + + DEBUG ironic.conductor.task_manager [-] Attempting to get exclusive lock on node d7e2aed8-50a9-4427-baaa-f8f595e2ceb3 (for node update) __init__ /usr/lib/python3.6/site-packages/ironic/conductor/task_manager.py:233 + DEBUG ironic_lib.json_rpc.server [-] RPC error NodeLocked: Node d7e2aed8-50a9-4427-baaa-f8f595e2ceb3 is locked by host 192.168.57.53, please retry after the current operation is completed. _handle_error /usr/lib/python3.6/site-packages/ironic_lib/json_rpc/server.py:179 + + The events right before this failure will provide you a clue on why the lock + is held. + +#. Find the last successful **exclusive** locking event before the failure, for + example:: + + DEBUG ironic.conductor.task_manager [-] Attempting to get exclusive lock on node d7e2aed8-50a9-4427-baaa-f8f595e2ceb3 (for provision action manage) __init__ /usr/lib/python3.6/site-packages/ironic/conductor/task_manager.py:233 + DEBUG ironic.conductor.task_manager [-] Node d7e2aed8-50a9-4427-baaa-f8f595e2ceb3 successfully reserved for provision action manage (took 0.01 seconds) reserve_node /usr/lib/python3.6/site-packages/ironic/conductor/task_manager.py:350 + DEBUG ironic.common.states [-] Exiting old state 'enroll' in response to event 'manage' on_exit /usr/lib/python3.6/site-packages/ironic/common/states.py:307 + DEBUG ironic.common.states [-] Entering new state 'verifying' in response to event 'manage' on_enter /usr/lib/python3.6/site-packages/ironic/common/states.py:313 + + This is your root cause, the lock is held because of the BMC credentials + verification. + +#. Find when the lock is released (if at all). The messages look like this:: + + DEBUG ironic.conductor.task_manager [-] Successfully released exclusive lock for provision action manage on node d7e2aed8-50a9-4427-baaa-f8f595e2ceb3 (lock was held 60.02 sec) release_resources /usr/lib/python3.6/site-packages/ironic/conductor/task_manager.py:447 + + The message tells you the reason the lock was held (``for provision action + manage``) and the amount of time it was held (60.02 seconds, which is way + too much for accessing a BMC). + +Unfortunately, due to the way the conductor is designed, it is not possible to +gracefully break a stuck lock held in ``*-ing`` states. As the last resort, you +may need to restart the affected conductor. See `Why are my nodes stuck in a +"-ing" state?`_. diff --git a/driver-requirements.txt b/driver-requirements.txt index 5239fe73c..da312468e 100644 --- a/driver-requirements.txt +++ b/driver-requirements.txt @@ -4,10 +4,10 @@ # python projects they should package as optional dependencies for Ironic. # These are available on pypi -proliantutils>=2.11.0 +proliantutils>=2.13.0 pysnmp>=4.3.0,<5.0.0 python-scciclient>=0.8.0 -python-dracclient>=5.1.0,<8.0.0 +python-dracclient>=5.1.0,<9.0.0 python-xclarityclient>=0.1.6 # Ansible-deploy interface diff --git a/ironic/common/images.py b/ironic/common/images.py index 939fcb676..30eaed9e3 100644 --- a/ironic/common/images.py +++ b/ironic/common/images.py @@ -120,7 +120,8 @@ def create_vfat_image(output_file, files_info=None, parameters=None, # NOTE: FAT filesystem label can be up to 11 characters long. # TODO(sbaker): use ironic_lib.utils.mkfs when rootwrap has been # removed - utils.execute('mkfs', '-t', 'vfat', '-n', 'ir-vfd-de', output_file) + utils.execute('mkfs', '-t', 'vfat', '-n', + 'ir-vfd-dev', output_file) except processutils.ProcessExecutionError as e: raise exception.ImageCreationFailed(image_type='vfat', error=e) @@ -135,10 +136,16 @@ def create_vfat_image(output_file, files_info=None, parameters=None, file_contents = '\n'.join(params_list) utils.write_to_file(parameters_file, file_contents) + file_list = os.listdir(tmpdir) + + if not file_list: + return + + file_list = [os.path.join(tmpdir, item) for item in file_list] + # use mtools to copy the files into the image in a single # operation - utils.execute('mcopy', '-s', '%s/*' % tmpdir, - '-i', output_file, '::') + utils.execute('mcopy', '-s', *file_list, '-i', output_file, '::') except Exception as e: LOG.exception("vfat image creation failed. Error: %s", e) diff --git a/ironic/common/wsgi_service.py b/ironic/common/wsgi_service.py index e7bbe9dcd..abfe4b1f2 100644 --- a/ironic/common/wsgi_service.py +++ b/ironic/common/wsgi_service.py @@ -10,6 +10,9 @@ # License for the specific language governing permissions and limitations # under the License. +import socket + +from ironic_lib import utils as il_utils from oslo_concurrency import processutils from oslo_service import service from oslo_service import wsgi @@ -46,10 +49,18 @@ class WSGIService(service.ServiceBase): _("api_workers value of %d is invalid, " "must be greater than 0.") % self.workers) - self.server = wsgi.Server(CONF, name, self.app, - host=CONF.api.host_ip, - port=CONF.api.port, - use_ssl=use_ssl) + if CONF.api.unix_socket: + il_utils.unlink_without_raise(CONF.api.unix_socket) + self.server = wsgi.Server(CONF, name, self.app, + socket_family=socket.AF_UNIX, + socket_file=CONF.api.unix_socket, + socket_mode=CONF.api.unix_socket_mode, + use_ssl=use_ssl) + else: + self.server = wsgi.Server(CONF, name, self.app, + host=CONF.api.host_ip, + port=CONF.api.port, + use_ssl=use_ssl) def start(self): """Start serving this service using loaded configuration. @@ -64,6 +75,8 @@ class WSGIService(service.ServiceBase): :returns: None """ self.server.stop() + if CONF.api.unix_socket: + il_utils.unlink_without_raise(CONF.unix_socket) def wait(self): """Wait for the service to stop serving this API. diff --git a/ironic/conf/api.py b/ironic/conf/api.py index dcf235edd..2b0e9a824 100644 --- a/ironic/conf/api.py +++ b/ironic/conf/api.py @@ -15,9 +15,20 @@ # under the License. from oslo_config import cfg +from oslo_config import types as cfg_types from ironic.common.i18n import _ + +class Octal(cfg_types.Integer): + + def __call__(self, value): + if isinstance(value, int): + return value + else: + return int(str(value), 8) + + opts = [ cfg.HostAddressOpt('host_ip', default='0.0.0.0', @@ -26,6 +37,11 @@ opts = [ cfg.PortOpt('port', default=6385, help=_('The TCP port on which ironic-api listens.')), + cfg.StrOpt('unix_socket', + help=_('Unix socket to listen on. Disables host_ip and port.')), + cfg.Opt('unix_socket_mode', type=Octal(), + help=_('File mode (an octal number) of the unix socket to ' + 'listen on. Ignored if unix_socket is not set.')), cfg.IntOpt('max_limit', default=1000, mutable=True, diff --git a/ironic/conf/deploy.py b/ironic/conf/deploy.py index 32f53644a..7a7fb37d7 100644 --- a/ironic/conf/deploy.py +++ b/ironic/conf/deploy.py @@ -128,7 +128,10 @@ opts = [ help=_('Default boot option to use when no boot option is ' 'requested in node\'s driver_info. Defaults to ' '"local". Prior to the Ussuri release, the default ' - 'was "netboot".')), + 'was "netboot".'), + deprecated_for_removal=True, + deprecated_reason=_('Support for network boot will be removed ' + 'after the Yoga release.')), cfg.StrOpt('default_boot_mode', choices=[(boot_modes.UEFI, _('UEFI boot mode')), (boot_modes.LEGACY_BIOS, _('Legacy BIOS boot mode'))], diff --git a/ironic/conf/redfish.py b/ironic/conf/redfish.py index eddf3e013..3cc9fe015 100644 --- a/ironic/conf/redfish.py +++ b/ironic/conf/redfish.py @@ -90,6 +90,21 @@ opts = [ default=60, help=_('Number of seconds to wait between checking for ' 'failed firmware update tasks')), + cfg.StrOpt('firmware_source', + choices=[('http', _('If firmware source URL is also HTTP, then ' + 'serve from original location, otherwise ' + 'copy to ironic\'s HTTP server. Default.')), + ('local', _('Download from original location and ' + 'server from ironic\'s HTTP server.')), + ('swift', _('If firmware source URL is also Swift, ' + 'serve from original location, otherwise ' + 'copy to ironic\'s Swift server.'))], + default='http', + mutable=True, + help=_('Specifies how firmware image should be served. Whether ' + 'from its original location using the firmware source ' + 'URL directly, or should serve it from ironic\'s Swift ' + 'or HTTP server.')), cfg.IntOpt('raid_config_status_interval', min=0, default=60, diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py index 2dcd8a819..c171f81b1 100644 --- a/ironic/drivers/modules/agent.py +++ b/ironic/drivers/modules/agent.py @@ -505,6 +505,11 @@ class AgentDeploy(CustomAgentDeploy): validate_http_provisioning_configuration(node) validate_image_proxies(node) + capabilities = utils.parse_instance_info_capabilities(node) + if 'boot_option' in capabilities: + LOG.warning("The boot_option capability has been deprecated, " + "please unset it for node %s", node.uuid) + @METRICS.timer('AgentDeployMixin.write_image') @base.deploy_step(priority=80) @task_manager.require_exclusive_lock diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py index e1aabb500..d09a616ea 100644 --- a/ironic/drivers/modules/deploy_utils.py +++ b/ironic/drivers/modules/deploy_utils.py @@ -604,12 +604,6 @@ def validate_image_properties(task, deploy_info): "%(properties)s") % {'image': image_href, 'properties': props}) -def get_default_boot_option(): - """Gets the default boot option.""" - # TODO(TheJulia): Deprecated: Remove after Ussuri. - return CONF.deploy.default_boot_option - - def get_boot_option(node): """Gets the boot option. diff --git a/ironic/drivers/modules/drac/bios.py b/ironic/drivers/modules/drac/bios.py index 8ea3ff51f..2ee565768 100644 --- a/ironic/drivers/modules/drac/bios.py +++ b/ironic/drivers/modules/drac/bios.py @@ -71,7 +71,7 @@ class DracWSManBIOS(base.BIOSInterface): reason=_("Unable to import dracclient.exceptions library")) @METRICS.timer('DracWSManBIOS.apply_configuration') - @base.clean_step(priority=0, argsinfo=_args_info) + @base.clean_step(priority=0, argsinfo=_args_info, requires_ramdisk=False) @base.deploy_step(priority=0, argsinfo=_args_info) def apply_configuration(self, task, settings): """Apply the BIOS configuration to the node @@ -352,7 +352,7 @@ class DracWSManBIOS(base.BIOSInterface): manager_utils.notify_conductor_resume_deploy(task) @METRICS.timer('DracWSManBIOS.factory_reset') - @base.clean_step(priority=0) + @base.clean_step(priority=0, requires_ramdisk=False) @base.deploy_step(priority=0) def factory_reset(self, task): """Reset the BIOS settings of the node to the factory default. @@ -418,7 +418,7 @@ class DracWSManBIOS(base.BIOSInterface): node.timestamp_driver_internal_info('factory_reset_time') # rebooting the server to apply factory reset value - client.set_power_state('REBOOT') + task.driver.power.reboot(task) # This method calls node.save(), bios_config_job_id will be # saved automatically diff --git a/ironic/drivers/modules/drac/management.py b/ironic/drivers/modules/drac/management.py index f4c77662a..df6942611 100644 --- a/ironic/drivers/modules/drac/management.py +++ b/ironic/drivers/modules/drac/management.py @@ -767,7 +767,7 @@ class DracWSManManagement(base.ManagementInterface): @METRICS.timer('DracManagement.reset_idrac') @base.verify_step(priority=0) - @base.clean_step(priority=0) + @base.clean_step(priority=0, requires_ramdisk=False) def reset_idrac(self, task): """Reset the iDRAC. @@ -782,7 +782,7 @@ class DracWSManManagement(base.ManagementInterface): @METRICS.timer('DracManagement.known_good_state') @base.verify_step(priority=0) - @base.clean_step(priority=0) + @base.clean_step(priority=0, requires_ramdisk=False) def known_good_state(self, task): """Reset the iDRAC, Clear the job queue. @@ -798,7 +798,7 @@ class DracWSManManagement(base.ManagementInterface): @METRICS.timer('DracManagement.clear_job_queue') @base.verify_step(priority=0) - @base.clean_step(priority=0) + @base.clean_step(priority=0, requires_ramdisk=False) def clear_job_queue(self, task): """Clear the job queue. diff --git a/ironic/drivers/modules/drac/raid.py b/ironic/drivers/modules/drac/raid.py index c2a063ec0..ae06f0dfa 100644 --- a/ironic/drivers/modules/drac/raid.py +++ b/ironic/drivers/modules/drac/raid.py @@ -1601,7 +1601,7 @@ class DracWSManRAID(base.RAIDInterface): ), "required": False, } - }) + }, requires_ramdisk=False) def create_configuration(self, task, create_root_volume=True, create_nonroot_volumes=True, @@ -1698,7 +1698,7 @@ class DracWSManRAID(base.RAIDInterface): return _create_virtual_disks(task, node) @METRICS.timer('DracRAID.delete_configuration') - @base.clean_step(priority=0) + @base.clean_step(priority=0, requires_ramdisk=False) @base.deploy_step(priority=0) def delete_configuration(self, task): """Delete the RAID configuration. diff --git a/ironic/drivers/modules/redfish/firmware_utils.py b/ironic/drivers/modules/redfish/firmware_utils.py index 35e4bb1f2..c73cb80dd 100644 --- a/ironic/drivers/modules/redfish/firmware_utils.py +++ b/ironic/drivers/modules/redfish/firmware_utils.py @@ -11,11 +11,20 @@ # License for the specific language governing permissions and limitations # under the License. +import os +import shutil +import tempfile +from urllib import parse as urlparse + import jsonschema from oslo_log import log +from oslo_utils import fileutils from ironic.common import exception from ironic.common.i18n import _ +from ironic.common import image_service +from ironic.common import swift +from ironic.conf import CONF LOG = log.getLogger(__name__) @@ -26,22 +35,35 @@ _UPDATE_FIRMWARE_SCHEMA = { # list of firmware update images "items": { "type": "object", - "required": ["url"], + "required": ["url", "checksum"], "properties": { "url": { "description": "URL for firmware file", "type": "string", "minLength": 1 }, + "checksum": { + "description": "SHA1 checksum for firmware file", + "type": "string", + "minLength": 1 + }, "wait": { "description": "optional wait time for firmware update", "type": "integer", "minimum": 1 + }, + "source": + { + "description": "optional firmware_source to override global " + "setting for firmware file", + "type": "string", + "enum": ["http", "local", "swift"] } }, "additionalProperties": False } } +_FIRMWARE_SUBDIR = 'firmware' def validate_update_firmware_args(firmware_images): @@ -56,3 +78,180 @@ def validate_update_firmware_args(firmware_images): raise exception.InvalidParameterValue( _('Invalid firmware update %(firmware_images)s. Errors: %(err)s') % {'firmware_images': firmware_images, 'err': err}) + + +def get_swift_temp_url(parsed_url): + """Gets Swift temporary URL + + :param parsed_url: Parsed URL from URL in format + swift://container/[sub-folder/]file + :returns: Swift temporary URL + """ + return swift.SwiftAPI().get_temp_url( + parsed_url.netloc, parsed_url.path.lstrip('/'), + CONF.redfish.swift_object_expiry_timeout) + + +def download_to_temp(node, url): + """Downloads to temporary location from given URL + + :param node: Node for which to download to temporary location + :param url: URL to download from + :returns: File path of temporary location file is downloaded to + """ + parsed_url = urlparse.urlparse(url) + scheme = parsed_url.scheme.lower() + if scheme not in ('http', 'swift', 'file'): + raise exception.InvalidParameterValue( + _('%(scheme)s is not supported for %(url)s.') + % {'scheme': scheme, 'url': parsed_url.geturl()}) + + tempdir = os.path.join(tempfile.gettempdir(), node.uuid) + os.makedirs(tempdir, exist_ok=True) + temp_file = os.path.join( + tempdir, + os.path.basename(parsed_url.path)) + LOG.debug('For node %(node)s firmware at %(url)s will be downloaded to ' + 'temporary location at %(temp_file)s', + {'node': node.uuid, 'url': url, 'temp_file': temp_file}) + if scheme == 'http': + with open(temp_file, 'wb') as tf: + image_service.HttpImageService().download(url, tf) + elif scheme == 'swift': + swift_url = get_swift_temp_url(parsed_url) + with open(temp_file, 'wb') as tf: + image_service.HttpImageService().download(swift_url, tf) + elif scheme == 'file': + with open(temp_file, 'wb') as tf: + image_service.FileImageService().download( + parsed_url.path, tf) + + return temp_file + + +def verify_checksum(node, checksum, file_path): + """Verify checksum. + + :param node: Node for which file to verify checksum + :param checksum: Expected checksum value + :param file_path: File path for which to verify checksum + :raises RedfishError: When checksum does not match + """ + calculated_checksum = fileutils.compute_file_checksum( + file_path, algorithm='sha1') + if checksum != calculated_checksum: + raise exception.RedfishError( + _('For node %(node)s firmware file %(temp_file)s checksums do not ' + 'match. Expected: %(checksum)s, calculated: ' + '%(calculated_checksum)s.') + % {'node': node.uuid, 'temp_file': file_path, 'checksum': checksum, + 'calculated_checksum': calculated_checksum}) + + +def stage(node, source, temp_file): + """Stage temporary file to configured location + + :param node: Node for which to stage the file + :param source: Where to stage the file. Corresponds to + CONF.redfish.firmware_source. + :param temp_file: File path of temporary file to stage + :returns: Tuple of staged URL and source (http or swift) that needs + cleanup of staged files afterwards. + :raises RedfishError: If staging to HTTP server has failed. + """ + staged_url = None + filename = os.path.basename(temp_file) + if source in ('http', 'local'): + http_url = CONF.deploy.external_http_url or CONF.deploy.http_url + staged_url = urlparse.urljoin( + http_url, "/".join([_FIRMWARE_SUBDIR, node.uuid, filename])) + staged_folder = os.path.join( + CONF.deploy.http_root, _FIRMWARE_SUBDIR, node.uuid) + staged_path = os.path.join(staged_folder, filename) + LOG.debug('For node %(node)s temporary file %(temp_file)s will be ' + 'hard-linked or copied to %(staged_path)s and served over ' + '%(staged_url)s', + {'node': node.uuid, 'temp_file': temp_file, + 'staged_path': staged_path, 'staged_url': staged_url}) + os.makedirs(staged_folder, exist_ok=True) + try: + os.link(temp_file, staged_path) + os.chmod(temp_file, CONF.redfish.file_permission) + except OSError as oserror: + LOG.debug("Could not hardlink file %(temp_file)s to location " + "%(staged_path)s. Will try to copy it. Error: %(error)s", + {'temp_file': temp_file, 'staged_path': staged_path, + 'error': oserror}) + try: + shutil.copyfile(temp_file, staged_path) + os.chmod(staged_path, CONF.redfish.file_permission) + except IOError as ioerror: + raise exception.RedfishError( + _('For %(node)s failed to copy firmware file ' + '%(temp_file)s to HTTP server root. Error %(error)s') + % {'node': node.uuid, 'temp_file': temp_file, + 'error': ioerror}) + + elif source == 'swift': + container = CONF.redfish.swift_container + timeout = CONF.redfish.swift_object_expiry_timeout + swift_api = swift.SwiftAPI() + object_name = "/".join([node.uuid, filename]) + swift_api.create_object( + container, + object_name, + temp_file, + object_headers={'X-Delete-After': str(timeout)}) + staged_url = swift_api.get_temp_url( + container, object_name, timeout) + LOG.debug('For node %(node)s temporary file at %(temp_file)s will be ' + 'served from Swift temporary URL %(staged_url)s', + {'node': node.uuid, 'temp_file': temp_file, + 'staged_url': staged_url}) + + need_cleanup = 'swift' if source == 'swift' else 'http' + return staged_url, need_cleanup + + +def cleanup(node): + """Clean up staged files + + :param node: Node for which to clean up. Should contain + 'firmware_cleanup' entry in `driver_internal_info` to indicate + source(s) to be cleaned up. + """ + # Cleaning up temporary just in case there is something when staging + # to http or swift has failed. + temp_dir = os.path.join(tempfile.gettempdir(), node.uuid) + LOG.debug('For node %(node)s cleaning up temporary files, if any, from ' + '%(temp_dir)s.', {'node': node.uuid, 'temp_dir': temp_dir}) + shutil.rmtree(temp_dir, ignore_errors=True) + + cleanup = node.driver_internal_info.get('firmware_cleanup') + if not cleanup: + return + + if 'http' in cleanup: + http_dir = os.path.join( + CONF.deploy.http_root, _FIRMWARE_SUBDIR, node.uuid) + LOG.debug('For node %(node)s cleaning up files from %(http_dir)s.', + {'node': node.uuid, 'http_dir': http_dir}) + shutil.rmtree(http_dir, ignore_errors=True) + + if 'swift' in cleanup: + swift_api = swift.SwiftAPI() + container = CONF.redfish.swift_container + LOG.debug('For node %(node)s cleaning up files from Swift container ' + '%(container)s.', + {'node': node.uuid, 'container': container}) + _, objects = swift_api.connection.get_container(container) + for o in objects: + name = o.get('name') + if name and name.startswith(node.uuid): + try: + swift_api.delete_object(container, name) + except exception.SwiftOperationError as error: + LOG.warning('For node %(node)s failed to clean up ' + '%(object)s. Error: %(error)s', + {'node': node.uuid, 'object': name, + 'error': error}) diff --git a/ironic/drivers/modules/redfish/management.py b/ironic/drivers/modules/redfish/management.py index cb56a821b..a669d09bc 100644 --- a/ironic/drivers/modules/redfish/management.py +++ b/ironic/drivers/modules/redfish/management.py @@ -14,6 +14,7 @@ # under the License. import collections +from urllib.parse import urlparse from ironic_lib import metrics_utils from oslo_log import log @@ -799,7 +800,8 @@ class RedfishManagement(base.ManagementInterface): """ firmware_update = firmware_updates[0] - firmware_url = firmware_update['url'] + firmware_url, need_cleanup = self._stage_firmware_file( + node, firmware_update) LOG.debug('Applying firmware %(firmware_image)s to node ' '%(node_uuid)s', @@ -809,8 +811,15 @@ class RedfishManagement(base.ManagementInterface): task_monitor = update_service.simple_update(firmware_url) firmware_update['task_monitor'] = task_monitor.task_monitor_uri - node.set_driver_internal_info('firmware_updates', - firmware_updates) + node.set_driver_internal_info('firmware_updates', firmware_updates) + + if need_cleanup: + fw_cleanup = node.driver_internal_info.get('firmware_cleanup') + if not fw_cleanup: + fw_cleanup = [need_cleanup] + elif need_cleanup not in fw_cleanup: + fw_cleanup.append(need_cleanup) + node.set_driver_internal_info('firmware_cleanup', fw_cleanup) def _continue_firmware_updates(self, task, update_service, firmware_updates): @@ -860,13 +869,18 @@ class RedfishManagement(base.ManagementInterface): manager_utils.node_power_action(task, states.REBOOT) def _clear_firmware_updates(self, node): - """Clears firmware updates from driver_internal_info + """Clears firmware updates artifacts + + Clears firmware updates from driver_internal_info and any files + that were staged. Note that the caller must have an exclusive lock on the node. :param node: the node to clear the firmware updates from """ + firmware_utils.cleanup(node) node.del_driver_internal_info('firmware_updates') + node.del_driver_internal_info('firmware_cleanup') node.save() @METRICS.timer('RedfishManagement._query_firmware_update_failed') @@ -1012,6 +1026,56 @@ class RedfishManagement(base.ManagementInterface): {'node': node.uuid, 'firmware_image': current_update['url']}) + def _stage_firmware_file(self, node, firmware_update): + """Stage firmware update according to configuration. + + :param node: Node for which to stage the firmware file + :param firmware_update: Firmware update to stage + :returns: Tuple of staged URL and source that needs cleanup of + staged files afterwards. If not staging, then return + original URL and None for source that needs cleanup. + :raises IronicException: If something goes wrong with staging. + """ + try: + url = firmware_update['url'] + parsed_url = urlparse(url) + scheme = parsed_url.scheme.lower() + source = (firmware_update.get('source') + or CONF.redfish.firmware_source).lower() + + # Keep it simple, in further processing TLS does not matter + if scheme == 'https': + scheme = 'http' + + # If source and scheme is HTTP, then no staging, + # returning original location + if scheme == 'http' and source == scheme: + LOG.debug('For node %(node)s serving firmware from original ' + 'location %(url)s', {'node': node.uuid, 'url': url}) + return url, None + + # If source and scheme is Swift, then not moving, but + # returning Swift temp URL + if scheme == 'swift' and source == scheme: + temp_url = firmware_utils.get_swift_temp_url(parsed_url) + LOG.debug('For node %(node)s serving original firmware at ' + '%(url)s via Swift temporary url %(temp_url)s', + {'node': node.uuid, 'url': url, + 'temp_url': temp_url}) + return temp_url, None + + # For remaining, download the image to temporary location + temp_file = firmware_utils.download_to_temp(node, url) + + firmware_utils.verify_checksum( + node, firmware_update.get('checksum'), temp_file) + + return firmware_utils.stage(node, source, temp_file) + + except exception.IronicException as error: + firmware_utils.cleanup(node) + raise error + def get_secure_boot_state(self, task): """Get the current secure boot state for the node. diff --git a/ironic/tests/unit/common/test_images.py b/ironic/tests/unit/common/test_images.py index f699fb7ce..f3fbbbf77 100644 --- a/ironic/tests/unit/common/test_images.py +++ b/ironic/tests/unit/common/test_images.py @@ -307,20 +307,22 @@ class FsImageTestCase(base.TestCase): mkdir_mock.assert_any_call('root_dir', exist_ok=True) mkdir_mock.assert_any_call('root_dir/sub_dir', exist_ok=True) + @mock.patch.object(os, 'listdir', autospec=True) @mock.patch.object(images, '_create_root_fs', autospec=True) @mock.patch.object(utils, 'tempdir', autospec=True) @mock.patch.object(utils, 'write_to_file', autospec=True) @mock.patch.object(utils, 'execute', autospec=True) def test_create_vfat_image( self, execute_mock, write_mock, - tempdir_mock, create_root_fs_mock): + tempdir_mock, create_root_fs_mock, os_listdir_mock): mock_file_handle = mock.MagicMock(spec=io.BytesIO) - mock_file_handle.__enter__.return_value = 'tempdir' + mock_file_handle.__enter__.return_value = '/tempdir' tempdir_mock.return_value = mock_file_handle parameters = {'p1': 'v1'} files_info = {'a': 'b'} + os_listdir_mock.return_value = ['b', 'qwe'] images.create_vfat_image('tgt_file', parameters=parameters, files_info=files_info, parameters_file='qwe', fs_size_kib=1000) @@ -328,13 +330,15 @@ class FsImageTestCase(base.TestCase): execute_mock.assert_has_calls([ mock.call('dd', 'if=/dev/zero', 'of=tgt_file', 'count=1', 'bs=1000KiB'), - mock.call('mkfs', '-t', 'vfat', '-n', 'ir-vfd-de', 'tgt_file'), - mock.call('mcopy', '-s', 'tempdir/*', '-i', 'tgt_file', '::') + mock.call('mkfs', '-t', 'vfat', '-n', 'ir-vfd-dev', 'tgt_file'), + mock.call('mcopy', '-s', '/tempdir/b', '/tempdir/qwe', '-i', + 'tgt_file', '::') ]) - parameters_file_path = os.path.join('tempdir', 'qwe') + parameters_file_path = os.path.join('/tempdir', 'qwe') write_mock.assert_called_once_with(parameters_file_path, 'p1=v1') - create_root_fs_mock.assert_called_once_with('tempdir', files_info) + create_root_fs_mock.assert_called_once_with('/tempdir', files_info) + os_listdir_mock.assert_called_once_with('/tempdir') @mock.patch.object(utils, 'execute', autospec=True) def test_create_vfat_image_dd_fails(self, execute_mock): diff --git a/ironic/tests/unit/drivers/modules/drac/test_bios.py b/ironic/tests/unit/drivers/modules/drac/test_bios.py index e24267f95..ab56fed0e 100644 --- a/ironic/tests/unit/drivers/modules/drac/test_bios.py +++ b/ironic/tests/unit/drivers/modules/drac/test_bios.py @@ -22,6 +22,7 @@ Test class for DRAC BIOS configuration specific methods from unittest import mock from dracclient import exceptions as drac_exceptions +from oslo_utils import importutils from oslo_utils import timeutils from ironic.common import exception @@ -36,6 +37,8 @@ from ironic import objects from ironic.tests.unit.drivers.modules.drac import utils as test_utils from ironic.tests.unit.objects import utils as obj_utils +drac_constants = importutils.try_import('dracclient.constants') + INFO_DICT = test_utils.INFO_DICT @@ -73,6 +76,7 @@ class DracWSManBIOSConfigurationTestCase(test_utils.BaseDracTest): } self.mock_client.commit_pending_bios_changes.return_value = \ "JID_5678" + self.mock_client.get_power_state.return_value = drac_constants.POWER_ON @mock.patch.object(drac_common, 'parse_driver_info', autospec=True) diff --git a/ironic/tests/unit/drivers/modules/redfish/test_firmware_utils.py b/ironic/tests/unit/drivers/modules/redfish/test_firmware_utils.py index 60c66c024..e2c6e75b2 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_firmware_utils.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_firmware_utils.py @@ -11,7 +11,18 @@ # License for the specific language governing permissions and limitations # under the License. +import os +import shutil +import tempfile +from unittest import mock +from urllib.parse import urlparse + +from oslo_utils import fileutils + from ironic.common import exception +from ironic.common import image_service +from ironic.common import swift +from ironic.conf import CONF from ironic.drivers.modules.redfish import firmware_utils from ironic.tests import base @@ -22,10 +33,12 @@ class FirmwareUtilsTestCase(base.TestCase): firmware_images = [ { "url": "http://192.0.2.10/BMC_4_22_00_00.EXE", + "checksum": "aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d", "wait": 300 }, { - "url": "https://192.0.2.10/NIC_19.0.12_A00.EXE" + "url": "https://192.0.2.10/NIC_19.0.12_A00.EXE", + "checksum": "9f6227549221920e312fed2cfc6586ee832cc546" } ] firmware_utils.validate_update_firmware_args(firmware_images) @@ -33,6 +46,7 @@ class FirmwareUtilsTestCase(base.TestCase): def test_validate_update_firmware_args_not_list(self): firmware_images = { "url": "http://192.0.2.10/BMC_4_22_00_00.EXE", + "checksum": "aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d", "wait": 300 } self.assertRaisesRegex( @@ -43,10 +57,12 @@ class FirmwareUtilsTestCase(base.TestCase): firmware_images = [ { "url": "http://192.0.2.10/BMC_4_22_00_00.EXE", + "checksum": "aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d", "wait": 300, }, { "url": "https://192.0.2.10/NIC_19.0.12_A00.EXE", + "checksum": "9f6227549221920e312fed2cfc6586ee832cc546", "something": "unknown" } ] @@ -58,9 +74,11 @@ class FirmwareUtilsTestCase(base.TestCase): firmware_images = [ { "url": "http://192.0.2.10/BMC_4_22_00_00.EXE", + "checksum": "aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d", "wait": 300, }, { + "checksum": "9f6227549221920e312fed2cfc6586ee832cc546", "wait": 300 } ] @@ -72,6 +90,34 @@ class FirmwareUtilsTestCase(base.TestCase): def test_validate_update_firmware_args_url_not_string(self): firmware_images = [{ "url": 123, + "checksum": "aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d", + "wait": 300 + }] + self.assertRaisesRegex( + exception.InvalidParameterValue, "123 is not of type 'string'", + firmware_utils.validate_update_firmware_args, firmware_images) + + def test_validate_update_firmware_args_checksum_missing(self): + firmware_images = [ + { + "url": "http://192.0.2.10/BMC_4_22_00_00.EXE", + "checksum": "aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d", + "wait": 300, + }, + { + "url": "https://192.0.2.10/NIC_19.0.12_A00.EXE", + "wait": 300 + } + ] + self.assertRaisesRegex( + exception.InvalidParameterValue, + "'checksum' is a required property", + firmware_utils.validate_update_firmware_args, firmware_images) + + def test_validate_update_firmware_args_checksum_not_string(self): + firmware_images = [{ + "url": "http://192.0.2.10/BMC_4_22_00_00.EXE", + "checksum": 123, "wait": 300 }] self.assertRaisesRegex( @@ -81,8 +127,335 @@ class FirmwareUtilsTestCase(base.TestCase): def test_validate_update_firmware_args_wait_not_int(self): firmware_images = [{ "url": "http://192.0.2.10/BMC_4_22_00_00.EXE", + "checksum": "aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d", "wait": 'abc' }] self.assertRaisesRegex( exception.InvalidParameterValue, "'abc' is not of type 'integer'", firmware_utils.validate_update_firmware_args, firmware_images) + + def test_validate_update_firmware_args_source_not_known(self): + firmware_images = [{ + "url": "http://192.0.2.10/BMC_4_22_00_00.EXE", + "checksum": "aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d", + "source": "abc" + }] + self.assertRaisesRegex( + exception.InvalidParameterValue, "'abc' is not one of", + firmware_utils.validate_update_firmware_args, firmware_images) + + @mock.patch.object(swift, 'SwiftAPI', autospec=True) + def test_get_swift_temp_url(self, mock_swift_api): + mock_swift_api.return_value.get_temp_url.return_value = 'http://temp' + parsed_url = urlparse("swift://firmware/sub/bios.exe") + + result = firmware_utils.get_swift_temp_url(parsed_url) + + self.assertEqual(result, 'http://temp') + mock_swift_api.return_value.get_temp_url.assert_called_with( + 'firmware', 'sub/bios.exe', + CONF.redfish.swift_object_expiry_timeout) + + @mock.patch.object(tempfile, 'gettempdir', autospec=True) + @mock.patch.object(os, 'makedirs', autospec=True) + @mock.patch.object(image_service, 'HttpImageService', autospec=True) + def test_download_to_temp_http( + self, mock_http_image_service, mock_makedirs, mock_gettempdir): + node = mock.Mock(uuid='9f0f6795-f74e-4b5a-850e-72f586a92435') + mock_gettempdir.return_value = '/tmp' + http_url = 'http://example.com/bios.exe' + + with mock.patch.object(firmware_utils, 'open', mock.mock_open(), + create=True) as mock_open: + result = firmware_utils.download_to_temp(node, http_url) + + exp_result = '/tmp/9f0f6795-f74e-4b5a-850e-72f586a92435/bios.exe' + exp_temp_dir = '/tmp/9f0f6795-f74e-4b5a-850e-72f586a92435' + mock_makedirs.assert_called_with(exp_temp_dir, exist_ok=True) + self.assertEqual(result, exp_result) + mock_http_image_service.return_value.download.assert_called_with( + http_url, mock_open.return_value) + mock_open.assert_has_calls([mock.call(exp_result, 'wb')]) + + @mock.patch.object(tempfile, 'gettempdir', autospec=True) + @mock.patch.object(os, 'makedirs', autospec=True) + @mock.patch.object(image_service, 'HttpImageService', autospec=True) + @mock.patch.object(swift, 'SwiftAPI', autospec=True) + def test_download_to_temp_swift( + self, mock_swift_api, mock_http_image_service, mock_makedirs, + mock_gettempdir): + node = mock.Mock(uuid='9f0f6795-f74e-4b5a-850e-72f586a92435') + mock_gettempdir.return_value = '/tmp' + swift_url = 'swift://firmware/sub/bios.exe' + temp_swift_url = 'http://swift_temp' + mock_swift_api.return_value.get_temp_url.return_value = temp_swift_url + + with mock.patch.object(firmware_utils, 'open', mock.mock_open(), + create=True) as mock_open: + result = firmware_utils.download_to_temp(node, swift_url) + + exp_result = '/tmp/9f0f6795-f74e-4b5a-850e-72f586a92435/bios.exe' + exp_temp_dir = '/tmp/9f0f6795-f74e-4b5a-850e-72f586a92435' + mock_makedirs.assert_called_with(exp_temp_dir, exist_ok=True) + self.assertEqual(result, exp_result) + mock_http_image_service.return_value.download.assert_called_with( + temp_swift_url, mock_open.return_value) + mock_open.assert_has_calls([mock.call(exp_result, 'wb')]) + + @mock.patch.object(tempfile, 'gettempdir', autospec=True) + @mock.patch.object(os, 'makedirs', autospec=True) + @mock.patch.object(image_service, 'FileImageService', autospec=True) + def test_download_to_temp_file( + self, mock_file_image_service, mock_makedirs, + mock_gettempdir): + node = mock.Mock(uuid='9f0f6795-f74e-4b5a-850e-72f586a92435') + mock_gettempdir.return_value = '/tmp' + file_url = 'file:///firmware/bios.exe' + + with mock.patch.object(firmware_utils, 'open', mock.mock_open(), + create=True) as mock_open: + result = firmware_utils.download_to_temp(node, file_url) + + exp_result = '/tmp/9f0f6795-f74e-4b5a-850e-72f586a92435/bios.exe' + exp_temp_dir = '/tmp/9f0f6795-f74e-4b5a-850e-72f586a92435' + mock_makedirs.assert_called_with(exp_temp_dir, exist_ok=True) + self.assertEqual(result, exp_result) + mock_file_image_service.return_value.download.assert_called_with( + '/firmware/bios.exe', mock_open.return_value) + mock_open.assert_has_calls([mock.call(exp_result, 'wb')]) + + def test_download_to_temp_invalid(self): + node = mock.Mock(uuid='9f0f6795-f74e-4b5a-850e-72f586a92435') + self.assertRaises( + exception.InvalidParameterValue, + firmware_utils.download_to_temp, node, 'ftp://firmware/bios.exe') + + @mock.patch.object(fileutils, 'compute_file_checksum', autospec=True) + def test_verify_checksum(self, mock_compute_file_checksum): + checksum = 'aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d' + file_path = '/tmp/bios.exe' + mock_compute_file_checksum.return_value = checksum + node = mock.Mock(uuid='9f0f6795-f74e-4b5a-850e-72f586a92435') + + firmware_utils.verify_checksum(node, checksum, file_path) + + mock_compute_file_checksum.assert_called_with( + file_path, algorithm='sha1') + + @mock.patch.object(fileutils, 'compute_file_checksum', autospec=True) + def test_verify_checksum_mismatch(self, mock_compute_file_checksum): + checksum1 = 'aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d' + checksum2 = '9f6227549221920e312fed2cfc6586ee832cc546' + file_path = '/tmp/bios.exe' + mock_compute_file_checksum.return_value = checksum1 + node = mock.Mock(uuid='9f0f6795-f74e-4b5a-850e-72f586a92435') + + self.assertRaises( + exception.RedfishError, firmware_utils.verify_checksum, node, + checksum2, file_path) + mock_compute_file_checksum.assert_called_with( + file_path, algorithm='sha1') + + @mock.patch.object(os, 'makedirs', autospec=True) + @mock.patch.object(shutil, 'copyfile', autospec=True) + @mock.patch.object(os, 'link', autospec=True) + @mock.patch.object(os, 'chmod', autospec=True) + def test_stage_http(self, mock_chmod, mock_link, mock_copyfile, + mock_makedirs): + CONF.deploy.http_url = 'http://10.0.0.2' + CONF.deploy.external_http_url = None + CONF.deploy.http_root = '/httproot' + node = mock.Mock(uuid='55cdaba0-1123-4622-8b37-bb52dd6285d3') + + staged_url, need_cleanup = firmware_utils.stage( + node, 'http', '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe') + + self.assertEqual(staged_url, + 'http://10.0.0.2/firmware/' + '55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe') + self.assertEqual(need_cleanup, 'http') + mock_makedirs.assert_called_with( + '/httproot/firmware/55cdaba0-1123-4622-8b37-bb52dd6285d3', + exist_ok=True) + mock_link.assert_called_with( + '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe', + '/httproot/firmware/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe') + mock_chmod.assert_called_with( + '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe', + CONF.redfish.file_permission) + mock_copyfile.assert_not_called() + + @mock.patch.object(os, 'makedirs', autospec=True) + @mock.patch.object(shutil, 'copyfile', autospec=True) + @mock.patch.object(os, 'link', autospec=True) + @mock.patch.object(os, 'chmod', autospec=True) + def test_stage_http_copyfile(self, mock_chmod, mock_link, mock_copyfile, + mock_makedirs): + CONF.deploy.http_url = 'http://10.0.0.2' + CONF.deploy.external_http_url = None + CONF.deploy.http_root = '/httproot' + node = mock.Mock(uuid='55cdaba0-1123-4622-8b37-bb52dd6285d3') + mock_link.side_effect = OSError + + staged_url, need_cleanup = firmware_utils.stage( + node, 'http', '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe') + + self.assertEqual(staged_url, + 'http://10.0.0.2/firmware/' + '55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe') + self.assertEqual(need_cleanup, 'http') + mock_makedirs.assert_called_with( + '/httproot/firmware/55cdaba0-1123-4622-8b37-bb52dd6285d3', + exist_ok=True) + mock_link.assert_called_with( + '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe', + '/httproot/firmware/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe') + mock_copyfile.assert_called_with( + '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe', + '/httproot/firmware/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe') + mock_chmod.assert_called_with( + '/httproot/firmware/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe', + CONF.redfish.file_permission) + + @mock.patch.object(os, 'makedirs', autospec=True) + @mock.patch.object(shutil, 'copyfile', autospec=True) + @mock.patch.object(os, 'link', autospec=True) + @mock.patch.object(os, 'chmod', autospec=True) + def test_stage_http_copyfile_fails(self, mock_chmod, mock_link, + mock_copyfile, mock_makedirs): + CONF.deploy.http_url = 'http://10.0.0.2' + CONF.deploy.external_http_url = None + CONF.deploy.http_root = '/httproot' + node = mock.Mock(uuid='55cdaba0-1123-4622-8b37-bb52dd6285d3') + mock_link.side_effect = OSError + mock_copyfile.side_effect = IOError + + self.assertRaises(exception.RedfishError, firmware_utils.stage, + node, 'http', + '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe') + + mock_makedirs.assert_called_with( + '/httproot/firmware/55cdaba0-1123-4622-8b37-bb52dd6285d3', + exist_ok=True) + mock_link.assert_called_with( + '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe', + '/httproot/firmware/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe') + mock_copyfile.assert_called_with( + '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe', + '/httproot/firmware/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe') + mock_chmod.assert_not_called() + + @mock.patch.object(os, 'makedirs', autospec=True) + @mock.patch.object(shutil, 'copyfile', autospec=True) + @mock.patch.object(shutil, 'rmtree', autospec=True) + @mock.patch.object(os, 'link', autospec=True) + @mock.patch.object(os, 'chmod', autospec=True) + def test_stage_local_external(self, mock_chmod, mock_link, mock_rmtree, + mock_copyfile, mock_makedirs): + CONF.deploy.http_url = 'http://10.0.0.2' + CONF.deploy.external_http_url = 'http://90.0.0.9' + CONF.deploy.http_root = '/httproot' + node = mock.Mock(uuid='55cdaba0-1123-4622-8b37-bb52dd6285d3') + + staged_url, need_cleanup = firmware_utils.stage( + node, 'local', + '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe') + + self.assertEqual(staged_url, + 'http://90.0.0.9/firmware/' + '55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe') + self.assertEqual(need_cleanup, 'http') + mock_makedirs.assert_called_with( + '/httproot/firmware/55cdaba0-1123-4622-8b37-bb52dd6285d3', + exist_ok=True) + mock_link.assert_called_with( + '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe', + '/httproot/firmware/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe') + mock_chmod.assert_called_with( + '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe', + CONF.redfish.file_permission) + mock_copyfile.assert_not_called() + + @mock.patch.object(swift, 'SwiftAPI', autospec=True) + def test_stage_swift(self, mock_swift_api): + node = mock.Mock(uuid='55cdaba0-1123-4622-8b37-bb52dd6285d3') + mock_swift_api.return_value.get_temp_url.return_value = 'http://temp' + temp_file = '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe' + + staged_url, need_cleanup = firmware_utils.stage( + node, 'swift', temp_file) + + self.assertEqual(staged_url, 'http://temp') + self.assertEqual(need_cleanup, 'swift') + exp_object_name = '55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe' + mock_swift_api.return_value.create_object.assert_called_with( + CONF.redfish.swift_container, + exp_object_name, temp_file, + object_headers={'X-Delete-After': + str(CONF.redfish.swift_object_expiry_timeout)}) + mock_swift_api.return_value.get_temp_url.assert_called_with( + CONF.redfish.swift_container, exp_object_name, + CONF.redfish.swift_object_expiry_timeout) + + @mock.patch.object(shutil, 'rmtree', autospec=True) + @mock.patch.object(tempfile, 'gettempdir', autospec=True) + @mock.patch.object(swift, 'SwiftAPI', autospec=True) + def test_cleanup(self, mock_swift_api, mock_gettempdir, mock_rmtree): + mock_gettempdir.return_value = '/tmp' + CONF.deploy.http_root = '/httproot' + node = mock.Mock( + uuid='55cdaba0-1123-4622-8b37-bb52dd6285d3', + driver_internal_info={'firmware_cleanup': ['http', 'swift']}) + object_name = '55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe' + get_container = mock_swift_api.return_value.connection.get_container + get_container.return_value = (mock.Mock(), [{'name': object_name}]) + + firmware_utils.cleanup(node) + + mock_rmtree.assert_any_call( + '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3', + ignore_errors=True) + mock_rmtree.assert_any_call( + '/httproot/firmware/55cdaba0-1123-4622-8b37-bb52dd6285d3', + ignore_errors=True) + mock_swift_api.return_value.delete_object.assert_called_with( + CONF.redfish.swift_container, object_name) + + @mock.patch.object(shutil, 'rmtree', autospec=True) + @mock.patch.object(tempfile, 'gettempdir', autospec=True) + def test_cleanup_notstaged(self, mock_gettempdir, mock_rmtree): + mock_gettempdir.return_value = '/tmp' + node = mock.Mock( + uuid='55cdaba0-1123-4622-8b37-bb52dd6285d3', + driver_internal_info={'something': 'else'}) + + firmware_utils.cleanup(node) + + mock_rmtree.assert_any_call( + '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3', + ignore_errors=True) + + @mock.patch.object(shutil, 'rmtree', autospec=True) + @mock.patch.object(tempfile, 'gettempdir', autospec=True) + @mock.patch.object(swift, 'SwiftAPI', autospec=True) + @mock.patch.object(firmware_utils.LOG, 'warning', autospec=True) + def test_cleanup_swift_fails(self, mock_warning, mock_swift_api, + mock_gettempdir, mock_rmtree): + mock_gettempdir.return_value = '/tmp' + node = mock.Mock( + uuid='55cdaba0-1123-4622-8b37-bb52dd6285d3', + driver_internal_info={'firmware_cleanup': ['swift']}) + object_name = '55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe' + get_container = mock_swift_api.return_value.connection.get_container + get_container.return_value = (mock.Mock(), [{'name': object_name}]) + mock_swift_api.return_value.delete_object.side_effect =\ + exception.SwiftOperationError + + firmware_utils.cleanup(node) + + mock_rmtree.assert_any_call( + '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3', + ignore_errors=True) + mock_swift_api.return_value.delete_object.assert_called_with( + CONF.redfish.swift_container, object_name) + mock_warning.assert_called_once() diff --git a/ironic/tests/unit/drivers/modules/redfish/test_management.py b/ironic/tests/unit/drivers/modules/redfish/test_management.py index b46700664..93aae5de8 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_management.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_management.py @@ -27,8 +27,10 @@ from ironic.common import indicator_states from ironic.common import states from ironic.conductor import task_manager from ironic.conductor import utils as manager_utils +from ironic.conf import CONF from ironic.drivers.modules import deploy_utils from ironic.drivers.modules.redfish import boot as redfish_boot +from ironic.drivers.modules.redfish import firmware_utils from ironic.drivers.modules.redfish import management as redfish_mgmt from ironic.drivers.modules.redfish import utils as redfish_utils from ironic.tests.unit.db import base as db_base @@ -834,22 +836,145 @@ class RedfishManagementTestCase(db_base.DbTestCase): mock_update_service = mock.Mock() mock_update_service.simple_update.return_value = mock_task_monitor mock_get_update_service.return_value = mock_update_service + CONF.redfish.firmware_source = 'http' with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: task.node.save = mock.Mock() - task.driver.management.update_firmware(task, - [{'url': 'test1'}, - {'url': 'test2'}]) + task.driver.management.update_firmware( + task, + [{'url': 'http://test1', + 'checksum': 'aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d'}, + {'url': 'http://test2', + 'checksum': '9f6227549221920e312fed2cfc6586ee832cc546'}]) + + mock_get_update_service.assert_called_once_with(task.node) + mock_update_service.simple_update.assert_called_once_with( + 'http://test1') + self.assertIsNotNone(task.node + .driver_internal_info['firmware_updates']) + self.assertEqual( + [{'task_monitor': '/task/123', 'url': 'http://test1', + 'checksum': 'aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d'}, + {'url': 'http://test2', + 'checksum': '9f6227549221920e312fed2cfc6586ee832cc546'}], + task.node.driver_internal_info['firmware_updates']) + self.assertIsNone( + task.node.driver_internal_info.get('firmware_cleanup')) + mock_set_async_step_flags.assert_called_once_with( + task.node, reboot=True, skip_current_step=True, polling=True) + mock_get_async_step_return_state.assert_called_once_with( + task.node) + mock_node_power_action.assert_called_once_with(task, states.REBOOT) + + @mock.patch.object(redfish_mgmt.RedfishManagement, '_stage_firmware_file', + autospec=True) + @mock.patch.object(deploy_utils, 'build_agent_options', + spec_set=True, autospec=True) + @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, 'prepare_ramdisk', + spec_set=True, autospec=True) + @mock.patch.object(manager_utils, 'node_power_action', autospec=True) + @mock.patch.object(deploy_utils, 'get_async_step_return_state', + autospec=True) + @mock.patch.object(deploy_utils, 'set_async_step_flags', autospec=True) + @mock.patch.object(redfish_utils, 'get_update_service', autospec=True) + def test_update_firmware_stage( + self, mock_get_update_service, mock_set_async_step_flags, + mock_get_async_step_return_state, mock_node_power_action, + mock_prepare, build_mock, mock_stage): + build_mock.return_value = {'a': 'b'} + mock_task_monitor = mock.Mock() + mock_task_monitor.task_monitor_uri = '/task/123' + mock_update_service = mock.Mock() + mock_update_service.simple_update.return_value = mock_task_monitor + mock_get_update_service.return_value = mock_update_service + mock_stage.return_value = ('http://staged/test1', 'http') + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + task.node.save = mock.Mock() + + task.driver.management.update_firmware( + task, + [{'url': 'http://test1', + 'checksum': 'aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d'}, + {'url': 'http://test2', + 'checksum': '9f6227549221920e312fed2cfc6586ee832cc546'}]) mock_get_update_service.assert_called_once_with(task.node) - mock_update_service.simple_update.assert_called_once_with('test1') + mock_update_service.simple_update.assert_called_once_with( + 'http://staged/test1') self.assertIsNotNone(task.node .driver_internal_info['firmware_updates']) self.assertEqual( - [{'task_monitor': '/task/123', 'url': 'test1'}, - {'url': 'test2'}], + [{'task_monitor': '/task/123', 'url': 'http://test1', + 'checksum': 'aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d'}, + {'url': 'http://test2', + 'checksum': '9f6227549221920e312fed2cfc6586ee832cc546'}], task.node.driver_internal_info['firmware_updates']) + self.assertIsNotNone( + task.node.driver_internal_info['firmware_cleanup']) + self.assertEqual( + ['http'], task.node.driver_internal_info['firmware_cleanup']) + mock_set_async_step_flags.assert_called_once_with( + task.node, reboot=True, skip_current_step=True, polling=True) + mock_get_async_step_return_state.assert_called_once_with( + task.node) + mock_node_power_action.assert_called_once_with(task, states.REBOOT) + + @mock.patch.object(redfish_mgmt.RedfishManagement, '_stage_firmware_file', + autospec=True) + @mock.patch.object(deploy_utils, 'build_agent_options', + spec_set=True, autospec=True) + @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, 'prepare_ramdisk', + spec_set=True, autospec=True) + @mock.patch.object(manager_utils, 'node_power_action', autospec=True) + @mock.patch.object(deploy_utils, 'get_async_step_return_state', + autospec=True) + @mock.patch.object(deploy_utils, 'set_async_step_flags', autospec=True) + @mock.patch.object(redfish_utils, 'get_update_service', autospec=True) + def test_update_firmware_stage_both( + self, mock_get_update_service, mock_set_async_step_flags, + mock_get_async_step_return_state, mock_node_power_action, + mock_prepare, build_mock, mock_stage): + build_mock.return_value = {'a': 'b'} + mock_task_monitor = mock.Mock() + mock_task_monitor.task_monitor_uri = '/task/123' + mock_update_service = mock.Mock() + mock_update_service.simple_update.return_value = mock_task_monitor + mock_get_update_service.return_value = mock_update_service + mock_stage.return_value = ('http://staged/test1', 'http') + info = self.node.driver_internal_info + info['firmware_cleanup'] = ['swift'] + self.node.driver_internal_info = info + self.node.save() + + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + task.node.save = mock.Mock() + + task.driver.management.update_firmware( + task, + [{'url': 'http://test1', + 'checksum': 'aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d'}, + {'url': 'http://test2', + 'checksum': '9f6227549221920e312fed2cfc6586ee832cc546'}]) + + mock_get_update_service.assert_called_once_with(task.node) + mock_update_service.simple_update.assert_called_once_with( + 'http://staged/test1') + self.assertIsNotNone(task.node + .driver_internal_info['firmware_updates']) + self.assertEqual( + [{'task_monitor': '/task/123', 'url': 'http://test1', + 'checksum': 'aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d'}, + {'url': 'http://test2', + 'checksum': '9f6227549221920e312fed2cfc6586ee832cc546'}], + task.node.driver_internal_info['firmware_updates']) + self.assertIsNotNone( + task.node.driver_internal_info['firmware_cleanup']) + self.assertEqual( + ['swift', 'http'], + task.node.driver_internal_info['firmware_cleanup']) mock_set_async_step_flags.assert_called_once_with( task.node, reboot=True, skip_current_step=True, polling=True) mock_get_async_step_return_state.assert_called_once_with( @@ -1218,9 +1343,10 @@ class RedfishManagementTestCase(db_base.DbTestCase): driver_internal_info = { 'something': 'else', 'firmware_updates': [ - {'task_monitor': '/task/123', 'url': 'test1'}, - {'url': 'test2'}]} + {'task_monitor': '/task/123', 'url': 'http://test1'}, + {'url': 'http://test2'}]} self.node.driver_internal_info = driver_internal_info + CONF.redfish.firmware_source = 'http' management = redfish_mgmt.RedfishManagement() with task_manager.acquire(self.context, self.node.uuid, @@ -1230,19 +1356,88 @@ class RedfishManagementTestCase(db_base.DbTestCase): management._continue_firmware_updates( task, mock_update_service, - [{'task_monitor': '/task/123', 'url': 'test1'}, - {'url': 'test2'}]) + [{'task_monitor': '/task/123', 'url': 'http://test1'}, + {'url': 'http://test2'}]) self.assertTrue(mock_log.called) - mock_update_service.simple_update.assert_called_once_with('test2') + mock_update_service.simple_update.assert_called_once_with( + 'http://test2') self.assertIsNotNone( task.node.driver_internal_info['firmware_updates']) self.assertEqual( - [{'url': 'test2', 'task_monitor': '/task/987'}], + [{'url': 'http://test2', 'task_monitor': '/task/987'}], task.node.driver_internal_info['firmware_updates']) task.node.save.assert_called_once_with() mock_node_power_action.assert_called_once_with(task, states.REBOOT) + @mock.patch.object(firmware_utils, 'download_to_temp', autospec=True) + @mock.patch.object(firmware_utils, 'verify_checksum', autospec=True) + @mock.patch.object(firmware_utils, 'stage', autospec=True) + def test__stage_firmware_file_https(self, mock_stage, mock_verify_checksum, + mock_download_to_temp): + CONF.redfish.firmware_source = 'local' + firmware_update = {'url': 'https://test1', 'checksum': 'abc'} + node = mock.Mock() + mock_download_to_temp.return_value = '/tmp/test1' + mock_stage.return_value = ('http://staged/test1', 'http') + + management = redfish_mgmt.RedfishManagement() + + staged_url, needs_cleanup = management._stage_firmware_file( + node, firmware_update) + + self.assertEqual(staged_url, 'http://staged/test1') + self.assertEqual(needs_cleanup, 'http') + mock_download_to_temp.assert_called_with(node, 'https://test1') + mock_verify_checksum.assert_called_with(node, 'abc', '/tmp/test1') + mock_stage.assert_called_with(node, 'local', '/tmp/test1') + + @mock.patch.object(firmware_utils, 'download_to_temp', autospec=True) + @mock.patch.object(firmware_utils, 'verify_checksum', autospec=True) + @mock.patch.object(firmware_utils, 'stage', autospec=True) + @mock.patch.object(firmware_utils, 'get_swift_temp_url', autospec=True) + def test__stage_firmware_file_swift( + self, mock_get_swift_temp_url, mock_stage, mock_verify_checksum, + mock_download_to_temp): + CONF.redfish.firmware_source = 'swift' + firmware_update = {'url': 'swift://container/bios.exe'} + node = mock.Mock() + mock_get_swift_temp_url.return_value = 'http://temp' + + management = redfish_mgmt.RedfishManagement() + + staged_url, needs_cleanup = management._stage_firmware_file( + node, firmware_update) + + self.assertEqual(staged_url, 'http://temp') + self.assertIsNone(needs_cleanup) + mock_download_to_temp.assert_not_called() + mock_verify_checksum.assert_not_called() + mock_stage.assert_not_called() + + @mock.patch.object(firmware_utils, 'cleanup', autospec=True) + @mock.patch.object(firmware_utils, 'download_to_temp', autospec=True) + @mock.patch.object(firmware_utils, 'verify_checksum', autospec=True) + @mock.patch.object(firmware_utils, 'stage', autospec=True) + def test__stage_firmware_file_error(self, mock_stage, mock_verify_checksum, + mock_download_to_temp, mock_cleanup): + node = mock.Mock() + firmware_update = {'url': 'https://test1'} + CONF.redfish.firmware_source = 'local' + firmware_update = {'url': 'https://test1'} + node = mock.Mock() + mock_download_to_temp.return_value = '/tmp/test1' + mock_stage.side_effect = exception.IronicException + + management = redfish_mgmt.RedfishManagement() + self.assertRaises(exception.IronicException, + management._stage_firmware_file, node, + firmware_update) + mock_download_to_temp.assert_called_with(node, 'https://test1') + mock_verify_checksum.assert_called_with(node, None, '/tmp/test1') + mock_stage.assert_called_with(node, 'local', '/tmp/test1') + mock_cleanup.assert_called_with(node) + @mock.patch.object(redfish_utils, 'get_system', autospec=True) def test_get_secure_boot_state(self, mock_get_system): fake_system = mock_get_system.return_value diff --git a/releasenotes/notes/add-more-sources-redfish-firmware-update-3da89f10dc0f8d21.yaml b/releasenotes/notes/add-more-sources-redfish-firmware-update-3da89f10dc0f8d21.yaml new file mode 100644 index 000000000..559ae2271 --- /dev/null +++ b/releasenotes/notes/add-more-sources-redfish-firmware-update-3da89f10dc0f8d21.yaml @@ -0,0 +1,14 @@ +--- +features: + - | + For ``redfish`` and ``idrac-redfish`` management interface + ``firmware_update`` clean step adds Swift, HTTP service and file system + support to serve and Ironic's HTTP and Swift service to stage files. Also + adds mandatory parameter ``checksum`` for file checksum verification. + +upgrade: + - | + For ``redfish`` and ``idrac-redfish`` management interface + ``firmware_update`` clean step there is now mandatory ``checksum`` + parameter necessary. Update existing clean steps to include it, otherwise + clean step will fail with error "'checksum' is a required property". diff --git a/releasenotes/notes/idrac-wsman-clean-steps-not-require-ramdisk-ca98aa5c0a88f727.yaml b/releasenotes/notes/idrac-wsman-clean-steps-not-require-ramdisk-ca98aa5c0a88f727.yaml new file mode 100644 index 000000000..fa478defe --- /dev/null +++ b/releasenotes/notes/idrac-wsman-clean-steps-not-require-ramdisk-ca98aa5c0a88f727.yaml @@ -0,0 +1,5 @@ +--- +features: + - | + Adds support for ``idrac-wsman`` RAID, BIOS and management clean steps to + be run without IPA when disabling ramdisk during cleaning. diff --git a/releasenotes/notes/known-issue-idrac-firmware-swift-721a19cac796e1ae.yaml b/releasenotes/notes/known-issue-idrac-firmware-swift-721a19cac796e1ae.yaml new file mode 100644 index 000000000..0b53e1e5c --- /dev/null +++ b/releasenotes/notes/known-issue-idrac-firmware-swift-721a19cac796e1ae.yaml @@ -0,0 +1,8 @@ +--- +issues: + - | + When using iDRAC with Swift to stage firmware update files in Management + interface ``firmware_update`` clean step of ``redfish`` or ``idrac`` + hardware type, the cleaning fails with error "An internal error occurred. + Unable to complete the specified operation." in iDRAC job. Until this is + fixed, use HTTP service to stage firmware files for iDRAC. diff --git a/releasenotes/notes/netboot-deprecation-fe5751a47df2d0b7.yaml b/releasenotes/notes/netboot-deprecation-fe5751a47df2d0b7.yaml new file mode 100644 index 000000000..9c8df1106 --- /dev/null +++ b/releasenotes/notes/netboot-deprecation-fe5751a47df2d0b7.yaml @@ -0,0 +1,14 @@ +--- +deprecations: + - | + Booting final instances via network (as opposed to via a local bootloader) + is now deprecated, except for the cases of booting from volume or the + ramdisk deploy interface. + + Network boot for whole disk images only works reliable for legacy (BIOS) + boot. In case of partition images, there is no way to update the kernel, + which makes this approach insecure. + + Users of partition images must ensure that they either contain the + ``grub-install`` binary, enough EFI artifacts to boot the operating + system or a legacy boot partition. diff --git a/releasenotes/notes/unix-socket-48e8f1caf4cb19f9.yaml b/releasenotes/notes/unix-socket-48e8f1caf4cb19f9.yaml new file mode 100644 index 000000000..14fefaf73 --- /dev/null +++ b/releasenotes/notes/unix-socket-48e8f1caf4cb19f9.yaml @@ -0,0 +1,5 @@ +--- +features: + - | + Supports listening on a Unix socket instead of a normal TCP socket. + This is useful with an HTTP server such as nginx in proxy mode. @@ -131,7 +131,7 @@ max-complexity=19 # [H203] Use assertIs(Not)None to check for None. # [H204] Use assert(Not)Equal to check for equality. # [H205] Use assert(Greater|Less)(Equal) for comparison. -# [H210] Require ‘autospec’, ‘spec’, or ‘spec_set’ in mock.patch/mock.patch.object calls +# [H210] Require 'autospec', 'spec', or 'spec_set' in mock.patch/mock.patch.object calls # [H904] Delay string interpolations at logging calls. enable-extensions=H106,H203,H204,H205,H210,H904 # [E402] Module level import not at top of file diff --git a/zuul.d/ironic-jobs.yaml b/zuul.d/ironic-jobs.yaml index a5a8f9b50..ff7727b7a 100644 --- a/zuul.d/ironic-jobs.yaml +++ b/zuul.d/ironic-jobs.yaml @@ -248,9 +248,6 @@ IRONIC_ENABLED_POWER_INTERFACES: redfish IRONIC_ENABLED_MANAGEMENT_INTERFACES: redfish IRONIC_AUTOMATED_CLEAN_ENABLED: False - # TODO(TheJulia): We need to excise netboot from - # jobs at some point. - IRONIC_DEFAULT_BOOT_OPTION: netboot IRONIC_ENABLED_BOOT_INTERFACES: redfish-virtual-media SWIFT_ENABLE_TEMPURLS: True SWIFT_TEMPURL_KEY: secretkey @@ -276,6 +273,7 @@ tempest_test_regex: test_baremetal_introspection devstack_localrc: IRONIC_BOOT_MODE: bios + IRONIC_DEFAULT_BOOT_OPTION: netboot IRONIC_INSPECTOR_MANAGED_BOOT: True IRONIC_INSPECTOR_NODE_NOT_FOUND_HOOK: '' IRONIC_AUTOMATED_CLEAN_ENABLED: False @@ -324,7 +322,7 @@ - job: name: ironic-tempest-wholedisk-bios-snmp-pxe - description: SNMP power, no-op management, netboot and whole disk images. + description: SNMP power, no-op management and whole disk images. parent: ironic-base vars: devstack_localrc: @@ -343,7 +341,6 @@ vars: devstack_localrc: IRONIC_AUTOMATED_CLEAN_ENABLED: False - IRONIC_DEFAULT_BOOT_OPTION: netboot - job: name: ironic-tempest-partition-bios-ipmi-pxe @@ -607,6 +604,7 @@ devstack_localrc: ENABLE_TENANT_TUNNELS: False ENABLE_TENANT_VLANS: True + FORCE_CONFIG_DRIVE: True HOST_TOPOLOGY: multinode HOST_TOPOLOGY_ROLE: subnode IRONIC_AUTOMATED_CLEAN_ENABLED: False @@ -676,7 +674,6 @@ IRONIC_IPXE_ENABLED: False IRONIC_RAMDISK_TYPE: tinyipa IRONIC_AUTOMATED_CLEAN_ENABLED: False - IRONIC_DEFAULT_BOOT_OPTION: netboot IRONIC_VM_SPECS_RAM: 4096 - job: |