From 87c35944498458ffb871f13842bcc08ffb23d704 Mon Sep 17 00:00:00 2001 From: Ciaran Concannon Date: Wed, 24 Jul 2024 02:23:29 -0700 Subject: [PATCH] Fix missing MTD error handling - add specific error Summary: Currently, upgrades that fail because of a blank /proc/mtd are being classed as `UNSAFE_TO_REBOOT` errors. This is because to classify as a `BAD_FLASH_CHIP` error, Flashy is grepping for the particular string *'Cannot access MTD device'* whereas the error on the BMC is returning: ``` root@rsw006-oob:~# cat /proc/mtd dev: size erasesize name root@rsw006-oob:~# fw_printenv bootargs Cannot open /dev/mtd1: No such file or directory ``` Let's fix this by skipping the string grepping logic completely, as we already know that we cannot see MTD at this stage Also added as separate error class, as missing MTD is a specific issue that we see quite frequently and good to classify it separately Test Plan: Unit tests ``` $ ./tools/flashy/scripts/run_unit_tests.sh ... ... ... 142/142 unit tests passed ``` --- Will handle exceptions in oobgrader and test in following diff Reviewed By: doranand Differential Revision: D60115096 fbshipit-source-id: 77e0cb6aa18fdb71e4c711d3951c7282aab23b6a --- .../common/04_ensure_flash_available.go | 14 +++++--------- .../common/04_ensure_flash_available_test.go | 17 +++-------------- tools/flashy/lib/step/error.go | 18 ++++++++++++++++++ 3 files changed, 26 insertions(+), 23 deletions(-) diff --git a/tools/flashy/checks_and_remediations/common/04_ensure_flash_available.go b/tools/flashy/checks_and_remediations/common/04_ensure_flash_available.go index a88acdc31ed4..b6c2a7046bf1 100644 --- a/tools/flashy/checks_and_remediations/common/04_ensure_flash_available.go +++ b/tools/flashy/checks_and_remediations/common/04_ensure_flash_available.go @@ -75,17 +75,13 @@ func ensureFlashAvailable(stepParams step.StepParams) step.StepExitError { } // Check for mtdparts aleady being set. - cmd = []string{"fw_printenv", "bootargs"} + cmd = []string{"ls", "/dev/mtd*"} _, err, stdout, stderr = utils.RunCommand(cmd, 30*time.Second) if err != nil { - if strings.Contains(stderr, "Cannot access MTD device") { - errMsg := errors.Errorf( - "U-Boot environment is inaccessible: broken flash chip?" + - " Error code: %v, stderr: %v", err, stderr) - return step.ExitBadFlashChip{Err: errMsg} - } - log.Printf("fw_printenv doesn't work: %v, stderr: %v", err, stderr) - return nil + errMsg := errors.Errorf( + "Broken flash chip? U-Boot environment is inaccessible." + + " Error code: %v, stderr: %v", err, stderr) + return step.ExitMissingMtd{Err: errMsg} } // Override mtdparts on next boot. diff --git a/tools/flashy/checks_and_remediations/common/04_ensure_flash_available_test.go b/tools/flashy/checks_and_remediations/common/04_ensure_flash_available_test.go index 163eb27cb87b..58afb2d18187 100644 --- a/tools/flashy/checks_and_remediations/common/04_ensure_flash_available_test.go +++ b/tools/flashy/checks_and_remediations/common/04_ensure_flash_available_test.go @@ -103,18 +103,7 @@ func TestEnsureFlashAvailable(t *testing.T) { want: step.ExitMustReboot{Err: errors.Errorf("Forcing reboot for new bootargs to take effect")}, }, { - name: "fw_printenv broken because reasons", - vbootUtilExists: false, - lfOpenBMC: false, - failGrep: false, - failPrint: true, - failSet: false, - printOutput: "", - grepOutput: "", - want: nil, - }, - { - name: "fw_printenv broken because missing flash", + name: "missing flash", vbootUtilExists: false, lfOpenBMC: false, failGrep: false, @@ -122,7 +111,7 @@ func TestEnsureFlashAvailable(t *testing.T) { failSet: false, printOutput: "Cannot access MTD device /dev/mtd1: No such file or directory", grepOutput: "", - want: step.ExitBadFlashChip{Err: errors.Errorf("U-Boot environment is inaccessible: broken flash chip? Error code: err1, stderr: Cannot access MTD device /dev/mtd1: No such file or directory")}, + want: step.ExitMissingMtd{Err: errors.Errorf("Broken flash chip? U-Boot environment is inaccessible. Error code: err1, stderr: Cannot access MTD device /dev/mtd1: No such file or directory")}, }, { name: "fw_setenv broken", @@ -149,7 +138,7 @@ func TestEnsureFlashAvailable(t *testing.T) { } else { return 0, nil, tc.grepOutput, "" } - } else if (cmdArr[0] == "fw_printenv") { + } else if (cmdArr[0] == "ls") { if (tc.failPrint) { return 1, errors.Errorf("err1"), "", tc.printOutput } else { diff --git a/tools/flashy/lib/step/error.go b/tools/flashy/lib/step/error.go index 9bc9391f5a0b..072135fff37f 100644 --- a/tools/flashy/lib/step/error.go +++ b/tools/flashy/lib/step/error.go @@ -36,6 +36,7 @@ const ( FLASHY_ERROR_SAFE_TO_REBOOT = 42 FLASHY_ERROR_UNSAFE_TO_REBOOT = 52 FLASHY_ERROR_BROKEN_SYSTEM = 53 + FLASHY_ERROR_MISSING_MTD = 54 ) type StepExitError interface { @@ -56,6 +57,10 @@ type ExitBadFlashChip struct { Err error } +type ExitMissingMtd struct { + Err error +} + type ExitMustReboot struct { Err error } @@ -100,6 +105,18 @@ func (e ExitBadFlashChip) GetType() string { return "ExitBadFlashChip" } +func (e ExitMissingMtd) GetError() string { + return e.Err.Error() +} + +func (e ExitMissingMtd) GetExitCode() int { + return FLASHY_ERROR_MISSING_MTD +} + +func (e ExitMissingMtd) GetType() string { + return "ExitMissingMtd" +} + func (e ExitMustReboot) GetError() string { return e.Err.Error() } @@ -157,6 +174,7 @@ func HandleStepError(err StepExitError) { case ExitUnsafeToReboot, ExitUnknownError, ExitBadFlashChip, + ExitMissingMtd, ExitMustReboot: encodeExitError(err)