Address @dadgar feedback

This commit is contained in:
Armon Dadgar 2017-08-23 13:49:08 -07:00
parent e7586a80df
commit 76a03f2d8e
4 changed files with 68 additions and 64 deletions

View file

@ -4,10 +4,57 @@ import (
"time"
metrics "github.com/armon/go-metrics"
lru "github.com/hashicorp/golang-lru"
"github.com/hashicorp/nomad/acl"
"github.com/hashicorp/nomad/nomad/structs"
)
const (
// policyCacheSize is the number of ACL policies to keep cached. Policies have a fetching cost
// so we keep the hot policies cached to reduce the ACL token resolution time.
policyCacheSize = 64
// aclCacheSize is the number of ACL objects to keep cached. ACLs have a parsing and
// construction cost, so we keep the hot objects cached to reduce the ACL token resolution time.
aclCacheSize = 64
// tokenCacheSize is the number of ACL tokens to keep cached. Tokens have a fetching cost,
// so we keep the hot tokens cached to reduce the lookups.
tokenCacheSize = 64
)
// clientACLResolver holds the state required for client resolution
// of ACLs
type clientACLResolver struct {
// aclCache is used to maintain the parsed ACL objects
aclCache *lru.TwoQueueCache
// policyCache is used to maintain the fetched policy objects
policyCache *lru.TwoQueueCache
// tokenCache is used to maintain the fetched token objects
tokenCache *lru.TwoQueueCache
}
// init is used to setup the client resolver state
func (c *clientACLResolver) init() error {
// Create the ACL object cache
var err error
c.aclCache, err = lru.New2Q(aclCacheSize)
if err != nil {
return err
}
c.policyCache, err = lru.New2Q(policyCacheSize)
if err != nil {
return err
}
c.tokenCache, err = lru.New2Q(tokenCacheSize)
if err != nil {
return err
}
return nil
}
// cachedACLValue is used to manage ACL Token or Policy TTLs
type cachedACLValue struct {
Token *structs.ACLToken
@ -17,17 +64,17 @@ type cachedACLValue struct {
// Age is the time since the token was cached
func (c *cachedACLValue) Age() time.Duration {
return time.Now().Sub(c.CacheTime)
return time.Since(c.CacheTime)
}
// resolveToken is used to translate an ACL Token Secret ID into
// ResolveToken is used to translate an ACL Token Secret ID into
// an ACL object, nil if ACLs are disabled, or an error.
func (c *Client) resolveToken(secretID string) (*acl.ACL, error) {
func (c *Client) ResolveToken(secretID string) (*acl.ACL, error) {
// Fast-path if ACLs are disabled
if !c.config.ACLEnabled {
return nil, nil
}
defer metrics.MeasureSince([]string{"client", "acl", "resolveToken"}, time.Now())
defer metrics.MeasureSince([]string{"client", "acl", "resolve_token"}, time.Now())
// Resolve the token value
token, err := c.resolveTokenValue(secretID)

View file

@ -98,7 +98,7 @@ func TestClient_ACL_resolvePolicies(t *testing.T) {
}
}
func TestClient_ACL_resolveToken_Disabled(t *testing.T) {
func TestClient_ACL_ResolveToken_Disabled(t *testing.T) {
s1, _ := testServer(t, nil)
defer s1.Shutdown()
testutil.WaitForLeader(t, s1.RPC)
@ -109,12 +109,12 @@ func TestClient_ACL_resolveToken_Disabled(t *testing.T) {
defer c1.Shutdown()
// Should always get nil when disabled
aclObj, err := c1.resolveToken("blah")
aclObj, err := c1.ResolveToken("blah")
assert.Nil(t, err)
assert.Nil(t, aclObj)
}
func TestClient_ACL_resolveToken(t *testing.T) {
func TestClient_ACL_ResolveToken(t *testing.T) {
s1, _ := testServer(t, nil)
defer s1.Shutdown()
testutil.WaitForLeader(t, s1.RPC)
@ -139,26 +139,26 @@ func TestClient_ACL_resolveToken(t *testing.T) {
assert.Nil(t, err)
// Test the client resolution
out, err := c1.resolveToken(token.SecretID)
out, err := c1.ResolveToken(token.SecretID)
assert.Nil(t, err)
assert.NotNil(t, out)
// Test caching
out2, err := c1.resolveToken(token.SecretID)
out2, err := c1.ResolveToken(token.SecretID)
assert.Nil(t, err)
if out != out2 {
t.Fatalf("should be cached")
}
// Test management token
out3, err := c1.resolveToken(token2.SecretID)
out3, err := c1.ResolveToken(token2.SecretID)
assert.Nil(t, err)
if acl.ManagementACL != out3 {
t.Fatalf("should be management")
}
// Test bad token
out4, err := c1.resolveToken(structs.GenerateUUID())
out4, err := c1.ResolveToken(structs.GenerateUUID())
assert.Equal(t, structs.TokenNotFound, err)
assert.Nil(t, out4)
}

View file

@ -18,7 +18,6 @@ import (
consulapi "github.com/hashicorp/consul/api"
"github.com/hashicorp/consul/lib"
"github.com/hashicorp/go-multierror"
lru "github.com/hashicorp/golang-lru"
"github.com/hashicorp/nomad/client/allocdir"
"github.com/hashicorp/nomad/client/config"
"github.com/hashicorp/nomad/client/driver"
@ -78,18 +77,6 @@ const (
// allocSyncRetryIntv is the interval on which we retry updating
// the status of the allocation
allocSyncRetryIntv = 5 * time.Second
// policyCacheSize is the number of ACL policies to keep cached. Policies have a fetching cost
// so we keep the hot policies cached to reduce the ACL token resolution time.
policyCacheSize = 64
// aclCacheSize is the number of ACL objects to keep cached. ACLs have a parsing and
// construction cost, so we keep the hot objects cached to reduce the ACL token resolution time.
aclCacheSize = 64
// tokenCacheSize is the number of ACL tokens to keep cached. Tokens have a fetching cost,
// so we keep the hot tokens cached to reduce the lookups.
tokenCacheSize = 64
)
// ClientStatsReporter exposes all the APIs related to resource usage of a Nomad
@ -164,14 +151,8 @@ type Client struct {
// in the node automatically
garbageCollector *AllocGarbageCollector
// aclCache is used to maintain the parsed ACL objects
aclCache *lru.TwoQueueCache
// policyCache is used to maintain the fetched policy objects
policyCache *lru.TwoQueueCache
// tokenCache is used to maintain the fetched token objects
tokenCache *lru.TwoQueueCache
// clientACLResolver holds the ACL resolution state
clientACLResolver
}
var (
@ -192,19 +173,6 @@ func NewClient(cfg *config.Config, consulCatalog consul.CatalogAPI, consulServic
}
tlsWrap = tw
}
// Create the ACL object cache
aclCache, err := lru.New2Q(aclCacheSize)
if err != nil {
return nil, err
}
policyCache, err := lru.New2Q(policyCacheSize)
if err != nil {
return nil, err
}
tokenCache, err := lru.New2Q(tokenCacheSize)
if err != nil {
return nil, err
}
// Create the client
c := &Client{
@ -220,9 +188,6 @@ func NewClient(cfg *config.Config, consulCatalog consul.CatalogAPI, consulServic
servers: newServerList(),
triggerDiscoveryCh: make(chan struct{}),
serversDiscoveredCh: make(chan struct{}),
aclCache: aclCache,
policyCache: policyCache,
tokenCache: tokenCache,
}
// Initialize the client
@ -230,6 +195,11 @@ func NewClient(cfg *config.Config, consulCatalog consul.CatalogAPI, consulServic
return nil, fmt.Errorf("failed to initialize client: %v", err)
}
// Initialize the ACL state
if err := c.clientACLResolver.init(); err != nil {
return nil, fmt.Errorf("failed to initialize ACL state: %v", err)
}
// Add the stats collector
statsCollector := stats.NewHostStatsCollector(logger, c.config.AllocDir)
c.hostStatsCollector = statsCollector

View file

@ -279,7 +279,9 @@ func ACLPolicyListHash(policies []*ACLPolicy) string {
// CompileACLObject compiles a set of ACL policies into an ACL object with a cache
func CompileACLObject(cache *lru.TwoQueueCache, policies []*ACLPolicy) (*acl.ACL, error) {
// Sort the policies to ensure consistent ordering
sort.Sort(ACLPolicyList(policies))
sort.Slice(policies, func(i, j int) bool {
return policies[i].Name < policies[j].Name
})
// Determine the cache key
cacheKey := ACLPolicyListHash(policies)
@ -308,18 +310,3 @@ func CompileACLObject(cache *lru.TwoQueueCache, policies []*ACLPolicy) (*acl.ACL
cache.Add(cacheKey, aclObj)
return aclObj, nil
}
// ACLPolicyList is used to sort a set of policies by name
type ACLPolicyList []*ACLPolicy
func (l ACLPolicyList) Len() int {
return len(l)
}
func (l ACLPolicyList) Swap(i, j int) {
l[i], l[j] = l[j], l[i]
}
func (l ACLPolicyList) Less(i, j int) bool {
return l[i].Name < l[j].Name
}