Revert "[tools] Move logic for running commands over serial to serial package."
This reverts commit a74fc61e5ab07ff4212df7b8f5a93ee04c9ad6db.
diff --git a/tools/botanist/cmd/run.go b/tools/botanist/cmd/run.go
index 08588f6..f735d52 100644
--- a/tools/botanist/cmd/run.go
+++ b/tools/botanist/cmd/run.go
@@ -338,13 +338,15 @@
if sshAddr.IP == nil {
// Reachable when ResolveIP times out because no error is returned.
// Invoke `threads` over serial if possible to dump process state to logs.
- socket, err := serial.NewSocket(ctx, socketPath)
- if err != nil {
- logger.Errorf(ctx, "newSerialSocket failed: %v", err)
+ // Invokes the command twice to identify hanging processes.
+ if conn, err := net.Dial("unix", socketPath); err != nil {
+ logger.Errorf(ctx, "failed to open serial socket %s to invoke threads: %s", socketPath, err)
} else {
- defer socket.Close()
- if err := serial.RunDiagnostics(ctx, socket); err != nil {
- logger.Errorf(ctx, "failed to run diagnostics over serial socket: %v", err)
+ for i := 0; i < 2; i++ {
+ if _, err := io.WriteString(conn, fmt.Sprintf("\r\nthreads --all-processes\r\n")); err != nil {
+ logger.Errorf(ctx, "failed to send threads over serial socket: %s", err)
+ }
+ time.Sleep(5 * time.Second)
}
}
return fmt.Errorf("%s for %s", constants.FailedToResolveIPErrorMsg, t.Nodename())
diff --git a/tools/serial/BUILD.gn b/tools/serial/BUILD.gn
index b8855f0..5479310 100644
--- a/tools/serial/BUILD.gn
+++ b/tools/serial/BUILD.gn
@@ -10,13 +10,11 @@
"serial.go",
"serial_darwin.go",
"serial_linux.go",
- "serial_test.go",
"server.go",
"server_test.go",
]
deps = [
"//third_party/golibs:golang.org/x/sys",
- "//tools/lib/iomisc",
"//tools/lib/logger",
]
}
@@ -24,9 +22,5 @@
go_test("tests") {
output_name = "serial_tests"
gopackages = [ "go.fuchsia.dev/fuchsia/tools/serial" ]
- deps = [
- ":serial",
- "//third_party/golibs:github.com/google/go-cmp",
- "//third_party/golibs:golang.org/x/sync",
- ]
+ deps = [ ":serial" ]
}
diff --git a/tools/serial/serial.go b/tools/serial/serial.go
index ab9f116..83fb6b5 100644
--- a/tools/serial/serial.go
+++ b/tools/serial/serial.go
@@ -6,35 +6,13 @@
package serial
import (
- "context"
- "fmt"
"io"
- "net"
- "strings"
- "time"
-
- "go.fuchsia.dev/fuchsia/tools/lib/iomisc"
- "go.fuchsia.dev/fuchsia/tools/lib/logger"
)
const (
defaultBaudRate = 115200
-
- // Printed to the serial console when ready to accept user input.
- consoleCursor = "\n$"
)
-var diagnosticCmds = []Command{
- {[]string{"k", "threadload"}, 200 * time.Millisecond}, // Turn on threadload
- {[]string{"k", "threadq"}, 5 * time.Second}, // Turn on threadq and wait 5 sec
- {[]string{"k", "cpu", "sev"}, 5 * time.Second}, // Send a SEV and wait 5 sec
- {[]string{"k", "threadload"}, 200 * time.Millisecond}, // Turn off threadload
- {[]string{"k", "threadq"}, 0}, // Turn off threadq
- // Invoke the threads command twice to identify hanging processes.
- {[]string{"threads", "--all-processes"}, 5 * time.Minute},
- {[]string{"threads", "--all-processes"}, 5 * time.Minute},
-}
-
// Open opens a new serial port using defaults.
func Open(name string) (io.ReadWriteCloser, error) {
return OpenWithOptions(name, defaultBaudRate)
@@ -44,63 +22,3 @@
func OpenWithOptions(name string, baudRate int) (io.ReadWriteCloser, error) {
return open(name, baudRate)
}
-
-// Command contains the command to run over serial and the expected duration
-// to wait for the command to complete.
-type Command struct {
- Cmd []string
- SleepDuration time.Duration
-}
-
-func asSerialCmd(cmd []string) string {
- // The UART kernel driver expects a command to be followed by \r\n.
- // Send a leading \r\n for all commands as there may be characters in the buffer already
- // that we need to clear first.
- return fmt.Sprintf("\r\n%s\r\n", strings.Join(cmd, " "))
-}
-
-// NewSocket opens a connection on the provided `socketPath`.
-func NewSocket(ctx context.Context, socketPath string) (io.ReadWriteCloser, error) {
- if socketPath == "" {
- return nil, fmt.Errorf("serialSocketPath not set")
- }
- socket, err := net.Dial("unix", socketPath)
- if err != nil {
- return nil, fmt.Errorf("failed to open serial socket connection: %v", err)
- }
- // Trigger a new cursor print by sending a newline. This may do nothing if the
- // system was not ready to process input, but in that case it will print a
- // new cursor anyways when it is ready to receive input.
- io.WriteString(socket, asSerialCmd([]string{}))
- // Look for the cursor, which should indicate that the console is ready for input.
- ctx, cancel := context.WithTimeout(ctx, 5*time.Second)
- defer cancel()
- m := iomisc.NewMatchingReader(socket, [][]byte{[]byte(consoleCursor)})
- if _, err = iomisc.ReadUntilMatch(ctx, m); err != nil {
- return nil, fmt.Errorf("failed to find cursor: %v", err)
- }
- return socket, nil
-}
-
-// RunCommands writes the provided commands to the serial socket.
-func RunCommands(ctx context.Context, socket io.ReadWriteCloser, cmds []Command) error {
- for _, cmd := range cmds {
- logger.Debugf(ctx, "running over serial: %v", cmd.Cmd)
-
- if _, err := io.WriteString(socket, asSerialCmd(cmd.Cmd)); err != nil {
- return fmt.Errorf("failed to write to serial socket: %v", err)
- }
-
- if cmd.SleepDuration > 0 {
- logger.Debugf(ctx, "sleeping for %v", cmd.SleepDuration)
- time.Sleep(cmd.SleepDuration)
- }
- }
- return nil
-}
-
-// RunDiagnostics runs a series of diagnostic commands over serial.
-func RunDiagnostics(ctx context.Context, socket io.ReadWriteCloser) error {
- logger.Debugf(ctx, "attempting to run diagnostics over serial")
- return RunCommands(ctx, socket, diagnosticCmds)
-}
diff --git a/tools/serial/serial_test.go b/tools/serial/serial_test.go
deleted file mode 100644
index 64bd414..0000000
--- a/tools/serial/serial_test.go
+++ /dev/null
@@ -1,119 +0,0 @@
-// Copyright 2021 The Fuchsia Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-package serial
-
-import (
- "context"
- "fmt"
- "io"
- "math/rand"
- "net"
- "os"
- "testing"
- "time"
-
- "github.com/google/go-cmp/cmp"
- "go.fuchsia.dev/fuchsia/tools/lib/iomisc"
- "golang.org/x/sync/errgroup"
-)
-
-type fakeSerialServer struct {
- received []byte
- shutdownString string
- socketPath string
- listeningChan chan bool
-}
-
-func (s *fakeSerialServer) Serve() error {
- listener, err := net.Listen("unix", s.socketPath)
- if err != nil {
- s.listeningChan <- false
- return fmt.Errorf("Listen(%s) failed: %v", s.socketPath, err)
- }
- defer listener.Close()
- s.listeningChan <- true
- conn, err := listener.Accept()
- if err != nil {
- return fmt.Errorf("Accept() failed: %v", err)
- }
- defer conn.Close()
- // Signal we're ready to accept input.
- if _, err := conn.Write([]byte(consoleCursor)); err != nil {
- return fmt.Errorf("conn.Write() failed: %v", err)
- }
- reader := iomisc.NewMatchingReader(conn, [][]byte{[]byte(s.shutdownString)})
- for {
- buf := make([]byte, 1024)
- bytesRead, err := reader.Read(buf)
- s.received = append(s.received, buf[:bytesRead]...)
- if err != nil {
- if err == io.EOF {
- return nil
- }
- return fmt.Errorf("conn.Read() failed: %v", err)
- }
- }
-}
-
-func TestRunCommands(t *testing.T) {
- socketPath := fmt.Sprintf("%d.sock", rand.Uint32())
- defer os.Remove(socketPath)
- server := fakeSerialServer{
- shutdownString: "dm shutdown",
- socketPath: socketPath,
- listeningChan: make(chan bool),
- }
- eg := errgroup.Group{}
- eg.Go(server.Serve)
-
- if !<-server.listeningChan {
- t.Fatalf("fakeSerialServer.Serve() returned: %v", eg.Wait())
- }
-
- clientSocket, err := NewSocket(context.Background(), socketPath)
- if err != nil {
- t.Fatalf("NewSocket() failed: %v", err)
- }
- diagnosticCmds = []Command{
- {
- Cmd: []string{"foo"},
- // Ensure we don't waste time sleeping in this test.
- SleepDuration: time.Microsecond,
- },
- {
- Cmd: []string{"bar"},
- // Ensure we don't waste time sleeping in this test.
- SleepDuration: time.Microsecond,
- },
- {
- // This is a hack to ensure the shutdown command gets sent to the serial server.
- // Rather than introduce a new synchronization mechanism, just use the code under test's
- // existing mechanism for sending commands.
- Cmd: []string{server.shutdownString},
- // Ensure we don't waste time sleeping in this test.
- SleepDuration: time.Microsecond,
- },
- }
- // RunDiagnostics calls RunCommands so this covers both functions for testability.
- err = RunDiagnostics(context.Background(), clientSocket)
- if err != nil {
- t.Errorf("RunDiagnostics failed: %v", err)
- }
- if err = eg.Wait(); err != nil {
- t.Errorf("server returned: %v", err)
- }
- if err = clientSocket.Close(); err != nil {
- t.Errorf("clientSocket.Close() returned: %v", err)
- }
- // Verify that each command was seen in the received data, and in the
- // proper order. The first command run was an empty command in NewSocket().
- expectedCommands := asSerialCmd([]string{})
- for _, cmd := range diagnosticCmds {
- expectedCommands += asSerialCmd(cmd.Cmd)
- }
- if diff := cmp.Diff(expectedCommands, string(server.received)); diff != "" {
- t.Errorf("Unexpected server.received (-want +got):\n%s", diff)
- }
-}
diff --git a/tools/testing/testrunner/cmd/tester.go b/tools/testing/testrunner/cmd/tester.go
index 05cf1a9..a47d926 100644
--- a/tools/testing/testrunner/cmd/tester.go
+++ b/tools/testing/testrunner/cmd/tester.go
@@ -25,7 +25,6 @@
"go.fuchsia.dev/fuchsia/tools/lib/retry"
"go.fuchsia.dev/fuchsia/tools/lib/runner"
"go.fuchsia.dev/fuchsia/tools/net/sshutil"
- "go.fuchsia.dev/fuchsia/tools/serial"
"go.fuchsia.dev/fuchsia/tools/testing/runtests"
"go.fuchsia.dev/fuchsia/tools/testing/testrunner/constants"
"golang.org/x/crypto/ssh"
@@ -93,11 +92,6 @@
Close() error
}
-// For testability
-type serialClient interface {
- runDiagnostics(ctx context.Context) error
-}
-
// subprocessTester executes tests in local subprocesses.
type subprocessTester struct {
env []string
@@ -231,22 +225,6 @@
return nil
}
-type serialSocket struct {
- socketPath string
-}
-
-func (s *serialSocket) runDiagnostics(ctx context.Context) error {
- if s.socketPath == "" {
- return fmt.Errorf("serialSocketPath not set")
- }
- socket, err := serial.NewSocket(ctx, s.socketPath)
- if err != nil {
- return fmt.Errorf("newSerialSocket failed: %v", err)
- }
- defer socket.Close()
- return serial.RunDiagnostics(ctx, socket)
-}
-
// fuchsiaSSHTester executes fuchsia tests over an SSH connection.
type fuchsiaSSHTester struct {
client sshClient
@@ -255,7 +233,7 @@
localOutputDir string
perTestTimeout time.Duration
connectionErrorRetryBackoff retry.Backoff
- serialSocket serialClient
+ serialSocketPath string
}
// newFuchsiaSSHTester returns a fuchsiaSSHTester associated to a fuchsia
@@ -297,10 +275,55 @@
localOutputDir: localOutputDir,
perTestTimeout: perTestTimeout,
connectionErrorRetryBackoff: retry.NewConstantBackoff(time.Second),
- serialSocket: &serialSocket{serialSocketPath},
+ serialSocketPath: serialSocketPath,
}, nil
}
+func asSerialCmd(cmd []string) string {
+ // The UART kernel driver expects a command to be followed by \r\n.
+ // Send a leading \r\n for all commands as there may be characters in the buffer already
+ // that we need to clear first.
+ return fmt.Sprintf("\r\n%s\r\n", strings.Join(cmd, " "))
+}
+
+type serialDiagnosticCmd struct {
+ cmd []string
+ sleepDuration time.Duration
+}
+
+var serialDiagnosticCmds = []serialDiagnosticCmd{
+ {[]string{"k", "threadload"}, 200 * time.Millisecond}, // Turn on threadload
+ {[]string{"k", "threadq"}, 5 * time.Second}, // Turn on threadq and wait 5 sec
+ {[]string{"k", "cpu", "sev"}, 5 * time.Second}, // Send a SEV and wait 5 sec
+ {[]string{"k", "threadload"}, 200 * time.Millisecond}, // Turn off threadload
+ {[]string{"k", "threadq"}, 0}, // Turn off threadq
+}
+
+func (t *fuchsiaSSHTester) runSerialDiagnostics(ctx context.Context) error {
+ if t.serialSocketPath == "" {
+ return fmt.Errorf("serialSocketPath not set")
+ }
+ logger.Debugf(ctx, "attempting to run diagnostics over serial")
+ socket, err := newSerialSocket(ctx, t.serialSocketPath)
+ if err != nil {
+ return fmt.Errorf("newSerialSocket failed: %v", err)
+ }
+ defer socket.Close()
+ for _, cmd := range serialDiagnosticCmds {
+ logger.Debugf(ctx, "running over serial: %v", cmd.cmd)
+
+ if _, err := io.WriteString(socket, asSerialCmd(cmd.cmd)); err != nil {
+ return fmt.Errorf("failed to write to serial socket: %v", err)
+ }
+
+ if cmd.sleepDuration > 0 {
+ logger.Debugf(ctx, "sleeping for %v", cmd.sleepDuration)
+ time.Sleep(cmd.sleepDuration)
+ }
+ }
+ return nil
+}
+
func (t *fuchsiaSSHTester) reconnect(ctx context.Context) error {
if err := t.client.Reconnect(ctx); err != nil {
return fmt.Errorf("failed to reestablish SSH connection: %w", err)
@@ -370,7 +393,7 @@
testErr := t.runSSHCommandWithRetry(ctx, command, stdout, stderr)
if sshutil.IsConnectionError(testErr) {
- if err := t.serialSocket.runDiagnostics(ctx); err != nil {
+ if err := t.runSerialDiagnostics(ctx); err != nil {
logger.Warningf(ctx, "failed to run serial diagnostics: %v", err)
}
return nil, testErr
@@ -446,8 +469,25 @@
localOutputDir string
}
+func newSerialSocket(ctx context.Context, path string) (io.ReadWriteCloser, error) {
+ socket, err := net.Dial("unix", path)
+ if err != nil {
+ return nil, fmt.Errorf("failed to open serial socket connection: %v", err)
+ }
+ // Trigger a new cursor print by sending a newline. This may do nothing if the
+ // system was not ready to process input, but in that case it will print a
+ // new cursor anyways when it is ready to receive input.
+ io.WriteString(socket, asSerialCmd([]string{}))
+ // Look for the cursor, which should indicate that the console is ready for input.
+ m := iomisc.NewMatchingReader(socket, [][]byte{[]byte(serialConsoleCursor)})
+ if _, err = iomisc.ReadUntilMatch(ctx, m); err != nil {
+ return nil, fmt.Errorf("failed to find cursor: %v", err)
+ }
+ return socket, nil
+}
+
func newFuchsiaSerialTester(ctx context.Context, serialSocketPath string, perTestTimeout time.Duration) (*fuchsiaSerialTester, error) {
- socket, err := serial.NewSocket(ctx, serialSocketPath)
+ socket, err := newSerialSocket(ctx, serialSocketPath)
if err != nil {
return nil, err
}
@@ -479,6 +519,7 @@
if err != nil {
return nil, err
}
+ cmd := asSerialCmd(command)
logger.Debugf(ctx, "starting: %v", command)
// If a single read from the socket includes both the bytes that indicate the test started and the bytes
@@ -487,7 +528,7 @@
lastWrite := &lastWriteSaver{}
startedReader := iomisc.NewMatchingReader(io.TeeReader(t.socket, lastWrite), [][]byte{[]byte(runtests.StartedSignature + test.Name)})
for ctx.Err() == nil {
- if err := serial.RunCommands(ctx, t.socket, []serial.Command{{command, 0}}); err != nil {
+ if _, err := io.WriteString(t.socket, cmd); err != nil {
return nil, fmt.Errorf("failed to write to serial socket: %v", err)
}
startedCtx, cancel := newTestStartedContext(ctx)
diff --git a/tools/testing/testrunner/cmd/tester_test.go b/tools/testing/testrunner/cmd/tester_test.go
index fb1a787..5b33a0b 100644
--- a/tools/testing/testrunner/cmd/tester_test.go
+++ b/tools/testing/testrunner/cmd/tester_test.go
@@ -10,6 +10,7 @@
"fmt"
"io"
"io/ioutil"
+ "math/rand"
"net"
"os"
"path/filepath"
@@ -24,6 +25,7 @@
"go.fuchsia.dev/fuchsia/tools/lib/retry"
"go.fuchsia.dev/fuchsia/tools/net/sshutil"
"go.fuchsia.dev/fuchsia/tools/testing/runtests"
+ "golang.org/x/sync/errgroup"
"github.com/google/go-cmp/cmp"
)
@@ -59,15 +61,6 @@
return err
}
-type fakeSerialClient struct {
- runCalls int
-}
-
-func (c *fakeSerialClient) runDiagnostics(_ context.Context) error {
- c.runCalls++
- return nil
-}
-
type fakeCmdRunner struct {
runErrs []error
runCalls int
@@ -273,12 +266,40 @@
runErrs: c.runErrs,
}
copier := &fakeDataSinkCopier{}
- serialSocket := &fakeSerialClient{}
tester := fuchsiaSSHTester{
client: client,
copier: copier,
connectionErrorRetryBackoff: &retry.ZeroBackoff{},
- serialSocket: serialSocket,
+ }
+ eg := errgroup.Group{}
+ serialServer := fakeSerialServer{
+ received: make([]byte, 1024),
+ shutdownString: "shutdown",
+ socketPath: fmt.Sprintf("%d.sock", rand.Uint32()),
+ listeningChan: make(chan bool),
+ }
+ if c.wantConnErr {
+ serialDiagnosticCmds = []serialDiagnosticCmd{
+ {
+ cmd: []string{"foo"},
+ // Ensure we don't waste time sleeping in this test.
+ sleepDuration: time.Microsecond,
+ },
+ {
+ // This is a hack to ensure the shutdown command gets sent to the serial server.
+ // Rather than introduce a new synchronization mechanism, just use the code under test's
+ // existing mechanism for sending commands.
+ cmd: []string{serialServer.shutdownString},
+ // Ensure we don't waste time sleeping in this test.
+ sleepDuration: time.Microsecond,
+ },
+ }
+ tester.serialSocketPath = serialServer.socketPath
+ defer os.Remove(serialServer.socketPath)
+ eg.Go(serialServer.Serve)
+ if !<-serialServer.listeningChan {
+ t.Fatalf("fakeSerialServer.Serve() returned: %v", eg.Wait())
+ }
}
defer func() {
if err := tester.Close(); err != nil {
@@ -335,11 +356,20 @@
}
if c.wantConnErr {
- if serialSocket.runCalls != 1 {
- t.Errorf("called RunSerialDiagnostics() %d times", serialSocket.runCalls)
+ if err = eg.Wait(); err != nil {
+ t.Errorf("serialServer.Serve() failed: %v", err)
}
- } else if serialSocket.runCalls > 0 {
- t.Errorf("unexpectedly called RunSerialDiagnostics()")
+ // Verify that each command was seen in the received data, and in the
+ // proper order. Ignore the shutdown command we appended at the end
+ searchPos := 0
+ for _, cmd := range serialDiagnosticCmds[:len(serialDiagnosticCmds)-2] {
+ index := bytes.Index(serialServer.received[searchPos:], []byte(asSerialCmd(cmd.cmd)))
+ if index == -1 {
+ t.Errorf("SSHTester did find the command \"%v\" in the proper order, all received: %s", cmd, string(serialServer.received))
+ } else {
+ searchPos = index + 1
+ }
+ }
}
})
}
@@ -422,6 +452,44 @@
}
}
+func TestNewSerialSocket(t *testing.T) {
+ socketPath := fmt.Sprintf("%d.sock", rand.Uint32())
+ defer os.Remove(socketPath)
+ server := fakeSerialServer{
+ shutdownString: "dm shutdown",
+ socketPath: socketPath,
+ listeningChan: make(chan bool),
+ }
+ eg := errgroup.Group{}
+ eg.Go(server.Serve)
+
+ if !<-server.listeningChan {
+ t.Fatalf("fakeSerialServer.Serve() returned: %v", eg.Wait())
+ }
+
+ clientSocket, err := newSerialSocket(context.Background(), socketPath)
+ if err != nil {
+ t.Fatalf("newSerialSocket() failed: %v", err)
+ }
+ bytesWritten, err := clientSocket.Write([]byte(server.shutdownString))
+ if err != nil {
+ t.Errorf("clientSocket.Write() failed: %v", err)
+ }
+ if bytesWritten != len(server.shutdownString) {
+ t.Errorf("clientSocket.Write() wrote %d bytes, wanted %d", bytesWritten, len(server.shutdownString))
+ }
+ if err = eg.Wait(); err != nil {
+ t.Errorf("server returned: %v", err)
+ }
+ if err = clientSocket.Close(); err != nil {
+ t.Errorf("clientSocket.Close() returned: %v", err)
+ }
+ // First newline should be sent by newSerialSocket to trigger a cursor.
+ if diff := cmp.Diff("\r\n\r\n"+server.shutdownString, string(server.received)); diff != "" {
+ t.Errorf("Unexpected server.received (-want +got):\n%s", diff)
+ }
+}
+
// fakeContext conforms to context.Context but lets us control the return
// value of Err().
type fakeContext struct {
