simpler, additional e2e functional test + auth test (istio#833)

Automatic merge from submit-queue

simpler, additional e2e functional test + auth test

This is for a couple things:

a) an e2e test (using the e2e framework) that has minimum  dependencies and fast start which can be use as a starting point for new tests that do not rely on bookinfo
b) exercise fortio images, ingress rule svc2sc
c) detect whether auth is working or not
d) bug fix in fortio when server errors out
e) faster test start and end (and updated README with the faster instructions)

_background/justification_
We all spent a lot of time debugging complex end2end test over the last few weeks,
Also people have been complaining that writing new tests is hard (which is partially true as I learned through writing one), and we also don't have test verifying auth is really on

This PR makes progress toward solving all 3 problems, introducing a simpler (than mixer/bookinfo) test. 

A simpler test lets us quickly smoke test basic features.

This is additional to tests that exists in pilot (as this is about end2end with all the actual components in place, same code as we release)

e2e is not exclusively "website tasks testing" and should have as many sane tests as we can have

A separate effort - though this simpler test does help already - is to optimize the runtime and probably split test into "every PR" tests and "nightly"... etc... 

```release-note
New simpler e2e functional tests, including an auth test.
```

Former-commit-id: fad04b1

devel/fortio/cmd/echosrv/echo.go

@@ -26,10 +26,11 @@ import (
 )
 
 var (
-	port = flag.Int("port", 8080, "default http port")
+	port      = flag.Int("port", 8080, "default http port")
+	debugPath = flag.String("debug-path", "/debug", "path for debug url, set to empty for no debug")
 )
 
 func main() {
 	flag.Parse()
-	fortio.EchoServer(*port)
+	fortio.EchoServer(*port, *debugPath)
 }

devel/fortio/cmd/fortio/main.go

@@ -69,6 +69,7 @@ var (
 	grpcFlag        = flag.Bool("grpc", false, "Use GRPC (health check) for load testing")
 	echoPortFlag    = flag.Int("http-port", 8080, "http echo server port")
 	grpcPortFlag    = flag.Int("grpc-port", 8079, "grpc port")
+	echoDbgPathFlag = flag.String("echo-debug-path", "/debug", "http echo server URI for debug, empty turns off that part (more secure)")
 
 	headersFlags flagList
 	percList     []float64
@@ -94,7 +95,7 @@ func main() {
 	case "load":
 		fortioLoad()
 	case "server":
-		go fortio.EchoServer(*echoPortFlag)
+		go fortio.EchoServer(*echoPortFlag, *echoDbgPathFlag)
 		pingServer(*grpcPortFlag)
 	case "grpcping":
 		grpcClient()

devel/fortio/http.go

@@ -59,7 +59,7 @@ func init() {
 
 // Version is the fortio package version (TODO:auto gen/extract).
 const (
-	Version       = "0.2.2"
+	Version       = "0.2.7"
 	userAgent     = "istio/fortio-" + Version
 	retcodeOffset = len("HTTP/1.X ")
 )
@@ -506,6 +506,7 @@ func DebugSummary(buf []byte, max int) string {
 func (c *BasicClient) readResponse(conn *net.TCPConn) {
 	max := len(c.buffer)
 	parsedHeaders := false
+	// TODO: safer to start with -1 and fix ok for http 1.0
 	c.code = http.StatusOK // In http 1.0 mode we don't bother parsing anything
 	endofHeadersStart := retcodeOffset + 3
 	keepAlive := c.keepAlive
@@ -514,10 +515,16 @@ func (c *BasicClient) readResponse(conn *net.TCPConn) {
 	for {
 		n, err := conn.Read(c.buffer[c.size:])
 		if err == io.EOF {
+			if c.size == 0 {
+				Errf("EOF before reading anything on %v %v", conn, c.dest)
+				c.code = -1
+			}
 			break
 		}
 		if err != nil {
 			Errf("Read error %v %v %d : %v", conn, c.dest, c.size, err)
+			c.code = -1
+			break
 		}
 		c.size += n
 		if Log(Debug) {
@@ -608,7 +615,7 @@ func (c *BasicClient) readResponse(conn *net.TCPConn) {
 					}
 				}
 			}
-		}
+		} // end of big if parse header
 		if c.size >= max {
 			if !keepAlive {
 				Errf("More data is available but stopping after %d, increase -httpbufferkb", max)
@@ -644,15 +651,17 @@ func (c *BasicClient) readResponse(conn *net.TCPConn) {
 			}
 			break // we're done!
 		}
-	}
+	} // end of big for loop
 	// Figure out whether to keep or close the socket:
 	if keepAlive && c.code == http.StatusOK {
 		c.socket = conn // keep the open socket
 	} else {
 		if err := conn.Close(); err != nil {
 			Errf("Close error %v %v %d : %v", conn, c.dest, c.size, err)
+		} else {
+			Debugf("Closed ok %v from %v after reading %d bytes", conn, c.dest, c.size)
 		}
-		// we cleared c.socket already
+		// we cleared c.socket in caller already
 	}
 }
 
@@ -669,7 +678,7 @@ func EchoHandler(w http.ResponseWriter, r *http.Request) {
 	if LogDebug() {
 		for name, headers := range r.Header {
 			for _, h := range headers {
-				fmt.Printf("%v: %v\n", name, h)
+				Debugf("Header %v: %v\n", name, h)
 			}
 		}
 	}
@@ -696,19 +705,45 @@ func EchoHandler(w http.ResponseWriter, r *http.Request) {
 	}
 }
 
-// DynamicHTTPServer listens on an available port and return it.
-func DynamicHTTPServer() int {
+func closingServer(listener net.Listener) (err error) {
+	for {
+		c, err := listener.Accept()
+		if err != nil {
+			Errf("Accept error in dummy server %v", err)
+			break
+		}
+		LogVf("Got connection from %v, closing", c.RemoteAddr())
+		err = c.Close()
+		if err != nil {
+			Errf("Close error in dummy server %v", err)
+			break
+		}
+	}
+	return
+}
+
+// DynamicHTTPServer listens on an available port, sets up an http or https
+// (when secure is true) server on it and returns the listening port.
+func DynamicHTTPServer(secure bool) int {
 	listener, err := net.Listen("tcp", ":0")
 	if err != nil {
 		Fatalf("Unable to listen to dynamic port: %v", err)
 	}
 	port := listener.Addr().(*net.TCPAddr).Port
 	Infof("Using port: %d", port)
-	go func(port int) {
-		if err := http.Serve(listener, nil); err != nil {
-			Fatalf("Unable to serve on %d: %v", port, err)
+	go func() {
+		var err error
+		if secure {
+			Errf("Secure setup not yet supported. Will just close incoming connections for now")
+			//err = http.ServeTLS(listener, nil, "", "") // go 1.9
+			err = closingServer(listener)
+		} else {
+			err = http.Serve(listener, nil)
+		}
+		if err != nil {
+			Fatalf("Unable to serve with secure=%v on %d: %v", secure, port, err)
 		}
-	}(port)
+	}()
 	return port
 }
 
@@ -814,10 +849,11 @@ func DebugHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 // EchoServer starts a debug / echo http server on the given port.
-func EchoServer(port int) {
+func EchoServer(port int, debugPath string) {
 	fmt.Printf("Fortio %s echo server listening on port %v\n", Version, port)
-
-	http.HandleFunc("/debug", DebugHandler)
+	if debugPath != "" {
+		http.HandleFunc(debugPath, DebugHandler)
+	}
 	http.HandleFunc("/", EchoHandler)
 	if err := http.ListenAndServe(fmt.Sprintf(":%d", port), nil); err != nil {
 		fmt.Println("Error starting server", err)

devel/fortio/httprunner_test.go

@@ -28,7 +28,7 @@ import (
 func TestHTTPRunner(t *testing.T) {
 	SetLogLevel(Info)
 	http.HandleFunc("/foo/", EchoHandler)
-	port := DynamicHTTPServer()
+	port := DynamicHTTPServer(false)
 	baseURL := fmt.Sprintf("http://localhost:%d/", port)
 
 	opts := HTTPRunnerOptions{
@@ -53,3 +53,23 @@ func TestHTTPRunner(t *testing.T) {
 		t.Errorf("Mismatch between requests %d and ok %v", totalReq, res.RetCodes)
 	}
 }
+
+func TestHTTPRunnerBadServer(t *testing.T) {
+	SetLogLevel(Info)
+	// Using http to an https server (or the current 'close all' dummy https server)
+	// should fail:
+	port := DynamicHTTPServer(true)
+	baseURL := fmt.Sprintf("http://localhost:%d/", port)
+
+	opts := HTTPRunnerOptions{
+		RunnerOptions: RunnerOptions{
+			QPS: 10,
+		},
+		URL: baseURL,
+	}
+	_, err := RunHTTPTest(&opts)
+	if err == nil {
+		t.Fatal("Expecting an error but didn't get it when connecting to bad server")
+	}
+	Infof("Got expected error from mismatch/bad server: %v", err)
+}

install/updateVersion.sh

@@ -161,15 +161,16 @@ export CA_HUB="${CA_HUB}"
 export CA_TAG="${CA_TAG}"
 export MIXER_HUB="${MIXER_HUB}"
 export MIXER_TAG="${MIXER_TAG}"
-export ISTIOCTL_URL="${ISTIOCTL_URL}"
 export PILOT_HUB="${PILOT_HUB}"
 export PILOT_TAG="${PILOT_TAG}"
+export ISTIOCTL_URL="${ISTIOCTL_URL}"
 export PROXY_TAG="${PROXY_TAG}"
 export ISTIO_NAMESPACE="${ISTIO_NAMESPACE}"
 export AUTH_DEBIAN_URL="${AUTH_DEBIAN_URL}"
 export PILOT_DEBIAN_URL="${PILOT_DEBIAN_URL}"
 export PROXY_DEBIAN_URL="${PROXY_DEBIAN_URL}"
-
+export FORTIO_HUB="${FORTIO_HUB}"
+export FORTIO_TAG="${FORTIO_TAG}"
 EOF
 }
 

istio.VERSION

@@ -4,12 +4,13 @@ export CA_HUB="docker.io/istio"
 export CA_TAG="0.2.7"
 export MIXER_HUB="docker.io/istio"
 export MIXER_TAG="0.2.7"
-export ISTIOCTL_URL="https://storage.googleapis.com/istio-release/releases/0.2.7/istioctl"
 export PILOT_HUB="docker.io/istio"
 export PILOT_TAG="0.2.7"
+export ISTIOCTL_URL="https://storage.googleapis.com/istio-release/releases/0.2.7/istioctl"
 export PROXY_TAG="5d544187a15e11b593e931902e2d74f8dca26ef7"
 export ISTIO_NAMESPACE="istio-system"
 export AUTH_DEBIAN_URL="https://storage.googleapis.com/istio-artifacts/auth/c433a20e495ec312ef702a6950893af67c7ebe9c/artifacts/debs"
 export PILOT_DEBIAN_URL="https://storage.googleapis.com/istio-artifacts/pilot/6b145c189aad8306b13af1725123bebfbc7eefd4/artifacts/debs"
 export PROXY_DEBIAN_URL="https://storage.googleapis.com/istio-artifacts/proxy/5d544187a15e11b593e931902e2d74f8dca26ef7/artifacts/debs"
-
+export FORTIO_HUB="gcr.io/istio-testing"
+export FORTIO_TAG="0.2.7"

tests/e2e/README.md

@@ -73,7 +73,15 @@ From the repo checkout root directory
 
 ```
 source istio.VERSION
-bazel run //tests/e2e/tests/mixer:go_default_test -- -alsologtostderr -test.v -v 2 -test.run TestDenials --skip_cleanup --auth_enable
+# Each time pilot SHA changes, get the matching istioctl binary
+# for instance on a mac:
+curl $ISTIOCTL_URL/istioctl-osx > ~/istioctl-osx ; chmod 755 ~/istioctl-osx
+# Each time the test code changes, rebuild the test, for instance:
+bazel build //tests/e2e/tests/simple:go_default_test
+# First time you want to run: deploy in namespace e2e and leave it running:
+./bazel-bin/tests/e2e/tests/simple/go_default_test -alsologtostderr -test.v -v 2  --skip_cleanup --namespace=e2e -istioctl ~/istioctl-osx --auth_enable
+# Subsequent runs if only the TestSimpleIngress (for instance) changes:
+./bazel-bin/tests/e2e/tests/simple/go_default_test -alsologtostderr -test.v -v 2  --skip_setup --skip_cleanup --namespace=e2e -istioctl ~/istioctl-osx --auth_enable --test.run TestSimpleIngress
 ```
 
 
@@ -132,6 +140,3 @@ Testing code or writing tests don't require knowledge of framework, it should be
 
 ### appManager.go
 `appManager` gather apps required for test into a array and deploy them while setup()
-
-
-

tests/e2e/framework/framework.go

@@ -148,7 +148,7 @@ func (t *testCleanup) init() error {
 
 func (t *testCleanup) cleanup() {
 	if t.skipCleanup {
-		glog.Info("Debug model, skip cleanup")
+		glog.Info("Dev mode (--skip_cleanup), skipping cleanup (removal of namespace/install)")
 		return
 	}
 	// Run tear down on all cleanable
@@ -167,6 +167,10 @@ func (t *testCleanup) cleanup() {
 // Fetch and save cluster tracing logs if logProvider specified
 // Logs are uploaded during test tear down
 func (c *CommonConfig) saveLogs(r int) error {
+	if c.Cleanup.skipCleanup {
+		glog.Info("Dev mode (--skip_cleanup), skipping log fetching")
+		return nil
+	}
 	if c.Info == nil {
 		glog.Warning("Skipping log saving as Info is not initialized")
 		return nil

tests/e2e/framework/kubernetes.go

@@ -72,6 +72,7 @@ type KubeInfo struct {
 
 	localCluster     bool
 	namespaceCreated bool
+	AuthEnabled      bool
 
 	// Istioctl installation
 	Istioctl *Istioctl
@@ -103,6 +104,7 @@ func newKubeInfo(tmpDir, runID string) (*KubeInfo, error) {
 		localCluster:     *localCluster,
 		Istioctl:         i,
 		AppManager:       a,
+		AuthEnabled:      *authEnable,
 	}, nil
 }
 

tests/e2e/tests/simple/BUILD.bazel

@@ -0,0 +1,22 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_test")
+
+go_test(
+    name = "go_default_test",
+    srcs = ["simple1_test.go"],
+    data = [":test_yamls"],
+    tags = ["manual"],
+    deps = [
+        "//devel/fortio:go_default_library",
+        "//tests/e2e/framework:go_default_library",
+        "//tests/e2e/util:go_default_library",
+        "@com_github_golang_glog//:go_default_library",
+    ],
+)
+
+filegroup(
+    name = "test_yamls",
+    srcs = [
+        "servicesNotInjected.yaml",
+        "servicesToBeInjected.yaml",
+    ],
+)

tests/e2e/tests/simple/servicesNotInjected.yaml

@@ -0,0 +1,31 @@
+# Client inside the cluster, not istio injected
+apiVersion: v1
+kind: Service
+metadata:
+  name: fortio-noistio
+spec:
+  ports:
+  - port: 8080
+    name: http-echo
+  - port: 8079
+    name: grpc-ping
+  selector:
+    app: fortio-noistio
+---
+apiVersion: apps/v1beta1
+kind: Deployment
+metadata:
+  name: raw-cli-deployement
+spec:
+  replicas: 1
+  template:
+    metadata:
+      labels:
+        app: fortio-noistio
+    spec:
+      containers:
+      - name: fortio-noistio
+        # That image runs the servers (so it will run forever) but has the
+        # /usr/local/bin/fortio client
+        image: {{.FortioImage}}
+        imagePullPolicy: Always