[botanist] Reland "[botanist] Refactor"
This is a reland of 163d0616b0422c582e8cd1c821a1c9e1f43ccb50
Original change's description:
> [botanist] Refactor
>
> Botanist has grown unwielidy over the past ~year and so this change
> returns it to a more manageable and scalable state. Much of the weight
> loss is due to the removal of log support in the multi-device case, which -
> interestingly - has not been used at all or relied upon since its introduction!
> (Indeed, we are creating files (with %d suffixes that we have up front
> not registered as task outputs, and so they never get shipped back to
> the recipe).)
>
> Also removed was botanist's care for not having streamed empty log
> content. This seems like something that should more reasonably be
> checked in the context of a recipe rather than something botanist should
> intimately care about.
>
> Tested: botanist is extensively integration tested being the tool that
> CQ itself consumes from source to run tests and manage device
> interaction. Also, this change can be viewed as bootstrapping
> testability for a more streamlined botanist codebase.
>
> Change-Id: I5809ec79e745339a4b970f8a109e2bb47fe2eef9
Change-Id: I02820760df551365ca64800b9bb73c52d0bc9823
diff --git a/tools/botanist/BUILD.gn b/tools/botanist/BUILD.gn
index e6026d8..1ac8747 100644
--- a/tools/botanist/BUILD.gn
+++ b/tools/botanist/BUILD.gn
@@ -54,11 +54,6 @@
deps = [ ":main" ]
}
-go_test("botanist_cmd_tests") {
- gopackages = [ "go.fuchsia.dev/fuchsia/tools/botanist/cmd" ]
- deps = [ ":main" ]
-}
-
go_test("botanist_lib_tests") {
gopackages = [ "go.fuchsia.dev/fuchsia/tools/botanist/lib" ]
deps = [ ":botanist_lib" ]
@@ -73,7 +68,6 @@
testonly = true
deps = [
- ":botanist_cmd_tests",
":botanist_lib_tests",
":botanist_target_tests",
]
diff --git a/tools/botanist/cmd/run.go b/tools/botanist/cmd/run.go
index 42a2711..eb54c46 100644
--- a/tools/botanist/cmd/run.go
+++ b/tools/botanist/cmd/run.go
@@ -12,7 +12,6 @@
"io/ioutil"
"net"
"os"
- "path/filepath"
"sync"
"time"
@@ -46,16 +45,16 @@
SSHKey() string
// Start starts the target.
- Start(ctx context.Context, images []bootserver.Image, args []string) error
+ Start(context.Context, []bootserver.Image, []string) error
// Restart restarts the target.
- Restart(ctx context.Context) error
+ Restart(context.Context) error
// Stop stops the target.
- Stop(ctx context.Context) error
+ Stop(context.Context) error
// Wait waits for the target to finish running.
- Wait(ctx context.Context) error
+ Wait(context.Context) error
}
// RunCommand is a Command implementation for booting a device and running a
@@ -76,12 +75,6 @@
// Timeout is the duration allowed for the command to finish execution.
timeout time.Duration
- // CmdStdout is the file to which the command's stdout will be redirected.
- cmdStdout string
-
- // CmdStderr is the file to which the command's stderr will be redirected.
- cmdStderr string
-
// SysloggerFile, if nonempty, is the file to where the system's logs will be written.
syslogFile string
@@ -121,8 +114,6 @@
f.BoolVar(&r.netboot, "netboot", false, "if set, botanist will not pave; but will netboot instead")
f.Var(&r.zirconArgs, "zircon-args", "kernel command-line arguments")
f.DurationVar(&r.timeout, "timeout", 10*time.Minute, "duration allowed for the command to finish execution.")
- f.StringVar(&r.cmdStdout, "stdout", "", "file to redirect the command's stdout into; if unspecified, it will be redirected to the process' stdout")
- f.StringVar(&r.cmdStderr, "stderr", "", "file to redirect the command's stderr into; if unspecified, it will be redirected to the process' stderr")
f.StringVar(&r.syslogFile, "syslog", "", "file to write the systems logs to")
f.StringVar(&r.sshKey, "ssh", "", "file containing a private SSH user key; if not provided, a private key will be generated.")
f.StringVar(&r.serialLogFile, "serial-log", "", "file to write the serial logs to.")
@@ -134,260 +125,6 @@
f.StringVar(&r.blobURL, "blobs", defaultBlobURL, "URL at which to serve a package repository's blobs")
}
-func (r *RunCommand) runCmd(ctx context.Context, args []string, t Target) error {
- nodename := t.Nodename()
- ip, err := t.IPv4Addr()
- if err == nil {
- logger.Infof(ctx, "IPv4 address of %s found: %s", nodename, ip)
- } else {
- logger.Errorf(ctx, "could not resolve IPv4 address of %s: %v", nodename, err)
- }
-
- env := append(
- os.Environ(),
- fmt.Sprintf("FUCHSIA_NODENAME=%s", nodename),
- fmt.Sprintf("FUCHSIA_IPV4_ADDR=%v", ip),
- fmt.Sprintf("FUCHSIA_SSH_KEY=%s", t.SSHKey()),
- )
-
- ctx, cancel := context.WithTimeout(ctx, r.timeout)
- defer cancel()
-
- stdout := os.Stdout
- if r.cmdStdout != "" {
- f, err := os.Create(r.cmdStdout)
- if err != nil {
- return err
- }
- defer f.Close()
- stdout = f
- }
- stderr := os.Stderr
- if r.cmdStderr != "" {
- f, err := os.Create(r.cmdStderr)
- if err != nil {
- return err
- }
- defer f.Close()
- stderr = f
- }
-
- runner := runner.SubprocessRunner{
- Env: env,
- }
- if err := runner.Run(ctx, args, stdout, stderr); err != nil {
- if ctx.Err() != nil {
- return fmt.Errorf("command timed out after %v", r.timeout)
- }
- return err
- }
- return nil
-}
-
-func getIndexedFilename(filename string, index int) string {
- ext := filepath.Ext(filename)
- name := filename[:len(filename)-len(ext)]
- return fmt.Sprintf("%s-%d%s", name, index, ext)
-}
-
-type targetSetup struct {
- targets []Target
- syslogs []*logWriter
- serialLogs []*logWriter
- err error
-}
-
-type logWriter struct {
- name string
- file io.ReadWriteCloser
- target Target
-}
-
-func (r *RunCommand) setupTargets(ctx context.Context, imgs []bootserver.Image, targets []Target) *targetSetup {
- var syslogs, serialLogs []*logWriter
- errs := make(chan error, len(targets))
- var wg sync.WaitGroup
- var setupErr error
-
- for i, t := range targets {
- var s *os.File
- var err error
- if r.syslogFile != "" {
- syslogFile := r.syslogFile
- if len(targets) > 1 {
- syslogFile = getIndexedFilename(r.syslogFile, i)
- }
- s, err = os.Create(syslogFile)
- if err != nil {
- setupErr = err
- break
- }
- syslogs = append(syslogs, &logWriter{
- name: syslogFile,
- file: s,
- target: t,
- })
- }
-
- zirconArgs := r.zirconArgs
- if t.Serial() != nil {
- if r.serialLogFile != "" {
- serialLogFile := r.serialLogFile
- if len(targets) > 1 {
- serialLogFile = getIndexedFilename(r.serialLogFile, i)
- }
- serialLog, err := os.Create(serialLogFile)
- if err != nil {
- setupErr = err
- break
- }
- serialLogs = append(serialLogs, &logWriter{
- name: serialLogFile,
- file: serialLog,
- target: t,
- })
-
- // Here we invoke the `dlog` command over serial to tail the existing log buffer into the
- // output file. This should give us everything since Zedboot boot, and new messages should
- // be written to directly to the serial port without needing to tail with `dlog -f`.
- if _, err = io.WriteString(t.Serial(), "\ndlog\n"); err != nil {
- logger.Errorf(ctx, "failed to tail zedboot dlog: %v", err)
- }
-
- go func(t Target, serialLog io.ReadWriteCloser) {
- for {
- _, err := io.Copy(serialLog, t.Serial())
- if err != nil && err != io.EOF {
- logger.Errorf(ctx, "failed to write serial log: %v", err)
- return
- }
- }
- }(t, serialLog)
- zirconArgs = append(zirconArgs, "kernel.bypass-debuglog=true")
- }
- // Modify the zirconArgs passed to the kernel on boot to enable serial on x64.
- // arm64 devices should already be enabling kernel.serial at compile time.
- zirconArgs = append(zirconArgs, "kernel.serial=legacy")
- }
-
- wg.Add(1)
- go func(t Target, s *os.File, zirconArgs []string) {
- defer wg.Done()
- if err := t.Start(ctx, imgs, zirconArgs); err != nil {
- errs <- err
- return
- }
- nodename := t.Nodename()
-
- if r.syslogFile != "" && s == nil {
- errs <- fmt.Errorf("syslog is nil.")
- return
- }
-
- // If having paved, SSH in and stream syslogs back to a file sink.
- if !r.netboot && s != nil {
- p, err := ioutil.ReadFile(t.SSHKey())
- if err != nil {
- errs <- err
- return
- }
- config, err := sshutil.DefaultSSHConfig(p)
- if err != nil {
- errs <- err
- return
- }
- client, err := sshutil.ConnectToNode(ctx, nodename, config)
- if err != nil {
- errs <- err
- return
- }
- syslogger := syslog.NewSyslogger(client, config)
- go func() {
- defer syslogger.Close()
- if err := syslogger.Stream(ctx, s, false); err != nil {
- errs <- err
- }
- }()
- }
- }(t, s, zirconArgs)
- }
- // Wait for all targets to finish starting.
- wg.Wait()
- // We can close the channel on the receiver end since we wait for all target goroutines to finish.
- close(errs)
- err, ok := <-errs
- if ok {
- setupErr = err
- }
- return &targetSetup{
- targets: targets,
- syslogs: syslogs,
- serialLogs: serialLogs,
- err: setupErr,
- }
-}
-
-func (r *RunCommand) runCmdWithTargets(ctx context.Context, targetSetup *targetSetup, args []string) error {
- for _, log := range targetSetup.syslogs {
- defer log.file.Close()
- }
- for _, log := range targetSetup.serialLogs {
- defer func(log *logWriter) {
- if err := log.target.Serial().Close(); err == nil {
- logger.Errorf(ctx, "serial output not closed yet.")
- }
- log.file.Close()
- }(log)
- }
- for _, t := range targetSetup.targets {
- defer func(t Target) {
- logger.Debugf(ctx, "stopping or rebooting the node %q\n", t.Nodename())
- if err := t.Stop(ctx); err == target.ErrUnimplemented {
- t.Restart(ctx)
- }
- }(t)
- }
-
- if targetSetup.err != nil {
- return targetSetup.err
- }
-
- errs := make(chan error)
-
- go func() {
- // Target doesn't matter for multi-device host tests. Just use first one.
- errs <- r.runCmd(ctx, args, targetSetup.targets[0])
- }()
-
- for _, t := range targetSetup.targets {
- go func(t Target) {
- if err := t.Wait(ctx); err != nil && err != target.ErrUnimplemented {
- errs <- err
- }
- }(t)
- }
-
- select {
- case err := <-errs:
- return err
- case <-ctx.Done():
- }
- return nil
-}
-
-func checkEmptyLogs(ctx context.Context, logs []*logWriter) error {
- for _, log := range logs {
- b, err := ioutil.ReadFile(log.name)
- if err != nil {
- return err
- }
- if len(b) == 0 {
- return fmt.Errorf("log is empty.")
- }
- }
- return nil
-}
-
func (r *RunCommand) execute(ctx context.Context, args []string) error {
var bootMode bootserver.Mode
if r.netboot {
@@ -417,21 +154,159 @@
var targets []Target
for _, obj := range objs {
- t, err := DeriveTarget(ctx, obj, opts)
+ t, err := deriveTarget(ctx, obj, opts)
if err != nil {
return err
}
targets = append(targets, t)
}
+ if len(targets) == 0 {
+ return fmt.Errorf("no targets found")
+ }
+
+ // This is the primary target that a command will be run against and that
+ // logs will be streamed from.
+ t0 := targets[0]
+
+ errs := make(chan error)
+
+ serial := t0.Serial()
+ if serial != nil && r.serialLogFile != "" {
+ // Modify the zirconArgs passed to the kernel on boot to enable serial on x64.
+ // arm64 devices should already be enabling kernel.serial at compile time.
+ r.zirconArgs = append(r.zirconArgs, "kernel.serial=legacy")
+ // Force serial output to the console instead of buffering it.
+ r.zirconArgs = append(r.zirconArgs, "kernel.bypass-debuglog=true")
+
+ serialLog, err := os.Create(r.serialLogFile)
+ if err != nil {
+ return err
+ }
+ defer serialLog.Close()
+ go func() {
+ if _, err := io.Copy(serialLog, serial); err != nil {
+ errs <- fmt.Errorf("failed to write serial log: %v", err)
+ }
+ }()
+ }
ctx, cancel := context.WithCancel(ctx)
defer cancel()
- targetSetup := r.setupTargets(ctx, imgs, targets)
- if err := r.runCmdWithTargets(ctx, targetSetup, args); err != nil {
- return err
+ // Defer asynchronously restarts of each target.
+ defer func() {
+ var wg sync.WaitGroup
+ for _, t := range targets {
+ wg.Add(1)
+ go func(t Target) {
+ defer wg.Done()
+ logger.Debugf(ctx, "stopping or rebooting the node %q\n", t.Nodename())
+ if err := t.Stop(ctx); err == target.ErrUnimplemented {
+ t.Restart(ctx)
+ }
+ }(t)
+ }
+ wg.Wait()
+ }()
+
+ // We wait until targets have started before running the subcommand against the zeroth one.
+ var wg sync.WaitGroup
+ for _, t := range targets {
+ wg.Add(1)
+ go func(t Target) {
+ if err := t.Start(ctx, imgs, r.zirconArgs); err != nil {
+ wg.Done()
+ errs <- err
+ return
+ }
+ wg.Done()
+ if err := t.Wait(ctx); err != nil && err != target.ErrUnimplemented {
+ errs <- err
+ }
+ }(t)
}
- return checkEmptyLogs(ctx, append(targetSetup.syslogs, targetSetup.serialLogs...))
+ go func() {
+ wg.Wait()
+ errs <- r.runAgainstTarget(ctx, t0, args)
+ }()
+
+ select {
+ case err := <-errs:
+ return err
+ case <-ctx.Done():
+ }
+ return nil
+}
+
+func (r *RunCommand) runAgainstTarget(ctx context.Context, t Target, args []string) error {
+ subprocessEnv := map[string]string{
+ "FUCHSIA_NODENAME": t.Nodename(),
+ }
+
+ // If |netboot| is true, then we assume that fuchsia is not provisioned
+ // with a netstack; in this case, do not try to establish a connection.
+ if !r.netboot {
+ p, err := ioutil.ReadFile(t.SSHKey())
+ if err != nil {
+ return err
+ }
+ config, err := sshutil.DefaultSSHConfig(p)
+ if err != nil {
+ return err
+ }
+ client, err := sshutil.ConnectToNode(ctx, t.Nodename(), config)
+ if err != nil {
+ return err
+ }
+ defer client.Close()
+ subprocessEnv["FUCHSIA_SSH_KEY"] = t.SSHKey()
+
+ ip, err := t.IPv4Addr()
+ if err != nil {
+ logger.Errorf(ctx, "could not resolve IPv4 address of %s: %v", t.Nodename(), err)
+ } else if ip != nil {
+ logger.Infof(ctx, "IPv4 address of %s found: %s", t.Nodename(), ip)
+ subprocessEnv["FUCHSIA_IPV4_ADDR"] = ip.String()
+ }
+
+ if r.syslogFile != "" {
+ s, err := os.Create(r.syslogFile)
+ if err != nil {
+ return err
+ }
+ defer s.Close()
+ syslogger := syslog.NewSyslogger(client, config)
+ defer syslogger.Close()
+
+ go func() {
+ err := syslogger.Stream(ctx, s, false)
+ // TODO(fxbug.dev/43518): when 1.13 is available, spell this as
+ // `err != nil && errors.Is(err, context.Canceled) && errors.Is(err, context.DeadlineExceeded)`
+ if err != nil && ctx.Err() == nil {
+ logger.Errorf(ctx, "syslog streaming interrupted: %v", err)
+ }
+ }()
+ }
+ }
+
+ // Run the provided command against t0, adding |subprocessEnv| into
+ // its environment.
+ environ := os.Environ()
+ for k, v := range subprocessEnv {
+ environ = append(environ, fmt.Sprintf("%s=%s", k, v))
+ }
+ runner := runner.SubprocessRunner{
+ Env: environ,
+ }
+
+ ctx, cancel := context.WithTimeout(ctx, r.timeout)
+ defer cancel()
+
+ err := runner.Run(ctx, args, os.Stdout, os.Stderr)
+ if ctx.Err() == context.DeadlineExceeded {
+ return fmt.Errorf("command timed out after %v", r.timeout)
+ }
+ return err
}
func (r *RunCommand) Execute(ctx context.Context, f *flag.FlagSet, _ ...interface{}) subcommands.ExitStatus {
@@ -446,7 +321,7 @@
return subcommands.ExitSuccess
}
-func DeriveTarget(ctx context.Context, obj []byte, opts target.Options) (Target, error) {
+func deriveTarget(ctx context.Context, obj []byte, opts target.Options) (Target, error) {
type typed struct {
Type string `json:"type"`
}
diff --git a/tools/botanist/cmd/run_test.go b/tools/botanist/cmd/run_test.go
deleted file mode 100644
index 802945d..0000000
--- a/tools/botanist/cmd/run_test.go
+++ /dev/null
@@ -1,136 +0,0 @@
-// Copyright 2019 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 main
-
-import (
- "context"
- "fmt"
- "io"
- "io/ioutil"
- "os"
- "path/filepath"
- "strings"
- "testing"
- "time"
-
- "go.fuchsia.dev/fuchsia/tools/bootserver/lib"
- "go.fuchsia.dev/fuchsia/tools/botanist/target"
- "go.fuchsia.dev/fuchsia/tools/lib/color"
- "go.fuchsia.dev/fuchsia/tools/lib/logger"
-)
-
-func TestExecute(t *testing.T) {
- t.Run("close logs after cmd finishes", func(t *testing.T) {
- tmpDir, err := ioutil.TempDir("", fmt.Sprintf("test-data"))
- if err != nil {
- t.Fatalf("failed to create temp dir: %v", err)
- }
- defer os.RemoveAll(tmpDir)
- syslogPath := filepath.Join(tmpDir, "syslog.txt")
- serialLogPath := filepath.Join(tmpDir, "serial.txt")
-
- // create empty script to pass as cmd to run.
- scriptPath := filepath.Join(tmpDir, "test_script.sh")
- if err = ioutil.WriteFile(scriptPath, []byte("#!/bin/bash\n# don't do anything."), 0755); err != nil {
- t.Fatalf("Failed to write to script: %s", err)
- }
-
- cmd := &RunCommand{
- syslogFile: syslogPath,
- netboot: true,
- timeout: time.Second * 5,
- serialLogFile: serialLogPath,
- }
- loggerOutFile := filepath.Join(tmpDir, "logger_out.txt")
- loggerOut, err := os.Create(loggerOutFile)
- if err != nil {
- t.Fatalf("failed to create log file: %v", err)
- }
-
- serialName := filepath.Join(tmpDir, "real_serial.txt")
- serialWriter, err := os.Create(serialName)
- if err != nil {
- t.Fatalf("failed to create mock serial writer: %v", err)
- }
- defer serialWriter.Close()
- testLogger := logger.NewLogger(logger.DebugLevel, color.NewColor(color.ColorAuto), loggerOut, loggerOut, "botanist ")
- ctx := logger.WithLogger(context.Background(), testLogger)
- var targets []Target
- for i := 0; i < 2; i++ {
- var serial io.ReadWriteCloser
- if i == 0 {
- serial = serialWriter
- }
- target, err := target.NewMockTarget(ctx, fmt.Sprintf("mock-target-%d", i+1), serial)
- if err != nil {
- t.Fatalf("failed to create new target: %v", err)
- }
- targets = append(targets, target)
- }
-
- // cmd.execute() calls setupTargets() followed by runCmdWithTargets().
- // The logs should be open for writing to when runCmdWithTargets()
- // is called and closed after it finishes.
- targetSetup := cmd.setupTargets(ctx, []bootserver.Image{}, targets)
- for _, log := range targetSetup.syslogs {
- if _, err := io.WriteString(log.file, "File is open!"); err != nil {
- t.Fatalf("File is not open: %v", err)
- }
- }
- if err = cmd.runCmdWithTargets(ctx, targetSetup, []string{scriptPath}); err != nil {
- t.Fatalf("Execute failed with error: %v", err)
- }
- for _, log := range targetSetup.syslogs {
- if _, err := io.WriteString(log.file, "File is closed!"); err == nil {
- t.Fatalf("File is open: %v", err)
- }
- }
-
- loggerOut.Close()
- log, err := ioutil.ReadFile(loggerOutFile)
- if err != nil {
- t.Fatalf("%v", err)
- }
- if strings.Contains(string(log), "serial output not closed yet") {
- t.Fatalf("Failed to close syslog/serial logs after stopping the target.")
- }
- })
-
- t.Run("fail for empty logs", func(t *testing.T) {
- tmpDir, err := ioutil.TempDir("", fmt.Sprintf("test-data"))
- if err != nil {
- t.Fatalf("failed to create temp dir: %v", err)
- }
- defer os.RemoveAll(tmpDir)
- syslogPath := filepath.Join(tmpDir, "syslog.txt")
-
- // create empty script to pass as cmd to run.
- scriptPath := filepath.Join(tmpDir, "test_script.sh")
- if err = ioutil.WriteFile(scriptPath, []byte("#!/bin/bash\n# don't do anything."), 0755); err != nil {
- t.Fatalf("Failed to write to script: %s", err)
- }
-
- cmd := &RunCommand{
- syslogFile: syslogPath,
- netboot: true,
- timeout: time.Second * 5,
- }
- ctx := context.Background()
- target, err := target.NewMockTarget(ctx, "mock-target", nil)
- if err != nil {
- t.Fatalf("failed to create new target: %v", err)
- }
- targets := []Target{target}
-
- targetSetup := cmd.setupTargets(ctx, []bootserver.Image{}, targets)
- if err = cmd.runCmdWithTargets(ctx, targetSetup, []string{scriptPath}); err != nil {
- t.Fatalf("Execute failed with error: %v", err)
- }
- // Logs should be empty.
- if err = checkEmptyLogs(ctx, append(targetSetup.syslogs, targetSetup.serialLogs...)); err == nil {
- t.Fatalf("Did not fail for empty logs")
- }
- })
-}
diff --git a/tools/botanist/cmd/zedboot.go b/tools/botanist/cmd/zedboot.go
index b127f69..82a8b68 100644
--- a/tools/botanist/cmd/zedboot.go
+++ b/tools/botanist/cmd/zedboot.go
@@ -11,6 +11,7 @@
"io/ioutil"
"os"
"strings"
+ "sync"
"time"
"go.fuchsia.dev/fuchsia/tools/bootserver/lib"
@@ -120,58 +121,59 @@
defer cancel()
errs := make(chan error)
- var serialLogs []*logWriter
- for i, device := range devices {
- if device.Serial() != nil {
- if cmd.serialLogFile != "" {
- serialLogFile := cmd.serialLogFile
- if len(devices) > 1 {
- serialLogFile = getIndexedFilename(cmd.serialLogFile, i)
- }
- serialLog, err := os.Create(serialLogFile)
- if err != nil {
- return err
- }
- serialLogs = append(serialLogs, &logWriter{
- name: serialLogFile,
- file: serialLog})
+ if serial := devices[0].Serial(); serial != nil && cmd.serialLogFile != "" {
+ defer serial.Close()
- // Here we invoke the `dlog` command over serial to tail the existing log buffer into the
- // output file. This should give us everything since Zedboot boot, and new messages should
- // be written to directly to the serial port without needing to tail with `dlog -f`.
- if _, err = io.WriteString(device.Serial(), "\ndlog\n"); err != nil {
- logger.Errorf(ctx, "failed to tail zedboot dlog: %v", err)
- }
+ // Modify the zirconArgs passed to the kernel on boot to enable serial on x64.
+ // arm64 devices should already be enabling kernel.serial at compile time.
+ cmdlineArgs = append(cmdlineArgs, "kernel.serial=legacy")
+ // Force serial output to the console instead of buffering it.
+ cmdlineArgs = append(cmdlineArgs, "kernel.bypass-debuglog=true")
- go func(device *target.DeviceTarget) {
- for {
- _, err := io.Copy(serialLog, device.Serial())
- if err != nil && err != io.EOF {
- logger.Errorf(ctx, "failed to write serial log: %v", err)
- return
- }
- }
- }(device)
- cmdlineArgs = append(cmdlineArgs, "kernel.bypass-debuglog=true")
- }
- // Modify the cmdlineArgs passed to the kernel on boot to enable serial on x64.
- // arm64 devices should already be enabling kernel.serial at compile time.
- cmdlineArgs = append(cmdlineArgs, "kernel.serial=legacy")
+ serialLog, err := os.Create(cmd.serialLogFile)
+ if err != nil {
+ return err
}
+ defer serialLog.Close()
+ // Here we invoke the `dlog` command over serial to tail the existing log buffer into the
+ // output file. This should give us everything since Zedboot boot, and new messages should
+ // be written to directly to the serial port without needing to tail with `dlog -f`.
+ if _, err = io.WriteString(serial, "\ndlog\n"); err != nil {
+ return fmt.Errorf("failed to tail zedboot dlog: %v", err)
+ }
+ go func() {
+ if _, err := io.Copy(serialLog, serial); err != nil {
+ errs <- fmt.Errorf("failed to write serial log: %v", err)
+ }
+ }()
+ }
+ // Defer asynchronously restarts of each device.
+ defer func() {
+ var wg sync.WaitGroup
+ for _, device := range devices {
+ wg.Add(1)
+ go func(device *target.DeviceTarget) {
+ defer wg.Done()
+ device.Restart(ctx)
+ }(device)
+ }
+ wg.Wait()
+ }()
+
+ var wg sync.WaitGroup
+ for _, device := range devices {
+ wg.Add(1)
go func(device *target.DeviceTarget) {
+ defer wg.Done()
if err := device.Start(ctx, imgs, cmdlineArgs); err != nil {
errs <- err
}
}(device)
}
- for _, log := range serialLogs {
- defer log.file.Close()
- }
- for _, device := range devices {
- defer device.Restart(ctx)
- }
+
go func() {
+ wg.Wait()
// We execute tests here against the 0th device, there may be N devices
// in the test bed but all other devices are driven by the tests not
// the test runner.
@@ -182,9 +184,8 @@
case err := <-errs:
return err
case <-ctx.Done():
+ return nil
}
-
- return checkEmptyLogs(ctx, serialLogs)
}
func (cmd *ZedbootCommand) Execute(ctx context.Context, f *flag.FlagSet, _ ...interface{}) subcommands.ExitStatus {
diff --git a/tools/botanist/target/device.go b/tools/botanist/target/device.go
index 78ef799..c69261f 100644
--- a/tools/botanist/target/device.go
+++ b/tools/botanist/target/device.go
@@ -16,7 +16,6 @@
"go.fuchsia.dev/fuchsia/tools/bootserver/lib"
"go.fuchsia.dev/fuchsia/tools/botanist/lib"
- "go.fuchsia.dev/fuchsia/tools/lib/logger"
"go.fuchsia.dev/fuchsia/tools/net/netboot"
"go.fuchsia.dev/fuchsia/tools/net/netutil"
"go.fuchsia.dev/fuchsia/tools/net/tftp"
@@ -28,6 +27,9 @@
const (
// The duration we allow for the netstack to come up when booting.
netstackTimeout = 90 * time.Second
+
+ // Command to dump the zircon debug log over serial.
+ dlogCmd = "\ndlog\n"
)
// DeviceConfig contains the static properties of a target device.
@@ -90,10 +92,11 @@
if config.Serial != "" {
s, err = serial.Open(config.Serial)
if err != nil {
- // TODO(IN-????): This should be returned as an error, but we don't want to fail any
- // test runs for misconfigured serial until it is actually required to complete certain
- // tasks.
- logger.Errorf(ctx, "unable to open %s: %v", config.Serial, err)
+ return nil, fmt.Errorf("unable to open %s: %v", config.Serial, err)
+ }
+ // Dump the existing serial debug log buffer.
+ if _, err := io.WriteString(s, dlogCmd); err != nil {
+ return nil, fmt.Errorf("failed to tail serial logs: %v", err)
}
}
addr, err := netutil.GetNodeAddress(ctx, config.Network.Nodename, false)
@@ -186,12 +189,12 @@
}
// Stop stops the device.
-func (t *DeviceTarget) Stop(ctx context.Context) error {
+func (t *DeviceTarget) Stop(context.Context) error {
return ErrUnimplemented
}
// Wait waits for the device target to stop.
-func (t *DeviceTarget) Wait(ctx context.Context) error {
+func (t *DeviceTarget) Wait(context.Context) error {
return ErrUnimplemented
}
