Merge pull request #5039 from tamird/grpc-http2-broken

server: introduce a new port for HTTP requests

acceptance/cluster/cluster.go

@@ -17,7 +17,6 @@
 package cluster
 
 import (
-	"net"
 	"testing"
 
 	"github.com/cockroachdb/cockroach/client"
@@ -34,8 +33,6 @@ type Cluster interface {
 	NewClient(*testing.T, int) (*client.DB, *stop.Stopper)
 	// PGUrl returns a URL string for the given node postgres server.
 	PGUrl(int) string
-	// Addr returns the TCP address for the given node.
-	Addr(int) *net.TCPAddr
 	// Assert verifies that the cluster state is as expected (i.e. no unexpected
 	// restarts or node deaths occurred). Tests can call this periodically to
 	// ascertain cluster health.

acceptance/cluster/docker.go

@@ -32,6 +32,7 @@ import (
 	"github.com/docker/engine-api/types"
 	"github.com/docker/engine-api/types/container"
 	"github.com/docker/engine-api/types/network"
+	"github.com/docker/go-connections/nat"
 	"golang.org/x/net/context"
 
 	"github.com/cockroachdb/cockroach/util/log"
@@ -243,21 +244,21 @@ func (c *Container) Inspect() (types.ContainerJSON, error) {
 }
 
 // Addr returns the TCP address to connect to.
-func (c *Container) Addr() *net.TCPAddr {
+func (c *Container) Addr(port nat.Port) *net.TCPAddr {
 	containerInfo, err := c.Inspect()
 	if err != nil {
 		return nil
 	}
-	bindings, ok := containerInfo.NetworkSettings.Ports[defaultTCP]
+	bindings, ok := containerInfo.NetworkSettings.Ports[port]
 	if !ok || len(bindings) == 0 {
 		return nil
 	}
-	port, err := strconv.Atoi(bindings[0].HostPort)
+	portNum, err := strconv.Atoi(bindings[0].HostPort)
 	if err != nil {
 		return nil
 	}
 	return &net.TCPAddr{
 		IP:   dockerIP(),
-		Port: port,
+		Port: portNum,
 	}
 }

acceptance/cluster/localcluster.go

@@ -54,7 +54,9 @@ const (
 	builderImageFull = builderImage + ":" + builderTag
 )
 
-const defaultTCP nat.Port = base.DefaultPort + "/tcp"
+// DefaultTCP is the default SQL/RPC port specification.
+const DefaultTCP nat.Port = base.DefaultPort + "/tcp"
+const defaultHTTP nat.Port = base.DefaultHTTPPort + "/tcp"
 
 var cockroachImage = flag.String("i", builderImageFull, "the docker image to run")
 var cockroachBinary = flag.String("b", defaultBinary(), "the binary to run (if image == "+builderImage+")")
@@ -369,7 +371,8 @@ func (l *LocalCluster) createRoach(node *testNode, vols *Container, env []string
 			Hostname: hostname,
 			Image:    *cockroachImage,
 			ExposedPorts: map[nat.Port]struct{}{
-				defaultTCP: {},
+				DefaultTCP:  {},
+				defaultHTTP: {},
 			},
 			Entrypoint: entrypoint,
 			Env:        env,
@@ -410,7 +413,6 @@ func (l *LocalCluster) startNode(node *testNode) {
 		"start",
 		"--certs=/certs",
 		"--host=" + node.nodeStr,
-		"--port=" + base.DefaultPort,
 		"--alsologtostderr=INFO",
 	}
 	for _, store := range node.stores {
@@ -442,7 +444,7 @@ func (l *LocalCluster) startNode(node *testNode) {
   logs:      %[3]s/cockroach.INFO
   pprof:     docker exec -it %[4]s /bin/bash -c 'go tool pprof /cockroach <(wget --no-check-certificate -qO- https://$(hostname):%[5]s/debug/pprof/heap)'
   cockroach: %[6]s`,
-		node.Name(), "https://"+node.Addr().String(), locallogDir, node.Container.id[:5], base.DefaultPort, cmd)
+		node.Name(), "https://"+node.Addr(DefaultTCP).String(), locallogDir, node.Container.id[:5], base.DefaultHTTPPort, cmd)
 }
 
 func (l *LocalCluster) processEvent(event events.Message) bool {
@@ -647,18 +649,13 @@ func (l *LocalCluster) NewClient(t *testing.T, i int) (*roachClient.DB, *stop.St
 		User:  security.NodeUser,
 		Certs: l.CertsDir,
 	}, nil, stopper)
-	sender, err := roachClient.NewSender(rpcContext, l.Addr(i).String())
+	sender, err := roachClient.NewSender(rpcContext, l.Nodes[i].Addr(DefaultTCP).String())
 	if err != nil {
 		t.Fatal(err)
 	}
 	return roachClient.NewDB(sender), stopper
 }
 
-// Addr returns the TCP address for the given node.
-func (l *LocalCluster) Addr(i int) *net.TCPAddr {
-	return l.Nodes[i].Addr()
-}
-
 // PGUrl returns a URL string for the given node postgres server.
 func (l *LocalCluster) PGUrl(i int) string {
 	certUser := security.RootUser
@@ -670,7 +667,7 @@ func (l *LocalCluster) PGUrl(i int) string {
 	pgURL := url.URL{
 		Scheme:   "postgres",
 		User:     url.User(certUser),
-		Host:     l.Addr(i).String(),
+		Host:     l.Nodes[i].Addr(DefaultTCP).String(),
 		RawQuery: options.Encode(),
 	}
 	return pgURL.String()
@@ -693,5 +690,5 @@ func (l *LocalCluster) Restart(i int) error {
 
 // URL returns the base url.
 func (l *LocalCluster) URL(i int) string {
-	return "https://" + l.Addr(i).String()
+	return "https://" + l.Nodes[i].Addr(defaultHTTP).String()
 }

acceptance/terrafarm/farmer.go

@@ -29,8 +29,6 @@ import (
 
 	"github.com/cockroachdb/cockroach/base"
 	"github.com/cockroachdb/cockroach/client"
-	"github.com/cockroachdb/cockroach/rpc"
-	"github.com/cockroachdb/cockroach/security"
 	"github.com/cockroachdb/cockroach/util/retry"
 	"github.com/cockroachdb/cockroach/util/stop"
 )
@@ -168,28 +166,14 @@ func (f *Farmer) Exec(i int, cmd string) error {
 
 // NewClient implements the Cluster interface.
 func (f *Farmer) NewClient(t *testing.T, i int) (*client.DB, *stop.Stopper) {
-	// TODO(tschottdorf,mberhault): TLS all the things!
-	stopper := stop.NewStopper()
-	rpcContext := rpc.NewContext(&base.Context{
-		User: security.RootUser,
-	}, nil, stopper)
-	sender, err := client.NewSender(rpcContext, f.Addr(i).String())
-	if err != nil {
-		t.Fatal(err)
-	}
-	return client.NewDB(sender), stopper
+	panic("unimplemented")
 }
 
 // PGUrl returns a URL string for the given node postgres server.
 func (f *Farmer) PGUrl(i int) string {
 	panic("unimplemented")
 }
 
-// Addr returns the TCP address for the given node.
-func (f *Farmer) Addr(i int) *net.TCPAddr {
-	panic("unimplemented")
-}
-
 // WaitReady waits until the infrastructure is in a state that *should* allow
 // for a healthy cluster. Currently, this means waiting for the load balancer
 // to resolve from all nodes.
@@ -268,7 +252,7 @@ func (f *Farmer) Restart(i int) error {
 
 // URL returns the HTTP(s) endpoint.
 func (f *Farmer) URL(i int) string {
-	return "http://" + net.JoinHostPort(f.Nodes()[i], base.DefaultPort)
+	return "http://" + net.JoinHostPort(f.Nodes()[i], base.DefaultHTTPPort)
 }
 
 func (f *Farmer) logf(format string, args ...interface{}) {

acceptance/util_test.go

@@ -241,7 +241,7 @@ func testDocker(t *testing.T, name string, cmd []string) error {
 	l := StartCluster(t, readConfigFromFlags()).(*cluster.LocalCluster)
 
 	defer l.AssertAndStop(t)
-	addr := l.Nodes[0].Addr()
+	addr := l.Nodes[0].Addr(cluster.DefaultTCP)
 	containerConfig := container.Config{
 		Image: fmt.Sprintf(image + ":" + postgresTestTag),
 		Env: []string{

base/context.go

@@ -41,10 +41,19 @@ const (
 	// https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.xhtml?search=cockroachdb
 	DefaultPort = "26257"
 
+	// The default port for HTTP-for-humans.
+	DefaultHTTPPort = "8080"
+
 	// NetworkTimeout is the timeout used for network operations.
 	NetworkTimeout = 3 * time.Second
 )
 
+type lazyTLSConfig struct {
+	once      sync.Once
+	tlsConfig *tls.Config
+	err       error
+}
+
 // Context is embedded by server.Context. A base context is not meant to be
 // used directly, but embedding contexts should call ctx.InitDefaults().
 type Context struct {
@@ -60,12 +69,11 @@ type Context struct {
 	// the server is running or the user passed in client calls.
 	User string
 
-	// Protects both clientTLSConfig and serverTLSConfig.
-	tlsConfigMu sync.Mutex
 	// clientTLSConfig is the loaded client tlsConfig. It is initialized lazily.
-	clientTLSConfig *tls.Config
+	clientTLSConfig lazyTLSConfig
+
 	// serverTLSConfig is the loaded server tlsConfig. It is initialized lazily.
-	serverTLSConfig *tls.Config
+	serverTLSConfig lazyTLSConfig
 
 	// httpClient is a lazily-initialized http client.
 	// It should be accessed through Context.GetHTTPClient() which will
@@ -108,25 +116,19 @@ func (ctx *Context) GetClientTLSConfig() (*tls.Config, error) {
 		return nil, nil
 	}
 
-	ctx.tlsConfigMu.Lock()
-	defer ctx.tlsConfigMu.Unlock()
-
-	if ctx.clientTLSConfig != nil {
-		return ctx.clientTLSConfig, nil
-	}
-
-	if ctx.Certs != "" {
-		cfg, err := security.LoadClientTLSConfig(ctx.Certs, ctx.User)
-		if err != nil {
-			return nil, util.Errorf("error setting up client TLS config: %s", err)
+	ctx.clientTLSConfig.once.Do(func() {
+		if ctx.Certs != "" {
+			ctx.clientTLSConfig.tlsConfig, ctx.clientTLSConfig.err = security.LoadClientTLSConfig(ctx.Certs, ctx.User)
+			if ctx.clientTLSConfig.err != nil {
+				ctx.clientTLSConfig.err = util.Errorf("error setting up client TLS config: %s", ctx.clientTLSConfig.err)
+			}
+		} else {
+			log.Println("no certificates directory specified: using insecure TLS")
+			ctx.clientTLSConfig.tlsConfig = security.LoadInsecureClientTLSConfig()
 		}
-		ctx.clientTLSConfig = cfg
-	} else {
-		log.Println("no certificates directory specified: using insecure TLS")
-		ctx.clientTLSConfig = security.LoadInsecureClientTLSConfig()
-	}
+	})
 
-	return ctx.clientTLSConfig, nil
+	return ctx.clientTLSConfig.tlsConfig, ctx.clientTLSConfig.err
 }
 
 // GetServerTLSConfig returns the context server TLS config, initializing it if needed.
@@ -138,24 +140,18 @@ func (ctx *Context) GetServerTLSConfig() (*tls.Config, error) {
 		return nil, nil
 	}
 
-	ctx.tlsConfigMu.Lock()
-	defer ctx.tlsConfigMu.Unlock()
-
-	if ctx.serverTLSConfig != nil {
-		return ctx.serverTLSConfig, nil
-	}
-
-	if ctx.Certs == "" {
-		return nil, util.Errorf("--insecure=false, but --certs is empty. We need a certs directory")
-	}
-
-	cfg, err := security.LoadServerTLSConfig(ctx.Certs, ctx.User)
-	if err != nil {
-		return nil, util.Errorf("error setting up server TLS config: %s", err)
-	}
-	ctx.serverTLSConfig = cfg
+	ctx.serverTLSConfig.once.Do(func() {
+		if ctx.Certs != "" {
+			ctx.serverTLSConfig.tlsConfig, ctx.serverTLSConfig.err = security.LoadServerTLSConfig(ctx.Certs, ctx.User)
+			if ctx.serverTLSConfig.err != nil {
+				ctx.serverTLSConfig.err = util.Errorf("error setting up client TLS config: %s", ctx.serverTLSConfig.err)
+			}
+		} else {
+			ctx.serverTLSConfig.err = util.Errorf("--insecure=false, but --certs is empty. We need a certs directory")
+		}
+	})
 
-	return ctx.serverTLSConfig, nil
+	return ctx.serverTLSConfig.tlsConfig, ctx.serverTLSConfig.err
 }
 
 // GetHTTPClient returns the context http client, initializing it

cli/cli_test.go

@@ -22,6 +22,7 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
+	"net"
 	"os"
 	"path/filepath"
 	"reflect"
@@ -163,7 +164,15 @@ func (c cliTest) RunWithArgs(a []string) {
 		fmt.Println(err)
 	}
 	args = append(args, fmt.Sprintf("--host=%s", h))
-	args = append(args, fmt.Sprintf("--port=%s", p))
+	if a[0] == "node" || a[0] == "quit" {
+		_, httpPort, err := net.SplitHostPort(c.HTTPAddr())
+		if err != nil {
+			fmt.Println(err)
+		}
+		args = append(args, fmt.Sprintf("--http-port=%s", httpPort))
+	} else {
+		args = append(args, fmt.Sprintf("--port=%s", p))
+	}
 	// Always load test certs.
 	args = append(args, fmt.Sprintf("--certs=%s", c.certsDir))
 	args = append(args, a[1:]...)

cli/flags.go

@@ -34,7 +34,7 @@ import (
 var maxResults int64
 
 var connURL string
-var connUser, connHost, connPort, connDBName string
+var connUser, connHost, connPort, httpPort, connDBName string
 
 // cliContext is the CLI Context used for the command-line client.
 var cliContext = NewContext()
@@ -113,6 +113,9 @@ provide the password on standard input.`),
 	"server_port": wrapText(`
 The port to bind to.`),
 
+	"http_port": wrapText(`
+The host:port to bind for HTTP requests.`),
+
 	"store": wrapText(`
 The file path to a storage device. This flag must be specified separately for
 each storage device, for example:`) + `
@@ -257,6 +260,7 @@ func initFlags(ctx *Context) {
 		// Server flags.
 		f.StringVar(&connHost, "host", "", usage("server_host"))
 		f.StringVarP(&connPort, "port", "p", base.DefaultPort, usage("server_port"))
+		f.StringVar(&httpPort, "http-port", base.DefaultHTTPPort, usage("http_port"))
 		f.StringVar(&ctx.Attrs, "attrs", ctx.Attrs, usage("attrs"))
 		f.VarP(&ctx.Stores, "store", "s", usage("store"))
 
@@ -320,12 +324,17 @@ func initFlags(ctx *Context) {
 	}
 
 	// Commands that need the cockroach port.
-	simpleCmds := []*cobra.Command{kvCmd, nodeCmd, rangeCmd, exterminateCmd, quitCmd}
+	simpleCmds := []*cobra.Command{kvCmd, rangeCmd, exterminateCmd}
 	for _, cmd := range simpleCmds {
 		f := cmd.PersistentFlags()
 		f.StringVarP(&connPort, "port", "p", base.DefaultPort, usage("client_port"))
 	}
 
+	for _, cmd := range []*cobra.Command{nodeCmd, quitCmd} {
+		f := cmd.PersistentFlags()
+		f.StringVar(&httpPort, "http-port", base.DefaultHTTPPort, usage("http_port"))
+	}
+
 	// Commands that establish a SQL connection.
 	sqlCmds := []*cobra.Command{sqlShellCmd, userCmd, zoneCmd}
 	for _, cmd := range sqlCmds {
@@ -349,5 +358,6 @@ func init() {
 
 	cobra.OnInitialize(func() {
 		cliContext.Addr = net.JoinHostPort(connHost, connPort)
+		cliContext.HTTPAddr = net.JoinHostPort(connHost, httpPort)
 	})
 }

cli/node.go

@@ -55,7 +55,7 @@ func runLsNodes(cmd *cobra.Command, args []string) error {
 
 	// Extract Node IDs from NodeStatuses.
 	nodeStatuses := map[string][]status.NodeStatus{}
-	if err := getJSON(cliContext.Addr, server.PathForNodeStatus(""), &nodeStatuses); err != nil {
+	if err := getJSON(cliContext.HTTPAddr, server.PathForNodeStatus(""), &nodeStatuses); err != nil {
 		return err
 	}
 
@@ -103,15 +103,15 @@ func runStatusNode(cmd *cobra.Command, args []string) error {
 	case 0:
 		// Show status for all nodes.
 		jsonResponse := map[string][]status.NodeStatus{}
-		if err := getJSON(cliContext.Addr, server.PathForNodeStatus(""), &jsonResponse); err != nil {
+		if err := getJSON(cliContext.HTTPAddr, server.PathForNodeStatus(""), &jsonResponse); err != nil {
 			return err
 		}
 		nodeStatuses = jsonResponse["d"]
 
 	case 1:
 		nodeStatus := status.NodeStatus{}
 		nodeID := args[0]
-		if err := getJSON(cliContext.Addr, server.PathForNodeStatus(nodeID), &nodeStatus); err != nil {
+		if err := getJSON(cliContext.HTTPAddr, server.PathForNodeStatus(nodeID), &nodeStatus); err != nil {
 			return err
 		}
 		if nodeStatus.Desc.NodeID == 0 {