From 923dbcb95fb3eb0ea9b809ebe97f07b3e6874509 Mon Sep 17 00:00:00 2001
From: Kevin Leimkuhler <kleimkuhler@icloud.com>
Date: Fri, 16 Aug 2019 14:47:56 -0400
Subject: [PATCH] Address non-structural review comments

Signed-off-by: Kevin Leimkuhler <kleimkuhler@icloud.com>
---
 controller/cmd/tap/main.go       |  4 ++++
 controller/tap/apiserver_test.go |  2 +-
 controller/tap/handlers.go       |  9 ++++++++-
 controller/tap/server.go         | 10 ++++------
 controller/tap/server_test.go    |  2 +-
 go.mod                           |  1 -
 go.sum                           |  2 --
 7 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/controller/cmd/tap/main.go b/controller/cmd/tap/main.go
index e9910c06fa1be..5911a02d784c0 100644
--- 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
index e4fbf7d5cabc8..1aae1af481a07 100644
--- 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
index 74566f49824f3..4438d35dce97a 100644
--- 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
index f5ecc749b36d0..66479264b2e84 100644
--- 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
index 076b926395ba4..de9d293452434 100644
--- 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
index 86f2dde1bd1a2..935736040acfe 100644
--- 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
index ee00e88cd44e4..868ec4f96cc06 100644
--- 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=