[warm-reboot]: added pre-check for ISSU file #915

vadymhlushko-mlnx · 2020-05-14T11:43:08Z

Signed-off-by: Vadym Hlushko [email protected]

- What I did
Added pre-check for file /host/warmboot/issu_bank.txt in warm-reboot script.
Added instruction on how to recover issu_bank.txt.

- How I did it
Modify the 'fast-reboot' script

- How to verify it
Need to somehow corrupt /host/warmboot/issu_bank.txt (list below) and then run the warm-reboot command.

Remove issu_bank.txt
Clear issu_bank.txt
Change characters count in issu_bank.txt
Write invalid content into issu_bank.txt

- Previous command output (if the output of a command-line utility has changed)
root@arc-switch1041:~# warm-reboot
Warning: Stopping telemetry.service, but it can still be activated by:
telemetry.timer

- New command output (if the output of a command-line utility has changed)
root@arc-switch1041:~# warm-reboot
(/host/warmboot/issu_bank.txt) does NOT exist or empty ...
To recover (/host/warmboot/issu_bank.txt) file, do the following:
$ docker exec -it syncd sx_api_dbg_generate_dump.py
$ docker exec -it syncd cat /tmp/sdkdump | grep 'ISSU Bank'
Command above will print the VALUE of ISSU BANK - 0 or 1, use this VALUE in the next command
$ printf VALUE > /host/warmboot/issu_bank.txt

Signed-off-by: Vadym Hlushko <[email protected]>

msftclas · 2020-05-14T11:43:20Z

All CLA requirements met.

qiluo-msft · 2020-05-19T18:28:22Z

scripts/fast-reboot

+        exit "${MLNX_ISSU_BANK_BROKEN}"
+    fi
+
+    issu_file_chars_count=`wc -m ${ISSU_BANK_FILE} | awk '{print $1}'`;


wc -m ${ISSU_BANK_FILE} | awk '{print $1}' [](start = 27, length = 42)

A more simple solution

stat -c %s FILE ``` #Closed

Did you try? This comment is still applicable.

In reply to: 427512540 [](ancestors = 427512540)

Did you try? This comment is still applicable.

In reply to: 427512540 [](ancestors = 427512540)

Fixed

scripts/fast-reboot

qiluo-msft

As comments

Signed-off-by: Vadym Hlushko <[email protected]>

yxieca · 2020-05-27T21:32:26Z

scripts/fast-reboot

    request_pre_shutdown

    wait_for_pre_shutdown_complete_or_fail

+    if [[ "x$sonic_asic_type" == x"mellanox" ]]; then
+        check_issu_bank_file


Sorry for the last minute issue.

How can we distinguish if the error message came out from line 531 or 523? Are you able to passing in a differentiating token to check issu_bank_file and use that to distinguish if the error happens before or after pre-shutdown?

Sorry for the last minute issue.

How can we distinguish if the error message came out from line 531 or 523? Are you able to passing in a differentiating token to check issu_bank_file and use that to distinguish if the error happens before or after pre-shutdown?

Fixed

Sorry I didn't see this earlier, even before pre-shutdown, the swss has been stopped. Exiting here will leave the device eventually in failure mode requires human interaction.

I think both check should generate log event stating problem found and mark if the problem was found before or after pre-shutdown, but not exiting.

Is there a chance that there was a problem before pre-shutdown, but pre-shutdown fixed it?

The only safe place to check and exit is here: https://github.com/Azure/sonic-utilities/blob/master/scripts/fast-reboot#L255

… issu_bank.txt was corrupted Signed-off-by: Vadym Hlushko <[email protected]>

qiluo-msft · 2020-05-29T17:00:00Z

scripts/fast-reboot


    if [[ ! -s "$ISSU_BANK_FILE" ]]; then
        error "(${ISSU_BANK_FILE}) does NOT exist or empty ..."
        recover_issu_bank_file_instruction
-        exit "${MLNX_ISSU_BANK_BROKEN}"
+	if [[ "$1" = true ]]; then


Please don't mix tab and space. We prefer 4 spaces for each indentation level. #Closed

yxieca · 2020-05-30T17:48:27Z

scripts/fast-reboot

+	if [[ "$1" = true ]]; then
+            exit "${MLNX_ISSU_BANK_BROKEN_BEFORE_PRE_SHUTDOWN}"
+	else
+            exit "${MLNX_ISSU_BANK_BROKEN_AFTER_PRE_SHUTDOWN}"


Sorry I missed this issue in my second round of review.

Exit here will cause the fast/warm reboot to abort. I don't think this is what we want here. Because at the time when this check happens, we've already shutdown some critical services, e.g. BGP, RADV. Depending on if pre-shutdown is called, we could also lose syncd. It is unfortunate that there was an issue. But I think at this point, we would have to proceed with reboot and take the consequence. The logic is that aborting reboot sequence will require human intervention to recover. Continue rebooting will cause data plane disruption in the future. That sucks, but that would self heal. Right?

I think here the best we can do is to generate a log message so that we have log to look into after reboot is done and learn what happened when.

Sorry I missed this issue in my second round of review.

Exit here will cause the fast/warm reboot to abort. I don't think this is what we want here. Because at the time when this check happens, we've already shutdown some critical services, e.g. BGP, RADV. Depending on if pre-shutdown is called, we could also lose syncd. It is unfortunate that there was an issue. But I think at this point, we would have to proceed with reboot and take the consequence. The logic is that aborting reboot sequence will require human intervention to recover. Continue rebooting will cause data plane disruption in the future. That sucks, but that would self heal. Right?

I think here the best we can do is to generate a log message so that we have log to look into after reboot is done and learn what happened when.

Fixed

Signed-off-by: Vadym Hlushko <[email protected]>

qiluo-msft · 2020-06-01T18:06:21Z

scripts/fast-reboot

@@ -483,10 +512,18 @@ systemctl stop swss
 if [[ "$REBOOT_TYPE" = "warm-reboot" || "$REBOOT_TYPE" = "fastfast-reboot" ]]; then
    initialize_pre_shutdown

+    if [[ "x$sonic_asic_type" == x"mellanox" ]]; then
+        check_issu_bank_file


check_issu_bank_file [](start = 8, length = 20)

This check is before request_pre_shutdown. If you find anything wrong, you may still exit the script and control/data plane are still running.

qiluo-msft

Notice new issue

…re call request_pre_shutdown Signed-off-by: Vadym Hlushko <[email protected]>

qiluo-msft · 2020-06-03T15:50:36Z

Retest this please

* [warm-reboot]: added pre-check for ISSU file Signed-off-by: Vadym Hlushko <[email protected]> * [warm-reboot]: fixed problems according to PR review comments Signed-off-by: Vadym Hlushko <[email protected]> * [warm-reboot]: added distinguish token and error code to clarify when issu_bank.txt was corrupted Signed-off-by: Vadym Hlushko <[email protected]> * [warm-reboot]: fixed indentation problems Signed-off-by: Vadym Hlushko <[email protected]> * [warm-reboot]: removed script interrupt when issu_bank.txt is broken Signed-off-by: Vadym Hlushko <[email protected]> * [warm-reboot]: added script interrupt if issu_bank.txt is broken before call request_pre_shutdown Signed-off-by: Vadym Hlushko <[email protected]>

[201911][thermal control] Backport changes from master branch (sonic-net#929) [201911][config] Support abbreviation (sonic-net#933) Add 'hw-management-generate-dump.sh' to 'show techsupport' command (sonic-net#934) [fwutil]: Update fwutil to v2.0.0.0. (sonic-net#942) Fixes bug for PFCWD feature parameters (sonic-net#838) Fixed fast-reboot for BFN platform (sonic-net#871) [config] Add 'interface transceiver' subgroup with 'lpmode' and 'reset' subcommands (sonic-net#904) [warm-reboot]: added pre-check for ISSU file (sonic-net#915) [config] Don't attempt to restart disabled services (sonic-net#944)

[warm-reboot]: added pre-check for ISSU file

f8a9a52

Signed-off-by: Vadym Hlushko <[email protected]>

stepanblyschak previously approved these changes May 15, 2020

View reviewed changes

liat-grozovik requested a review from qiluo-msft May 19, 2020 11:21

qiluo-msft requested review from yxieca and pavel-shirshov May 19, 2020 18:20

qiluo-msft reviewed May 19, 2020

View reviewed changes

scripts/fast-reboot Show resolved Hide resolved

qiluo-msft reviewed May 19, 2020

View reviewed changes

[warm-reboot]: fixed problems according to PR review comments

04359cd

Signed-off-by: Vadym Hlushko <[email protected]>

vadymhlushko-mlnx dismissed stepanblyschak’s stale review via 04359cd May 27, 2020 10:54

qiluo-msft previously approved these changes May 27, 2020

View reviewed changes

yxieca reviewed May 27, 2020

View reviewed changes

[warm-reboot]: added distinguish token and error code to clarify when…

0b292a9

… issu_bank.txt was corrupted Signed-off-by: Vadym Hlushko <[email protected]>

vadymhlushko-mlnx dismissed qiluo-msft’s stale review via 0b292a9 May 29, 2020 14:07

qiluo-msft reviewed May 29, 2020

View reviewed changes

yxieca reviewed May 30, 2020

View reviewed changes

Vadym Hlushko added 2 commits June 1, 2020 14:12

[warm-reboot]: fixed indentation problems

70db2cf

Signed-off-by: Vadym Hlushko <[email protected]>

[warm-reboot]: removed script interrupt when issu_bank.txt is broken

f3521de

Signed-off-by: Vadym Hlushko <[email protected]>

qiluo-msft reviewed Jun 1, 2020

View reviewed changes

qiluo-msft requested changes Jun 1, 2020

View reviewed changes

[warm-reboot]: added script interrupt if issu_bank.txt is broken befo…

1b8029b

…re call request_pre_shutdown Signed-off-by: Vadym Hlushko <[email protected]>

vadymhlushko-mlnx requested a review from qiluo-msft June 3, 2020 08:51

qiluo-msft approved these changes Jun 3, 2020

View reviewed changes

liat-grozovik added Enhancement Platform: Mellanox Request for 201911 Branch labels Jun 9, 2020

stepanblyschak approved these changes Jun 9, 2020

View reviewed changes

yxieca approved these changes Jun 9, 2020

View reviewed changes

yxieca merged commit 1ddd3f2 into sonic-net:master Jun 9, 2020

abdosi added the Included in 201911 Branch label Jun 16, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[warm-reboot]: added pre-check for ISSU file #915

[warm-reboot]: added pre-check for ISSU file #915

vadymhlushko-mlnx commented May 14, 2020

msftclas commented May 14, 2020 •

edited

Loading

qiluo-msft May 19, 2020 •

edited

Loading

qiluo-msft May 26, 2020

vadymhlushko-mlnx May 27, 2020

qiluo-msft left a comment

yxieca May 27, 2020

vadymhlushko-mlnx May 29, 2020

yxieca Jun 3, 2020 •

edited

Loading

yxieca Jun 3, 2020

qiluo-msft May 29, 2020 •

edited

Loading

vadymhlushko-mlnx Jun 1, 2020

yxieca May 30, 2020

vadymhlushko-mlnx Jun 1, 2020

qiluo-msft Jun 1, 2020

vadymhlushko-mlnx Jun 3, 2020

qiluo-msft left a comment

qiluo-msft commented Jun 3, 2020

[warm-reboot]: added pre-check for ISSU file #915

[warm-reboot]: added pre-check for ISSU file #915

Conversation

vadymhlushko-mlnx commented May 14, 2020

msftclas commented May 14, 2020 • edited Loading

qiluo-msft May 19, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yxieca Jun 3, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft May 29, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft left a comment

Choose a reason for hiding this comment

qiluo-msft commented Jun 3, 2020

msftclas commented May 14, 2020 •

edited

Loading

qiluo-msft May 19, 2020 •

edited

Loading

yxieca Jun 3, 2020 •

edited

Loading

qiluo-msft May 29, 2020 •

edited

Loading