[fwutil] Add `fwutil update all` to support the automatic platform component fw updates by sujinmkang · Pull Request #1242 · sonic-net/sonic-utilities

sujinmkang · 2020-11-16T08:05:43Z

- What I did
Add the support for the automatic platform component fw updates
- How I did it
Add fwutil auto_update interfaces
- How to verify it
Work with dell to verify the auto_update interfaces with ssd firmware update.
- Previous command output (if the output of a command-line utility has changed)
- New command output (if the output of a command-line utility has changed)
New added command outputs are available in the following HLD:
sonic-net/SONiC#648

sujinmkang · 2020-11-16T08:13:22Z

@rkdevi27 and @padmanarayana please review this new PR.

lgtm-com · 2020-11-16T08:15:54Z

This pull request introduces 1 alert when merging 6d6633f into f46c27e - view on LGTM.com

new alerts:

1 for Unused import

lgtm-com · 2020-11-17T06:11:03Z

This pull request introduces 1 alert when merging 53ec476 into 1c45ca1 - view on LGTM.com

new alerts:

1 for Unused import

lgtm-com · 2020-11-23T08:17:09Z

This pull request introduces 2 alerts when merging f4574c9 into f9eb739 - view on LGTM.com

new alerts:

1 for Unused import
1 for Wrong number of arguments in a call

lgtm-com · 2020-11-24T08:15:05Z

This pull request introduces 2 alerts when merging 7efd058 into 9dc58ea - view on LGTM.com

new alerts:

1 for Unused import
1 for Wrong number of arguments in a call

nazariig · 2020-11-24T09:38:25Z

scripts/reboot

+    debug "Handling task file for boot type ${REBOOT_TYPE}”
+    ${DEVPATH}/${PLATFORM}/${PLATFORM_FWUTIL_AU_REBOOT_HANDLE} ${REBOOT_TYPE} || PLATFORM_FW_AU_RC=$?
+    if [[ $PLATFORM_FW_AU_RC -ne 0 ]]; then
+        error "Failed to handle the platform firmware auto-update for ${REBOOT_TYPE} Exit code: $PLATFORM_FW_AU_RC"


@sujinmkang please double check whether error is available in this context

lgtm-com · 2020-11-24T23:52:37Z

This pull request introduces 2 alerts when merging f29a5b4 into 5b8da56 - view on LGTM.com

new alerts:

1 for Unused import
1 for Wrong number of arguments in a call

lgtm-com · 2020-11-25T09:23:29Z

This pull request introduces 1 alert when merging 15cb89d into 5b8da56 - view on LGTM.com

new alerts:

1 for Unused import

lgtm-com · 2020-11-27T00:14:25Z

This pull request introduces 1 alert when merging a2ac11c into 8f3b22e - view on LGTM.com

new alerts:

1 for Unused import

sujinmkang · 2020-11-30T08:28:40Z

@rkdevi27 @nazariig @padmanarayana can you please help review this PR?

nazariig · 2020-11-30T11:14:46Z

scripts/reboot

+    debug "Handling task file for boot type ${REBOOT_TYPE}”
+    ${DEVPATH}/${PLATFORM}/${PLATFORM_FWUTIL_AU_REBOOT_HANDLE} ${REBOOT_TYPE} || PLATFORM_FW_AU_RC=$?
+    if [[ $PLATFORM_FW_AU_RC -ne 0 ]]; then
+        debug "Failed to handle the platform firmware auto-update for ${REBOOT_TYPE} Exit code: $PLATFORM_FW_AU_RC"


@sujinmkang debug "ERROR: Failed to handle the platform firmware auto-update for ${REBOOT_TYPE} Exit code: $PLATFORM_FW_AU_RC"

nazariig · 2020-11-30T11:17:06Z

scripts/fast-reboot

+    debug "Handling task file for boot type ${REBOOT_TYPE}”
+    ${DEVPATH}/${PLATFORM}/${PLATFORM_FWUTIL_AU_REBOOT_HANDLE} ${REBOOT_TYPE} || PLATFORM_FW_AU_RC=$?
+    if [[ $PLATFORM_FW_AU_RC -ne 0 ]]; then
+        debug "Failed to handle the platform firmware auto-update for ${REBOOT_TYPE} Exit code: $PLATFORM_FW_AU_RC"


@sujinmkang debug "ERROR: Failed to handle the platform firmware auto-update for ${REBOOT_TYPE} Exit code: $PLATFORM_FW_AU_RC"

lgtm-com · 2020-11-30T18:07:15Z

This pull request introduces 1 alert when merging 393fc18 into a5b78cf - view on LGTM.com

new alerts:

1 for Unused import

lgtm-com · 2021-04-22T19:07:49Z

This pull request introduces 1 alert when merging 68cdb1e into 59ed6f3 - view on LGTM.com

new alerts:

1 for Unused import

lgtm-com · 2021-04-27T18:16:12Z

This pull request introduces 1 alert when merging f46a3b4 into 9dba93f - view on LGTM.com

new alerts:

1 for Unused import

components upgrade.

lgtm-com · 2021-05-04T22:11:31Z

This pull request introduces 1 alert when merging 228e47f into 9492eab - view on LGTM.com

new alerts:

1 for Unused import

lgtm-com · 2021-05-18T21:24:22Z

This pull request introduces 1 alert when merging 4261570 into ad801bf - view on LGTM.com

new alerts:

1 for Unused import

… fwutil auto-update (#7721) Why I did it The Mellanox platform is required to support the fwutil auto-update feature defined here This is to allow switches, when performing SONiC upgrades to choose whether to perform firmware upgrades that may interrupt the data plane through a cold boot. How I did it Two methods were added to the component implementations for mellanox. In the base Component class we add a default function that chooses to skip the installation of any firmware unless the cold boot option is provided. This is because the Mellanox platform, by default, does not support installing firmware on ONIE, the CPLD, or the BIOS "on-the-fly". In the ComponentSSD class we add a function that behaves similarly but uses the Mellanox specific SSD firmware upgrade tool to check if the current SSD supports being upgraded on the fly in order to decide whether to skip or perform the installation. How to verify it Unit tests are included with this PR. These test will run on build of target sonic-mellanox.bin You may also perform fwutil auto-update ... commands after sonic-net/sonic-utilities#1242 is merged in.

lgtm-com · 2021-07-02T22:44:41Z

This pull request introduces 1 alert when merging da1e4ef into 4818360 - view on LGTM.com

new alerts:

1 for Unused import

… fwutil auto-update (sonic-net#7721) Why I did it The Mellanox platform is required to support the fwutil auto-update feature defined here This is to allow switches, when performing SONiC upgrades to choose whether to perform firmware upgrades that may interrupt the data plane through a cold boot. How I did it Two methods were added to the component implementations for mellanox. In the base Component class we add a default function that chooses to skip the installation of any firmware unless the cold boot option is provided. This is because the Mellanox platform, by default, does not support installing firmware on ONIE, the CPLD, or the BIOS "on-the-fly". In the ComponentSSD class we add a function that behaves similarly but uses the Mellanox specific SSD firmware upgrade tool to check if the current SSD supports being upgraded on the fly in order to decide whether to skip or perform the installation. How to verify it Unit tests are included with this PR. These test will run on build of target sonic-mellanox.bin You may also perform fwutil auto-update ... commands after sonic-net/sonic-utilities#1242 is merged in.

lgtm-com · 2021-08-22T22:55:37Z

This pull request introduces 1 alert when merging dd59bea into cd3ee78 - view on LGTM.com

new alerts:

1 for Unused import

ArunSaravananBalachandran · 2021-08-25T13:18:24Z

fwutil/main.py

        cli_abort(ctx, str(e))


+# 'updates' subcommand


Please change it to 'update'

fwutil/lib.py

…rectly.

lgtm-com · 2021-08-31T07:24:07Z

This pull request introduces 1 alert when merging 9c6436a into 720b650 - view on LGTM.com

new alerts:

1 for Unused local variable

fwutil/lib.py

ArunSaravananBalachandran · 2021-09-07T14:44:52Z

fwutil/lib.py

+        for comp_path, comp_au_status in data.items():
+            if not self.__is_dict(comp_au_status):


Since the value for comp_path is a list, please iterate over it seperately.

Suggested change

for comp_path, comp_au_status in data.items():

if not self.__is_dict(comp_au_status):

for comp_path, au_status in data.items():

for comp_au_status in au_status:

if not self.__is_dict(comp_au_status):

ArunSaravananBalachandran · 2021-09-07T14:46:08Z

fwutil/lib.py

+
+        comp_au_status['comp'] = component_path
+        comp_au_status['status'] = status
+        comp_au_status['info'] = info


Please add version details.

@ArunSaravananBalachandran I fixed only the parsing part based on the last HLD update. But the last HLD update is a little gap between the requirement for reboot_handler. I will set up a meeting sometime tomorrow to revise the HLD again.

auto update status should be reset after reboot

…vailable version information from get_available_firmware_version() api

ArunSaravananBalachandran

@sujinmkang, could you please add a check (like below) in reboot_pre_check in the reboot scripts to prevent the reboot if a firmware auto update was scheduled for a different reboot type.

fast-reboot

    # Make sure firmware auto update is not scheduled for a different reboot
    FIRMWARE_AU_STATUS_DIR="/tmp/firmwareupdate"
    FW_AU_TASK_FILE_REGEX="${FIRMWARE_AU_STATUS_DIR}/*_fw_au_task"
    case "$REBOOT_TYPE" in
        "fast-reboot")
            FW_AU_TASK_FILE_EXP="${FIRMWARE_AU_STATUS_DIR}/fast_fw_au_task"
            ;;
        "warm-reboot")
            FW_AU_TASK_FILE_EXP="${FIRMWARE_AU_STATUS_DIR}/warm_fw_au_task"
            ;;
    esac
    FW_AU_TASK_FILE=$(compgen -G ${FW_AU_TASK_FILE_REGEX}) || true
    if [[ -n "${FW_AU_TASK_FILE}" ]] && [[ ! -f "${FW_AU_TASK_FILE_EXP}" ]]; then
        error "Firmware auto update scheduled for a different reboot: ${FW_AU_TASK_FILE}"
        exit "${EXIT_FAILURE}"
    fi

reboot

    # Make sure firmware auto update is not scheduled for a different reboot
    FIRMWARE_AU_STATUS_DIR="/tmp/firmwareupdate"
    FW_AU_TASK_FILE_REGEX="${FIRMWARE_AU_STATUS_DIR}/*_fw_au_task"
    FW_AU_TASK_FILE_EXP="${FIRMWARE_AU_STATUS_DIR}/cold_fw_au_task"
    FW_AU_TASK_FILE=$(compgen -G ${FW_AU_TASK_FILE_REGEX}) || true
    if [[ -n "${FW_AU_TASK_FILE}" ]] && [[ ! -f "${FW_AU_TASK_FILE_EXP}" ]]; then
        VERBOSE=yes debug "Firmware auto update scheduled for a different reboot: ${FW_AU_TASK_FILE}"
        exit 1
    fi

ArunSaravananBalachandran · 2021-10-19T16:33:05Z

fwutil/lib.py

                elif len(value1) == 2 and self.VERSION_KEY not in value1:
                    missing_key = self.VERSION_KEY
                    break
+                elif len(value1) == 3 and self.UTILITY_KEY not in value1:
+                    missing_key = self.UTILITY_KEY
+                    break


As both VERSION_KEY and UTILITY_KEY are optional, please remove these checks since the above will break even if one of them is not present.

ArunSaravananBalachandran · 2021-10-19T16:48:35Z

fwutil/lib.py

-        if missing_key is not None:
+        if missing_key is not None and missing_key is not self.UTILITY_KEY and missing_key is not self.VERSION_KEY:


This change can be reverted, once the above checks have been removed.

…lict boot type of fw update

support

6d6633f

sujinmkang requested a review from nazariig November 16, 2020 08:06

sujinmkang mentioned this pull request Nov 16, 2020

Fwutil Auto-update support #1165

Closed

move get_au_status to ComponentStatusProvider

53ec476

add platform_fw_au_reboot_plugin

f4574c9

review comments

7efd058

nazariig reviewed Nov 24, 2020

View reviewed changes

review comments

f29a5b4

sujinmkang added 2 commits November 25, 2020 01:12

type_cast

853d276

type_cast2

15cb89d

review comments

a2ac11c

nazariig reviewed Nov 30, 2020

View reviewed changes

review comments

393fc18

:Merge branch 'master' into fwutil_rebase

68cdb1e

Changes based on hld

f46a3b4

update to support the combined platform with modular and non-modular

228e47f

components upgrade.

Review comments from dell

4261570

alexrallen mentioned this pull request May 26, 2021

[Mellanox] Implement auto_firmware_update platform API for to support fwutil auto-update sonic-net/sonic-buildimage#7721

Merged

Merge branch 'master' into fwutil_rebase

da1e4ef

review comments

dd59bea

fix lgtm

ef3ca37

ArunSaravananBalachandran reviewed Aug 25, 2021

View reviewed changes

fwutil/lib.py Outdated Show resolved Hide resolved

ArunSaravananBalachandran reviewed Aug 25, 2021

View reviewed changes

fwutil/lib.py Outdated Show resolved Hide resolved

Update the ComponentStatusProvider to parse the fw_au_status file cor…

9c6436a

…rectly.

fix lgtm

fb538e9

ArunSaravananBalachandran reviewed Sep 7, 2021

View reviewed changes

fwutil/lib.py Outdated Show resolved Hide resolved

ArunSaravananBalachandran reviewed Sep 7, 2021

View reviewed changes

sujinmkang added 3 commits October 5, 2021 08:32

Fix review feedback

0a666fe

Fix the after-reboot remaining auto update status issue

1029c54

auto update status should be reset after reboot

Ignore VERSION_KEY as a missing_key since some platform can get the a…

a496b82

…vailable version information from get_available_firmware_version() api

ArunSaravananBalachandran suggested changes Oct 19, 2021

View reviewed changes

Remove unnecessary missing key check and add reboot precheck for conf…

be7782a

…lict boot type of fw update

sujinmkang changed the title ~~[fwutil] Add fwutil auto_update to support the automatic platform component fw updates~~ [fwutil] Add fwutil update all to support the automatic platform component fw updates Oct 26, 2021

ArunSaravananBalachandran approved these changes Oct 26, 2021

View reviewed changes

sujinmkang requested review from prgeor and vdahiya12 October 26, 2021 18:09

vdahiya12 approved these changes Oct 27, 2021

View reviewed changes

sujinmkang merged commit 3a8ab73 into sonic-net:master Oct 27, 2021

		for comp_path, comp_au_status in data.items():
		if not self.__is_dict(comp_au_status):

		if missing_key is not None:
		if missing_key is not None and missing_key is not self.UTILITY_KEY and missing_key is not self.VERSION_KEY:

Conversation

sujinmkang commented Nov 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sujinmkang commented Nov 16, 2020

Uh oh!

lgtm-com bot commented Nov 16, 2020

Uh oh!

lgtm-com bot commented Nov 17, 2020

Uh oh!

lgtm-com bot commented Nov 23, 2020

Uh oh!

lgtm-com bot commented Nov 24, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lgtm-com bot commented Nov 24, 2020

Uh oh!

lgtm-com bot commented Nov 25, 2020

Uh oh!

lgtm-com bot commented Nov 27, 2020

Uh oh!

sujinmkang commented Nov 30, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lgtm-com bot commented Nov 30, 2020

Uh oh!

lgtm-com bot commented Apr 22, 2021

Uh oh!

lgtm-com bot commented Apr 27, 2021

Uh oh!

lgtm-com bot commented May 4, 2021

Uh oh!

lgtm-com bot commented May 18, 2021

Uh oh!

lgtm-com bot commented Jul 2, 2021

Uh oh!

lgtm-com bot commented Aug 22, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lgtm-com bot commented Aug 31, 2021

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ArunSaravananBalachandran left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sujinmkang commented Nov 16, 2020 •

edited

Loading