Merge pull request #3108 from hashicorp/b-3013-stats-tls

Fix TLS support in api pkg / cli
hashicorp · Aug 28, 2017 · 63b715f · 63b715f
2 parents 9512709 + 52857a6
commit 63b715f
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 125 deletions.
diff --git a/api/allocations.go b/api/allocations.go
@@ -48,43 +48,24 @@ func (a *Allocations) Info(allocID string, q *QueryOptions) (*Allocation, *Query
 }
 
 func (a *Allocations) Stats(alloc *Allocation, q *QueryOptions) (*AllocResourceUsage, error) {
-	node, _, err := a.client.Nodes().Info(alloc.NodeID, q)
-	if err != nil {
-		return nil, err
-	}
-	if node.Status == "down" {
-		return nil, NodeDownErr
-	}
-	if node.HTTPAddr == "" {
-		return nil, fmt.Errorf("http addr of the node where alloc %q is running is not advertised", alloc.ID)
-	}
-	client, err := NewClient(a.client.config.CopyConfig(node.HTTPAddr, node.TLSEnabled))
+	nodeClient, err := a.client.GetNodeClient(alloc.NodeID, q)
 	if err != nil {
 		return nil, err
 	}
+
 	var resp AllocResourceUsage
-	_, err = client.query("/v1/client/allocation/"+alloc.ID+"/stats", &resp, nil)
+	_, err = nodeClient.query("/v1/client/allocation/"+alloc.ID+"/stats", &resp, nil)
 	return &resp, err
 }
 
 func (a *Allocations) GC(alloc *Allocation, q *QueryOptions) error {
-	node, _, err := a.client.Nodes().Info(alloc.NodeID, q)
-	if err != nil {
-		return err
-	}
-	if node.Status == "down" {
-		return NodeDownErr
-	}
-	if node.HTTPAddr == "" {
-		return fmt.Errorf("http addr of the node where alloc %q is running is not advertised", alloc.ID)
-	}
-	client, err := NewClient(a.client.config.CopyConfig(node.HTTPAddr, node.TLSEnabled))
+	nodeClient, err := a.client.GetNodeClient(alloc.NodeID, q)
 	if err != nil {
 		return err
 	}
 
 	var resp struct{}
-	_, err = client.query("/v1/client/allocation"+alloc.ID+"/gc", &resp, nil)
+	_, err = nodeClient.query("/v1/client/allocation/"+alloc.ID+"/gc", &resp, nil)
 	return err
 }
 

diff --git a/api/api.go b/api/api.go
@@ -110,20 +110,22 @@ type Config struct {
 	TLSConfig *TLSConfig
 }
 
-// CopyConfig copies the configuration with a new address
-func (c *Config) CopyConfig(address string, tlsEnabled bool) *Config {
+// ClientConfig copies the configuration with a new client address, region, and
+// whether the client has TLS enabled.
+func (c *Config) ClientConfig(region, address string, tlsEnabled bool) *Config {
 	scheme := "http"
 	if tlsEnabled {
 		scheme = "https"
 	}
 	config := &Config{
 		Address:    fmt.Sprintf("%s://%s", scheme, address),
-		Region:     c.Region,
+		Region:     region,
 		HttpClient: c.HttpClient,
 		HttpAuth:   c.HttpAuth,
 		WaitTime:   c.WaitTime,
-		TLSConfig:  c.TLSConfig,
+		TLSConfig:  c.TLSConfig.Copy(),
 	}
+	config.TLSConfig.TLSServerName = fmt.Sprintf("client.%s.nomad", c.Region)
 
 	return config
 }
@@ -153,6 +155,16 @@ type TLSConfig struct {
 	Insecure bool
 }
 
+func (t *TLSConfig) Copy() *TLSConfig {
+	if t == nil {
+		return nil
+	}
+
+	nt := new(TLSConfig)
+	*nt = *t
+	return nt
+}
+
 // DefaultConfig returns a default configuration for the client
 func DefaultConfig() *Config {
 	config := &Config{
@@ -285,6 +297,30 @@ func (c *Client) SetRegion(region string) {
 	c.config.Region = region
 }
 
+// GetNodeClient returns a new Client that will dial the specified node. If the
+// QueryOptions is set, its region will be used.
+func (c *Client) GetNodeClient(nodeID string, q *QueryOptions) (*Client, error) {
+	node, _, err := c.Nodes().Info(nodeID, q)
+	if err != nil {
+		return nil, err
+	}
+	if node.Status == "down" {
+		return nil, NodeDownErr
+	}
+	if node.HTTPAddr == "" {
+		return nil, fmt.Errorf("http addr of node %q (%s) is not advertised", node.Name, nodeID)
+	}
+
+	region := c.config.Region
+	if q != nil && q.Region != "" {
+		region = q.Region
+	}
+
+	// Get an API client for the node
+	conf := c.config.ClientConfig(region, node.HTTPAddr, node.TLSEnabled)
+	return NewClient(conf)
+}
+
 // request is used to help build up a request
 type request struct {
 	config *Config

diff --git a/api/fs.go b/api/fs.go
@@ -49,58 +49,9 @@ func (c *Client) AllocFS() *AllocFS {
 	return &AllocFS{client: c}
 }
 
-// getNodeClient returns a Client that will dial the node. If the QueryOptions
-// is set, the function will ensure that it is initialized and that the Params
-// field is valid.
-func (a *AllocFS) getNodeClient(node *Node, allocID string, q **QueryOptions) (*Client, error) {
-	if node.HTTPAddr == "" {
-		return nil, fmt.Errorf("http addr of the node where alloc %q is running is not advertised", allocID)
-	}
-
-	region := ""
-	if q != nil && *q != nil && (*q).Region != "" {
-		region = (*q).Region
-	} else if a.client.config.Region != "" {
-		// Use the region from the client
-		region = a.client.config.Region
-	} else {
-		// Use the region from the agent
-		agentRegion, err := a.client.Agent().Region()
-		if err != nil {
-			return nil, err
-		}
-		region = agentRegion
-	}
-
-	// Get an API client for the node
-	conf := a.client.config.CopyConfig(node.HTTPAddr, node.TLSEnabled)
-	conf.TLSConfig.TLSServerName = fmt.Sprintf("client.%s.nomad", region)
-	nodeClient, err := NewClient(conf)
-	if err != nil {
-		return nil, err
-	}
-
-	// Set the query params
-	if q == nil {
-		return nodeClient, nil
-	}
-
-	if *q == nil {
-		*q = &QueryOptions{}
-	}
-	if actQ := *q; actQ.Params == nil {
-		actQ.Params = make(map[string]string)
-	}
-	return nodeClient, nil
-}
-
 // List is used to list the files at a given path of an allocation directory
 func (a *AllocFS) List(alloc *Allocation, path string, q *QueryOptions) ([]*AllocFileInfo, *QueryMeta, error) {
-	node, _, err := a.client.Nodes().Info(alloc.NodeID, &QueryOptions{})
-	if err != nil {
-		return nil, nil, err
-	}
-	nodeClient, err := a.getNodeClient(node, alloc.ID, &q)
+	nodeClient, err := a.client.GetNodeClient(alloc.NodeID, q)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -117,11 +68,7 @@ func (a *AllocFS) List(alloc *Allocation, path string, q *QueryOptions) ([]*Allo
 
 // Stat is used to stat a file at a given path of an allocation directory
 func (a *AllocFS) Stat(alloc *Allocation, path string, q *QueryOptions) (*AllocFileInfo, *QueryMeta, error) {
-	node, _, err := a.client.Nodes().Info(alloc.NodeID, &QueryOptions{})
-	if err != nil {
-		return nil, nil, err
-	}
-	nodeClient, err := a.getNodeClient(node, alloc.ID, &q)
+	nodeClient, err := a.client.GetNodeClient(alloc.NodeID, q)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -138,12 +85,7 @@ func (a *AllocFS) Stat(alloc *Allocation, path string, q *QueryOptions) (*AllocF
 // ReadAt is used to read bytes at a given offset until limit at the given path
 // in an allocation directory. If limit is <= 0, there is no limit.
 func (a *AllocFS) ReadAt(alloc *Allocation, path string, offset int64, limit int64, q *QueryOptions) (io.ReadCloser, error) {
-	node, _, err := a.client.Nodes().Info(alloc.NodeID, &QueryOptions{})
-	if err != nil {
-		return nil, err
-	}
-
-	nodeClient, err := a.getNodeClient(node, alloc.ID, &q)
+	nodeClient, err := a.client.GetNodeClient(alloc.NodeID, q)
 	if err != nil {
 		return nil, err
 	}
@@ -161,12 +103,7 @@ func (a *AllocFS) ReadAt(alloc *Allocation, path string, offset int64, limit int
 // Cat is used to read contents of a file at the given path in an allocation
 // directory
 func (a *AllocFS) Cat(alloc *Allocation, path string, q *QueryOptions) (io.ReadCloser, error) {
-	node, _, err := a.client.Nodes().Info(alloc.NodeID, &QueryOptions{})
-	if err != nil {
-		return nil, err
-	}
-
-	nodeClient, err := a.getNodeClient(node, alloc.ID, &q)
+	nodeClient, err := a.client.GetNodeClient(alloc.NodeID, q)
 	if err != nil {
 		return nil, err
 	}
@@ -190,12 +127,7 @@ func (a *AllocFS) Cat(alloc *Allocation, path string, q *QueryOptions) (io.ReadC
 func (a *AllocFS) Stream(alloc *Allocation, path, origin string, offset int64,
 	cancel <-chan struct{}, q *QueryOptions) (<-chan *StreamFrame, error) {
 
-	node, _, err := a.client.Nodes().Info(alloc.NodeID, q)
-	if err != nil {
-		return nil, err
-	}
-
-	nodeClient, err := a.getNodeClient(node, alloc.ID, &q)
+	nodeClient, err := a.client.GetNodeClient(alloc.NodeID, q)
 	if err != nil {
 		return nil, err
 	}
@@ -259,14 +191,12 @@ func (a *AllocFS) Stream(alloc *Allocation, path, origin string, offset int64,
 func (a *AllocFS) Logs(alloc *Allocation, follow bool, task, logType, origin string,
 	offset int64, cancel <-chan struct{}, q *QueryOptions) (<-chan *StreamFrame, error) {
 
-	node, _, err := a.client.Nodes().Info(alloc.NodeID, q)
+	nodeClient, err := a.client.GetNodeClient(alloc.NodeID, q)
 	if err != nil {
 		return nil, err
 	}
-
-	nodeClient, err := a.getNodeClient(node, alloc.ID, &q)
-	if err != nil {
-		return nil, err
+	if q == nil {
+		q = &QueryOptions{}
 	}
 	q.Params["follow"] = strconv.FormatBool(follow)
 	q.Params["task"] = task

diff --git a/api/nodes.go b/api/nodes.go
@@ -1,7 +1,6 @@
 package api
 
 import (
-	"fmt"
 	"sort"
 	"strconv"
 )
@@ -73,38 +72,25 @@ func (n *Nodes) ForceEvaluate(nodeID string, q *WriteOptions) (string, *WriteMet
 }
 
 func (n *Nodes) Stats(nodeID string, q *QueryOptions) (*HostStats, error) {
-	node, _, err := n.client.Nodes().Info(nodeID, q)
-	if err != nil {
-		return nil, err
-	}
-	if node.HTTPAddr == "" {
-		return nil, fmt.Errorf("http addr of the node %q is running is not advertised", nodeID)
-	}
-	client, err := NewClient(n.client.config.CopyConfig(node.HTTPAddr, node.TLSEnabled))
+	nodeClient, err := n.client.GetNodeClient(nodeID, q)
 	if err != nil {
 		return nil, err
 	}
 	var resp HostStats
-	if _, err := client.query("/v1/client/stats", &resp, nil); err != nil {
+	if _, err := nodeClient.query("/v1/client/stats", &resp, nil); err != nil {
 		return nil, err
 	}
 	return &resp, nil
 }
 
 func (n *Nodes) GC(nodeID string, q *QueryOptions) error {
-	node, _, err := n.client.Nodes().Info(nodeID, q)
-	if err != nil {
-		return err
-	}
-	if node.HTTPAddr == "" {
-		return fmt.Errorf("http addr of the node %q is running is not advertised", nodeID)
-	}
-	client, err := NewClient(n.client.config.CopyConfig(node.HTTPAddr, node.TLSEnabled))
+	nodeClient, err := n.client.GetNodeClient(nodeID, q)
 	if err != nil {
 		return err
 	}
+
 	var resp struct{}
-	_, err = client.query("/v1/client/gc", &resp, nil)
+	_, err = nodeClient.query("/v1/client/gc", &resp, nil)
 	return err
 }