Review fixes and logging integration

go/examples/pingpong/pp_integration/pingpong.go

@@ -32,26 +32,19 @@ func main() {
 }
 
 func realMain() int {
-	if err := integration.Init(); err != nil {
+	if err := integration.Init("pp_integration"); err != nil {
 		fmt.Fprintf(os.Stderr, "Failed to init: %s\n", err)
 		return 1
 	}
 	defer log.LogPanicAndExit()
-	asList, err := integration.LoadASList()
-	if err != nil {
-		fmt.Fprintf(os.Stderr, "Failed to load AS-list: %s\n", err)
-		return 1
-	}
-	// TODO(lukedirtwalker) we should enable logging
-	// depending on the main log parameter it should either go to a file or to console stderr.
+	defer log.Flush()
 	in := integration.NewBinaryIntegration("./bin/pingpong",
-		[]string{"-mode", "client", "-sciondFromIA", "-count", "1",
+		[]string{"-mode", "client", "-sciondFromIA", "-log.console", "debug", "-count", "1",
 			"-local", integration.SrcIAReplace + ",[127.0.0.1]:0",
 			"-remote", integration.DstIAReplace + ",[127.0.0.1]:" + serverPort},
-		[]string{"-mode", "server", "-sciondFromIA",
+		[]string{"-mode", "server", "-sciondFromIA", "-log.console", "debug",
 			"-local", integration.DstIAReplace + ",[127.0.0.1]:" + serverPort})
-	pairs := integration.GenerateAllSrcDst(integration.AllIAs(asList), integration.AllIAs(asList))
-	if err = runTests(in, pairs); err != nil {
+	if err := runTests(in, integration.IAPairs()); err != nil {
 		fmt.Fprintf(os.Stderr, "Failed to run tests: %s\n", err)
 		return 1
 	}
@@ -71,10 +64,12 @@ func runTests(in integration.Integration, pairs []integration.IAPair) error {
 			}
 			defer c.Close()
 		}
+		// give the servers some time to start.
+		time.Sleep(100 * time.Millisecond)
 		// Now start the clients for srcDest pair
 		for i, conn := range pairs {
 			log.Info(fmt.Sprintf("Test %v: %v -> %v (%v/%v)",
-				in.Name(), conn.Src, conn.Dst, i, len(pairs)))
+				in.Name(), conn.Src, conn.Dst, i+1, len(pairs)))
 
 			if err := integration.RunClient(in, conn, 1*time.Second); err != nil {
 				fmt.Fprintf(os.Stderr, "Error during client execution: %s\n", err)

go/lib/integration/binary.go

@@ -16,10 +16,13 @@ package integration
 
 import (
 	"context"
+	"io"
 	"os/exec"
 	"strings"
 
 	"github.com/scionproto/scion/go/lib/addr"
+	"github.com/scionproto/scion/go/lib/log"
+	"github.com/scionproto/scion/go/lib/log/logparse"
 )
 
 const (
@@ -58,6 +61,11 @@ func (bi *binaryIntegration) StartServer(ctx context.Context, dst addr.IA) (Wait
 	r := &binaryWaiter{
 		exec.CommandContext(ctx, bi.name, args...),
 	}
+	ep, err := r.StderrPipe()
+	if err != nil {
+		return nil, err
+	}
+	go redirectLog("Server", "ServerErr", ep)
 	return r, r.Start()
 }
 
@@ -67,6 +75,11 @@ func (bi *binaryIntegration) StartClient(ctx context.Context, src, dst addr.IA)
 	r := &binaryWaiter{
 		exec.CommandContext(ctx, bi.name, args...),
 	}
+	ep, err := r.StderrPipe()
+	if err != nil {
+		return nil, err
+	}
+	go redirectLog("Client", "ClientErr", ep)
 	return r, r.Start()
 }
 
@@ -81,6 +94,31 @@ func replacePattern(pattern string, replacement string, args []string) []string
 	return argsCopy
 }
 
+func redirectLog(name, pName string, ep io.ReadCloser) {
+	defer ep.Close()
+	logparse.ParseFrom(ep, pName, pName, func(e logparse.LogEntry) {
+		logLogEntry(name, e)
+	})
+}
+
+func logLogEntry(name string, e logparse.LogEntry) {
+	var logFun func(string, ...interface{})
+	switch e.Level {
+	case logparse.LvlDebug:
+		logFun = log.Debug
+	case logparse.LvlInfo:
+		logFun = log.Info
+	case logparse.LvlWarn:
+		logFun = log.Warn
+	case logparse.LvlError:
+		logFun = log.Error
+	case logparse.LvlCrit:
+		logFun = log.Crit
+	}
+	indent := strings.Repeat(" ", len(name)+2)
+	logFun(name + ": " + strings.Join(e.Lines, "\n"+indent))
+}
+
 var _ Waiter = (*binaryWaiter)(nil)
 
 type binaryWaiter struct {

go/lib/integration/integration.go

@@ -22,13 +22,41 @@ import (
 	"io"
 	"math/rand"
 	"os"
+	"strings"
 	"time"
 
 	"github.com/scionproto/scion/go/lib/addr"
 	"github.com/scionproto/scion/go/lib/log"
 	"github.com/scionproto/scion/go/lib/util"
 )
 
+type iaArgs []addr.IA
+
+func (a *iaArgs) String() string {
+	rawIAs := make([]string, len(*a))
+	for i, ia := range *a {
+		rawIAs[i] = ia.String()
+	}
+	return strings.Join(rawIAs, ",")
+}
+
+func (a *iaArgs) Set(value string) error {
+	rawIAs := strings.Split(value, ",")
+	for _, rawIA := range rawIAs {
+		ia, err := addr.IAFromString(rawIA)
+		if err != nil {
+			return err
+		}
+		*a = append(*a, ia)
+	}
+	return nil
+}
+
+var (
+	srcIAs iaArgs
+	dstIAs iaArgs
+)
+
 // Integration can be used to run integration tests.
 type Integration interface {
 	// Name returns the name of the test
@@ -50,44 +78,53 @@ type Waiter interface {
 	Wait() error
 }
 
-// Init initializes the integration test, it adds and validates the command line flags.
-func Init() error {
+// Init initializes the integration test, it adds and validates the command line flags,
+// and initializes logging.
+func Init(name string) error {
 	addTestFlags()
-	return validateFlags()
+	return validateFlags(name)
 }
 
 func addTestFlags() {
 	log.AddLogConsFlags()
+	log.AddLogFileFlags()
+	flag.Var(&srcIAs, "src", "Source ISD-ASes (comma separated list)")
+	flag.Var(&dstIAs, "dst", "Destination ISD-ASes (comma separated list)")
 }
 
-func validateFlags() error {
+func validateFlags(name string) error {
 	flag.Parse()
-	if err := log.SetupFromFlags(""); err != nil {
+	if err := log.SetupFromFlags(name); err != nil {
 		fmt.Fprintf(os.Stderr, "ERROR: %s\n", err)
 		flag.Usage()
 		return err
 	}
+	asList, err := util.LoadASList("gen/as_list.yml")
+	if err != nil {
+		return err
+	}
+	if len(srcIAs) == 0 {
+		srcIAs = asList.AllASes()
+	}
+	if len(dstIAs) == 0 {
+		dstIAs = asList.AllASes()
+	}
 	return nil
 }
 
-// LoadASList loads the AS list from the as_list yaml file.
-func LoadASList() (*util.ASList, error) {
-	return util.LoadASList("gen/as_list.yml")
-}
-
-// AllIAs returns all IA from asList.
-func AllIAs(asList *util.ASList) []addr.IA {
-	return append([]addr.IA(nil), append(asList.Core, asList.NonCore...)...)
-}
-
 // IAPair is a source, destination pair. The client (Src) will dial the server (Dst).
 type IAPair struct {
 	Src addr.IA
 	Dst addr.IA
 }
 
+// IAPairs returns all IAPairs that should be tested.
+func IAPairs() []IAPair {
+	return generateAllSrcDst(srcIAs, dstIAs)
+}
+
 // GenerateAllSrcDst generates the cartesian product shuffle(srcASes) x shuffle(dstASes).
-func GenerateAllSrcDst(srcASes, dstASes []addr.IA) []IAPair {
+func generateAllSrcDst(srcASes, dstASes []addr.IA) []IAPair {
 	shuffle(len(srcASes), func(i, j int) {
 		srcASes[i], srcASes[j] = srcASes[j], srcASes[i]
 	})
@@ -124,7 +161,7 @@ func (s *serverStop) Close() error {
 
 // StartServer runs a server. The server can be stopped by calling Close() on the returned Closer.
 func StartServer(in Integration, dst addr.IA) (io.Closer, error) {
-	serverCtx, serverCancel := context.WithCancel(context.Background()) // add method to run a single server that returns a closer (go chanel to close).
+	serverCtx, serverCancel := context.WithCancel(context.Background())
 	s, err := in.StartServer(serverCtx, dst)
 	if err != nil {
 		serverCancel()

go/lib/util/aslist.go

@@ -68,3 +68,8 @@ func parse(names []string) ([]addr.IA, error) {
 	}
 	return iaList, nil
 }
+
+// AllASes returns all ASes in the ASList as a slice.
+func (al *ASList) AllASes() []addr.IA {
+	return append([]addr.IA(nil), append(al.Core, al.NonCore...)...)
+}