diff --git a/confluent_client/bin/nodefirmware b/confluent_client/bin/nodefirmware index 32875a58..8f247989 100755 --- a/confluent_client/bin/nodefirmware +++ b/confluent_client/bin/nodefirmware @@ -56,11 +56,14 @@ components = ['all'] argparser = optparse.OptionParser( usage="Usage: " - "%prog [list][updatestatus][update [--backup ]]|[]") + "%prog [list][updatestatus][update [--backup] " + "[--parameterfile ] ]|[]") argparser.add_option('-b', '--backup', action='store_true', help='Target a backup bank rather than primary') argparser.add_option('-p', '--parameterfile', type='string', - help='When updating, use the specified parameter file') + help='When updating, use the specified parameter file; see ' + 'nodefirmware(8), some platforms need it to say what ' + 'kind of firmware the image holds') argparser.add_option('-m', '--maxnodes', type='int', help='When updating, prompt if more than the specified ' 'number of servers will be affected') diff --git a/confluent_client/doc/man/nodefirmware.ronn b/confluent_client/doc/man/nodefirmware.ronn index 0c3b9bb1..3fc3a018 100644 --- a/confluent_client/doc/man/nodefirmware.ronn +++ b/confluent_client/doc/man/nodefirmware.ronn @@ -3,7 +3,7 @@ nodefirmware(8) -- Report firmware information on confluent nodes ## SYNOPSIS -`nodefirmware [list][updatestatus][update [--backup ]]|[]` +`nodefirmware [list][updatestatus][update [--backup] [--parameterfile ] ]|[]` ## DESCRIPTION @@ -37,11 +37,31 @@ the out of band facilities. Firmware updates can end in one of three states: be affected * `-p PARAMETERFILE`, `--parameterfile=PARAMETERFILE`: - For updating, a parameter file to provide along with the update payload + For updating, a parameter file to provide along with the update payload. See + PARAMETER FILE below * `-h`, `--help`: Show help message and exit +## PARAMETER FILE + +The parameter file given with `-p` holds JSON that is sent alongside the image. +Its keys are the parts a Redfish multipart update accepts, so `UpdateParameters` +for the standard fields and `OemParameters` for whatever a vendor asks for on top +of them. Without one, an update names no targets, which means "whatever this +image is for", and that is what most platforms want. + +Some platforms will not take an image without being told what kind of firmware it +holds, and refuse the update rather than guess. On an AMI MegaRAC that is an +`ImageType` under `OemParameters`: + + {"OemParameters": {"ImageType": "BIOS"}} + +Attempting the update without it names the types the platform accepts in the +error, so there is no need to look them up first. The value is checked against +the same list before anything is uploaded, so a name the platform does not accept +is refused rather than sent. + ## EXAMPLES * Pull firmware from a node: diff --git a/confluent_server/aiohmi/redfish/oem/ami/megarac.py b/confluent_server/aiohmi/redfish/oem/ami/megarac.py index e0d84100..7c69fdc8 100644 --- a/confluent_server/aiohmi/redfish/oem/ami/megarac.py +++ b/confluent_server/aiohmi/redfish/oem/ami/megarac.py @@ -12,6 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +import os + import aiohmi.redfish.oem.generic as generic import aiohmi.util.webclient as webclient from urllib.parse import urlencode @@ -54,29 +56,88 @@ class OEMHandler(generic.OEMHandler): msgents.append(self.format_messages(msg)) return ';'.join(msgents) - async def update_firmware(self, filename, data=None, progress=None, bank=None, otherfields=()): + # What to keep across a firmware update, for the builds that let us say. + # SDR is excluded because a new firmware image is expected to bring its own. + _preserveconfig = { + 'Syslog': True, + 'NTP': True, + 'Network': True, + 'Authentication': True, + 'EXTLOG': True, + 'FRU': True, + 'IPMI': True, + 'KVM': True, + 'REDFISH': True, + 'SDR': False, + 'SEL': True, + 'SNMP': True, + 'SSH': True, + 'WEB': True, + } + + async def _preserve_configuration(self): + """Ask the bmc to keep its configuration across the update. + + Builds differ in which settings they are willing to preserve, and they + reject the whole request if it names one they do not know, so send only + the intersection with what this bmc advertises. + """ + usd = await self._do_web_request('/redfish/v1/UpdateService', cache=False) + advertised = usd.get('Oem', {}).get('AMIUpdateService', {}).get( + 'PreserveConfiguration', None) + if not advertised: + return + preserve = {k: v for k, v in self._preserveconfig.items() if k in advertised} + if not preserve: + return await self._do_web_request('/redfish/v1/UpdateService', { - "Oem": { - "AMIUpdateService": { - "@odata.type": "#AMIUpdateService.v1_0_0.AMIUpdateService", - "PreserveConfiguration": { - "Syslog": True, - "NTP": True, - "Network": True, - "Authentication": True, - "EXTLOG": True, - "FRU": True, - "IPMI": True, - "KVM": True, - "REDFISH": True, - "SDR": False, - "SEL": True, - "SNMP": True, - "SSH": True, - "WEB": True - } - }}}, method='PATCH', etag='*') - return await super().update_firmware(filename, data, progress, bank, otherfields) + 'Oem': { + 'AMIUpdateService': { + '@odata.type': '#AMIUpdateService.v1_0_0.AMIUpdateService', + 'PreserveConfiguration': preserve, + }}}, method='PATCH', etag='*') + + async def _allowed_imagetypes(self): + actinfo = await self._do_web_request( + '/redfish/v1/UpdateService/SimpleUpdateActionInfo') + for param in actinfo.get('Parameters', []): + if param.get('Name', None) == 'ImageType': + return param.get('AllowableValues', []) + return [] + + async def _checked_imagetype(self, imagetype, filename): + """Check the image type the parameter file gave against what is accepted. + + Flashing the wrong kind of image is not something to be clever about, so + the type is asked for rather than worked out from the file. + """ + allowed = await self._allowed_imagetypes() + choices = ', '.join(allowed) if allowed else 'BMC, BIOS' + if not imagetype: + raise pygexc.InvalidParameterValue( + 'This bmc needs to be told what kind of firmware "{0}" is, ' + 'one of: {1}'.format(os.path.basename(filename), choices)) + if allowed and imagetype not in allowed: + raise pygexc.InvalidParameterValue( + '"{0}" is not a firmware image type this bmc accepts, it ' + 'offers: {1}'.format(imagetype, choices)) + return imagetype + + async def update_firmware(self, filename, data=None, progress=None, bank=None, otherfields=()): + await self._preserve_configuration() + otherfields = dict(otherfields) if otherfields else {} + oemparams = dict(otherfields.get('OemParameters', None) or {}) + imagetype = await self._checked_imagetype( + oemparams.get('ImageType', None), filename) + oemparams['ImageType'] = imagetype + otherfields['OemParameters'] = oemparams + # Updating the bmc takes the bmc away mid flight, which is expected + # rather than a failure to watch the update + self._updateresetsbmc = imagetype == 'BMC' + try: + return await super().update_firmware(filename, data, progress, bank, otherfields) + finally: + self._updateresetsbmc = False async def get_wc(self): diff --git a/confluent_server/aiohmi/redfish/oem/generic.py b/confluent_server/aiohmi/redfish/oem/generic.py index a6c9ef37..9b737181 100644 --- a/confluent_server/aiohmi/redfish/oem/generic.py +++ b/confluent_server/aiohmi/redfish/oem/generic.py @@ -17,6 +17,7 @@ from fnmatch import fnmatch import json import os import re +import time import uuid import base64 @@ -1427,6 +1428,14 @@ class OEMHandler(object): async def update_firmware(self, filename, data=None, progress=None, bank=None, otherfields=()): # disable cache to make sure we trigger the token renewal logic if needed usd, upurl, ismultipart = await self.retrieve_firmware_upload_url() + if ismultipart: + # A multipart push has to carry an UpdateParameters part beside the + # image, and implementations are entitled to reject the request + # when it is missing. Targets empty means "whatever this image is + # for", and OperationApplyTime is left out because it is optional + # and some implementations refuse the ones they do not implement. + otherfields = dict(otherfields) if otherfields else {} + otherfields.setdefault('UpdateParameters', {'Targets': []}) try: uploadthread = await webclient.make_uploader( self.webclient, upurl, filename, data, formname='UpdateFile', formwrap=ismultipart, @@ -1465,6 +1474,20 @@ class OEMHandler(object): monitorurl = rsp['@odata.id'] return await self.monitor_update_progress(monitorurl, progress) + # How long to keep waiting for a bmc that is expected to restart while it + # applies an update to itself. Observed reset to serving again is a bit over + # three minutes, so this leaves generous room without hanging forever. + _resetgracetime = 300 + + async def _bmc_is_back(self): + """Check whether the redfish service is serving requests again.""" + try: + rsp, status = await self.webclient.grab_json_response_with_status( + '/redfish/v1/') + except Exception: + return False + return status == 200 and bool(rsp) + async def monitor_update_progress(self, monitorurl, progress): complete = False phase = "apply" @@ -1473,12 +1496,30 @@ class OEMHandler(object): # the validating phase; add a retry here so we don't exit the loop in this case retry = 3 pct = 0.0 + # Updating the bmc itself takes the bmc, and with it the task we are + # watching, away for minutes. That is the update working, not the + # monitoring failing, so wait it out and take the bmc coming back + # with the task gone as the update having landed. + deadline = None + if getattr(self, '_updateresetsbmc', False): + deadline = time.monotonic() + self._resetgracetime while not complete and retry > 0: try: pgress = await self._do_web_request(monitorurl, cache=False) - except socket.timeout: + except (socket.timeout, exc.PyghmiException, OSError): pgress = None if not pgress: + if deadline is not None: + if time.monotonic() > deadline: + raise Exception( + 'The bmc did not return within {0} seconds of ' + 'starting to apply its firmware'.format( + self._resetgracetime)) + await asyncio.sleep(10) + if await self._bmc_is_back(): + progress({'phase': phase, 'progress': 100.0}) + return 'pending' + continue retry -= 1 await asyncio.sleep(3) continue