From b145e04d5d108ca2dc42439af9167ad3e55450b6 Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Mon, 28 Aug 2017 14:41:21 -0700 Subject: [PATCH] Refactor GetNodeClient weirdness - No need to for a pointer to a pointer - Properly set and use QueryOptions.Region --- api/allocations.go | 4 +-- api/api.go | 79 ++++++++++++++++++++++------------------------ api/fs.go | 15 +++++---- api/nodes.go | 14 ++------ 4 files changed, 52 insertions(+), 60 deletions(-) diff --git a/api/allocations.go b/api/allocations.go index 42ac087f6..1af3f2533 100644 --- a/api/allocations.go +++ b/api/allocations.go @@ -48,7 +48,7 @@ func (a *Allocations) Info(allocID string, q *QueryOptions) (*Allocation, *Query } func (a *Allocations) Stats(alloc *Allocation, q *QueryOptions) (*AllocResourceUsage, error) { - nodeClient, err := a.client.GetNodeClient(alloc.NodeID, &q) + nodeClient, err := a.client.GetNodeClient(alloc.NodeID, q) if err != nil { return nil, err } @@ -59,7 +59,7 @@ func (a *Allocations) Stats(alloc *Allocation, q *QueryOptions) (*AllocResourceU } func (a *Allocations) GC(alloc *Allocation, q *QueryOptions) error { - nodeClient, err := a.client.GetNodeClient(alloc.NodeID, &q) + nodeClient, err := a.client.GetNodeClient(alloc.NodeID, q) if err != nil { return err } diff --git a/api/api.go b/api/api.go index d5e56a07d..5f46383d3 100644 --- a/api/api.go +++ b/api/api.go @@ -111,19 +111,20 @@ type Config struct { } // CopyConfig copies the configuration with a new address -func (c *Config) CopyConfig(address string, tlsEnabled bool) *Config { +func (c *Config) CopyConfig(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 +154,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{ @@ -286,10 +297,28 @@ func (c *Client) SetRegion(region string) { } // GetNodeClient returns a new Client that will dial the specified node. If the -// QueryOptions is set, the function will ensure that it is initialized and -// that the Params field is valid. -func (c *Client) GetNodeClient(nodeID string, q **QueryOptions) (*Client, error) { - node, _, err := c.Nodes().Info(nodeID, &QueryOptions{}) +// QueryOptions is set, its region will be used. +func (c *Client) GetNodeClient(nodeID string, q *QueryOptions) (*Client, error) { + if q == nil { + q = &QueryOptions{} + } + + // If region is unset, set from config or agent if available + if q.Region == "" { + if c.config.Region != "" { + // Use the region from the client + q.Region = c.config.Region + } else { + // Use the region from the agent + agentRegion, err := c.Agent().Region() + if err != nil { + return nil, err + } + q.Region = agentRegion + } + } + + node, _, err := c.Nodes().Info(nodeID, q) if err != nil { return nil, err } @@ -300,41 +329,9 @@ func (c *Client) GetNodeClient(nodeID string, q **QueryOptions) (*Client, error) return nil, fmt.Errorf("http addr of node %q (%s) is not advertised", node.Name, nodeID) } - region := "" - if q != nil && *q != nil && (*q).Region != "" { - region = (*q).Region - } else if c.config.Region != "" { - // Use the region from the client - region = c.config.Region - } else { - // Use the region from the agent - agentRegion, err := c.Agent().Region() - if err != nil { - return nil, err - } - region = agentRegion - } - // Get an API client for the node - conf := c.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 + conf := c.config.CopyConfig(q.Region, node.HTTPAddr, node.TLSEnabled) + return NewClient(conf) } // request is used to help build up a request diff --git a/api/fs.go b/api/fs.go index efe65650e..e2fbf8181 100644 --- a/api/fs.go +++ b/api/fs.go @@ -51,7 +51,7 @@ func (c *Client) AllocFS() *AllocFS { // 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) { - nodeClient, err := a.client.GetNodeClient(alloc.NodeID, &q) + nodeClient, err := a.client.GetNodeClient(alloc.NodeID, q) if err != nil { return nil, nil, err } @@ -68,7 +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) { - nodeClient, err := a.client.GetNodeClient(alloc.NodeID, &q) + nodeClient, err := a.client.GetNodeClient(alloc.NodeID, q) if err != nil { return nil, nil, err } @@ -85,7 +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) { - nodeClient, err := a.client.GetNodeClient(alloc.NodeID, &q) + nodeClient, err := a.client.GetNodeClient(alloc.NodeID, q) if err != nil { return nil, err } @@ -103,7 +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) { - nodeClient, err := a.client.GetNodeClient(alloc.NodeID, &q) + nodeClient, err := a.client.GetNodeClient(alloc.NodeID, q) if err != nil { return nil, err } @@ -127,7 +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) { - nodeClient, err := a.client.GetNodeClient(alloc.NodeID, &q) + nodeClient, err := a.client.GetNodeClient(alloc.NodeID, q) if err != nil { return nil, err } @@ -191,10 +191,13 @@ 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) { - nodeClient, err := a.client.GetNodeClient(alloc.NodeID, &q) + nodeClient, err := a.client.GetNodeClient(alloc.NodeID, q) if err != nil { return nil, err } + if q == nil { + q = &QueryOptions{} + } q.Params["follow"] = strconv.FormatBool(follow) q.Params["task"] = task q.Params["type"] = logType diff --git a/api/nodes.go b/api/nodes.go index b1bdb5054..50a159628 100644 --- a/api/nodes.go +++ b/api/nodes.go @@ -1,7 +1,6 @@ package api import ( - "fmt" "sort" "strconv" ) @@ -73,26 +72,19 @@ 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 { - nodeClient, err := n.client.GetNodeClient(nodeID, &q) + nodeClient, err := n.client.GetNodeClient(nodeID, q) if err != nil { return err }