Make migrations log output to stdout

Migrations were logging to a mix of stdout and stderr.  This was due to the individual migration binaries logging non-error output to stdout, while the migration library (which downloads and executed these migrations) was logging to stderr.  This inconsistency can be confusing.  Also, previous versions of go-ipfs wrote non-error output to stdout. This PR fixes this so that non-error output from the migrations library is written to stdout.

Added test to look for expected log output.
This commit is contained in:
gammazero 2021-04-05 20:19:09 -07:00
parent ef866a1400
commit 7c8df87cd0
2 changed files with 35 additions and 16 deletions

View File

@ -38,7 +38,9 @@ func RunMigration(ctx context.Context, fetcher Fetcher, targetVer int, ipfsDir s
return fmt.Errorf("downgrade not allowed from %d to %d", fromVer, targetVer)
}
log.Print("Looking for suitable migration binaries.")
logger := log.New(os.Stdout, "", 0)
logger.Print("Looking for suitable migration binaries.")
migrations, binPaths, err := findMigrations(ctx, fromVer, targetVer)
if err != nil {
@ -54,7 +56,7 @@ func RunMigration(ctx context.Context, fetcher Fetcher, targetVer int, ipfsDir s
}
}
log.Println("Need", len(missing), "migrations, downloading.")
logger.Println("Need", len(missing), "migrations, downloading.")
tmpDir, err := ioutil.TempDir("", "migrations")
if err != nil {
@ -62,9 +64,9 @@ func RunMigration(ctx context.Context, fetcher Fetcher, targetVer int, ipfsDir s
}
defer os.RemoveAll(tmpDir)
fetched, err := fetchMigrations(ctx, fetcher, missing, tmpDir)
fetched, err := fetchMigrations(ctx, fetcher, missing, tmpDir, logger)
if err != nil {
log.Print("Failed to download migrations.")
logger.Print("Failed to download migrations.")
return err
}
for i := range missing {
@ -77,13 +79,13 @@ func RunMigration(ctx context.Context, fetcher Fetcher, targetVer int, ipfsDir s
revert = true
}
for _, migration := range migrations {
log.Println("Running migration", migration, "...")
err = runMigration(ctx, binPaths[migration], ipfsDir, revert)
logger.Println("Running migration", migration, "...")
err = runMigration(ctx, binPaths[migration], ipfsDir, revert, logger)
if err != nil {
return fmt.Errorf("migration %s failed: %s", migration, err)
}
}
log.Printf("Success: fs-repo migrated to version %d.\n", targetVer)
logger.Printf("Success: fs-repo migrated to version %d.\n", targetVer)
return nil
}
@ -142,14 +144,14 @@ func findMigrations(ctx context.Context, from, to int) ([]string, map[string]str
return migrations, binPaths, nil
}
func runMigration(ctx context.Context, binPath, ipfsDir string, revert bool) error {
func runMigration(ctx context.Context, binPath, ipfsDir string, revert bool, logger *log.Logger) error {
pathArg := fmt.Sprintf("-path=%s", ipfsDir)
var cmd *exec.Cmd
if revert {
log.Println(" => Running:", binPath, pathArg, "-verbose=true -revert")
logger.Println(" => Running:", binPath, pathArg, "-verbose=true -revert")
cmd = exec.CommandContext(ctx, binPath, pathArg, "-verbose=true", "-revert")
} else {
log.Println(" => Running:", binPath, pathArg, "-verbose=true")
logger.Println(" => Running:", binPath, pathArg, "-verbose=true")
cmd = exec.CommandContext(ctx, binPath, pathArg, "-verbose=true")
}
cmd.Stdout = os.Stdout
@ -159,7 +161,7 @@ func runMigration(ctx context.Context, binPath, ipfsDir string, revert bool) err
// fetchMigrations downloads the requested migrations, and returns a slice with
// the paths of each binary, in the same order specified by needed.
func fetchMigrations(ctx context.Context, fetcher Fetcher, needed []string, destDir string) ([]string, error) {
func fetchMigrations(ctx context.Context, fetcher Fetcher, needed []string, destDir string, logger *log.Logger) ([]string, error) {
osv, err := osWithVariant()
if err != nil {
return nil, err
@ -173,21 +175,21 @@ func fetchMigrations(ctx context.Context, fetcher Fetcher, needed []string, dest
bins := make([]string, len(needed))
// Download and unpack all requested migrations concurrently.
for i, name := range needed {
log.Printf("Downloading migration: %s...", name)
logger.Printf("Downloading migration: %s...", name)
go func(i int, name string) {
defer wg.Done()
dist := path.Join(distMigsRoot, name)
ver, err := LatestDistVersion(ctx, fetcher, dist, false)
if err != nil {
log.Printf("could not get latest version of migration %s: %s", name, err)
logger.Printf("could not get latest version of migration %s: %s", name, err)
return
}
loc, err := FetchBinary(ctx, fetcher, dist, ver, name, destDir)
if err != nil {
log.Printf("could not download %s: %s", name, err)
logger.Printf("could not download %s: %s", name, err)
return
}
log.Printf("Downloaded and unpacked migration: %s (%s)", loc, ver)
logger.Printf("Downloaded and unpacked migration: %s (%s)", loc, ver)
bins[i] = loc
}(i, name)
}

View File

@ -2,7 +2,9 @@ package migrations
import (
"context"
"fmt"
"io/ioutil"
"log"
"os"
"path/filepath"
"strings"
@ -126,7 +128,10 @@ func TestFetchMigrations(t *testing.T) {
defer os.RemoveAll(tmpDir)
needed := []string{"fs-repo-1-to-2", "fs-repo-2-to-3"}
fetched, err := fetchMigrations(ctx, fetcher, needed, tmpDir)
buf := new(strings.Builder)
buf.Grow(256)
logger := log.New(buf, "", 0)
fetched, err := fetchMigrations(ctx, fetcher, needed, tmpDir, logger)
if err != nil {
t.Fatal(err)
}
@ -137,6 +142,18 @@ func TestFetchMigrations(t *testing.T) {
t.Error("expected file to exist:", bin)
}
}
// Check expected log output
for _, mig := range needed {
logOut := fmt.Sprintf("Downloading migration: %s", mig)
if !strings.Contains(buf.String(), logOut) {
t.Fatalf("did not find expected log output %q", logOut)
}
logOut = fmt.Sprintf("Downloaded and unpacked migration: %s", filepath.Join(tmpDir, mig))
if !strings.Contains(buf.String(), logOut) {
t.Fatalf("did not find expected log output %q", logOut)
}
}
}
func TestRunMigrations(t *testing.T) {