Repository: istio/fortio (go)

Issue:
eliminate (most) log.Fatalf from library code
log.Critf instead and return nil or similar (original error) and let the called crash instead of the library


Pull request:
Adding fnet.Proxy* for tcp proxying and -P multi flag for cmd line
Start of #160. Also Fixes #205 (less log.Fatalf)

This is to do ingress->svc1->svc2  without closing connections unlike the /fetch/ endpoint does

Patch:
diff --git a/Dockerfile b/Dockerfile
index 4e19b0d..f62bbcb 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -6,27 +6,34 @@ COPY . fortio
 RUN make -C fortio submodule
 # Putting spaces in linker replaced variables is hard but does work.
 RUN echo "$(date +'%Y-%m-%d %H:%M') $(cd fortio; git rev-parse HEAD)" > /build-info.txt
+RUN echo "-s -X istio.io/fortio/ui.resourcesDir=/usr/local/lib/fortio -X main.defaultDataDir=/var/lib/istio/fortio \
+  -X \"istio.io/fortio/version.buildInfo=$(cat /build-info.txt)\" \
+  -X istio.io/fortio/version.tag=$(cd fortio; git describe --tags) \
+  -X istio.io/fortio/version.gitstatus=$(cd fortio; git status --porcelain | wc -l)" > /link-flags.txt
 # Sets up the static directory outside of the go source tree and
 # the default data directory to a /var/lib/... volume
 RUN go version
-RUN CGO_ENABLED=0 GOOS=linux go build -a -ldflags \
-  "-s -X istio.io/fortio/ui.resourcesDir=/usr/local/lib/fortio -X main.defaultDataDir=/var/lib/istio/fortio \
-  -X \"istio.io/fortio/version.buildInfo=$(cat /build-info.txt)\" \
-  -X istio.io/fortio/version.tag=$(cd fortio; git describe --tags) \
-  -X istio.io/fortio/version.gitstatus=$(cd fortio; git status --porcelain | wc -l)" \
-  -o fortio.bin istio.io/fortio
+RUN CGO_ENABLED=0 GOOS=linux go build -a -ldflags "$(cat /link-flags.txt)" -o fortio_go1.10.bin istio.io/fortio
+RUN ./fortio_go1.10.bin version
 # Check we still build with go 1.8 (and macos does not break)
 RUN /usr/local/go/bin/go version
-RUN CGO_ENABLED=0 GOOS=darwin /usr/local/go/bin/go build -a -ldflags -s -o fortio.go18.mac istio.io/fortio
+RUN CGO_ENABLED=0 GOOS=darwin /usr/local/go/bin/go build -a -ldflags "$(cat /link-flags.txt)" -o fortio_go1.8.mac istio.io/fortio
+# Build with 1.8 for perf comparaison
+#RUN CGO_ENABLED=0 GOOS=linux /usr/local/go/bin/go build -a -ldflags "$(cat /link-flags.txt)" -o fortio_go1.8.bin istio.io/fortio
+#RUN ./fortio_go1.8.bin version
 # Just check it stays compiling on Windows (would need to set the rsrcDir too)
 RUN CGO_ENABLED=0 GOOS=windows go build -a -o fortio.exe istio.io/fortio
+#RUN ln -s /usr/local/bin/fortio_go1.8 /usr/local/bin/fortio
+#RUN tar cvf /tmp/symlink.tar /usr/local/bin/fortio
 # Minimal image with just the binary and certs
 FROM scratch as release
 # NOTE: the list of files here, if updated, must be changed in release/Dockerfile.in too
 COPY --from=build /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/
 COPY --from=build /go/src/istio.io/fortio/ui/static /usr/local/lib/fortio/static
 COPY --from=build /go/src/istio.io/fortio/ui/templates /usr/local/lib/fortio/templates
-COPY --from=build /go/src/istio.io/fortio.bin /usr/local/bin/fortio
+#COPY --from=build /go/src/istio.io/fortio_go1.10.bin /usr/local/bin/fortio_go1.10
+#COPY --from=build /go/src/istio.io/fortio_go1.8.bin /usr/local/bin/fortio_go1.8
+COPY --from=build /go/src/istio.io/fortio_go1.10.bin /usr/local/bin/fortio
 EXPOSE 8079
 EXPOSE 8080
 EXPOSE 8081
diff --git a/README.md b/README.md
index d889028..544f932 100644
--- a/README.md
+++ b/README.md
@@ -10,7 +10,7 @@ Fortio (Φορτίο) is [Istio](https://istio.io/)'s load testing tool. Fortio
 
 The name fortio comes from greek [φορτίο](https://translate.google.com/translate_tts?q=Φορτίο&tl=el&tk=452076.38818&client=t) which means load/burden.
 
-Fortio is a fast, small, reusable, embeddable go library as well as a command line tool and server process, the server includes a simple web UI and graphical representation of the results (both a single latency graph and a multiple results comparative min, max, avg and percentiles graphs).
+Fortio is a fast, small (3Mb docker image, minimal dependencies), reusable, embeddable go library as well as a command line tool and server process, the server includes a simple web UI and graphical representation of the results (both a single latency graph and a multiple results comparative min, max, avg and percentiles graphs).
 
 ## Installation
 
@@ -44,12 +44,15 @@ Fortio can be an http or grpc load generator, gathering statistics using the `lo
 Φορτίο 0.8.0 usage:
 	fortio command [flags] target
 where command is one of: load (load testing), server (starts grpc ping and http
-echo/ui/redirect servers), grpcping (grpc client), report (report only UI
+echo/ui/redirect/proxy servers), grpcping (grpc client), report (report only UI
 server), redirect (redirect only server), or curl (single URL debug).  where
 target is a url (http load tests) or host:port (grpc health test) and flags are:
   -H value
 	Additional Header(s)
   -L	Follow redirects (implies -std-client) - do not use for load test
+  -P value
+	Proxies to run, e.g -P "localport1 dest_host1:dest_port1" 
+	-P "[::1]:0 www.google.com:443" ...
   -a	Automatically save JSON result with filename based on labels & timestamp
   -allow-initial-errors
 	Allow and don't abort on initial warmup errors
diff --git a/echosrv/echo.go b/echosrv/echo.go
index a23c096..ec3a82f 100644
--- a/echosrv/echo.go
+++ b/echosrv/echo.go
@@ -21,6 +21,7 @@ package main
 
 import (
 	"flag"
+	"os"
 
 	"istio.io/fortio/fhttp"
 )
@@ -32,6 +33,8 @@ var (
 
 func main() {
 	flag.Parse()
-	fhttp.Serve(*port, *debugPath)
+	if _, addr := fhttp.Serve(*port, *debugPath); addr == nil {
+		os.Exit(1) // error already logged
+	}
 	select {}
 }
diff --git a/fgrpc/grpcrunner.go b/fgrpc/grpcrunner.go
index ce10ecd..1d3a4ed 100644
--- a/fgrpc/grpcrunner.go
+++ b/fgrpc/grpcrunner.go
@@ -159,7 +159,10 @@ func RunGRPCTest(o *GRPCRunnerOptions) (*GRPCRunnerResults, error) {
 			}
 			total.RetCodes[k] += grpcstate[i].RetCodes[k]
 		}
+		// TODO: if grpc client needs 'cleanup'/Close like http one, do it on original NumThreads
 	}
+	// Cleanup state:
+	r.Options().ReleaseRunners()
 	for _, k := range keys {
 		fmt.Fprintf(out, "Health %s : %d\n", k.String(), total.RetCodes[k])
 	}
diff --git a/fgrpc/pingsrv.go b/fgrpc/pingsrv.go
index 2456610..cf3663f 100644
--- a/fgrpc/pingsrv.go
+++ b/fgrpc/pingsrv.go
@@ -53,6 +53,9 @@ func (s *pingSrv) Ping(c context.Context, in *PingMessage) (*PingMessage, error)
 // to be marked as SERVING.
 func PingServer(port string, healthServiceName string) int {
 	socket, addr := fnet.Listen("grpc '"+healthServiceName+"'", port)
+	if addr == nil {
+		return -1
+	}
 	grpcServer := grpc.NewServer()
 	reflection.Register(grpcServer)
 	healthServer := health.NewServer()
diff --git a/fhttp/http_server.go b/fhttp/http_server.go
index f526a29..701e9cb 100644
--- a/fhttp/http_server.go
+++ b/fhttp/http_server.go
@@ -158,6 +158,9 @@ func HTTPServer(name string, port string) (*http.ServeMux, *net.TCPAddr) {
 		Handler: m,
 	}
 	listener, addr := fnet.Listen(name, port)
+	if addr == nil {
+		return nil, nil // error already logged
+	}
 	go func() {
 		err := s.Serve(listener)
 		if err != nil {
@@ -308,6 +311,9 @@ func DebugHandler(w http.ResponseWriter, r *http.Request) {
 func Serve(port, debugPath string) (*http.ServeMux, *net.TCPAddr) {
 	startTime = time.Now()
 	mux, addr := HTTPServer("echo", port)
+	if addr == nil {
+		return nil, nil // error already logged
+	}
 	if debugPath != "" {
 		mux.HandleFunc(debugPath, DebugHandler)
 	}
@@ -373,6 +379,9 @@ func RedirectToHTTPSHandler(w http.ResponseWriter, r *http.Request) {
 // (Do not create a loop, make sure this is addressed from an ingress)
 func RedirectToHTTPS(port string) *net.TCPAddr {
 	m, a := HTTPServer("https redirector", port)
+	if m == nil {
+		return nil // error already logged
+	}
 	m.HandleFunc("/", RedirectToHTTPSHandler)
 	return a
 }
diff --git a/fhttp/httprunner.go b/fhttp/httprunner.go
index c05f1a6..e083197 100644
--- a/fhttp/httprunner.go
+++ b/fhttp/httprunner.go
@@ -141,6 +141,8 @@ func RunHTTPTest(o *HTTPRunnerOptions) (*HTTPRunnerResults, error) {
 		total.sizes.Transfer(httpstate[i].sizes)
 		total.headerSizes.Transfer(httpstate[i].headerSizes)
 	}
+	// Cleanup state:
+	r.Options().ReleaseRunners()
 	sort.Ints(keys)
 	totalCount := float64(total.DurationHistogram.Count)
 	fmt.Fprintf(out, "Sockets used: %d (for perfect keepalive, would be %d)\n", total.SocketCount, r.Options().NumThreads)
diff --git a/fnet/network.go b/fnet/network.go
index a3be284..479f9dd 100644
--- a/fnet/network.go
+++ b/fnet/network.go
@@ -16,8 +16,10 @@ package fnet // import "istio.io/fortio/fnet"
 
 import (
 	"fmt"
+	"io"
 	"net"
 	"strings"
+	"sync"
 
 	"istio.io/fortio/log"
 	"istio.io/fortio/version"
@@ -35,14 +37,14 @@ func NormalizePort(port string) string {
 // Listen returns a listener for the port. Port can be a port or a
 // bind address and a port (e.g. "8080" or "[::1]:8080"...). If the
 // port component is 0 a free port will be returned by the system.
-// This logs fatal on error and is meant for servers that must start.
-// For library use, we could extract into 2 functions, one returning
-// error,... if needed.
+// This logs critical on error and returns nil (is meant for servers
+// that must start).
 func Listen(name string, port string) (net.Listener, *net.TCPAddr) {
 	nPort := NormalizePort(port)
 	listener, err := net.Listen("tcp", nPort)
 	if err != nil {
-		log.Fatalf("Can't listen to %v: %v", nPort, err)
+		log.Critf("Can't listen to %v: %v", nPort, err)
+		return nil, nil
 	}
 	addr := listener.Addr().(*net.TCPAddr)
 	if len(name) > 0 {
@@ -54,7 +56,7 @@ func Listen(name string, port string) (net.Listener, *net.TCPAddr) {
 // ResolveDestination returns the TCP address of the "host:port" suitable for net.Dial.
 // nil in case of errors.
 func ResolveDestination(dest string) *net.TCPAddr {
-	i := strings.LastIndex(dest, ":")
+	i := strings.LastIndex(dest, ":") // important so [::1]:port works
 	if i < 0 {
 		log.Errf("Destination '%s' is not host:port format", dest)
 		return nil
@@ -67,17 +69,29 @@ func ResolveDestination(dest string) *net.TCPAddr {
 // Resolve returns the TCP address of the host,port suitable for net.Dial.
 // nil in case of errors.
 func Resolve(host string, port string) *net.TCPAddr {
-	addrs, err := net.LookupIP(host)
-	if err != nil {
-		log.Errf("Unable to lookup '%s' : %v", host, err)
-		return nil
+	dest := &net.TCPAddr{}
+	if strings.HasPrefix(host, "[") && strings.HasSuffix(host, "]") {
+		log.Debugf("host %s looks like an IPv6, stripping []", host)
+		host = host[1 : len(host)-1]
 	}
-	if len(addrs) > 1 && log.LogDebug() {
-		log.Debugf("Using only the first of the addresses for %s : %v", host, addrs)
+	isAddr := net.ParseIP(host)
+	var err error
+	if isAddr != nil {
+		log.Debugf("Host already an IP, will go to %s", isAddr)
+		dest.IP = isAddr
+	} else {
+		var addrs []net.IP
+		addrs, err = net.LookupIP(host)
+		if err != nil {
+			log.Errf("Unable to lookup '%s' : %v", host, err)
+			return nil
+		}
+		if len(addrs) > 1 && log.LogDebug() {
+			log.Debugf("Using only the first of the addresses for %s : %v", host, addrs)
+		}
+		log.Debugf("Will go to %s", addrs[0])
+		dest.IP = addrs[0]
 	}
-	log.Debugf("Will go to %s", addrs[0])
-	dest := &net.TCPAddr{}
-	dest.IP = addrs[0]
 	dest.Port, err = net.LookupPort("tcp", port)
 	if err != nil {
 		log.Errf("Unable to resolve port '%s' : %v", port, err)
@@ -86,15 +100,62 @@ func Resolve(host string, port string) *net.TCPAddr {
 	return dest
 }
 
-/*
-// Proxy starts a tcp proxy.
-func Proxy(port string, dest *net.TCPAddr) {
-	l, a := Listen("proxy", port)
+func transfer(wg *sync.WaitGroup, dst *net.TCPConn, src *net.TCPConn) {
+	n, oErr := io.Copy(dst, src) // keep original error for logs below
+	log.LogVf("Proxy: transferred %d bytes from %v to %v (err=%v)", n, src.RemoteAddr(), dst.RemoteAddr(), oErr)
+	err := src.CloseRead()
+	if err != nil { // We got an eof so it's already half closed.
+		log.LogVf("Proxy: semi expected error CloseRead on src %v: %v,%v", src.RemoteAddr(), err, oErr)
+	}
+	err = dst.CloseWrite()
+	if err != nil {
+		log.Errf("Proxy: error CloseWrite on dst %v: %v,%v", dst.RemoteAddr(), err, oErr)
+	}
+	wg.Done()
+}
+
+func handleProxyRequest(conn *net.TCPConn, dest *net.TCPAddr) {
 	d, err := net.DialTCP("tcp", nil, dest)
 	if err != nil {
-		log.Errf("Unable to connect to %v : %v", dest, err)
-		return nil
+		log.Errf("Proxy: unable to connect to %v for %v : %v", dest, conn.RemoteAddr(), err)
+		_ = conn.Close()
+		return
+	}
+	var wg sync.WaitGroup
+	wg.Add(2)
+	go transfer(&wg, d, conn)
+	transfer(&wg, conn, d)
+	wg.Wait()
+	log.LogVf("Proxy: both sides of transfer to %v for %v done", dest, conn.RemoteAddr())
+	// Not checking as we are closing/ending anyway - note: bad side effect of coverage...
+	_ = d.Close()
+	_ = conn.Close()
+}
+
+// Proxy starts a tcp proxy.
+func Proxy(port string, dest *net.TCPAddr) *net.TCPAddr {
+	listener, addr := Listen("proxy for "+dest.String(), port)
+	if addr == nil {
+		return nil // error already logged
 	}
+	go func() {
+		for {
+			conn, err := listener.Accept()
+			if err != nil {
+				log.Critf("Proxy: error accepting: %v", err) // will this loop with error?
+			} else {
+				tcpConn := conn.(*net.TCPConn)
+				log.LogVf("Proxy: Accepted proxy connection from %v for %v", conn.RemoteAddr(), dest)
+				// TODO limit number of go request, use worker pool, etc...
+				go handleProxyRequest(tcpConn, dest)
+			}
+		}
+	}()
+	return addr
+}
 
+// ProxyToDestination opens a proxy from the listenPort (or addr:port) and forwards
+// all traffic to destination (host:port)
+func ProxyToDestination(listenPort string, destination string) *net.TCPAddr {
+	return Proxy(listenPort, ResolveDestination(destination))
 }
-*/
diff --git a/fortio_main.go b/fortio_main.go
index d849cd3..c89f62c 100644
--- a/fortio_main.go
+++ b/fortio_main.go
@@ -26,6 +26,8 @@ import (
 	"runtime"
 	"strings"
 
+	"istio.io/fortio/fnet"
+
 	"istio.io/fortio/fgrpc"
 	"istio.io/fortio/fhttp"
 	"istio.io/fortio/log"
@@ -35,21 +37,31 @@ import (
 	"istio.io/fortio/version"
 )
 
-var httpOpts fhttp.HTTPOptions
-
 // -- Support for multiple instances of -H flag on cmd line:
-type flagList struct {
+type headersFlagList struct {
 }
 
-// Unclear when/why this is called and necessary
-func (f *flagList) String() string {
+func (f *headersFlagList) String() string {
 	return ""
 }
-func (f *flagList) Set(value string) error {
+func (f *headersFlagList) Set(value string) error {
 	return httpOpts.AddAndValidateExtraHeader(value)
 }
 
 // -- end of functions for -H support
+// -- Support for multiple proxies (-P) flags on cmd line:
+type proxiesFlagList struct {
+}
+
+func (f *proxiesFlagList) String() string {
+	return ""
+}
+func (f *proxiesFlagList) Set(value string) error {
+	proxies = append(proxies, value)
+	return nil
+}
+
+// -- end of functions for -P support
 
 // Prints usage
 func usage(msgs ...interface{}) {
@@ -58,7 +70,7 @@ func usage(msgs ...interface{}) {
 		version.Short(),
 		os.Args[0],
 		"where command is one of: load (load testing), server (starts grpc ping and",
-		"http echo/ui/redirect servers), grpcping (grpc client), report (report only UI",
+		"http echo/ui/redirect/proxy servers), grpcping (grpc client), report (report only UI",
 		"server), redirect (redirect only server), or curl (single URL debug).",
 		"where target is a url (http load tests) or host:port (grpc health test)",
 		"and flags are:")
@@ -98,9 +110,13 @@ var (
 	curlFlag   = flag.Bool("curl", false, "Just fetch the content once")
 	labelsFlag = flag.String("labels", "",
 		"Additional config data/labels to add to the resulting JSON, defaults to target URL and hostname")
-	staticDirFlag  = flag.String("static-dir", "", "Absolute path to the dir containing the static files dir")
-	dataDirFlag    = flag.String("data-dir", defaultDataDir, "Directory where JSON results are stored/read")
-	headersFlags   flagList
+	staticDirFlag = flag.String("static-dir", "", "Absolute path to the dir containing the static files dir")
+	dataDirFlag   = flag.String("data-dir", defaultDataDir, "Directory where JSON results are stored/read")
+	headersFlags  headersFlagList
+	httpOpts      fhttp.HTTPOptions
+	proxiesFlags  proxiesFlagList
+	proxies       = make([]string, 0)
+
 	defaultDataDir = "."
 
 	followRedirectsFlag    = flag.Bool("L", false, "Follow redirects (implies -std-client) - do not use for load test")
@@ -128,6 +144,7 @@ var (
 
 func main() {
 	flag.Var(&headersFlags, "H", "Additional Header(s)")
+	flag.Var(&proxiesFlags, "P", "Proxies to run, e.g -P \"localport1 dest_host1:dest_port1\" -P \"[::1]:0 www.google.com:443\" ...")
 	flag.IntVar(&fhttp.BufferSizeKb, "httpbufferkb", fhttp.BufferSizeKb,
 		"Size of the buffer (max data size) for the optimized http client in kbytes")
 	flag.BoolVar(&fhttp.CheckConnectionClosedHeader, "httpccch", fhttp.CheckConnectionClosedHeader,
@@ -177,14 +194,25 @@ func main() {
 		if *redirectFlag != "disabled" {
 			fhttp.RedirectToHTTPS(*redirectFlag)
 		}
-		ui.Report(baseURL, *echoPortFlag, *staticDirFlag, *dataDirFlag)
+		if !ui.Report(baseURL, *echoPortFlag, *staticDirFlag, *dataDirFlag) {
+			os.Exit(1) // error already logged
+		}
 	case "server":
 		isServer = true
 		fgrpc.PingServer(*grpcPortFlag, fgrpc.DefaultHealthServiceName)
 		if *redirectFlag != "disabled" {
 			fhttp.RedirectToHTTPS(*redirectFlag)
 		}
-		ui.Serve(baseURL, *echoPortFlag, *echoDbgPathFlag, *uiPathFlag, *staticDirFlag, *dataDirFlag, percList)
+		if !ui.Serve(baseURL, *echoPortFlag, *echoDbgPathFlag, *uiPathFlag, *staticDirFlag, *dataDirFlag, percList) {
+			os.Exit(1) // error already logged
+		}
+		for _, proxy := range proxies {
+			s := strings.SplitN(proxy, " ", 2)
+			if len(s) != 2 {
+				log.Errf("Invalid syntax for proxy \"%s\", should be \"localAddr destHost:destPort\"", proxy)
+			}
+			fnet.ProxyToDestination(s[0], s[1])
+		}
 	case "grpcping":
 		grpcClient()
 	default:
diff --git a/periodic/periodic.go b/periodic/periodic.go
index 0f6f3ba..7afb8ab 100644
--- a/periodic/periodic.go
+++ b/periodic/periodic.go
@@ -66,6 +66,13 @@ func (r *RunnerOptions) MakeRunners(rr Runnable) {
 	}
 }
 
+// ReleaseRunners clear the runners state.
+func (r *RunnerOptions) ReleaseRunners() {
+	for idx := range r.Runners {
+		r.Runners[idx] = nil
+	}
+}
+
 // Aborter is the object controlling Abort() of the runs.
 type Aborter struct {
 	sync.Mutex
@@ -256,14 +263,18 @@ func (r *RunnerOptions) Abort() {
 	}
 }
 
-// internal version, returning the concrete implementation.
+// internal version, returning the concrete implementation. logical std::move
 func newPeriodicRunner(opts *RunnerOptions) *periodicRunner {
 	r := &periodicRunner{*opts} // by default just copy the input params
+	opts.ReleaseRunners()
+	opts.Stop = nil
 	r.Normalize()
 	return r
 }
 
 // NewPeriodicRunner constructs a runner from input parameters/options.
+// The options will be moved and normalized to the returned object, do
+// not use the original options after this call, call Options() instead.
 // Abort() must be called if Run() is not called.
 func NewPeriodicRunner(params *RunnerOptions) PeriodicRunner {
 	return newPeriodicRunner(params)
diff --git a/ui/uihandler.go b/ui/uihandler.go
index 1668344..eeb5a1e 100644
--- a/ui/uihandler.go
+++ b/ui/uihandler.go
@@ -790,13 +790,16 @@ func downloadOne(w http.ResponseWriter, client *fhttp.Client, name string, u str
 
 // Serve starts the fhttp.Serve() plus the UI server on the given port
 // and paths (empty disables the feature). uiPath should end with /
-// (be a 'directory' path)
-func Serve(baseurl, port, debugpath, uipath, staticRsrcDir string, datadir string, percentileList []float64) {
+// (be a 'directory' path). Returns true if server is started successfully.
+func Serve(baseurl, port, debugpath, uipath, staticRsrcDir string, datadir string, percentileList []float64) bool {
 	baseURL = baseurl
 	startTime = time.Now()
 	mux, addr := fhttp.Serve(port, debugpath)
+	if addr == nil {
+		return false // Error already logged
+	}
 	if uipath == "" {
-		return
+		return true
 	}
 	fhttp.SetupPPROF(mux)
 	uiPath = uipath
@@ -854,16 +857,20 @@ func Serve(baseurl, port, debugpath, uipath, staticRsrcDir string, datadir strin
 	}
 	fmt.Printf(uiMsg + "\n")
 	defaultPercentileList = percentileList
+	return true
 }
 
 // Report starts the browsing only UI server on the given port.
 // Similar to Serve with only the read only part.
-func Report(baseurl, port, staticRsrcDir string, datadir string) {
+func Report(baseurl, port, staticRsrcDir string, datadir string) bool {
 	// drop the pprof default handlers [shouldn't be needed with custom mux but better safe than sorry]
 	http.DefaultServeMux = http.NewServeMux()
 	baseURL = baseurl
 	extraBrowseLabel = ", report only limited UI"
 	mux, addr := fhttp.HTTPServer("report", port)
+	if addr == nil {
+		return false
+	}
 	setHostAndPort(port, addr)
 	uiMsg := fmt.Sprintf("Browse only UI started - visit:\nhttp://%s/", urlHostPort)
 	if !strings.Contains(port, ":") {
@@ -888,6 +895,7 @@ func Report(baseurl, port, staticRsrcDir string, datadir string) {
 	}
 	fsd := http.FileServer(http.Dir(dataDir))
 	mux.Handle(uiPath+"data/", LogAndFilterDataRequest(http.StripPrefix(uiPath+"data", fsd)))
+	return true
 }
 
 // setHostAndPort takes hostport in the form of hostname:port, ip:port or :port,


Test patch:
diff --git a/fgrpc/pingsrv_test.go b/fgrpc/pingsrv_test.go
index b7aea1f..1502794 100644
--- a/fgrpc/pingsrv_test.go
+++ b/fgrpc/pingsrv_test.go
@@ -17,6 +17,7 @@ package fgrpc
 
 import (
 	"fmt"
+	"strconv"
 	"testing"
 
 	"google.golang.org/grpc/health/grpc_health_v1"
@@ -47,4 +48,9 @@ func TestPingServer(t *testing.T) {
 	if r, err := GrpcHealthCheck(addr, false, "willfail", 1); err == nil || r != nil {
 		t.Errorf("Was expecting error when using unknown service, didn't get one, got %+v", r)
 	}
+	// 2nd server on same port should fail to bind:
+	newPort := PingServer(strconv.Itoa(port), "will fail")
+	if newPort != -1 {
+		t.Errorf("Didn't expect 2nd server on same port to succeed: %d %d", newPort, port)
+	}
 }
diff --git a/fhttp/http_test.go b/fhttp/http_test.go
index e9d4f91..b0e9d29 100644
--- a/fhttp/http_test.go
+++ b/fhttp/http_test.go
@@ -22,6 +22,7 @@ import (
 	"net/http"
 	"net/url"
 	"os"
+	"strconv"
 	"strings"
 	"testing"
 	"time"
@@ -755,6 +756,14 @@ func TestFetchAndOnBehalfOf(t *testing.T) {
 	}
 }
 
+func TestServeError(t *testing.T) {
+	_, addr := Serve("0", "")
+	mux2, addr2 := Serve(strconv.Itoa(addr.Port), "")
+	if mux2 != nil || addr2 != nil {
+		t.Errorf("2nd Serve() on same port %d should have failed: %v %v", addr.Port, mux2, addr2)
+	}
+}
+
 func TestRedirector(t *testing.T) {
 	addr := RedirectToHTTPS(":0")
 	relativeURL := "/foo/bar?some=param&anotherone"
@@ -768,6 +777,12 @@ func TestRedirector(t *testing.T) {
 	if !bytes.Contains(data, []byte("Location: https://foo.istio.io"+relativeURL)) {
 		t.Errorf("Result %s doesn't contain Location: redirect", DebugSummary(data, 1024))
 	}
+	// 2nd one should fail
+	addr2 := RedirectToHTTPS(strconv.Itoa(addr.Port))
+	if addr2 != nil {
+		t.Errorf("2nd RedirectToHTTPS() on same port %d should have failed: %v", addr.Port, addr2)
+
+	}
 }
 
 var testNeedEscape = "<a href='http://google.com'>link</a>"
diff --git a/fhttp/httprunner_test.go b/fhttp/httprunner_test.go
index 471c788..75e6b4a 100644
--- a/fhttp/httprunner_test.go
+++ b/fhttp/httprunner_test.go
@@ -103,6 +103,7 @@ func testHTTPNotLeaking(t *testing.T, opts *HTTPRunnerOptions) {
 	res, err = RunHTTPTest(opts)
 	// it takes a while for the connections to close with std client (!) why isn't CloseIdleConnections() synchronous
 	runtime.GC()
+	runtime.GC() // 2x to clean up more... (#178)
 	ngAfter := runtime.NumGoroutine()
 	t.Logf("Number of go routine after 2nd test %d", ngAfter)
 	if err != nil {
@@ -113,8 +114,8 @@ func testHTTPNotLeaking(t *testing.T, opts *HTTPRunnerOptions) {
 	if opts.Exactly != httpOk {
 		t.Errorf("Run2: Mismatch between requested calls %d and ok %v", numCalls, res.RetCodes)
 	}
-	// allow for ~5 goroutine variance, as we use 50 if we leak it will show (was failing before #167)
-	if ngAfter > ngBefore2+5 {
+	// allow for ~8 goroutine variance, as we use 50 if we leak it will show (was failing before #167)
+	if ngAfter > ngBefore2+8 {
 		t.Errorf("Goroutines after test %d, expected it to stay near %d", ngAfter, ngBefore2)
 	}
 	if !opts.DisableFastClient {
diff --git a/fnet/network_test.go b/fnet/network_test.go
index cf7b56d..710ad9c 100644
--- a/fnet/network_test.go
+++ b/fnet/network_test.go
@@ -15,10 +15,13 @@
 package fnet
 
 import (
+	"net"
 	"strconv"
+	"strings"
 	"testing"
 
 	"istio.io/fortio/log"
+	"istio.io/fortio/version"
 )
 
 func TestNormalizePort(t *testing.T) {
@@ -58,7 +61,7 @@ func TestNormalizePort(t *testing.T) {
 }
 
 func TestListen(t *testing.T) {
-	l, a := Listen("test listen", "0")
+	l, a := Listen("test listen1", "0")
 	if l == nil || a == nil {
 		t.Fatalf("Unexpected nil in Listen() %v %v", l, a)
 	}
@@ -69,22 +72,14 @@ func TestListen(t *testing.T) {
 }
 
 func TestListenFailure(t *testing.T) {
-	reached := false
-	defer func(rp *bool) {
-		if r := recover(); r == nil {
-			t.Error("expected a panic from listen, didn't get one")
-		}
-		if !*rp {
-			t.Error("didn't reach expected statement")
-		}
-	}(&reached)
-	_, a1 := Listen("test listen1", "0")
+	_, a1 := Listen("test listen2", "0")
 	if a1.Port == 0 {
 		t.Errorf("Unexpected 0 port after listen %+v", a1)
 	}
-	reached = true // last reached statement
-	_, _ = Listen("this should fail", strconv.Itoa(a1.Port))
-	t.Error("should not reach this")
+	l, a := Listen("this should fail", strconv.Itoa(a1.Port))
+	if l != nil || a != nil {
+		t.Errorf("listen that should error got %v %v instead of nil", l, a)
+	}
 }
 
 func TestResolveDestination(t *testing.T) {
@@ -122,6 +117,60 @@ func TestResolveDestinationMultipleIps(t *testing.T) {
 		t.Error("got nil address for google")
 	}
 }
+
+func TestProxy(t *testing.T) {
+	addr := ProxyToDestination(":0", "www.google.com:80")
+	dAddr := net.TCPAddr{Port: addr.Port}
+	d, err := net.DialTCP("tcp", nil, &dAddr)
+	if err != nil {
+		t.Fatalf("can't connect to our proxy: %v", err)
+	}
+	defer d.Close()
+	data := "HEAD / HTTP/1.0\r\nUser-Agent: fortio-unit-test-" + version.Long() + "\r\n\r\n"
+	d.Write([]byte(data))
+	d.CloseWrite()
+	res := make([]byte, 4096)
+	n, err := d.Read(res)
+	if err != nil {
+		t.Errorf("read error with proxy: %v", err)
+	}
+	resStr := string(res[:n])
+	expectedStart := "HTTP/1.0 200 OK\r\n"
+	if !strings.HasPrefix(resStr, expectedStart) {
+		t.Errorf("Unexpected reply '%q', expected starting with '%q'", resStr, expectedStart)
+	}
+}
+
+func TestProxyErrors(t *testing.T) {
+	addr := ProxyToDestination(":0", "doesnotexist.istio.io:80")
+	dAddr := net.TCPAddr{Port: addr.Port}
+	d, err := net.DialTCP("tcp", nil, &dAddr)
+	if err != nil {
+		t.Fatalf("can't connect to our proxy: %v", err)
+	}
+	defer d.Close()
+	res := make([]byte, 4096)
+	n, err := d.Read(res)
+	if err == nil {
+		t.Errorf("didn't get expected error with proxy %d", n)
+	}
+	// 2nd proxy on same port should fail
+	addr2 := ProxyToDestination(strconv.Itoa(addr.Port), "www.google.com:80")
+	if addr2 != nil {
+		t.Errorf("Second proxy on same port should have failed, got %+v", addr2)
+	}
+}
+func TestResolveIpV6(t *testing.T) {
+	addr := Resolve("[::1]", "http")
+	addrStr := addr.String()
+	expected := "[::1]:80"
+	if addrStr != expected {
+		t.Errorf("Got '%s' instead of '%s'", addrStr, expected)
+	}
+}
+
+// --- max logging for tests
+
 func init() {
 	log.SetLogLevel(log.Debug)
 }
diff --git a/periodic/periodic_test.go b/periodic/periodic_test.go
index 331f3c2..e8756e6 100644
--- a/periodic/periodic_test.go
+++ b/periodic/periodic_test.go
@@ -65,6 +65,7 @@ func TestNewPeriodicRunner(t *testing.T) {
 			t.Errorf("threads: with %d input got %d, not as expected %d",
 				tst.numThreads, r.NumThreads, tst.expectedNumThreads)
 		}
+		r.ReleaseRunners()
 	}
 }
 
@@ -113,6 +114,7 @@ func TestStart(t *testing.T) {
 	if count != 2 {
 		t.Errorf("Test executed unexpected number of times %d instead minimum 2", count)
 	}
+	r.Options().ReleaseRunners()
 }
 
 func TestStartMaxQps(t *testing.T) {
@@ -139,6 +141,7 @@ func TestStartMaxQps(t *testing.T) {
 	if count != expected {
 		t.Errorf("MaxQpsTest executed unexpected number of times %d instead %d", count, expected)
 	}
+	r.Options().ReleaseRunners()
 }
 
 func TestExactlyLargeDur(t *testing.T) {
@@ -164,6 +167,7 @@ func TestExactlyLargeDur(t *testing.T) {
 	if count != expected {
 		t.Errorf("Exact count executed unexpected number of times %d instead %d", count, expected)
 	}
+	r.Options().ReleaseRunners()
 }
 
 func TestExactlySmallDur(t *testing.T) {
@@ -189,6 +193,7 @@ func TestExactlySmallDur(t *testing.T) {
 	if count != expected {
 		t.Errorf("Exact count executed unexpected number of times %d instead %d", count, expected)
 	}
+	r.Options().ReleaseRunners()
 }
 
 func TestExactlyMaxQps(t *testing.T) {
@@ -214,6 +219,7 @@ func TestExactlyMaxQps(t *testing.T) {
 	if count != expected {
 		t.Errorf("Exact count executed unexpected number of times %d instead %d", count, expected)
 	}
+	r.Options().ReleaseRunners()
 }
 
 func TestID(t *testing.T) {
@@ -263,12 +269,13 @@ func TestInfiniteDurationAndAbort(t *testing.T) {
 		r.Options().Abort()
 	}()
 	r.Run()
-	if count < 9 || count > 12 {
-		t.Errorf("Test executed unexpected number of times %d instead of 9-12", count)
+	if count < 9 || count > 13 {
+		t.Errorf("Test executed unexpected number of times %d instead of 9-13", count)
 	}
 	// Same with infinite qps
 	count = 0
 	o.QPS = -1 // infinite qps
+	r.Options().ReleaseRunners()
 	r = NewPeriodicRunner(&o)
 	r.Options().MakeRunners(&c)
 	go func() {
@@ -279,8 +286,9 @@ func TestInfiniteDurationAndAbort(t *testing.T) {
 		gAbortMutex.Unlock()
 	}()
 	r.Run()
-	if count != 3 { // should get 3 in 140ms
-		t.Errorf("Test executed unexpected number of times %d instead of %d", count, 3)
+	r.Options().ReleaseRunners()
+	if count < 2 || count > 4 { // should get 3 in 140ms
+		t.Errorf("Test executed unexpected number of times %d instead of 3 (2-4)", count)
 	}
 }
 
@@ -302,8 +310,9 @@ func TestExactlyAndAbort(t *testing.T) {
 		r.Options().Abort()
 	}()
 	res := r.Run()
-	if count < 9 || count > 12 {
-		t.Errorf("Test executed unexpected number of times %d instead of 9-12", count)
+	r.Options().ReleaseRunners()
+	if count < 9 || count > 13 {
+		t.Errorf("Test executed unexpected number of times %d instead of 9-13", count)
 	}
 	if !strings.Contains(res.RequestedDuration, "exactly 100 calls, interrupted after") {
 		t.Errorf("Got '%s' and didn't find expected aborted", res.RequestedDuration)
@@ -323,14 +332,16 @@ func TestSleepFallingBehind(t *testing.T) {
 	r.Options().MakeRunners(&c)
 	count = 0
 	res := r.Run()
+	r.Options().ReleaseRunners()
 	expected := int64(3 * 4) // can start 3 50ms in 140ms * 4 threads
 	// Check the count both from the histogram and from our own test counter:
 	actual := res.DurationHistogram.Count
-	if actual != expected {
+	if actual > expected+2 || actual < expected-2 {
 		t.Errorf("Extra high qps executed unexpected number of times %d instead %d", actual, expected)
 	}
-	if count != expected {
-		t.Errorf("Extra high qps executed unexpected number of times %d instead %d", count, expected)
+	// check histogram and our counter got same result
+	if count != actual {
+		t.Errorf("Extra high qps internal counter %d doesn't match histogram %d for expected %d", count, actual, expected)
 	}
 }