cosmovisor: various small improvements (#7275)

{,cosmosvisor/}Makefile: Add Makefile to cosmovisor/
subdirectory and integrate build target into the
top-directory Makefile.

cosmovisor/:

Create Golang-idiomatic cmd/ subdirectory to make
`cosmosvisor` binary (instead of a binary named `cmd`)
automatically installable with `go get`.

Minor changes to error handling:

- Plain error strings bring more value than the whole
  stacktrace in case of dumb configuration errors.

- Output errors to /dev/stderr instead of /dev/stdout.

Remove dependency on github.com/pkg/errors.

.github/workflows/test.yml

@@ -29,7 +29,7 @@ jobs:
             .mod
             .sum
       - name: Run cosmovisor tests
-        run: cd cosmovisor; go test .
+        run: cd cosmovisor; make
         if: "env.GIT_DIFF != ''"
   split-test-files:
     runs-on: ubuntu-latest

Makefile

@@ -93,7 +93,10 @@ build-simd: go.sum
 build-simd-linux: go.sum
 	LEDGER_ENABLED=false GOOS=linux GOARCH=amd64 $(MAKE) build-simd
 
-.PHONY: build build-simd build-simd-linux
+cosmovisor:
+	$(MAKE) -C cosmovisor cosmovisor
+
+.PHONY: build build-simd build-simd-linux cosmovisor
 
 mocks: $(MOCKS_DIR)
 	mockgen -source=client/account_retriever.go -package mocks -destination tests/mocks/account_retriever.go

cosmovisor/Makefile

@@ -0,0 +1,12 @@
+#!/usr/bin/make -f
+
+
+all: cosmovisor test
+
+cosmovisor:
+	go build -mod=readonly ./cmd/cosmovisor
+
+test:
+	go test -mod=readonly -race ./...
+
+.PHONY: all cosmovisor test

cosmovisor/args.go

@@ -1,11 +1,11 @@
 package cosmovisor
 
 import (
+	"errors"
+	"fmt"
 	"net/url"
 	"os"
 	"path/filepath"
-
-	"github.com/pkg/errors"
 )
 
 const (
@@ -80,8 +80,7 @@ func (cfg *Config) CurrentBin() (string, error) {
 	}
 
 	// and return the binary
-	dest = filepath.Join(dest, "bin", cfg.Name)
+	return filepath.Join(dest, "bin", cfg.Name), nil
-	return dest, nil
 }
 
 // GetConfigFromEnv will read the environmental variables into a config
@@ -91,15 +90,19 @@ func GetConfigFromEnv() (*Config, error) {
 		Home: os.Getenv("DAEMON_HOME"),
 		Name: os.Getenv("DAEMON_NAME"),
 	}
+
 	if os.Getenv("DAEMON_ALLOW_DOWNLOAD_BINARIES") == "true" {
 		cfg.AllowDownloadBinaries = true
 	}
+
 	if os.Getenv("DAEMON_RESTART_AFTER_UPGRADE") == "true" {
 		cfg.RestartAfterUpgrade = true
 	}
+
 	if err := cfg.validate(); err != nil {
 		return nil, err
 	}
+
 	return cfg, nil
 }
 
@@ -110,6 +113,7 @@ func (cfg *Config) validate() error {
 	if cfg.Name == "" {
 		return errors.New("DAEMON_NAME is not set")
 	}
+
 	if cfg.Home == "" {
 		return errors.New("DAEMON_HOME is not set")
 	}
@@ -121,10 +125,11 @@ func (cfg *Config) validate() error {
 	// ensure the root directory exists
 	info, err := os.Stat(cfg.Root())
 	if err != nil {
-		return errors.Wrap(err, "cannot stat home dir")
+		return fmt.Errorf("cannot stat home dir: %w", err)
 	}
+
 	if !info.IsDir() {
-		return errors.Errorf("%s is not a directory", info.Name())
+		return fmt.Errorf("%s is not a directory", info.Name())
 	}
 
 	return nil

cosmovisor/cmd/cosmovisor/main.go

@@ -4,13 +4,12 @@ import (
 	"fmt"
 	"os"
 
-	cosmovisor "github.com/cosmos/cosmos-sdk/cosmovisor"
+	"github.com/cosmos/cosmos-sdk/cosmovisor"
 )
 
 func main() {
-	err := Run(os.Args[1:])
+	if err := Run(os.Args[1:]); err != nil {
-	if err != nil {
+		fmt.Fprintf(os.Stderr, "%+v\n", err)
-		fmt.Printf("%+v\n", err)
 		os.Exit(1)
 	}
 }
@@ -21,8 +20,8 @@ func Run(args []string) error {
 	if err != nil {
 		return err
 	}
-	doUpgrade, err := cosmovisor.LaunchProcess(cfg, args, os.Stdout, os.Stderr)
 
+	doUpgrade, err := cosmovisor.LaunchProcess(cfg, args, os.Stdout, os.Stderr)
 	// if RestartAfterUpgrade, we launch after a successful upgrade (only condition LaunchProcess returns nil)
 	for cfg.RestartAfterUpgrade && err == nil && doUpgrade {
 		doUpgrade, err = cosmovisor.LaunchProcess(cfg, args, os.Stdout, os.Stderr)

cosmovisor/go.mod

@@ -5,6 +5,5 @@ go 1.14
 require (
 	github.com/hashicorp/go-getter v1.4.1
 	github.com/otiai10/copy v1.2.0
-	github.com/pkg/errors v0.9.1
 	github.com/stretchr/testify v1.6.1
 )

cosmovisor/go.sum

@@ -69,8 +69,6 @@ github.com/otiai10/curr v1.0.0/go.mod h1:LskTG5wDwr8Rs+nNQ+1LlxRjAtTZZjtJW4rMXl6
 github.com/otiai10/mint v1.3.0/go.mod h1:F5AjcsTsWUqX+Na9fpHb52P8pcRX2CI6A3ctIT91xUo=
 github.com/otiai10/mint v1.3.1 h1:BCmzIS3n71sGfHB5NMNDB3lHYPz8fWSkCAErHed//qc=
 github.com/otiai10/mint v1.3.1/go.mod h1:/yxELlJQ0ufhjUwhshSj+wFjZ78CnZ48/1wtmBH1OTc=
-github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
-github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=

cosmovisor/process.go

@@ -2,12 +2,11 @@ package cosmovisor
 
 import (
 	"bufio"
+	"fmt"
 	"io"
 	"os/exec"
 	"strings"
 	"sync"
-
-	"github.com/pkg/errors"
 )
 
 // LaunchProcess runs a subprocess and returns when the subprocess exits,
@@ -15,11 +14,11 @@ import (
 func LaunchProcess(cfg *Config, args []string, stdout, stderr io.Writer) (bool, error) {
 	bin, err := cfg.CurrentBin()
 	if err != nil {
-		return false, errors.Wrap(err, "error creating symlink to genesis")
+		return false, fmt.Errorf("error creating symlink to genesis: %w", err)
 	}
-	err = EnsureBinary(bin)
+
-	if err != nil {
+	if err := EnsureBinary(bin); err != nil {
-		return false, errors.Wrap(err, "current binary invalid")
+		return false, fmt.Errorf("current binary invalid: %w", err)
 	}
 
 	cmd := exec.Command(bin, args...)
@@ -27,16 +26,17 @@ func LaunchProcess(cfg *Config, args []string, stdout, stderr io.Writer) (bool,
 	if err != nil {
 		return false, err
 	}
+
 	errpipe, err := cmd.StderrPipe()
 	if err != nil {
 		return false, err
 	}
+
 	scanOut := bufio.NewScanner(io.TeeReader(outpipe, stdout))
 	scanErr := bufio.NewScanner(io.TeeReader(errpipe, stderr))
 
-	err = cmd.Start()
+	if err := cmd.Start(); err != nil {
-	if err != nil {
+		return false, fmt.Errorf("launching process %s %s: %w", bin, strings.Join(args, " "), err)
-		return false, errors.Wrapf(err, "launching process %s %s", bin, strings.Join(args, " "))
 	}
 
 	// three ways to exit - command ends, find regexp in scanOut, find regexp in scanErr
@@ -44,6 +44,7 @@ func LaunchProcess(cfg *Config, args []string, stdout, stderr io.Writer) (bool,
 	if err != nil {
 		return false, err
 	}
+
 	if upgradeInfo != nil {
 		return true, DoUpgrade(cfg, upgradeInfo)
 	}

cosmovisor/upgrade.go

@@ -2,6 +2,7 @@ package cosmovisor
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"net/url"
@@ -11,41 +12,38 @@ import (
 	"strings"
 
 	"github.com/hashicorp/go-getter"
-	"github.com/pkg/errors"
 )
 
 // DoUpgrade will be called after the log message has been parsed and the process has terminated.
 // We can now make any changes to the underlying directory without interference and leave it
 // in a state, so we can make a proper restart
 func DoUpgrade(cfg *Config, info *UpgradeInfo) error {
-	err := EnsureBinary(cfg.UpgradeBin(info.Name))
-
 	// Simplest case is to switch the link
+	err := EnsureBinary(cfg.UpgradeBin(info.Name))
 	if err == nil {
 		// we have the binary - do it
 		return cfg.SetCurrentUpgrade(info.Name)
 	}
-
 	// if auto-download is disabled, we fail
 	if !cfg.AllowDownloadBinaries {
-		return errors.Wrap(err, "binary not present, downloading disabled")
+		return fmt.Errorf("binary not present, downloading disabled: %w", err)
 	}
+
 	// if the dir is there already, don't download either
-	_, err = os.Stat(cfg.UpgradeDir(info.Name))
+	if _, err := os.Stat(cfg.UpgradeDir(info.Name)); !os.IsNotExist(err) {
-	if !os.IsNotExist(err) {
+		return errors.New("upgrade dir already exists, won't overwrite")
-		return errors.Errorf("upgrade dir already exists, won't overwrite")
 	}
 
 	// If not there, then we try to download it... maybe
 	if err := DownloadBinary(cfg, info); err != nil {
-		return errors.Wrap(err, "cannot download binary")
+		return fmt.Errorf("cannot download binary: %w", err)
 	}
 
 	// and then set the binary again
-	err = EnsureBinary(cfg.UpgradeBin(info.Name))
+	if err := EnsureBinary(cfg.UpgradeBin(info.Name)); err != nil {
-	if err != nil {
+		return fmt.Errorf("downloaded binary doesn't check out: %w", err)
-		return errors.Wrap(err, "downloaded binary doesn't check out")
 	}
+
 	return cfg.SetCurrentUpgrade(info.Name)
 }
 
@@ -77,7 +75,7 @@ func DownloadBinary(cfg *Config, info *UpgradeInfo) error {
 func MarkExecutable(path string) error {
 	info, err := os.Stat(path)
 	if err != nil {
-		return errors.Wrap(err, "stating binary")
+		return fmt.Errorf("stating binary: %w", err)
 	}
 	// end early if world exec already set
 	if info.Mode()&0001 == 1 {
@@ -100,17 +98,18 @@ func GetDownloadURL(info *UpgradeInfo) (string, error) {
 	if _, err := url.Parse(doc); err == nil {
 		tmpDir, err := ioutil.TempDir("", "upgrade-manager-reference")
 		if err != nil {
-			return "", errors.Wrap(err, "create tempdir for reference file")
+			return "", fmt.Errorf("create tempdir for reference file: %w", err)
 		}
 		defer os.RemoveAll(tmpDir)
+
 		refPath := filepath.Join(tmpDir, "ref")
-		err = getter.GetFile(refPath, doc)
+		if err := getter.GetFile(refPath, doc); err != nil {
-		if err != nil {
+			return "", fmt.Errorf("downloading reference link %s: %w", doc, err)
-			return "", errors.Wrapf(err, "downloading reference link %s", doc)
 		}
+
 		refBytes, err := ioutil.ReadFile(refPath)
 		if err != nil {
-			return "", errors.Wrap(err, "reading downloaded reference")
+			return "", fmt.Errorf("reading downloaded reference: %w", err)
 		}
 		// if download worked properly, then we use this new file as the binary map to parse
 		doc = string(refBytes)
@@ -118,12 +117,13 @@ func GetDownloadURL(info *UpgradeInfo) (string, error) {
 
 	// check if it is the upgrade config
 	var config UpgradeConfig
-	err := json.Unmarshal([]byte(doc), &config)
+
-	if err == nil {
+	if err := json.Unmarshal([]byte(doc), &config); err == nil {
 		url, ok := config.Binaries[osArch()]
 		if !ok {
-			return "", errors.Errorf("cannot find binary for os/arch: %s", osArch())
+			return "", fmt.Errorf("cannot find binary for os/arch: %s", osArch())
 		}
+
 		return url, nil
 	}
 
@@ -138,6 +138,7 @@ func osArch() string {
 func (cfg *Config) SetCurrentUpgrade(upgradeName string) error {
 	// ensure named upgrade exists
 	bin := cfg.UpgradeBin(upgradeName)
+
 	if err := EnsureBinary(bin); err != nil {
 		return err
 	}
@@ -154,8 +155,9 @@ func (cfg *Config) SetCurrentUpgrade(upgradeName string) error {
 
 	// point to the new directory
 	if err := os.Symlink(upgrade, link); err != nil {
-		return errors.Wrap(err, "creating current symlink")
+		return fmt.Errorf("creating current symlink: %w", err)
 	}
+
 	return nil
 }
 
@@ -163,15 +165,18 @@ func (cfg *Config) SetCurrentUpgrade(upgradeName string) error {
 func EnsureBinary(path string) error {
 	info, err := os.Stat(path)
 	if err != nil {
-		return errors.Wrap(err, "cannot stat home dir")
+		return fmt.Errorf("cannot stat dir %s: %w", path, err)
 	}
+
 	if !info.Mode().IsRegular() {
-		return errors.Errorf("%s is not a regular file", info.Name())
+		return fmt.Errorf("%s is not a regular file", info.Name())
 	}
+
 	// this checks if the world-executable bit is set (we cannot check owner easily)
 	exec := info.Mode().Perm() & 0001
 	if exec == 0 {
-		return errors.Errorf("%s is not world executable", info.Name())
+		return fmt.Errorf("%s is not world executable", info.Name())
 	}
+
 	return nil
 }

cosmovisor/upgrade_test.go

@@ -12,7 +12,6 @@ import (
 
 	copy2 "github.com/otiai10/copy"
 
-	"github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -270,7 +269,7 @@ func TestDownloadBinary(t *testing.T) {
 func copyTestData(subdir string) (string, error) {
 	tmpdir, err := ioutil.TempDir("", "upgrade-manager-test")
 	if err != nil {
-		return "", errors.Wrap(err, "create temp dir")
+		return "", fmt.Errorf("couldn't create temporary directory: %w", err)
 	}
 
 	src := filepath.Join("testdata", subdir)
@@ -278,7 +277,8 @@ func copyTestData(subdir string) (string, error) {
 	err = copy2.Copy(src, tmpdir)
 	if err != nil {
 		os.RemoveAll(tmpdir)
-		return "", errors.Wrap(err, "copying files")
+		return "", fmt.Errorf("couldn't copy files: %w", err)
 	}
+
 	return tmpdir, nil
 }