Skip to content

Commit

Permalink
Kill mfg-tool processes on yosemite4
Browse files Browse the repository at this point in the history
Summary: On yosemite4, kill all `mfg-tool` processes before attempting to upgrade and avoid issues due to stuck `mfg-tool` processes (eg. OOMs)

Test Plan:
Unit tests, build+push flashy to a device with stuck mfg-tool processes:

```
~/openbmc/tools/flashy$ scripts/run_flashy_remote.sh --device mtd:bmc --imagepath flash-yosemite4 --host sled325442309-oob.31.atn1.facebook.com --dry-run
```

Run new flashy step, ensure it successfully kills mfg-tool processes:
```
root@sled325442309-oob:~# /run/flashy/flashy -install && /run/flashy/checks_and_remediations/yosemite4/00_kill_mfg_tool_processes -imagepath /run/upgrade/image -device mtd:bmc
2024/10/18 05:17:58 Installing flashy...
2024/10/18 05:17:58 Finished installing flashy
2024/10/18 05:17:58 Starting: checks_and_remediations/yosemite4/00_kill_mfg_tool_processes
2024/10/18 05:17:59 Found 4 mfg-tool running processes
2024/10/18 05:17:59 Killing mfg-tool process with pid=1427
2024/10/18 05:17:59 Killing mfg-tool process with pid=1775
2024/10/18 05:17:59 Killing mfg-tool process with pid=31338
2024/10/18 05:17:59 Killing mfg-tool process with pid=530
2024/10/18 05:17:59 Finished: checks_and_remediations/yosemite4/00_kill_mfg_tool_processes
```

Ensure it runs successfully if no `mfg-tool` processes found:

```
root@sled325442309-oob:~# /run/flashy/flashy -install && /run/flashy/checks_and_remediations/yosemite4/00_kill_mfg_tool_processes -imagepath /run/upgrade/image -device mtd:bmc
2024/10/18 05:18:06 Installing flashy...
2024/10/18 05:18:06 Finished installing flashy
2024/10/18 05:18:06 Starting: checks_and_remediations/yosemite4/00_kill_mfg_tool_processes
2024/10/18 05:18:06 Found 0 mfg-tool running processes
2024/10/18 05:18:06 Finished: checks_and_remediations/yosemite4/00_kill_mfg_tool_processes
```

Reviewed By: lsiudut

Differential Revision: D64548615

fbshipit-source-id: 18b5a034c63773e938861918ec97b11da454cb5a
  • Loading branch information
kawmarco authored and facebook-github-bot committed Oct 18, 2024
1 parent 7d6b915 commit ce71547
Show file tree
Hide file tree
Showing 5 changed files with 226 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/**
* Copyright 2020-present Facebook. All Rights Reserved.
*
* This program file is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License as published by the
* Free Software Foundation; version 2 of the License.
*
* This program is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program in a file named COPYING; if not, write to the
* Free Software Foundation, Inc.,
* 51 Franklin Street, Fifth Floor,
* Boston, MA 02110-1301 USA
*/

package remediations_yosemite4

import (
"github.com/facebook/openbmc/tools/flashy/lib/step"
"github.com/facebook/openbmc/tools/flashy/lib/utils"
"log"
"os"
"regexp"
"syscall"
)

func init() {
step.RegisterStep(killMfgToolProcesses)
}

func killMfgToolProcesses(stepParams step.StepParams) step.StepExitError {
// Find all running mfg-tool processes
mfgToolRegex := regexp.MustCompile(`(^|/)mfg-tool(\x00|$)`)
mfgToolProcs, err := utils.ListProcessesMatchingRegex(mfgToolRegex)
if err != nil {
return step.ExitSafeToReboot{Err: err}
}
log.Printf("Found %d mfg-tool running processes", len(mfgToolProcs))

// S458061: If there's more than one running mfg-tool process, kill them all
// to prevent stuck mfg-tool instances causing OOM problems during flashing
if len(mfgToolProcs) > 1 {
for _, proc := range mfgToolProcs {
log.Printf("Killing mfg-tool process with pid=%d", proc.Pid)
err := kill(proc)
if err != nil {
log.Printf("Failed to kill mfg-tool process with pid=%d: %v", proc.Pid, err)
}
}
}

return nil
}

var kill = func(proc *os.Process) error {
// Just a simple wrapper so it's easier to mock in tests
return proc.Signal(syscall.SIGKILL)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
/**
* Copyright 2020-present Facebook. All Rights Reserved.
*
* This program file is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License as published by the
* Free Software Foundation; version 2 of the License.
*
* This program is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program in a file named COPYING; if not, write to the
* Free Software Foundation, Inc.,
* 51 Franklin Street, Fifth Floor,
* Boston, MA 02110-1301 USA
*/

package remediations_yosemite4

import (
"github.com/facebook/openbmc/tools/flashy/lib/step"
"github.com/facebook/openbmc/tools/flashy/lib/utils"
"log"
"testing"
"bytes"
"os"
"regexp"
"strings"
)

func TestKillMfgToolProcesses(t *testing.T) {
// save log output into buf for testing
var logBuffer bytes.Buffer
log.SetOutput(&logBuffer)

// mock cleanup
realListProcessesMatchingRegex := utils.ListProcessesMatchingRegex
realKill := kill

defer func() {
log.SetOutput(os.Stderr)
kill = realKill
utils.ListProcessesMatchingRegex = realListProcessesMatchingRegex
}()

t.Run("Must not kill mfg-tool if there's a single process", func(t *testing.T) {
logBuffer.Reset()

// Simulate a single mfg-tool process
utils.ListProcessesMatchingRegex = func(regex *regexp.Regexp) ([]*os.Process, error) {
return []*os.Process{&os.Process{Pid: 1234}}, nil
}

// Mock kill
killCalled := false
kill = func (proc *os.Process) error {
killCalled = true
return nil;
}

res := killMfgToolProcesses(step.StepParams{})

if res != nil {
t.Errorf("Expected killMfgToolProcesses() to return nil, got %v", res)
}

if !strings.Contains(logBuffer.String(), "Found 1 mfg-tool running processes") {
t.Errorf("Expected 'Found 1 mfg-tool running processes' log message, got %v", logBuffer.String())
}

if killCalled {
t.Errorf("Expected killMfgToolProcesses() to not call kill() if there's a single mfg-tool process")
}
})

t.Run("Must kill all mfg-tool if there are multiple mfg-tool processes", func(t *testing.T) {
logBuffer.Reset()

// Simulate multiple mfg-tool processes
utils.ListProcessesMatchingRegex = func(regex *regexp.Regexp) ([]*os.Process, error) {
return []*os.Process{&os.Process{Pid: 1234}, &os.Process{Pid: 4567}}, nil
}

// Mock kill
killCalled := false
kill = func (proc *os.Process) error {
killCalled = true
return nil;
}

res := killMfgToolProcesses(step.StepParams{})

if res != nil {
t.Errorf("Expected killMfgToolProcesses() to return nil, got %v", res)
}

if !strings.Contains(logBuffer.String(), "Found 2 mfg-tool running processes") {
t.Errorf("Expected 'Found 2 mfg-tool running processes' log message, got %v", logBuffer.String())
}

// Ensure all mfg-tool processes were killed
if !strings.Contains(logBuffer.String(), "Killing mfg-tool process with pid=1234") || !strings.Contains(logBuffer.String(), "Killing mfg-tool process with pid=4567") {
t.Errorf("Expected logs showing all mfg-tool processes being killed got %v", logBuffer.String())
}

if !killCalled {
t.Errorf("Expected killMfgToolProcesses() to call kill() on multiple mfg-tool processes")
}
})


}
1 change: 1 addition & 0 deletions tools/flashy/install/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
_ "github.com/facebook/openbmc/tools/flashy/checks_and_remediations/wedge100"
_ "github.com/facebook/openbmc/tools/flashy/checks_and_remediations/yamp"
_ "github.com/facebook/openbmc/tools/flashy/checks_and_remediations/grandteton"
_ "github.com/facebook/openbmc/tools/flashy/checks_and_remediations/yosemite4"
_ "github.com/facebook/openbmc/tools/flashy/flash_procedure"
_ "github.com/facebook/openbmc/tools/flashy/utilities"
)
Expand Down
25 changes: 25 additions & 0 deletions tools/flashy/lib/utils/system.go
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,31 @@ var getOtherProcCmdlinePaths = func() []string {
return otherCmdlines
}

var ListProcessesMatchingRegex = func(regex *regexp.Regexp) ([]*os.Process, error) {
// Get all pids by listing /proc/[0-9]*/
allProcCmdlines, _ := fileutils.Glob("/proc/[0-9]*/cmdline")

procs := []*os.Process{}
pidRegex := regexp.MustCompile(`/proc/([0-9]+)/`)

for _, cmdlinePath := range allProcCmdlines {
buf, err := fileutils.ReadFile(cmdlinePath)

// Check if regex matches /proc/pid/cmdline and add to procs list
if err == nil && regex.Match(buf) {
pidRegexMatch := pidRegex.FindStringSubmatch(cmdlinePath)

pid, _ := strconv.Atoi(pidRegexMatch[1])
if proc, err := os.FindProcess(pid); err == nil {
procs = append(procs, proc)
}

}
}

return procs, nil
}

// Examine arguments for well known base names and refine the match.
var refineBaseNameMatch = func(baseName string, params []string) bool {
if baseName == "fw-util" {
Expand Down
24 changes: 24 additions & 0 deletions tools/flashy/lib/utils/system_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"reflect"
"testing"
"time"
"regexp"

"github.com/facebook/openbmc/tools/flashy/lib/fileutils"
"github.com/facebook/openbmc/tools/flashy/tests"
Expand Down Expand Up @@ -1069,3 +1070,26 @@ func TestIsDataPartitionMounted(t *testing.T) {
})
}
}

func TestListProcessesMatchingRegex(t *testing.T) {
// Tests using real /proc/ just to get some basic test coverage
t.Run("Process list matching .* must return all processes", func(t *testing.T) {
procs, err := ListProcessesMatchingRegex(regexp.MustCompile(`.*`))
if err != nil {
t.Errorf("Error listing processes: %v", err)
}
if len(procs) < 1 {
t.Errorf("Expected at least one process matching `.*`, got %d", len(procs))
}
})

t.Run("Process list matching ^\b$ must return no processes", func(t *testing.T) {
procs, err := ListProcessesMatchingRegex(regexp.MustCompile(`^\b$`))
if err != nil {
t.Errorf("Error listing processes: %v", err)
}
if len(procs) != 0 {
t.Errorf("Expected no process matching `^\b$`, got %d", len(procs))
}
})
}

0 comments on commit ce71547

Please sign in to comment.