Report metrics from tests

.github/workflows/unit_tests.yaml

@@ -38,9 +38,6 @@ jobs:
           max_attempts: 3
           timeout_minutes: 12
           command: |
-            # We want to validate that tests still pass, even if the metrics host
-            # points to a broken IP
-            echo "127.0.0.1 metriton.datawire.io" | sudo tee -a /etc/hosts
             make check-unit
   "windows":
     runs-on: windows-2019
@@ -65,11 +62,6 @@ jobs:
         run: choco install make
       - name: Build
         run: make build
-      - name: Update etc\hosts
-        run: |
-          # We want to validate that tests still pass, even if the metrics host
-          # points to a broken IP
-          echo "127.0.0.1 metriton.datawire.io" >> c:\windows\system32\drivers\etc\hosts
       - name: Lint
         run: make lint
       - name: Run tests
@@ -107,7 +99,4 @@ jobs:
           max_attempts: 3
           timeout_minutes: 10
           command: |
-            # We want to validate that tests still pass, even if the metrics host
-            # points to a broken IP
-            echo "127.0.0.1 metriton.datawire.io" | sudo tee -a /etc/hosts
             make check-unit

DEVELOPING.md

@@ -226,7 +226,7 @@ $ sudo id
 [sudo] password for lukeshu:
 uid=0(root) gid=0(root) groups=0(root)
 
-$ make make check-unit
+$ make check-unit
 [make] TELEPRESENCE_VERSION=v2.6.7-20-g9de10e316-1655892249
 ...
 ```
@@ -236,6 +236,18 @@ The first time you run the tests, you should use `make check`, to get
 binaries.  However, after that initial run, you can instead use
 `gotestsum` or `go test` if you prefer.
 
+### Test metric collection
+
+**When running in CI,** `make check-unit` and `make check-integration` will report the result of test
+runs to metriton, Ambassador Labs' metrics store. These reports include test name, running time, and
+result. They are reported by the tool at `tools/src/test-report`. This `test-report` tool will also
+visually modify test output; this happens even running locally, since the json output to go test
+is piped to the tool anyway:
+
+```console
+$ make check-unit
+```
+
 ## Building for Release
 
 See https://www.notion.so/datawire/To-Release-Telepresence-2-x-x-2752ef26968444b99d807979cde06f2f

Makefile

@@ -22,6 +22,8 @@ else
   TELEPRESENCE_VERSION := ${TELEPRESENCE_VERSION}
 endif
 
+SHELL:=$(shell which bash)
+
 $(if $(filter v2.%,$(TELEPRESENCE_VERSION)),\
   $(info [make] TELEPRESENCE_VERSION=$(TELEPRESENCE_VERSION)),\
   $(error TELEPRESENCE_VERSION variable is invalid: It must be a v2.* string, but is '$(TELEPRESENCE_VERSION)'))

build-aux/main.mk

@@ -83,6 +83,7 @@ generate: generate-clean
 generate: protoc $(tools/go-mkopensource) $(BUILDDIR)/$(shell go env GOVERSION).src.tar.gz
 	cd ./rpc && export GOFLAGS=-mod=mod && go mod tidy && go mod vendor && rm -rf vendor
 	cd ./pkg/vif/testdata/router && export GOFLAGS=-mod=mod && go mod tidy && go mod vendor && rm -rf vendor
+	cd ./tools/src/test-report && export GOFLAGS=-mod=mod && go mod tidy && go mod vendor && rm -rf vendor
 
 	export GOFLAGS=-mod=mod && go mod tidy && go mod vendor
 
@@ -333,22 +334,24 @@ format: lint-deps ## (QA) Automatically fix linter complaints
 check-all: check-integration check-unit ## (QA) Run the test suite
 
 .PHONY: check-unit
-check-unit: build-deps ## (QA) Run the test suite
+check-unit: build-deps $(tools/test-report) ## (QA) Run the test suite
 	# We run the test suite with TELEPRESENCE_LOGIN_DOMAIN set to localhost since that value
 	# is only used for extensions. Therefore, we want to validate that our tests, and
 	# telepresence, run without requiring any outside dependencies.
-	TELEPRESENCE_MAX_LOGFILES=300 TELEPRESENCE_LOGIN_DOMAIN=127.0.0.1 CGO_ENABLED=$(CGO_ENABLED) go test -failfast -timeout=20m ./cmd/... ./pkg/...
+	set -o pipefail
+	TELEPRESENCE_MAX_LOGFILES=300 SCOUT_DISABLE=1 TELEPRESENCE_LOGIN_DOMAIN=127.0.0.1 CGO_ENABLED=$(CGO_ENABLED) go test -json -failfast -timeout=20m ./cmd/... ./pkg/... | $(tools/test-report)
 
 .PHONY: check-integration
 ifeq ($(GOHOSTOS), linux)
-check-integration: client-image $(tools/helm) ## (QA) Run the test suite
+check-integration: client-image $(tools/test-report) $(tools/helm) ## (QA) Run the test suite
 else
-check-integration: build-deps $(tools/helm) ## (QA) Run the test suite
+check-integration: build-deps $(tools/test-report) $(tools/helm) ## (QA) Run the test suite
 endif
 	# We run the test suite with TELEPRESENCE_LOGIN_DOMAIN set to localhost since that value
 	# is only used for extensions. Therefore, we want to validate that our tests, and
 	# telepresence, run without requiring any outside dependencies.
-	TELEPRESENCE_MAX_LOGFILES=300 TELEPRESENCE_LOGIN_DOMAIN=127.0.0.1 CGO_ENABLED=$(CGO_ENABLED) go test -failfast -v -timeout=55m ./integration_test/...
+	set -o pipefail
+	TELEPRESENCE_MAX_LOGFILES=300 TELEPRESENCE_LOGIN_DOMAIN=127.0.0.1 CGO_ENABLED=$(CGO_ENABLED) go test -failfast -json -timeout=55m ./integration_test/... | $(tools/test-report)
 
 .PHONY: _login
 _login:

build-aux/tools.mk

@@ -79,10 +79,17 @@ $(TOOLSDIR)/$(PROTOLINT_TGZ):
 	mkdir -p $(@D)
 	tar -C $(@D) -zxmf $< protolint$(EXE) protoc-gen-protolint$(EXE)
 
-ifneq ($(GOHOSTOS),windows)
+# Test reporter
+# ==========
+#
+tools/test-report = $(TOOLSBINDIR)/test-report$(EXE)
+$(TOOLSBINDIR)/test-report$(EXE): $(TOOLSSRCDIR)/test-report/*.go $(TOOLSSRCDIR)/test-report/go.*
+	cd $(<D) && GOOS= GOARCH= go build -o $(abspath $@) *.go
+
 # Shellcheck
 # ==========
 #
+ifneq ($(GOHOSTOS),windows)
 tools/shellcheck = $(TOOLSBINDIR)/shellcheck
 SHELLCHECK_VERSION=0.8.0
 SHELLCHECK_ARCH=$(shell uname -m)

integration_test/itest/cluster.go

@@ -206,7 +206,7 @@ func (s *cluster) ensureQuit(ctx context.Context) {
 	_, _, _ = Telepresence(ctx, "quit", "-s") //nolint:dogsled // don't care about any of the returns
 
 	// Ensure that the daemon-socket is non-existent.
-	_ = rmAsRoot(socket.RootDaemonPath(ctx))
+	_ = rmAsRoot(ctx, socket.RootDaemonPath(ctx))
 }
 
 func (s *cluster) ensureExecutable(ctx context.Context, errs chan<- error, wg *sync.WaitGroup) {

integration_test/itest/rm_as_root_unix.go

@@ -4,8 +4,17 @@
 package itest
 
 //nolint:depguard // don't care about output or contexts
-import "os/exec"
+import (
+	"context"
+	"os/exec"
 
-func rmAsRoot(file string) error {
+	"github.com/telepresenceio/telepresence/v2/pkg/proc"
+)
+
+func rmAsRoot(ctx context.Context, file string) error {
+	// We just wanna make sure that the credentials are cached in a uniform way.
+	if err := proc.CacheAdmin(ctx, ""); err != nil {
+		return err
+	}
 	return exec.Command("sudo", "rm", "-f", file).Run()
 }

integration_test/itest/rm_as_root_windows.go

@@ -1,14 +1,15 @@
 package itest
 
 import (
+	"context"
 	"os"
 
 	"golang.org/x/sys/windows"
 
 	"github.com/telepresenceio/telepresence/v2/pkg/shellquote"
 )
 
-func rmAsRoot(file string) error {
+func rmAsRoot(ctx context.Context, file string) error {
 	cwd, _ := os.Getwd()
 	// UTF16PtrFromString can only fail if the argument contains a NUL byte. That will never happen here.
 	verbPtr, _ := windows.UTF16PtrFromString("runas")

pkg/proc/exec.go

@@ -115,3 +115,13 @@ func IsAdmin() bool {
 func Terminate(p *os.Process) error {
 	return terminate(p)
 }
+
+// CacheAdmin will ensure that the current process is able to invoke subprocesses with admin rights
+// without having to ask for the password again. This is needed among other things to make sure the
+// integration tests can see that a password is being asked for.
+func CacheAdmin(ctx context.Context, prompt string) error {
+	// These logs will get picked up by the test-reporter to make sure the user is asked for the password.
+	dlog.Info(ctx, "Asking for admin credentials")
+	defer dlog.Info(ctx, "Admin credentials acquired")
+	return cacheAdmin(ctx, prompt)
+}

pkg/proc/exec_unix.go

@@ -43,33 +43,43 @@ func startInBackground(includeEnv bool, args ...string) error {
 	return nil
 }
 
+func cacheAdmin(ctx context.Context, prompt string) error {
+	// If we're going to be prompting for the `sudo` password, we want to first provide
+	// the user with some info about exactly what we're prompting for.  We don't want to
+	// use `sudo`'s `--prompt` flag for this because (1) we don't want it to be
+	// re-displayed if they typo their password, and (2) it might be ignored anyway
+	// depending on `passprompt_override` in `/etc/sudoers`.  So we'll do a pre-flight
+	// `sudo --non-interactive true` to decide whether to display it.
+	//
+	// Note: Using `sudo --non-interactive --validate` does not work well in situations
+	// where the user has configured `myuser ALL=(ALL:ALL) NOPASSWD: ALL` in the sudoers
+	// file. Hence the use of `sudo --non-interactive true`. A plausible cause can be
+	// found in the first comment here:
+	// https://unix.stackexchange.com/questions/50584/why-sudo-timestamp-is-not-updated-when-nopasswd-is-set
+	needPwCmd := dexec.CommandContext(ctx, "sudo", "--non-interactive", "true")
+	needPwCmd.DisableLogging = true
+	if err := needPwCmd.Run(); err != nil {
+		if prompt != "" {
+			fmt.Println(prompt)
+		}
+		pwCmd := dexec.CommandContext(ctx, "sudo", "true")
+		pwCmd.DisableLogging = true
+		if err := pwCmd.Run(); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 func startInBackgroundAsRoot(ctx context.Context, args ...string) error {
 	if !isAdmin() {
-		// If we're going to be prompting for the `sudo` password, we want to first provide
-		// the user with some info about exactly what we're prompting for.  We don't want to
-		// use `sudo`'s `--prompt` flag for this because (1) we don't want it to be
-		// re-displayed if they typo their password, and (2) it might be ignored anyway
-		// depending on `passprompt_override` in `/etc/sudoers`.  So we'll do a pre-flight
-		// `sudo --non-interactive true` to decide whether to display it.
-		//
-		// Note: Using `sudo --non-interactive --validate` does not work well in situations
-		// where the user has configured `myuser ALL=(ALL:ALL) NOPASSWD: ALL` in the sudoers
-		// file. Hence the use of `sudo --non-interactive true`. A plausible cause can be
-		// found in the first comment here:
-		// https://unix.stackexchange.com/questions/50584/why-sudo-timestamp-is-not-updated-when-nopasswd-is-set
-		needPwCmd := dexec.CommandContext(ctx, "sudo", "--non-interactive", "true")
-		needPwCmd.DisableLogging = true
-		if err := needPwCmd.Run(); err != nil {
-			fmt.Printf("Need root privileges to run: %s\n", shellquote.ShellString(args[0], args[1:]))
-			// `sudo` won't be able to read the password from the terminal when we run
-			// it with Setpgid=true, so do a pre-flight `sudo true` to read the
-			// password, and then enforce that being re-used by passing
-			// `--non-interactive`.
-			pwCmd := dexec.CommandContext(ctx, "sudo", "true")
-			pwCmd.DisableLogging = true
-			if err := pwCmd.Run(); err != nil {
-				return err
-			}
+		// `sudo` won't be able to read the password from the terminal when we run
+		// it with Setpgid=true, so do a pre-flight `sudo true` (i.e. cacheAdmin) to read the
+		// password, and then enforce that being re-used by passing
+		// `--non-interactive`.
+		prompt := fmt.Sprintf("Need root privileges to run: %s", shellquote.ShellString(args[0], args[1:]))
+		if err := CacheAdmin(ctx, prompt); err != nil {
+			return err
 		}
 		args = append([]string{"sudo", "--non-interactive"}, args...)
 	}

pkg/proc/exec_windows.go

@@ -30,6 +30,11 @@ func createNewProcessGroup(cmd *exec.Cmd) {
 	cmd.SysProcAttr = &windows.SysProcAttr{CreationFlags: windows.CREATE_NEW_PROCESS_GROUP}
 }
 
+func cacheAdmin(_ context.Context, _ string) error {
+	// No-op on windows, there's no sudo caching. Runas will just pop a window open.
+	return nil
+}
+
 func startInBackground(_ bool, args ...string) error {
 	return shellExec("open", args[0], args[1:]...)
 }

pkg/vif/router_test.go

@@ -18,6 +18,7 @@ import (
 	"github.com/datawire/dlib/dgroup"
 	"github.com/datawire/dlib/dlog"
 	"github.com/telepresenceio/telepresence/v2/pkg/dos"
+	"github.com/telepresenceio/telepresence/v2/pkg/proc"
 	"github.com/telepresenceio/telepresence/v2/pkg/routing"
 	"github.com/telepresenceio/telepresence/v2/pkg/subnet"
 )
@@ -55,7 +56,7 @@ func (s *RoutingSuite) SetupSuite() {
 		s.Require().NoError(err)
 
 		// Run sudo to get a password prompt out of the way
-		err = dexec.CommandContext(context.Background(), "sudo", "true").Run()
+		err = proc.CacheAdmin(context.Background(), "")
 		s.Require().NoError(err)
 	}
 	// Make sure there's no existing route

tools/src/test-report/go.mod

@@ -0,0 +1,91 @@
+module local
+
+go 1.20
+
+replace github.com/telepresenceio/telepresence/rpc/v2 => ./../../../rpc
+
+replace github.com/telepresenceio/telepresence/v2 => ./../../..
+
+require (
+	github.com/datawire/metriton-go-client v0.1.1
+	github.com/fatih/color v1.15.0
+	github.com/telepresenceio/telepresence/v2 v2.0.0-00010101000000-000000000000
+	github.com/vbauerster/mpb/v8 v8.4.0
+)
+
+require (
+	github.com/VividCortex/ewma v1.2.0 // indirect
+	github.com/acarl005/stripansi v0.0.0-20180116102854-5a71ef0e047d // indirect
+	github.com/blang/semver v3.5.1+incompatible // indirect
+	github.com/datawire/dlib v1.3.1-0.20221228104658-e373c6d44961 // indirect
+	github.com/datawire/envconfig v0.0.0-20221012222025-09524dc7d59b // indirect
+	github.com/datawire/k8sapi v0.1.3 // indirect
+	github.com/davecgh/go-spew v1.1.1 // indirect
+	github.com/emicklei/go-restful/v3 v3.10.2 // indirect
+	github.com/evanphx/json-patch v5.6.0+incompatible // indirect
+	github.com/fsnotify/fsnotify v1.6.0 // indirect
+	github.com/go-errors/errors v1.4.2 // indirect
+	github.com/go-logr/logr v1.2.4 // indirect
+	github.com/go-openapi/jsonpointer v0.19.6 // indirect
+	github.com/go-openapi/jsonreference v0.20.2 // indirect
+	github.com/go-openapi/swag v0.22.3 // indirect
+	github.com/gogo/protobuf v1.3.2 // indirect
+	github.com/golang/protobuf v1.5.3 // indirect
+	github.com/google/btree v1.1.2 // indirect
+	github.com/google/gnostic v0.6.9 // indirect
+	github.com/google/go-cmp v0.5.9 // indirect
+	github.com/google/gofuzz v1.2.0 // indirect
+	github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 // indirect
+	github.com/google/uuid v1.3.0 // indirect
+	github.com/gregjones/httpcache v0.0.0-20190611155906-901d90724c79 // indirect
+	github.com/hashicorp/errwrap v1.1.0 // indirect
+	github.com/hashicorp/go-multierror v1.1.1 // indirect
+	github.com/imdario/mergo v0.3.15 // indirect
+	github.com/inconshreveable/mousetrap v1.1.0 // indirect
+	github.com/josharian/intern v1.0.1-0.20211109044230-42b52b674af5 // indirect
+	github.com/json-iterator/go v1.1.12 // indirect
+	github.com/liggitt/tabwriter v0.0.0-20181228230101-89fcab3d43de // indirect
+	github.com/mailru/easyjson v0.7.7 // indirect
+	github.com/mattn/go-colorable v0.1.13 // indirect
+	github.com/mattn/go-isatty v0.0.18 // indirect
+	github.com/mattn/go-runewidth v0.0.14 // indirect
+	github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
+	github.com/modern-go/reflect2 v1.0.2 // indirect
+	github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00 // indirect
+	github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
+	github.com/peterbourgon/diskv v2.0.1+incompatible // indirect
+	github.com/pkg/errors v0.9.1 // indirect
+	github.com/rivo/uniseg v0.4.4 // indirect
+	github.com/sirupsen/logrus v1.9.2 // indirect
+	github.com/spf13/cobra v1.7.0 // indirect
+	github.com/spf13/pflag v1.0.5 // indirect
+	github.com/telepresenceio/telepresence/rpc/v2 v2.14.1-rc.3 // indirect
+	github.com/xlab/treeprint v1.2.0 // indirect
+	go.starlark.net v0.0.0-20230302034142-4b1e35fe2254 // indirect
+	golang.org/x/exp v0.0.0-20230515195305-f3d0a9c9a5cc // indirect
+	golang.org/x/net v0.10.0 // indirect
+	golang.org/x/oauth2 v0.8.0 // indirect
+	golang.org/x/sys v0.9.0 // indirect
+	golang.org/x/term v0.8.0 // indirect
+	golang.org/x/text v0.9.0 // indirect
+	golang.org/x/time v0.3.0 // indirect
+	google.golang.org/appengine v1.6.7 // indirect
+	google.golang.org/genproto v0.0.0-20230410155749-daa745c078e1 // indirect
+	google.golang.org/grpc v1.55.0 // indirect
+	google.golang.org/protobuf v1.30.0 // indirect
+	gopkg.in/inf.v0 v0.9.1 // indirect
+	gopkg.in/yaml.v2 v2.4.0 // indirect
+	gopkg.in/yaml.v3 v3.0.1 // indirect
+	k8s.io/api v0.27.2 // indirect
+	k8s.io/apimachinery v0.27.2 // indirect
+	k8s.io/cli-runtime v0.27.2 // indirect
+	k8s.io/client-go v0.27.2 // indirect
+	k8s.io/klog/v2 v2.100.1 // indirect
+	k8s.io/kube-openapi v0.0.0-20230515203736-54b630e78af5 // indirect
+	k8s.io/utils v0.0.0-20230505201702-9f6742963106 // indirect
+	sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
+	sigs.k8s.io/kustomize/api v0.13.4 // indirect
+	sigs.k8s.io/kustomize/kyaml v0.14.2 // indirect
+	sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect
+	sigs.k8s.io/yaml v1.3.0 // indirect
+)