[transceiver/cdb_firmware_upgrade] Add CDB firmware download and activation tests - #26869
Open
pnakka28 wants to merge 3 commits into
Open
[transceiver/cdb_firmware_upgrade] Add CDB firmware download and activation tests#26869pnakka28 wants to merge 3 commits into
pnakka28 wants to merge 3 commits into
Conversation
Signed-off-by: Pavan Kalyan Nakka <pnakka@microsoft.com>
Collaborator
|
/azp run |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds end-to-end CMIS CDB firmware lifecycle coverage to the transceiver test suite by introducing pre-checks plus dedicated download (TC3) and activation (TC4) test cases, along with shared helpers/fixtures to stage firmware and validate operational safety signals (links, dmesg I2C errors, etc.).
Changes:
- Add new CDB firmware test cases for pre-checks (TC1/TC2), firmware download (TC3), and firmware activation (TC4).
- Introduce shared firmware operation helpers (download/run/commit/reset) and dmesg-scoped error scanning.
- Update CDB firmware conftest to stage firmware once per session and restore modules to original baseline before/after the session.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/transceiver/common/scenario_ops.py | Adds reusable sfputil reset operation helper for scenarios. |
| tests/transceiver/common/dmesg_helpers.py | New helper to watermark and scan dmesg for operation-window errors using monotonic timestamps. |
| tests/transceiver/common/cli_parser_helper.py | Adds centralized constants for sfputil show fwversion field labels. |
| tests/transceiver/common/cli_helpers.py | Adds firmware CLI wrappers (download/run/commit/reset) and CDB abort helper; switches DOM polling toggle to command. |
| tests/transceiver/cdb_firmware_upgrade/utils/firmware_utils.py | Refactors required firmware metadata selection to use per-port configured firmware_versions; tracks staged DUT paths. |
| tests/transceiver/cdb_firmware_upgrade/test_firmware_download.py | New TC3 test that runs the per-port firmware download operation and aggregates failures. |
| tests/transceiver/cdb_firmware_upgrade/test_firmware_activation.py | New TC4 test that activates firmware (including single-bank flow) and aggregates failures. |
| tests/transceiver/cdb_firmware_upgrade/test_firmware_abort_versions.py | New TC1/TC2 baseline checks for gold firmware state and CDB abort support. |
| tests/transceiver/cdb_firmware_upgrade/firmware_operations.py | New implementation of per-port firmware download/activation/restore operations with link and dmesg checks. |
| tests/transceiver/cdb_firmware_upgrade/conftest.py | Session fixtures for qualifying ports, staging firmware, per-test DOM polling disable, session baseline restore, and cleanup. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (6)
tests/transceiver/cdb_firmware_upgrade/conftest.py:196
- These pre/post-session restores run outside the function-scoped
dom_polling_disabledfixture, so firmware download/activation andshow fwversionaccess the modules while DOM polling is enabled. That violates the stated CDB prerequisite and can interleave I2C transactions during the safety restore itself; disable DOM polling around each restore while preserving its prior state.
failures, ports = run_firmware_op_on_ports(
duthost, port_attributes_dict, cdb_firmware_qualifying_ports,
lport_to_pport, required_firmware_metadata_for_all_transceivers,
restore_module_to_original,
tests/transceiver/cdb_firmware_upgrade/firmware_operations.py:254
- The dmesg scan happens before the mandatory
sfputil resetbelow, so I2C errors caused by reset/recovery are outside the checked operation window even though reset is part of activation. Perform the final scan after reset and port recovery (and keep the watermark available across failure paths).
if not failures:
failures += _scan_i2c_errors(duthost, dmesg_start_uptime, "activation")
tests/transceiver/cdb_firmware_upgrade/firmware_operations.py:259
thermalctld_stoppedis cleared even when_start_thermalctldreports failure. Thefinallyblock then skips its restart attempt and can leave thermal management stopped after the test. Clear the flag only after a successful restart so teardown retries failures.
thermal_err = _start_thermalctld(duthost, thermalctld_stopped)
thermalctld_stopped = False
if thermal_err:
failures.append(f"failed to restart thermalctld: {thermal_err}")
tests/transceiver/cdb_firmware_upgrade/conftest.py:202
- The pre-session restore rewrites the firmware banks before
test_firmware_versionsruns, so TC1 cannot detect a module that did not initially start on its configured gold/inactive firmware—the condition this PR says TC1 validates. Capture and assert the initial bank state before modifying it, or defer restoration until after the pre-check.
_restore("Pre-session")
tests/transceiver/common/cli_helpers.py:450
NO_CDBis printed with exit code 0, so this helper returns(“NO_CDB”, None)and the download path treats an unavailable CDB API as a successful abort. Convert this sentinel into an error before proceeding with firmware download.
result = duthost.shell('python3 -c "{}"'.format(pycode), module_ignore_errors=True)
if result.get("rc", RC_FAILURE) != 0:
return None, f"CDB abort failed with rc={result.get('rc')} ({_error_detail(result)})"
return " ".join(result.get("stdout_lines") or []).strip(), None
tests/transceiver/cdb_firmware_upgrade/firmware_operations.py:196
- The dual-bank check only verifies the new active version and bank letter. It can pass even if the old active firmware was not moved to the inactive bank, so it does not actually verify the bank swap claimed by TC4. Also assert
after Inactive Firmware == before Active Firmware.
This issue also appears in the following locations of the same file:
- line 253
- line 256
if dual_bank_supported:
if after_banks.get(FW_ACTIVE) != before_banks.get(FW_INACTIVE):
failures.append("firmware activation failed")
if after_banks.get(FW_RUNNING_IMAGE) == before_banks.get(FW_RUNNING_IMAGE):
failures.append("Running Image did not change after activation")
Signed-off-by: Pavan Kalyan Nakka <pnakka@microsoft.com>
Collaborator
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Collaborator
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
pnakka28
added
the
Transceiver Test Dev 🔌
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of PR
Summary:
Adds the two CDB firmware upgrade test cases to the transceiver suite: TC3 downloads a firmware image to a CMIS active-optical module's inactive bank and verifies the download, and TC4 activates the inactive-bank firmware and verifies the running image changed. Both the test cases serve as individual as well as base tests for future firmware upgrade test case.
Fixes #
Type of change
Back port request
Tracking issue/work item for backport/cherry-pick request:
Failure type:
Approach
What is the motivation for this PR?
The pre-checks (TC1/TC2) confirm each module starts on its gold firmware and advertises CDB abort support. The next step is to exercise the CDB firmware lifecycle end-to-end. TC3 and TC4 validate that
sfputil firmware download,run, andcommitwork on real CMIS active-optical modules.Prerequisites
sfputil show fwversionread the module over CDB and if DOM polling accesses the same module concurrently, its I2C/CDB access can interleave and corrupt the reply.firmware_versionsmust be available.firmware_versions,gold_firmware_version, andinactive_firmware_versionmust be configured.How did you do it?
transceiver/cdb_firmware_upgrade/test_firmware_download.pyandtest_firmware_activation.py.firmware_operations.py- containing all firmware related operations including download, activation etc.conftest.py- session-scoped fixtures that stage firmware once, restore every qualifying module to its original banks before and after the session, and disable DOM polling per test.cli_helpers,scenario_ops,dmesg_helpers,cli_parser_helper.How did you verify/test it?
Any platform specific information?
Supported testbed topology if it's a new test case?
Physical testbeds with real CMIS active-optical transceivers. Ports not supporting CDB are skipped automatically.
Documentation
N/A
Test Logs
MSFT ADO - 39030975