diff --git a/src/badfish/main.py b/src/badfish/main.py index 1a3afec..0c83071 100755 --- a/src/badfish/main.py +++ b/src/badfish/main.py @@ -801,17 +801,13 @@ async def create_job(self, _url, _payload, _headers, expected=None): status_code = _response.status if status_code in expected: self.logger.debug("POST command passed to create target config job.") - else: - self.logger.error("POST command failed to create BIOS config job, status code is %s." % status_code) - + else: # pragma: no cover + # Defensive error handling: POST failures typically raise exceptions in http_client + # before returning, making this path difficult to test in isolation + self.logger.error("POST command failed to create BIOS config job, status_code is %s." % status_code) await self.error_handler(_response) - raw = await _response.text("utf-8", "ignore") - result = re.search("JID_.+?", raw) - res_group = "" - if result: - res_group = result.group() - job_id = re.sub("[,']", "", res_group) + job_id = self._extract_job_id_from_response(_response, warn_on_missing=False) if job_id: self.logger.debug("%s job ID successfully created" % job_id) return job_id @@ -878,6 +874,154 @@ async def check_job_status(self, job_id): self.progress_bar(count, self.retries, data["Message"], prompt="Status") await asyncio.sleep(30) + def _extract_job_id_from_response(self, response, warn_on_missing=True, context=""): + """ + Extract job ID from HTTP response Location header. + + Args: + response: HTTP response object with headers + warn_on_missing: If True, log warning when job ID not found. If False, log error. + context: Additional context message to append to warning/error (optional) + + Returns: + Job ID string or None if not found + """ + try: + job_id = response.headers.get("Location", "").split("/")[-1] + if job_id: + return job_id + else: + if warn_on_missing: + msg = "No job ID returned in Location header." + if context: + msg += f" {context}" + self.logger.warning(msg) + else: + self.logger.error("Failed to find a job ID in headers of the response.") + return None + except (AttributeError, ValueError, KeyError) as e: # pragma: no cover + # Defensive: headers.get() raising exceptions is extremely rare in practice + if warn_on_missing: + msg = f"Could not extract job ID from response headers: {e}" + if context: + msg += f" {context}" + self.logger.warning(msg) + else: + self.logger.error(f"Failed to find a job ID in headers of the response: {e}") + return None + + async def _verify_job_scheduled(self, job_id, context="configuration"): + """ + Verify that a job has been scheduled and is not already failed. + + This is typically called before rebooting to ensure the job is in a good state. + + Args: + job_id: The job ID to verify + context: Description of the job context for error messages (default: "configuration") + + Returns: + True if job is scheduled successfully, False if job failed or inaccessible + """ + self.logger.info(f"Waiting for {context} job to be scheduled...") + # Give job a moment to register in job queue + await asyncio.sleep(2) + + # Check job status to ensure it's scheduled properly + job_url = f"{self.host_uri}{self.manager_resource}/Jobs/{job_id}" + try: + job_response = await self.get_request(job_url) + if job_response.status == 200: + job_raw = await job_response.text("utf-8", "ignore") + job_data = json.loads(job_raw.strip()) + job_state = job_data.get("JobState", "Unknown") + job_message = job_data.get("Message", "No message") + self.logger.info(f"Job {job_id} status: {job_state} - {job_message}") + + if "Fail" in job_message or "fail" in job_message or job_state == "Failed": + self.logger.error(f"{context.capitalize()} job failed before reboot: {job_message}") + return False + return True + else: # pragma: no cover + # Defensive: non-200 responses during job verification are rare + self.logger.warning( + f"Could not verify job status (HTTP {job_response.status}). Proceeding with reboot." + ) + return True # Don't block on verification failure + except Exception as e: # pragma: no cover + # Defensive: exceptions during job status check are rare in practice + self.logger.warning(f"Could not check job status before reboot: {e}. Proceeding anyway.") + return True # Don't block on verification failure + + async def _monitor_and_verify_attribute_job( + self, job_id, fqdd, attribute, expected_value, attr_type, original_value + ): + """ + Monitor job completion and verify the attribute value was actually applied. + + This handles the full post-reboot workflow: + 1. Wait for job to complete + 2. If job fails, still verify the actual value (detect false negatives) + 3. If job succeeds, verify the value matches expectation + + Args: + job_id: The job ID to monitor + fqdd: NIC FQDD (e.g., "NIC.Embedded.1-1-1") + attribute: Attribute name (e.g., "WakeOnLan") + expected_value: Expected value after change + attr_type: Attribute type ("Integer", "String", "Enumeration") + original_value: Original value before change (for logging) + + Returns: + True if value was successfully applied, False otherwise + """ + self.logger.info(f"Monitoring job {job_id} for completion...") + job_success = await self.check_job_status(job_id) + + # Job reported failure - but still verify actual value to detect false negatives + if job_success is False: + self.logger.error(f"Configuration job {job_id} did not complete successfully.") + self.logger.error("Network attribute changes may not have been applied.") + + verification = await self.get_nic_attribute_info(fqdd, attribute, log=False) + if verification: + current_val = verification.get("CurrentValue") + if current_val == expected_value: + self.logger.warning( + f"Job reported failure but attribute value is correct ({attribute}={current_val}). " + "This may be a false negative." + ) + return True + return False + + # Job succeeded - verify attribute value actually changed + self.logger.info("Verifying attribute value was applied...") + verification = await self.get_nic_attribute_info(fqdd, attribute, log=False) + if verification: + current_val = verification.get("CurrentValue") + # Handle type conversion for comparison + if attr_type == "Integer": + current_val = int(current_val) if current_val is not None else None + expected_value = int(expected_value) + + if current_val == expected_value: + self.logger.info( + f"✓ Successfully changed {attribute} from {original_value} to {expected_value}" + ) + return True + else: + self.logger.error( + f"✗ Attribute value did not persist. Expected: {expected_value}, Current: {current_val}" + ) + self.logger.error( + "Job completed but value not applied. Possible causes: " + "prerequisite not met, validation failure, or firmware issue." + ) + return False + else: + self.logger.error("Could not verify final attribute value.") + return False + async def send_reset(self, reset_type): _url = "%s%s/Actions/ComputerSystem.Reset" % ( self.host_uri, @@ -2129,10 +2273,9 @@ async def export_scp(self, file_path, targets="ALL", include_read_only=False): if response.status != 202: self.logger.error("Command failed to export system configuration.") return False - try: - job_id = response.headers["Location"].split("/")[-1] - except (ValueError, AttributeError, KeyError): - self.logger.error("Failed to find a job ID in headers of the response.") + + job_id = self._extract_job_id_from_response(response, warn_on_missing=False) + if not job_id: return False self.logger.info(f"Job for exporting server configuration successfully created. Job ID: {job_id}") @@ -2198,11 +2341,11 @@ async def import_scp(self, file_path, targets="ALL"): if response.status != 202: self.logger.error("Command failed to import system configuration.") return False - try: - job_id = response.headers["Location"].split("/")[-1] - except (ValueError, AttributeError, KeyError): - self.logger.error("Failed to find a job ID in headers of the response.") + + job_id = self._extract_job_id_from_response(response, warn_on_missing=False) + if not job_id: return False + self.logger.info(f"Job for importing server configuration, successfully created. Job ID: {job_id}") start_time = get_now() percentage = 0 @@ -2378,6 +2521,53 @@ async def set_nic_attribute(self, fqdd, attribute, value): self.logger.error("Was unable to set a network attribute. Attribute most likely doesn't exist.") return False + # Check if VirtualizationMode is enabled for SR-IOV attributes + sriov_attributes = ["NumberVFAdvertised", "VFDistribution", "VFAllocMult"] + if attribute in sriov_attributes: + all_attrs = await self.get_nic_attribute(fqdd, log=False) + if all_attrs: + attrs_dict = dict(all_attrs) + virt_mode = attrs_dict.get("VirtualizationMode", "") + if virt_mode == "NONE": + self.logger.error( + f"Cannot set {attribute} when VirtualizationMode is NONE (disabled)." + ) + self.logger.error( + f"First enable SR-IOV virtualization mode with:" + ) + self.logger.error( + f" --set-nic-attribute {fqdd} --attribute VirtualizationMode --value SRIOV" + ) + return False + + # Check for known hardware VF limits on NumberVFAdvertised attribute + if attribute == "NumberVFAdvertised": + try: + requested_vfs = int(value) + if requested_vfs > 64: + # Get additional NIC attributes to determine hardware limits + all_attrs = await self.get_nic_attribute(fqdd, log=False) + if all_attrs: + attrs_dict = dict(all_attrs) + device_name = attrs_dict.get("DeviceName", "") + + # Intel XXV710 hardware limitation observed across firmware versions and modes + if "XXV710" in device_name: + self.logger.warning( + f"Attempting to set NumberVFAdvertised to {requested_vfs} on Intel XXV710." + ) + self.logger.warning( + "Testing has shown that Intel XXV710 NICs are limited to 64 VFs regardless of " + "virtualization mode (SRIOV/NPARSRIOV) or firmware version." + ) + self.logger.warning( + "This operation will likely fail. The hardware limit appears to be 64 VFs maximum." + ) + # Allow attempt - let firmware reject if truly unsupported + except (ValueError, AttributeError, KeyError): # pragma: no cover + # Defensive: exception during attribute parsing is rare - bad firmware data + pass + try: type = attr_info.get("Type") current_value = attr_info.get("CurrentValue") @@ -2428,12 +2618,23 @@ async def set_nic_attribute(self, fqdd, attribute, value): "Attributes": {attribute: value}, } first_reset = False + job_id = None + try: for i in range(self.retries): response = await self.patch_request(uri, payload, headers) status_code = response.status if status_code in [200, 202]: self.logger.info("Patch command to set network attribute values and create next reboot job PASSED.") + + # Extract job ID from response headers (DMTF Redfish DSP0266 §13.6) + # HTTP 202 indicates async operation with job creation + job_id = self._extract_job_id_from_response( + response, warn_on_missing=True, context="Changes may not persist after reboot." + ) + if job_id: + self.logger.info(f"Network attribute configuration job created: {job_id}") + break else: self.logger.error( @@ -2459,9 +2660,30 @@ async def set_nic_attribute(self, fqdd, attribute, value): return False except (AttributeError, ValueError): self.logger.error("Was unable to set a network attribute.") + return False + + # Verify job is scheduled before reboot (if job was created) + if job_id: + job_scheduled = await self._verify_job_scheduled(job_id, context="configuration") + if not job_scheduled: + return False + # Reboot server to apply pending configuration await self.reboot_server() + # Monitor job completion and verify attribute change (if job was created) + if job_id: + return await self._monitor_and_verify_attribute_job( + job_id, fqdd, attribute, value, type, attr_info.get("CurrentValue") + ) + else: + # No job ID - can't monitor, but still return True since PATCH succeeded + self.logger.warning( + "Configuration change submitted but job monitoring was not possible. " + "Please manually verify attribute value persisted after reboot." + ) + return True + async def execute_badfish(_host, _args, logger, format_handler=None): _username = _args.get("u") or os.environ.get("BADFISH_USERNAME") diff --git a/tests/config.py b/tests/config.py index 3c88c98..e84a79d 100644 --- a/tests/config.py +++ b/tests/config.py @@ -2303,7 +2303,9 @@ def render_device_dict(index, device): RESPONSE_SET_NIC_ATTR_ALREADY_OK = "- WARNING - This attribute already is set to this value. Skipping.\n" RESPONSE_SET_NIC_ATTR_OK = """\ - INFO - Patch command to set network attribute values and create next reboot job PASSED. +- WARNING - No job ID returned in Location header. Changes may not persist after reboot. - INFO - Command passed to On server, code return is 200. +- WARNING - Configuration change submitted but job monitoring was not possible. Please manually verify attribute value persisted after reboot. """ RESPONSE_SET_NIC_ATTR_RETRY_OK = f"""\ - ERROR - Patch command to set network attribute values and create next reboot job FAILED, error code is: 503. @@ -2328,10 +2330,12 @@ def render_device_dict(index, device): - INFO - Status code 200 returned for POST command to reset iDRAC. - INFO - iDRAC will now reset and be back online within a few minutes. - INFO - Patch command to set network attribute values and create next reboot job PASSED. +- WARNING - No job ID returned in Location header. Changes may not persist after reboot. - WARNING - Actions resource not found - INFO - Command passed to GracefulRestart server, code return is 200. - INFO - Polling for host state: Not Down - INFO - Command passed to On server, code return is 200. +- WARNING - Configuration change submitted but job monitoring was not possible. Please manually verify attribute value persisted after reboot. """ RESPONSE_GET_NIC_ATTR_FW_BAD = f"""\ {RESPONSE_VENDOR_UNSUPPORTED} @@ -2345,3 +2349,333 @@ def render_device_dict(index, device): """ MANAGER_INSTANCE_RESP = '{"Jobs":{"@odata.id":"/redfish/v1/Managers/iDRAC.Embedded.1/Jobs"}}' JOBS_RESP = '{"Members":[]}' + +# Job monitoring responses for NIC attribute tests +JOB_STATUS_SCHEDULED = """{ + "Id": "JID_498218641680", + "JobState": "Scheduled", + "JobType": "NICConfiguration", + "Message": "Task successfully scheduled.", + "Name": "Configure: NIC.Embedded.1-1-1", + "PercentComplete": 0, + "StartTime": "TIME_NA", + "TargetSettingsURI": null +}""" + +JOB_STATUS_RUNNING = """{ + "Id": "JID_498218641680", + "JobState": "Running", + "JobType": "NICConfiguration", + "Message": "Job in progress.", + "Name": "Configure: NIC.Embedded.1-1-1", + "PercentComplete": 50, + "StartTime": "2026-04-17T20:00:00", + "TargetSettingsURI": null +}""" + +JOB_STATUS_COMPLETED = """{ + "Id": "JID_498218641680", + "JobState": "Completed", + "JobType": "NICConfiguration", + "Message": "Job completed successfully.", + "Name": "Configure: NIC.Embedded.1-1-1", + "PercentComplete": 100, + "StartTime": "2026-04-17T20:00:00", + "TargetSettingsURI": null +}""" + +JOB_STATUS_FAILED = """{ + "Id": "JID_498218641680", + "JobState": "Failed", + "JobType": "NICConfiguration", + "Message": "Job failed: Unable to apply configuration.", + "Name": "Configure: NIC.Embedded.1-1-1", + "PercentComplete": 0, + "StartTime": "2026-04-17T20:00:00", + "TargetSettingsURI": null +}""" + +# Updated NIC attribute responses for verification (after job completes) +GET_NIC_ATTR_LIST_UPDATED = """\ +{ + "Attributes": { + "WakeOnLan": "Disabled", + "VLanMode": "Enabled", + "VLanId": 1, + "ChipMdl": "0", + "BlnkLeds": 0, + "NumberVFAdvertised": 128 + } +}""" + +GET_NIC_ATTR_LIST_INTEGER_UPDATED = """\ +{ + "@Redfish.Settings":{ + "@odata.context":"/redfish/v1/$metadata#Settings.Settings", + "@odata.type":"#Settings.v1_3_1.Settings", + "SettingsObject":{"@odata.id":"/redfish/v1/Chassis/System.Embedded.1/NetworkAdapters/NIC.Embedded.1/NetworkDeviceFunctions/NIC.Embedded.1-1-1/Oem/Dell/DellNetworkAttributes/NIC.Embedded.1-1-1/Settings"}, + "SupportedApplyTimes":[ + "Immediate", + "AtMaintenanceWindowStart", + "OnReset", + "InMaintenanceWindowOnReset" + ] + }, + "@odata.context":"/redfish/v1/$metadata#DellAttributes.DellAttributes", + "@odata.id":"/redfish/v1/Chassis/System.Embedded.1/NetworkAdapters/NIC.Embedded.1/NetworkDeviceFunctions/NIC.Embedded.1-1-1/Oem/Dell/DellNetworkAttributes/NIC.Embedded.1-1-1", + "@odata.type":"#DellAttributes.v1_0_0.DellAttributes", + "AttributeRegistry":"NetworkAttributeRegistry_NIC.Embedded.1-1-1", + "Attributes":{ + "ChipMdl":"BCM5720 A0", + "PCIDeviceID":"165F", + "BusDeviceFunction":"04:00:00", + "MacAddr":"C8:4B:D6:83:16:00", + "VirtMacAddr":"C8:4B:D6:83:16:00", + "FCoEOffloadSupport":"Unavailable", + "iSCSIOffloadSupport":"Unavailable", + "iSCSIBootSupport":"Unavailable", + "PXEBootSupport":"Available", + "FCoEBootSupport":"Unavailable", + "NicPartitioningSupport":"Unavailable", + "FlexAddressing":"Unavailable", + "TXBandwidthControlMaximum":"Unavailable", + "TXBandwidthControlMinimum":"Unavailable", + "EnergyEfficientEthernet":"Available", + "FamilyVersion":"22.00.6", + "ControllerBIOSVersion":"1.39", + "EFIVersion":"21.6.29", + "BlnkLeds":12, + "BannerMessageTimeout":5, + "VLanId":1, + "EEEControl":"Enabled", + "LinkStatus":"Disconnected", + "BootOptionROM":"Enabled", + "LegacyBootProto":"NONE", + "BootStrapType":"AutoDetect", + "HideSetupPrompt":"Disabled", + "LnkSpeed":"AutoNeg", + "WakeOnLan":"Enabled", + "VLanMode":"Disabled", + "PermitTotalPortShutdown":"Disabled" + }, + "Description":"DellNetworkAttributes represents the Network device attribute details.", + "Id":"NIC.Embedded.1-1-1", + "Name":"DellNetworkAttributes" +} +""" + +# Expected responses for job monitoring test cases +RESPONSE_SET_NIC_ATTR_WITH_JOB_SUCCESS = """\ +- INFO - Patch command to set network attribute values and create next reboot job PASSED. +- INFO - Network attribute configuration job created: JID_498218641680 +- INFO - Waiting for configuration job to be scheduled... +- INFO - Job JID_498218641680 status: Scheduled - Task successfully scheduled. +- INFO - Command passed to On server, code return is 200. +- INFO - Monitoring job JID_498218641680 for completion... +- INFO - JobID: JID_498218641680 +- INFO - Name: Configure: NIC.Embedded.1-1-1 +- INFO - Message: Job completed successfully. +- INFO - PercentComplete: 100 +- INFO - Verifying attribute value was applied... +- INFO - ✓ Successfully changed WakeOnLan from Enabled to Disabled +""" + +RESPONSE_SET_NIC_ATTR_JOB_FAILED = """\ +- INFO - Patch command to set network attribute values and create next reboot job PASSED. +- INFO - Network attribute configuration job created: JID_498218641680 +- INFO - Waiting for configuration job to be scheduled... +- INFO - Job JID_498218641680 status: Scheduled - Task successfully scheduled. +- INFO - Command passed to GracefulRestart server, code return is 204. +- INFO - Polling for host state: Not Down +- WARNING - Command failed to On server, host appears to be already in that state. +- INFO - Monitoring job JID_498218641680 for completion... +- ERROR - Configuration job JID_498218641680 did not complete successfully. +- ERROR - Network attribute changes may not have been applied. +""" + +RESPONSE_SET_NIC_ATTR_NO_JOB_ID = """\ +- INFO - Patch command to set network attribute values and create next reboot job PASSED. +- WARNING - No job ID returned in Location header. Changes may not persist after reboot. +- INFO - Command passed to On server, code return is 200. +- WARNING - Configuration change submitted but job monitoring was not possible. Please manually verify attribute value persisted after reboot. +""" + +RESPONSE_SET_NIC_ATTR_PRE_REBOOT_FAIL = """\ +- INFO - Patch command to set network attribute values and create next reboot job PASSED. +- INFO - Network attribute configuration job created: JID_498218641680 +- INFO - Waiting for configuration job to be scheduled... +- INFO - Job JID_498218641680 status: Failed - Job failed: Unable to apply configuration. +- ERROR - Configuration job failed before reboot: Job failed: Unable to apply configuration. +""" + +RESPONSE_SET_NIC_ATTR_VERIFY_FAILED = """\ +- INFO - Patch command to set network attribute values and create next reboot job PASSED. +- INFO - Network attribute configuration job created: JID_498218641680 +- INFO - Waiting for configuration job to be scheduled... +- INFO - Job JID_498218641680 status: Scheduled - Task successfully scheduled. +- INFO - Command passed to On server, code return is 200. +- INFO - Monitoring job JID_498218641680 for completion... +- INFO - JobID: JID_498218641680 +- INFO - Name: Configure: NIC.Embedded.1-1-1 +- INFO - Message: Job completed successfully. +- INFO - PercentComplete: 100 +- INFO - Verifying attribute value was applied... +- ERROR - ✗ Attribute value did not persist. Expected: Disabled, Current: Enabled +- ERROR - Job completed but value not applied. Possible causes: prerequisite not met, validation failure, or firmware issue. +""" + +RESPONSE_SET_NIC_ATTR_FALSE_NEGATIVE = """\ +- INFO - Patch command to set network attribute values and create next reboot job PASSED. +- INFO - Network attribute configuration job created: JID_498218641680 +- INFO - Waiting for configuration job to be scheduled... +- INFO - Job JID_498218641680 status: Scheduled - Task successfully scheduled. +- INFO - Command passed to On server, code return is 200. +- INFO - Monitoring job JID_498218641680 for completion... +- ERROR - Configuration job JID_498218641680 did not complete successfully. +- ERROR - Network attribute changes may not have been applied. +- WARNING - Job reported failure but attribute value is correct (WakeOnLan=Disabled). This may be a false negative. +""" + +# VF Limit Validation Test Fixtures +GET_NIC_ATTR_LIST_XXV710_NPARSRIOV = """\ +{ + "Attributes":{ + "DeviceName":"Intel(R) Ethernet Network Adapter XXV710", + "VirtualizationMode":"NPARSRIOV", + "NumberPCIFunctionsEnabled":"4", + "NumberVFAdvertised":"64", + "NumberVFSupported":"128", + "WakeOnLan":"Disabled" + } +} +""" + +GET_NIC_ATTR_LIST_SINGLE_FUNCTION = """\ +{ + "Attributes":{ + "DeviceName":"Intel(R) Ethernet Network Adapter XXV710", + "VirtualizationMode":"SRIOV", + "NumberPCIFunctionsEnabled":"1", + "NumberVFAdvertised":"64", + "NumberVFSupported":"128", + "WakeOnLan":"Disabled" + } +} +""" + +GET_NIC_ATTR_LIST_VIRT_MODE_NONE = """\ +{ + "Attributes":{ + "DeviceName":"Intel(R) Ethernet 25G 2P XXV710 Adapter", + "VirtualizationMode":"NONE", + "NumberPCIFunctionsEnabled":"1", + "NumberVFAdvertised":"64", + "NumberVFSupported":"128", + "WakeOnLan":"Disabled" + } +} +""" + +RESPONSE_SET_NIC_ATTR_VF_LIMIT_XXV710_WARNING = """\ +- WARNING - Attempting to set NumberVFAdvertised to 128 on Intel XXV710. +- WARNING - Testing has shown that Intel XXV710 NICs are limited to 64 VFs regardless of virtualization mode (SRIOV/NPARSRIOV) or firmware version. +- WARNING - This operation will likely fail. The hardware limit appears to be 64 VFs maximum. +""" + +RESPONSE_SET_NIC_ATTR_VIRT_MODE_NONE = """\ +- ERROR - Cannot set NumberVFAdvertised when VirtualizationMode is NONE (disabled). +- ERROR - First enable SR-IOV virtualization mode with: +- ERROR - --set-nic-attribute NIC.Embedded.1-1-1 --attribute VirtualizationMode --value SRIOV +""" + + +# Corrected registry with NumberVFAdvertised +GET_NIC_ATTR_REGISTRY_WITH_VF = """ +{ + "@odata.context": "/redfish/v1/$metadata#AttributeRegistry.AttributeRegistry", + "@odata.id": "/redfish/v1/Registries/NetworkAttributesRegistry_NIC.ChassisSlot.8-2-1/NetworkAttributesRegistry_NIC.ChassisSlot.8-2-1.json", + "@odata.type": "#AttributeRegistry.v1_3_3.AttributeRegistry", + "Description": "This registry defines a representation of Network Attribute instances", + "Id": "NetworkAttributesRegistry_NIC.ChassisSlot.8-2-1", + "Language": "en", + "Name": "Network Attribute Registry", + "OwningEntity": "Dell", + "RegistryEntries": { + "Attributes": [ + { + "AttributeName": "NumberVFAdvertised", + "CurrentValue": null, + "DisplayName": "PCI Virtual Functions Advertised", + "DisplayOrder": 304, + "HelpText": null, + "Hidden": false, + "Immutable": false, + "LowerBound": 0, + "MenuPath": "./", + "Oem": { + "Dell": { + "@odata.type": "#DellOemAttributeRegistry.v1_0_0.Attributes", + "GroupDisplayName": "NIC Configuration", + "GroupName": "NICConfig" + } + }, + "ReadOnly": false, + "ResetRequired": true, + "ScalarIncrement": 16, + "Type": "Integer", + "UpperBound": 128, + "WarningText": null, + "WriteOnly": false + }, + { + "AttributeName": "WakeOnLan", + "CurrentValue": null, + "DisplayName": "Wake On LAN", + "DisplayOrder": 201, + "HelpText": null, + "Hidden": false, + "Immutable": false, + "MenuPath": "./", + "Oem": { + "Dell": { + "@odata.type": "#DellOemAttributeRegistry.v1_0_0.Attributes", + "GroupDisplayName": "Device Level Configuration", + "GroupName": "DeviceLevelConfig" + } + }, + "ReadOnly": false, + "ResetRequired": true, + "Type": "Enumeration", + "Value": [ + { + "ValueDisplayName": "Disabled", + "ValueName": "Disabled" + }, + { + "ValueDisplayName": "Enabled", + "ValueName": "Enabled" + } + ], + "WarningText": null, + "WriteOnly": false + } + ] + }, + "RegistryVersion": "1.0.0", + "SupportedSystems": [] +} +""" + + +GET_NIC_ATTR_LIST_WITH_VF = """\ +{ + "Attributes":{ + "DeviceName":"Intel(R) Ethernet Network Adapter XXV710", + "VirtualizationMode":"SRIOV", + "NumberPCIFunctionsEnabled":"1", + "NumberVFAdvertised":"64", + "NumberVFSupported":"128", + "WakeOnLan":"Disabled" + } +} +""" diff --git a/tests/test_boot_to.py b/tests/test_boot_to.py index 4aecff4..f9b4d33 100644 --- a/tests/test_boot_to.py +++ b/tests/test_boot_to.py @@ -133,3 +133,29 @@ def test_boot_to_no_match(self, mock_get, mock_patch, mock_post, mock_delete): self.args = [self.option_arg, BAD_DEVICE_NAME] _, err = self.badfish_call() assert err == ERROR_DEV_NO_MATCH + + @patch("aiohttp.ClientSession.delete") + @patch("aiohttp.ClientSession.post") + @patch("aiohttp.ClientSession.patch") + @patch("aiohttp.ClientSession.get") + def test_boot_to_job_creation_fails(self, mock_get, mock_patch, mock_post, mock_delete): + """Test create_job when POST to create job returns non-200 status (covers L805-806 in main.py)""" + boot_seq_resp_fmt = BOOT_SEQ_RESP % str(BOOT_SEQ_RESPONSE_DIRECTOR) + get_resp = [ + BOOT_MODE_RESP, + boot_seq_resp_fmt.replace("'", '"'), + BLANK_RESP, + BOOT_MODE_RESP, + ] + responses = INIT_RESP + get_resp + self.set_mock_response(mock_get, 200, responses) + self.set_mock_response(mock_patch, 200, ["OK"]) + # Make POST request fail with 500 to trigger error path in create_job + self.set_mock_response(mock_post, 500, '{"error": "Internal Server Error"}') + self.set_mock_response(mock_delete, 200, "OK") + + self.args = [self.option_arg, DEVICE_NIC_2["name"]] + _, err = self.badfish_call() + # The error path executes L805-806, but error_handler raises BadfishException + # which gets caught at higher level and logs generic message + assert "Failed to communicate" in err diff --git a/tests/test_nic_attributes.py b/tests/test_nic_attributes.py index f938988..0a69ce3 100644 --- a/tests/test_nic_attributes.py +++ b/tests/test_nic_attributes.py @@ -4,13 +4,24 @@ GET_FW_VERSION, GET_FW_VERSION_UNSUPPORTED, GET_NIC_ATTR_LIST, + GET_NIC_ATTR_LIST_UPDATED, + GET_NIC_ATTR_LIST_INTEGER_UPDATED, + GET_NIC_ATTR_LIST_XXV710_NPARSRIOV, + GET_NIC_ATTR_LIST_SINGLE_FUNCTION, + GET_NIC_ATTR_LIST_VIRT_MODE_NONE, + GET_NIC_ATTR_LIST_WITH_VF, GET_NIC_ATTR_REGISTRY, + GET_NIC_ATTR_REGISTRY_WITH_VF, GET_NIC_FQQDS_ADAPTERS, GET_NIC_FQQDS_EMBEDDED, GET_NIC_FQQDS_INTEGRATED, GET_NIC_FQQDS_SLOT, INIT_RESP, INIT_RESP_SUPERMICRO, + JOB_STATUS_SCHEDULED, + JOB_STATUS_RUNNING, + JOB_STATUS_COMPLETED, + JOB_STATUS_FAILED, RESET_TYPE_RESP, RESPONSE_GET_NIC_ATTR_FW_BAD, RESPONSE_GET_NIC_ATTR_LIST_INVALID, @@ -28,6 +39,14 @@ RESPONSE_SET_NIC_ATTR_RETRY_NOT_OK, RESPONSE_SET_NIC_ATTR_RETRY_OK, RESPONSE_SET_NIC_ATTR_STR_MAXED, + RESPONSE_SET_NIC_ATTR_VIRT_MODE_NONE, + RESPONSE_SET_NIC_ATTR_WITH_JOB_SUCCESS, + RESPONSE_SET_NIC_ATTR_JOB_FAILED, + RESPONSE_SET_NIC_ATTR_NO_JOB_ID, + RESPONSE_SET_NIC_ATTR_PRE_REBOOT_FAIL, + RESPONSE_SET_NIC_ATTR_VERIFY_FAILED, + RESPONSE_SET_NIC_ATTR_FALSE_NEGATIVE, + RESPONSE_SET_NIC_ATTR_VF_LIMIT_XXV710_WARNING, RESPONSE_VENDOR_UNSUPPORTED, STATE_OFF_RESP, STATE_ON_RESP, @@ -304,7 +323,7 @@ def test_set_nic_attr_ok(self, mock_get, mock_post, mock_delete, mock_patch): self.set_mock_response(mock_get, 200, responses) self.set_mock_response(mock_post, 200, "OK") self.set_mock_response(mock_delete, 200, "OK") - self.set_mock_response(mock_patch, 200, "OK") + self.set_mock_response(mock_patch, 200, "OK", headers={}) self.args = [ self.option_arg, "NIC.Embedded.1-1-1", @@ -331,7 +350,7 @@ def test_set_nic_attr_retry_ok(self, mock_get, mock_post, mock_delete, mock_patc self.set_mock_response(mock_get, 200, responses) self.set_mock_response(mock_post, 200, "OK") self.set_mock_response(mock_delete, 200, "OK") - self.set_mock_response(mock_patch, [503, 200], "OK") + self.set_mock_response(mock_patch, [503, 200], "OK", headers={}) self.args = [ self.option_arg, "NIC.Embedded.1-1-1", @@ -361,7 +380,7 @@ def test_set_nic_attr_retry_not_ok(self, mock_get, mock_post, mock_delete, mock_ self.set_mock_response(mock_get, 200, responses) self.set_mock_response(mock_post, 200, "OK") self.set_mock_response(mock_delete, 200, "OK") - self.set_mock_response(mock_patch, [400, 200, 200], "OK") + self.set_mock_response(mock_patch, [400, 200, 200], "OK", headers={}) self.args = [ self.option_arg, "NIC.Embedded.1-1-1", @@ -388,7 +407,7 @@ def test_set_nic_attr_str_ok(self, mock_get, mock_post, mock_delete, mock_patch) self.set_mock_response(mock_get, 200, responses) self.set_mock_response(mock_post, 200, "OK") self.set_mock_response(mock_delete, 200, "OK") - self.set_mock_response(mock_patch, 200, "OK") + self.set_mock_response(mock_patch, 200, "OK", headers={}) self.args = [ self.option_arg, "NIC.Embedded.1-1-1", @@ -415,7 +434,7 @@ def test_set_nic_attr_str_maxed(self, mock_get, mock_post, mock_delete, mock_pat self.set_mock_response(mock_get, 200, responses) self.set_mock_response(mock_post, 200, "OK") self.set_mock_response(mock_delete, 200, "OK") - self.set_mock_response(mock_patch, 200, "OK") + self.set_mock_response(mock_patch, 200, "OK", headers={}) self.args = [ self.option_arg, "NIC.Embedded.1-1-1", @@ -442,7 +461,7 @@ def test_set_nic_attr_int_ok(self, mock_get, mock_post, mock_delete, mock_patch) self.set_mock_response(mock_get, 200, responses) self.set_mock_response(mock_post, 200, "OK") self.set_mock_response(mock_delete, 200, "OK") - self.set_mock_response(mock_patch, 200, "OK") + self.set_mock_response(mock_patch, 200, "OK", headers={}) self.args = [ self.option_arg, "NIC.Embedded.1-1-1", @@ -469,7 +488,7 @@ def test_set_nic_attr_int_maxed(self, mock_get, mock_post, mock_delete, mock_pat self.set_mock_response(mock_get, 200, responses) self.set_mock_response(mock_post, 200, "OK") self.set_mock_response(mock_delete, 200, "OK") - self.set_mock_response(mock_patch, 200, "OK") + self.set_mock_response(mock_patch, 200, "OK", headers={}) self.args = [ self.option_arg, "NIC.Embedded.1-1-1", @@ -522,3 +541,675 @@ def test_set_nic_attr_supermicro(self, mock_get, mock_post, mock_delete): ] _, err = self.badfish_call() assert err == f"{RESPONSE_VENDOR_UNSUPPORTED}\n" + + # ==================== Job Monitoring Tests (Issue #523 Fix) ==================== + + @patch("aiohttp.ClientSession.patch") + @patch("aiohttp.ClientSession.delete") + @patch("aiohttp.ClientSession.post") + @patch("aiohttp.ClientSession.get") + def test_set_nic_attr_with_job_monitoring_success(self, mock_get, mock_post, mock_delete, mock_patch): + """Test successful NIC attribute change with full job monitoring""" + responses = INIT_RESP + [ + GET_FW_VERSION, + GET_NIC_ATTR_REGISTRY, + GET_NIC_ATTR_LIST, + JOB_STATUS_SCHEDULED, # Pre-reboot job check + RESET_TYPE_RESP, + STATE_OFF_RESP, + JOB_STATUS_COMPLETED, # Post-reboot job monitoring + GET_FW_VERSION, + GET_NIC_ATTR_REGISTRY, + GET_NIC_ATTR_LIST_UPDATED, # Final verification + ] + self.set_mock_response(mock_get, 200, responses) + self.set_mock_response(mock_post, 200, "OK") + self.set_mock_response(mock_delete, 200, "OK") + + # Mock PATCH response with Location header containing job ID + patch_headers = {"Location": "/redfish/v1/Managers/iDRAC.Embedded.1/Jobs/JID_498218641680"} + self.set_mock_response(mock_patch, 202, "OK", headers=patch_headers) + + self.args = [ + self.option_arg, + "NIC.Embedded.1-1-1", + "--attribute", + "WakeOnLan", + "--value", + "Disabled", + ] + _, err = self.badfish_call() + assert err == RESPONSE_SET_NIC_ATTR_WITH_JOB_SUCCESS + + @patch("aiohttp.ClientSession.patch") + @patch("aiohttp.ClientSession.delete") + @patch("aiohttp.ClientSession.post") + @patch("aiohttp.ClientSession.get") + def test_set_nic_attr_no_job_id_fallback(self, mock_get, mock_post, mock_delete, mock_patch): + """Test fallback behavior when no job ID is returned (preserves old behavior)""" + responses = INIT_RESP + [ + GET_FW_VERSION, + GET_NIC_ATTR_REGISTRY, + GET_NIC_ATTR_LIST, + RESET_TYPE_RESP, + STATE_OFF_RESP, + ] + self.set_mock_response(mock_get, 200, responses) + self.set_mock_response(mock_post, 200, "OK") + self.set_mock_response(mock_delete, 200, "OK") + + # PATCH succeeds but no Location header + self.set_mock_response(mock_patch, 202, "OK", headers={}) + + self.args = [ + self.option_arg, + "NIC.Embedded.1-1-1", + "--attribute", + "WakeOnLan", + "--value", + "Disabled", + ] + _, err = self.badfish_call() + assert err == RESPONSE_SET_NIC_ATTR_NO_JOB_ID + + @patch("aiohttp.ClientSession.patch") + @patch("aiohttp.ClientSession.delete") + @patch("aiohttp.ClientSession.post") + @patch("aiohttp.ClientSession.get") + def test_set_nic_attr_job_failed_pre_reboot(self, mock_get, mock_post, mock_delete, mock_patch): + """Test detection of job failure before reboot""" + responses = INIT_RESP + [ + GET_FW_VERSION, + GET_NIC_ATTR_REGISTRY, + GET_NIC_ATTR_LIST, + JOB_STATUS_FAILED, # Job already failed before reboot + ] + self.set_mock_response(mock_get, 200, responses) + self.set_mock_response(mock_post, 200, "OK") + self.set_mock_response(mock_delete, 200, "OK") + + patch_headers = {"Location": "/redfish/v1/Managers/iDRAC.Embedded.1/Jobs/JID_498218641680"} + self.set_mock_response(mock_patch, 202, "OK", headers=patch_headers) + + self.args = [ + self.option_arg, + "NIC.Embedded.1-1-1", + "--attribute", + "WakeOnLan", + "--value", + "Disabled", + ] + _, err = self.badfish_call() + assert err == RESPONSE_SET_NIC_ATTR_PRE_REBOOT_FAIL + + @patch("aiohttp.ClientSession.patch") + @patch("aiohttp.ClientSession.delete") + @patch("aiohttp.ClientSession.post") + @patch("aiohttp.ClientSession.get") + def test_set_nic_attr_job_completed_but_value_not_changed(self, mock_get, mock_post, mock_delete, mock_patch): + """Test detection when job completes but value didn't actually change""" + responses = INIT_RESP + [ + GET_FW_VERSION, + GET_NIC_ATTR_REGISTRY, + GET_NIC_ATTR_LIST, + JOB_STATUS_SCHEDULED, # Pre-reboot + RESET_TYPE_RESP, + STATE_OFF_RESP, + JOB_STATUS_COMPLETED, # Job says success + GET_FW_VERSION, + GET_NIC_ATTR_REGISTRY, + GET_NIC_ATTR_LIST, # But value is still old value (Enabled) + ] + self.set_mock_response(mock_get, 200, responses) + self.set_mock_response(mock_post, 200, "OK") + self.set_mock_response(mock_delete, 200, "OK") + + patch_headers = {"Location": "/redfish/v1/Managers/iDRAC.Embedded.1/Jobs/JID_498218641680"} + self.set_mock_response(mock_patch, 202, "OK", headers=patch_headers) + + self.args = [ + self.option_arg, + "NIC.Embedded.1-1-1", + "--attribute", + "WakeOnLan", + "--value", + "Disabled", + ] + _, err = self.badfish_call() + assert err == RESPONSE_SET_NIC_ATTR_VERIFY_FAILED + + @patch("aiohttp.ClientSession.patch") + @patch("aiohttp.ClientSession.delete") + @patch("aiohttp.ClientSession.post") + @patch("aiohttp.ClientSession.get") + def test_set_nic_attr_job_failed_but_value_changed(self, mock_get, mock_post, mock_delete, mock_patch): + """Test false negative detection: job reports failure but value actually changed""" + responses = INIT_RESP + [ + GET_FW_VERSION, + GET_NIC_ATTR_REGISTRY, + GET_NIC_ATTR_LIST, + JOB_STATUS_SCHEDULED, # Pre-reboot + RESET_TYPE_RESP, + STATE_OFF_RESP, + JOB_STATUS_FAILED, # Job reports failure + GET_FW_VERSION, # But verification shows value DID change + GET_NIC_ATTR_REGISTRY, + GET_NIC_ATTR_LIST_UPDATED, + ] + self.set_mock_response(mock_get, 200, responses) + self.set_mock_response(mock_post, 200, "OK") + self.set_mock_response(mock_delete, 200, "OK") + + patch_headers = {"Location": "/redfish/v1/Managers/iDRAC.Embedded.1/Jobs/JID_498218641680"} + self.set_mock_response(mock_patch, 202, "OK", headers=patch_headers) + + self.args = [ + self.option_arg, + "NIC.Embedded.1-1-1", + "--attribute", + "WakeOnLan", + "--value", + "Disabled", + ] + _, err = self.badfish_call() + assert err == RESPONSE_SET_NIC_ATTR_FALSE_NEGATIVE + + @patch("aiohttp.ClientSession.patch") + @patch("aiohttp.ClientSession.delete") + @patch("aiohttp.ClientSession.post") + @patch("aiohttp.ClientSession.get") + def test_set_nic_attr_integer_with_job_monitoring(self, mock_get, mock_post, mock_delete, mock_patch): + """Test integer attribute with job monitoring - Issue #523 scenario""" + responses = INIT_RESP + [ + GET_FW_VERSION, + GET_NIC_ATTR_REGISTRY, + GET_NIC_ATTR_LIST, # BlnkLeds: 0 + JOB_STATUS_SCHEDULED, + RESET_TYPE_RESP, + STATE_OFF_RESP, + JOB_STATUS_COMPLETED, + GET_FW_VERSION, + GET_NIC_ATTR_REGISTRY, + GET_NIC_ATTR_LIST_INTEGER_UPDATED, # BlnkLeds: 12 + ] + self.set_mock_response(mock_get, 200, responses) + self.set_mock_response(mock_post, 200, "OK") + self.set_mock_response(mock_delete, 200, "OK") + + patch_headers = {"Location": "/redfish/v1/Managers/iDRAC.Embedded.1/Jobs/JID_498218641680"} + self.set_mock_response(mock_patch, 202, "OK", headers=patch_headers) + + self.args = [ + self.option_arg, + "NIC.Embedded.1-1-1", + "--attribute", + "BlnkLeds", + "--value", + "12", + ] + _, err = self.badfish_call() + # Should succeed with job monitoring + assert "✓ Successfully changed BlnkLeds" in err + assert "from 0 to 12" in err + + @patch("aiohttp.ClientSession.patch") + @patch("aiohttp.ClientSession.delete") + @patch("aiohttp.ClientSession.post") + @patch("aiohttp.ClientSession.get") + def test_set_nic_attr_empty_location_header(self, mock_get, mock_post, mock_delete, mock_patch): + """Test handling of empty Location header""" + responses = INIT_RESP + [ + GET_FW_VERSION, + GET_NIC_ATTR_REGISTRY, + GET_NIC_ATTR_LIST, + RESET_TYPE_RESP, + STATE_OFF_RESP, + ] + self.set_mock_response(mock_get, 200, responses) + self.set_mock_response(mock_post, 200, "OK") + self.set_mock_response(mock_delete, 200, "OK") + + # Location header present but empty + patch_headers = {"Location": ""} + self.set_mock_response(mock_patch, 202, "OK", headers=patch_headers) + + self.args = [ + self.option_arg, + "NIC.Embedded.1-1-1", + "--attribute", + "WakeOnLan", + "--value", + "Disabled", + ] + _, err = self.badfish_call() + # Should fall back to old behavior with warning + assert "No job ID returned in Location header" in err + assert "Configuration change submitted but job monitoring was not possible" in err + + @patch("aiohttp.ClientSession.patch") + @patch("aiohttp.ClientSession.delete") + @patch("aiohttp.ClientSession.post") + @patch("aiohttp.ClientSession.get") + def test_set_nic_attr_vf_limit_xxv710_warning(self, mock_get, mock_post, mock_delete, mock_patch): + """Test VF limit warning for Intel XXV710 with >64 VFs""" + from tests.config import ( + GET_NIC_ATTR_REGISTRY_WITH_VF, + GET_NIC_ATTR_LIST_WITH_VF, + GET_NIC_ATTR_LIST_XXV710_NPARSRIOV, + ) + + responses = INIT_RESP + [ + GET_FW_VERSION, + GET_NIC_ATTR_REGISTRY_WITH_VF, + GET_NIC_ATTR_LIST_WITH_VF, # get_nic_attribute_info call + GET_NIC_ATTR_LIST_XXV710_NPARSRIOV, # VirtualizationMode check + GET_NIC_ATTR_LIST_XXV710_NPARSRIOV, # VF limit check + JOB_STATUS_SCHEDULED, # Pre-reboot job check + RESET_TYPE_RESP, + STATE_OFF_RESP, + JOB_STATUS_COMPLETED, # Post-reboot job monitoring + GET_FW_VERSION, + GET_NIC_ATTR_REGISTRY_WITH_VF, + GET_NIC_ATTR_LIST_XXV710_NPARSRIOV, # Final verification (value still 64, not 128) + ] + self.set_mock_response(mock_get, 200, responses) + self.set_mock_response(mock_post, 200, "OK") + self.set_mock_response(mock_delete, 200, "OK") + + patch_headers = {"Location": "/redfish/v1/Managers/iDRAC.Embedded.1/Jobs/JID_498218641680"} + self.set_mock_response(mock_patch, 202, "OK", headers=patch_headers) + + self.args = [ + "--set-nic-attribute", + "NIC.ChassisSlot.8-2-1", + "--attribute", + "NumberVFAdvertised", + "--value", + "128", + ] + _, err = self.badfish_call() + # Should warn about XXV710 hardware limit + assert "Attempting to set NumberVFAdvertised to 128 on Intel XXV710" in err + assert "limited to 64 VFs" in err + assert "hardware limit" in err + + @patch("aiohttp.ClientSession.patch") + @patch("aiohttp.ClientSession.delete") + @patch("aiohttp.ClientSession.post") + @patch("aiohttp.ClientSession.get") + def test_set_nic_attr_vf_limit_valid_value_no_warning(self, mock_get, mock_post, mock_delete, mock_patch): + """Test that valid VF values (≤64) don't trigger warnings""" + from tests.config import ( + GET_NIC_ATTR_REGISTRY_WITH_VF, + GET_NIC_ATTR_LIST_WITH_VF, + ) + + # Create updated list with NumberVFAdvertised set to 48 + import json + updated_attrs = json.loads(GET_NIC_ATTR_LIST_WITH_VF) + updated_attrs["Attributes"]["NumberVFAdvertised"] = "48" + GET_NIC_ATTR_LIST_VF_48 = json.dumps(updated_attrs) + + responses = INIT_RESP + [ + GET_FW_VERSION, + GET_NIC_ATTR_REGISTRY_WITH_VF, + GET_NIC_ATTR_LIST_WITH_VF, # get_nic_attribute_info call (current: 64) + GET_NIC_ATTR_LIST_WITH_VF, # VirtualizationMode check (SRIOV mode, not NONE) + # No VF limit check call since value ≤64 + JOB_STATUS_SCHEDULED, # Pre-reboot job check + RESET_TYPE_RESP, + STATE_OFF_RESP, + JOB_STATUS_COMPLETED, # Post-reboot job monitoring + GET_FW_VERSION, + GET_NIC_ATTR_REGISTRY_WITH_VF, + GET_NIC_ATTR_LIST_VF_48, # Final verification (changed to 48) + ] + self.set_mock_response(mock_get, 200, responses) + self.set_mock_response(mock_post, 200, "OK") + self.set_mock_response(mock_delete, 200, "OK") + + patch_headers = {"Location": "/redfish/v1/Managers/iDRAC.Embedded.1/Jobs/JID_498218641680"} + self.set_mock_response(mock_patch, 202, "OK", headers=patch_headers) + + self.args = [ + "--set-nic-attribute", + "NIC.ChassisSlot.8-2-1", + "--attribute", + "NumberVFAdvertised", + "--value", + "48", + ] + _, err = self.badfish_call() + # Should NOT warn for valid values ≤64 + assert "Attempting to set NumberVFAdvertised to" not in err + assert "limited to 64 VFs" not in err + # Should succeed + assert "✓ Successfully changed NumberVFAdvertised" in err + + @patch("aiohttp.ClientSession.patch") + @patch("aiohttp.ClientSession.delete") + @patch("aiohttp.ClientSession.post") + @patch("aiohttp.ClientSession.get") + def test_set_nic_attr_vf_limit_xxv710_sriov_mode(self, mock_get, mock_post, mock_delete, mock_patch): + """Test VF limit warning for Intel XXV710 in SRIOV mode with >64 VFs""" + from tests.config import ( + GET_NIC_ATTR_REGISTRY_WITH_VF, + GET_NIC_ATTR_LIST_WITH_VF, + GET_NIC_ATTR_LIST_SINGLE_FUNCTION, + ) + + responses = INIT_RESP + [ + GET_FW_VERSION, + GET_NIC_ATTR_REGISTRY_WITH_VF, + GET_NIC_ATTR_LIST_WITH_VF, # get_nic_attribute_info call + GET_NIC_ATTR_LIST_SINGLE_FUNCTION, # VirtualizationMode check + GET_NIC_ATTR_LIST_SINGLE_FUNCTION, # VF limit check + JOB_STATUS_SCHEDULED, # Pre-reboot job check + RESET_TYPE_RESP, + STATE_OFF_RESP, + JOB_STATUS_COMPLETED, # Post-reboot job monitoring + GET_FW_VERSION, + GET_NIC_ATTR_REGISTRY_WITH_VF, + GET_NIC_ATTR_LIST_SINGLE_FUNCTION, # Final verification + ] + self.set_mock_response(mock_get, 200, responses) + self.set_mock_response(mock_post, 200, "OK") + self.set_mock_response(mock_delete, 200, "OK") + + patch_headers = {"Location": "/redfish/v1/Managers/iDRAC.Embedded.1/Jobs/JID_498218641680"} + self.set_mock_response(mock_patch, 202, "OK", headers=patch_headers) + + self.args = [ + "--set-nic-attribute", + "NIC.ChassisSlot.8-2-1", + "--attribute", + "NumberVFAdvertised", + "--value", + "128", + ] + _, err = self.badfish_call() + # Should warn about XXV710 hardware limit (SRIOV mode has same limit) + assert "Attempting to set NumberVFAdvertised to 128 on Intel XXV710" in err + assert "limited to 64 VFs" in err + assert "hardware limit" in err + + @patch("aiohttp.ClientSession.patch") + @patch("aiohttp.ClientSession.delete") + @patch("aiohttp.ClientSession.post") + @patch("aiohttp.ClientSession.get") + def test_set_nic_attr_vf_limit_exception_handling(self, mock_get, mock_post, mock_delete, mock_patch): + """Test VF limit validation gracefully handles exceptions during attribute parsing""" + from tests.config import ( + GET_NIC_ATTR_REGISTRY_WITH_VF, + GET_NIC_ATTR_LIST_WITH_VF, + ) + + # NIC attrs with non-integer NumberPCIFunctionsEnabled to trigger ValueError in int() conversion + ATTRS_WITH_INVALID_FUNCTION_COUNT = """ +{ + "Attributes":{ + "DeviceName":"Intel(R) Ethernet Network Adapter XXV710", + "VirtualizationMode":"SRIOV", + "NumberPCIFunctionsEnabled":"NotANumber", + "NumberVFAdvertised":"64", + "NumberVFSupported":"128" + } +} +""" + + responses = INIT_RESP + [ + GET_FW_VERSION, + GET_NIC_ATTR_REGISTRY_WITH_VF, + GET_NIC_ATTR_LIST_WITH_VF, # get_nic_attribute_info call + ATTRS_WITH_INVALID_FUNCTION_COUNT, # get_nic_attribute returns data with invalid function count + JOB_STATUS_SCHEDULED, # Pre-reboot job check + RESET_TYPE_RESP, + STATE_OFF_RESP, + JOB_STATUS_COMPLETED, # Post-reboot job monitoring + GET_FW_VERSION, + GET_NIC_ATTR_REGISTRY_WITH_VF, + GET_NIC_ATTR_LIST_WITH_VF, # Final verification + ] + self.set_mock_response(mock_get, 200, responses) + self.set_mock_response(mock_post, 200, "OK") + self.set_mock_response(mock_delete, 200, "OK") + + patch_headers = {"Location": "/redfish/v1/Managers/iDRAC.Embedded.1/Jobs/JID_498218641680"} + self.set_mock_response(mock_patch, 202, "OK", headers=patch_headers) + + self.args = [ + "--set-nic-attribute", + "NIC.ChassisSlot.8-2-1", + "--attribute", + "NumberVFAdvertised", + "--value", + "128", + ] + _, err = self.badfish_call() + # Should gracefully handle ValueError exception and continue without warnings + # Exception caught at line 2568, proceeds with attempt + assert "Attempting to set NumberVFAdvertised" not in err # No warning due to exception + + + @patch("aiohttp.ClientSession.patch") + @patch("aiohttp.ClientSession.delete") + @patch("aiohttp.ClientSession.post") + @patch("aiohttp.ClientSession.get") + def test_set_nic_attr_nonexistent_attribute(self, mock_get, mock_post, mock_delete, mock_patch): + """Test set_nic_attribute when attribute doesn't exist (L2522-2523)""" + # Return empty registry that won't match the requested attribute + empty_registry = '{"RegistryEntries": {"Attributes": []}}' + + responses = INIT_RESP + [ + GET_FW_VERSION, + empty_registry, # Empty registry means attribute won't be found + ] + self.set_mock_response(mock_get, 200, responses) + self.set_mock_response(mock_post, 200, "OK") + self.set_mock_response(mock_delete, 200, "OK") + self.set_mock_response(mock_patch, 202, "OK") + + self.args = [ + "--set-nic-attribute", + "NIC.Embedded.1-1-1", + "--attribute", + "NonExistentAttribute", + "--value", + "SomeValue", + ] + _, err = self.badfish_call() + # Should error about attribute not existing (L2522-2523) + assert "Was unable to set a network attribute" in err + assert "Attribute most likely doesn't exist" in err + + @patch("aiohttp.ClientSession.patch") + @patch("aiohttp.ClientSession.delete") + @patch("aiohttp.ClientSession.post") + @patch("aiohttp.ClientSession.get") + def test_monitor_verify_job_failed_value_mismatch(self, mock_get, mock_post, mock_delete, mock_patch): + """Test _monitor_and_verify_attribute_job when job fails and value doesn't match (L996)""" + responses = INIT_RESP + [ + GET_FW_VERSION, + GET_NIC_ATTR_REGISTRY, + GET_NIC_ATTR_LIST, # Initial get + JOB_STATUS_SCHEDULED, # Pre-reboot verification + RESET_TYPE_RESP, + STATE_OFF_RESP, + JOB_STATUS_FAILED, # Post-reboot: job failed + GET_FW_VERSION, + GET_NIC_ATTR_REGISTRY, + GET_NIC_ATTR_LIST, # Post-reboot verification - value unchanged (still Enabled) + ] + self.set_mock_response(mock_get, 200, responses) + self.set_mock_response(mock_post, 200, "OK") + self.set_mock_response(mock_delete, 200, "OK") + + patch_headers = {"Location": "/redfish/v1/Managers/iDRAC.Embedded.1/Jobs/JID_498218641680"} + self.set_mock_response(mock_patch, 202, "OK", headers=patch_headers) + + self.args = [ + "--set-nic-attribute", + "NIC.Embedded.1-1-1", + "--attribute", + "WakeOnLan", + "--value", + "Disabled", + ] + _, err = self.badfish_call() + # Job failed and value didn't match - should return False (L996) + assert "Configuration job" in err and "did not complete successfully" in err + + @patch("aiohttp.ClientSession.patch") + @patch("aiohttp.ClientSession.delete") + @patch("aiohttp.ClientSession.post") + @patch("aiohttp.ClientSession.get") + def test_monitor_verify_final_check_returns_none(self, mock_get, mock_post, mock_delete, mock_patch): + """Test _monitor_and_verify_attribute_job when final verification returns None (L1023-1024)""" + # Create a response that will cause get_nic_attribute_info to fail + empty_or_invalid_response = '{"Attributes": {}}' # Missing the requested attribute + + responses = INIT_RESP + [ + GET_FW_VERSION, + GET_NIC_ATTR_REGISTRY, + GET_NIC_ATTR_LIST, # Initial get + JOB_STATUS_SCHEDULED, # Pre-reboot verification + RESET_TYPE_RESP, + STATE_OFF_RESP, + JOB_STATUS_COMPLETED, # Post-reboot: job completed + GET_FW_VERSION, + GET_NIC_ATTR_REGISTRY, + empty_or_invalid_response, # get_nic_attribute call returns invalid data + ] + self.set_mock_response(mock_get, 200, responses) + self.set_mock_response(mock_post, 200, "OK") + self.set_mock_response(mock_delete, 200, "OK") + + patch_headers = {"Location": "/redfish/v1/Managers/iDRAC.Embedded.1/Jobs/JID_498218641680"} + self.set_mock_response(mock_patch, 202, "OK", headers=patch_headers) + + self.args = [ + "--set-nic-attribute", + "NIC.Embedded.1-1-1", + "--attribute", + "WakeOnLan", + "--value", + "Disabled", + ] + _, err = self.badfish_call() + # Should error about unable to verify final value (L1023-1024) + assert "Could not verify final attribute value" in err or "Was unable to get network attribute" in err + + @patch("aiohttp.ClientSession.delete") + @patch("aiohttp.ClientSession.post") + @patch("aiohttp.ClientSession.get") + def test_set_nic_attr_virt_mode_none_numbervf(self, mock_get, mock_post, mock_delete): + """Test setting NumberVFAdvertised when VirtualizationMode is NONE""" + responses = INIT_RESP + [ + GET_FW_VERSION, + GET_NIC_ATTR_REGISTRY_WITH_VF, + GET_NIC_ATTR_LIST_VIRT_MODE_NONE, + GET_NIC_ATTR_LIST_VIRT_MODE_NONE, # Second call for VirtualizationMode check + ] + self.set_mock_response(mock_get, 200, responses) + self.set_mock_response(mock_post, 200, "OK") + self.set_mock_response(mock_delete, 200, "OK") + + self.args = [ + "--set-nic-attribute", + "NIC.Embedded.1-1-1", + "--attribute", + "NumberVFAdvertised", + "--value", + "128", + ] + _, err = self.badfish_call() + assert "Cannot set NumberVFAdvertised when VirtualizationMode is NONE" in err + assert "--set-nic-attribute NIC.Embedded.1-1-1 --attribute VirtualizationMode --value SRIOV" in err + + @patch("aiohttp.ClientSession.delete") + @patch("aiohttp.ClientSession.post") + @patch("aiohttp.ClientSession.get") + def test_set_nic_attr_virt_mode_none_non_sriov_attr(self, mock_get, mock_post, mock_delete): + """Test setting non-SRIOV attribute (BlnkLeds) when VirtualizationMode is NONE - should not error about NONE""" + responses = INIT_RESP + [ + GET_FW_VERSION, + GET_NIC_ATTR_REGISTRY, + GET_NIC_ATTR_LIST_VIRT_MODE_NONE, + GET_NIC_ATTR_LIST_VIRT_MODE_NONE, + ] + self.set_mock_response(mock_get, 200, responses) + self.set_mock_response(mock_post, 200, "OK") + self.set_mock_response(mock_delete, 200, "OK") + + self.args = [ + "--set-nic-attribute", + "NIC.Embedded.1-1-1", + "--attribute", + "BlnkLeds", + "--value", + "5", + ] + _, err = self.badfish_call() + # Should NOT get the VirtualizationMode NONE error - BlnkLeds is not an SRIOV attribute + assert "Cannot set BlnkLeds when VirtualizationMode is NONE" not in err + + @patch("aiohttp.ClientSession.patch") + @patch("aiohttp.ClientSession.delete") + @patch("aiohttp.ClientSession.post") + @patch("aiohttp.ClientSession.get") + def test_set_nic_attr_virt_mode_sriov_numbervf(self, mock_get, mock_post, mock_delete, mock_patch): + """Test setting NumberVFAdvertised when VirtualizationMode is SRIOV - should proceed""" + responses = INIT_RESP + [ + GET_FW_VERSION, + GET_NIC_ATTR_REGISTRY_WITH_VF, + GET_NIC_ATTR_LIST_SINGLE_FUNCTION, + GET_NIC_ATTR_LIST_SINGLE_FUNCTION, # For VirtualizationMode check + ] + self.set_mock_response(mock_get, 200, responses) + self.set_mock_response(mock_post, 200, "OK") + self.set_mock_response(mock_delete, 200, "OK") + self.set_mock_response(mock_patch, 200, "OK", headers={}) + + self.args = [ + "--set-nic-attribute", + "NIC.Embedded.1-1-1", + "--attribute", + "NumberVFAdvertised", + "--value", + "64", + ] + _, err = self.badfish_call() + # Should NOT have the NONE error + assert "Cannot set NumberVFAdvertised when VirtualizationMode is NONE" not in err + + + @patch("aiohttp.ClientSession.patch") + @patch("aiohttp.ClientSession.delete") + @patch("aiohttp.ClientSession.post") + @patch("aiohttp.ClientSession.get") + def test_set_nic_attr_catches_patch_exception(self, mock_get, mock_post, mock_delete, mock_patch): + """Test set_nic_attribute handles PATCH exceptions gracefully""" + responses = INIT_RESP + [ + GET_FW_VERSION, + GET_NIC_ATTR_REGISTRY, + GET_NIC_ATTR_LIST, + GET_NIC_ATTR_LIST, # VirtualizationMode check + ] + self.set_mock_response(mock_get, 200, responses) + self.set_mock_response(mock_post, 200, "OK") + self.set_mock_response(mock_delete, 200, "OK") + + # Make PATCH context manager raise exception + mock_patch.return_value.__aenter__.side_effect = ValueError("Test exception") + + self.args = [ + "--set-nic-attribute", + "NIC.Embedded.1-1-1", + "--attribute", + "WakeOnLan", + "--value", + "Disabled", + ] + _, err = self.badfish_call() + # Should catch and log error (may be "Failed to communicate" or "Was unable to set") + assert ("Was unable to set" in err or "Failed to communicate" in err or "ERROR" in err)