Address non-structural review comments

Signed-off-by: Kevin Leimkuhler <kleimkuhler@icloud.com>
linkerd · Aug 16, 2019 · 923dbcb · 923dbcb
1 parent f6362df
commit 923dbcb
Show file tree

Hide file tree

Showing 7 changed files with 18 additions and 12 deletions.
diff --git a/controller/cmd/tap/main.go b/controller/cmd/tap/main.go
@@ -1,6 +1,7 @@
 package main
 
 import (
+	"context"
 	"crypto/tls"
 	"flag"
 	"os"
@@ -69,4 +70,7 @@ func main() {
 	go admin.StartServer(*metricsAddr)
 
 	<-stop
+
+	log.Infof("shutting down APIServer on %s", *apiServerAddr)
+	apiServer.Shutdown(context.Background())
 }
diff --git a/controller/tap/apiserver_test.go b/controller/tap/apiserver_test.go
@@ -47,7 +47,7 @@ data:
 				t.Fatalf("NewFakeAPI returned an error: %s", err)
 			}
 
-			_, fakeGrpcServer := newGRPCTapServer(4190, "controller-ns", k8sAPI)
+			fakeGrpcServer := newGRPCTapServer(4190, "controller-ns", k8sAPI)
 
 			_, _, err = NewAPIServer("localhost:0", tls.Certificate{}, k8sAPI, fakeGrpcServer, false)
 			if !reflect.DeepEqual(err, exp.err) {

diff --git a/controller/tap/handlers.go b/controller/tap/handlers.go
@@ -168,7 +168,7 @@ func (h *handler) handleTap(w http.ResponseWriter, req *http.Request, p httprout
 		return
 	}
 
-	tapServer := tapByResourceServer{serverStream{w: flushableWriter, req: req}}
+	tapServer := tapByResourceServer{serverStream{w: flushableWriter, req: req, log: h.log}}
 	err = h.grpcTapServer.TapByResource(&tapReq, &tapServer)
 	if err != nil {
 		h.log.Error(err)
@@ -344,6 +344,13 @@ func renderJSONError(w http.ResponseWriter, err error, status int) {
 	w.Write(rsp)
 }
 
+// serverStream and tapByResourceServer provide functionality that satisfy the
+// tap.Tap_TapByResourceServer. This allows the tap APIServer to call
+// GRPCTapServer.TapByResource() directly, rather than make the request to an
+// actual gRPC over the network.
+//
+// TODO: Share this code with streamServer and destinationServer in
+// http_server.go.
 type serverStream struct {
 	w   protohttp.FlushableResponseWriter
 	req *http.Request

diff --git a/controller/tap/server.go b/controller/tap/server.go
@@ -29,7 +29,7 @@ const requireIDHeader = "l5d-require-id"
 const podIPIndex = "ip"
 const defaultMaxRps = 100.0
 
-// GRPCTapServer describes the gRPC server implementing tap.Tap_TapByResourceServer
+// GRPCTapServer describes the gRPC server implementing pb.TapServer
 type GRPCTapServer struct {
 	tapPort             uint
 	k8sAPI              *k8s.API
@@ -462,16 +462,14 @@ func NewGrpcTapServer(
 ) *GRPCTapServer {
 	k8sAPI.Pod().Informer().AddIndexers(cache.Indexers{podIPIndex: indexPodByIP})
 
-	_, srv := newGRPCTapServer(tapPort, controllerNamespace, k8sAPI)
-
-	return srv
+	return newGRPCTapServer(tapPort, controllerNamespace, k8sAPI)
 }
 
 func newGRPCTapServer(
 	tapPort uint,
 	controllerNamespace string,
 	k8sAPI *k8s.API,
-) (*grpc.Server, *GRPCTapServer) {
+) *GRPCTapServer {
 	srv := &GRPCTapServer{
 		tapPort:             tapPort,
 		k8sAPI:              k8sAPI,
@@ -481,7 +479,7 @@ func newGRPCTapServer(
 	s := prometheus.NewGrpcServer()
 	pb.RegisterTapServer(s, srv)
 
-	return s, srv
+	return srv
 }
 
 func indexPodByIP(obj interface{}) ([]string, error) {

diff --git a/controller/tap/server_test.go b/controller/tap/server_test.go
@@ -371,7 +371,7 @@ status:
 				t.Fatalf("Invalid port: %s", port)
 			}
 
-			_, fakeGrpcServer := newGRPCTapServer(uint(tapPort), "controller-ns", k8sAPI)
+			fakeGrpcServer := newGRPCTapServer(uint(tapPort), "controller-ns", k8sAPI)
 
 			k8sAPI.Sync()
 

diff --git a/go.mod b/go.mod
@@ -60,7 +60,6 @@ require (
 	github.com/projectcalico/libcalico-go v1.7.3
 	github.com/prometheus/client_golang v0.9.2
 	github.com/prometheus/common v0.0.0-20181126121408-4724e9255275
-	github.com/runconduit/conduit v18.9.1+incompatible
 	github.com/russross/blackfriday v1.5.2 // indirect
 	github.com/satori/go.uuid v1.2.0 // indirect
 	github.com/sergi/go-diff v1.0.0

diff --git a/go.sum b/go.sum
@@ -202,8 +202,6 @@ github.com/prometheus/procfs v0.0.0-20181204211112-1dc9a6cbc91a h1:9a8MnZMP0X2nL
 github.com/prometheus/procfs v0.0.0-20181204211112-1dc9a6cbc91a/go.mod h1:c3At6R/oaqEKCNdg8wHV1ftS6bRYblBhIjjI8uT2IGk=
 github.com/remyoudompheng/bigfft v0.0.0-20170806203942-52369c62f446/go.mod h1:uYEyJGbgTkfkS4+E/PavXkNJcbFIpEtjt2B0KDQ5+9M=
 github.com/rogpeppe/go-charset v0.0.0-20180617210344-2471d30d28b4/go.mod h1:qgYeAmZ5ZIpBWTGllZSQnw97Dj+woV0toclVaRGI8pc=
-github.com/runconduit/conduit v18.9.1+incompatible h1:qzkTq7XQLdK1haRbm+233vccaVsNRz4YdA1bftOSS28=
-github.com/runconduit/conduit v18.9.1+incompatible/go.mod h1:+wTabui8mGgdYdOV7l5n6sEKIzxsqPBAMLK3hpNMUmA=
 github.com/russross/blackfriday v1.5.2 h1:HyvC0ARfnZBqnXwABFeSZHpKvJHJJfPz81GNueLj0oo=
 github.com/russross/blackfriday v1.5.2/go.mod h1:JO/DiYxRf+HjHt06OyowR9PTA263kcR/rfWxYHBV53g=
 github.com/satori/go.uuid v1.2.0 h1:0uYX9dsZ2yD7q2RtLRtPSdGDWzjeM3TbMJP9utgA0ww=