From 124d83ef6d200148ae9e139193714e1bd39c8f27 Mon Sep 17 00:00:00 2001 From: Jess Yuen Date: Wed, 8 Apr 2020 02:21:11 -0700 Subject: [PATCH] Return Resource on cache Fetch * Return `Resource` rather than `Response` on fetch. Orchestrator needs the Resource object afterall in order to identify and notify watches. * Export `Resource` and `Response` members that will be used by calling classes. Signed-off-by: Jess Yuen --- internal/app/cache/cache.go | 58 ++++++++++++------------- internal/app/cache/cache_test.go | 72 ++++++++++++++++---------------- 2 files changed, 64 insertions(+), 66 deletions(-) diff --git a/internal/app/cache/cache.go b/internal/app/cache/cache.go index 8f06fe16..e5abff3b 100644 --- a/internal/app/cache/cache.go +++ b/internal/app/cache/cache.go @@ -7,16 +7,16 @@ import ( "sync" "time" + v2 "github.com/envoyproxy/go-control-plane/envoy/api/v2" + gcp_types "github.com/envoyproxy/go-control-plane/pkg/cache/types" gcp "github.com/envoyproxy/go-control-plane/pkg/cache/v2" "github.com/golang/groupcache/lru" "github.com/golang/protobuf/ptypes/any" - - v2 "github.com/envoyproxy/go-control-plane/envoy/api/v2" ) type Cache interface { - // Fetch returns the cached response if it exists. - Fetch(key string) (*Response, error) + // Fetch returns the cached resource if it exists. + Fetch(key string) (*Resource, error) // SetResponse sets the cache response and returns the list of requests. SetResponse(key string, resp v2.DiscoveryResponse) ([]*v2.DiscoveryRequest, error) @@ -33,14 +33,14 @@ type cache struct { // Response stores the raw Discovery Response alongside its marshaled resources. type Response struct { - raw v2.DiscoveryResponse - marshaledResources []*any.Any + Raw v2.DiscoveryResponse + MarshaledResources []gcp_types.MarshaledResource } type Resource struct { - resp *Response - requests []*v2.DiscoveryRequest - expirationTime time.Time + Resp *Response + Requests []*v2.DiscoveryRequest + ExpirationTime time.Time } // OnEvictFunc is a callback function for each eviction. Receives the key and cache value when called. @@ -72,7 +72,7 @@ func NewCache(maxEntries int, onEvicted OnEvictFunc, ttl time.Duration) (Cache, }, nil } -func (c *cache) Fetch(key string) (*Response, error) { +func (c *cache) Fetch(key string) (*Resource, error) { c.cacheMu.RLock() value, found := c.cache.Get(key) c.cacheMu.RUnlock() @@ -104,7 +104,7 @@ func (c *cache) Fetch(key string) (*Response, error) { return nil, nil } } - return resource.resp, nil + return &resource, nil } func (c *cache) SetResponse(key string, resp v2.DiscoveryResponse) ([]*v2.DiscoveryRequest, error) { @@ -115,14 +115,14 @@ func (c *cache) SetResponse(key string, resp v2.DiscoveryResponse) ([]*v2.Discov return nil, fmt.Errorf("failed to marshal resources for key: %s, err %v", key, err) } response := &Response{ - raw: resp, - marshaledResources: marshaledResources, + Raw: resp, + MarshaledResources: marshaledResources, } value, found := c.cache.Get(key) if !found { resource := Resource{ - resp: response, - expirationTime: c.getExpirationTime(time.Now()), + Resp: response, + ExpirationTime: c.getExpirationTime(time.Now()), } c.cache.Add(key, resource) return nil, nil @@ -131,11 +131,10 @@ func (c *cache) SetResponse(key string, resp v2.DiscoveryResponse) ([]*v2.Discov if !ok { return nil, fmt.Errorf("unable to cast cache value to type resource for key: %s", key) } - resource.resp = response - resource.expirationTime = c.getExpirationTime(time.Now()) + resource.Resp = response + resource.ExpirationTime = c.getExpirationTime(time.Now()) c.cache.Add(key, resource) - // TODO: Add logic that allows for notifying of watches. - return resource.requests, nil + return resource.Requests, nil } func (c *cache) AddRequest(key string, req v2.DiscoveryRequest) error { @@ -144,8 +143,8 @@ func (c *cache) AddRequest(key string, req v2.DiscoveryRequest) error { value, found := c.cache.Get(key) if !found { resource := Resource{ - requests: []*v2.DiscoveryRequest{&req}, - expirationTime: c.getExpirationTime(time.Now()), + Requests: []*v2.DiscoveryRequest{&req}, + ExpirationTime: c.getExpirationTime(time.Now()), } c.cache.Add(key, resource) return nil @@ -154,17 +153,17 @@ func (c *cache) AddRequest(key string, req v2.DiscoveryRequest) error { if !ok { return fmt.Errorf("unable to cast cache value to type resource for key: %s", key) } - resource.requests = append(resource.requests, &req) - resource.expirationTime = c.getExpirationTime(time.Now()) + resource.Requests = append(resource.Requests, &req) + resource.ExpirationTime = c.getExpirationTime(time.Now()) c.cache.Add(key, resource) return nil } func (r *Resource) isExpired(currentTime time.Time) bool { - if r.expirationTime.IsZero() { + if r.ExpirationTime.IsZero() { return false } - return r.expirationTime.Before(currentTime) + return r.ExpirationTime.Before(currentTime) } func (c *cache) getExpirationTime(currentTime time.Time) time.Time { @@ -174,18 +173,15 @@ func (c *cache) getExpirationTime(currentTime time.Time) time.Time { return time.Time{} } -func marshalResources(resources []*any.Any) ([]*any.Any, error) { - var marshaledResources []*any.Any +func marshalResources(resources []*any.Any) ([]gcp_types.MarshaledResource, error) { + var marshaledResources []gcp_types.MarshaledResource for _, resource := range resources { marshaledResource, err := gcp.MarshalResource(resource) if err != nil { return nil, err } - marshaledResources = append(marshaledResources, &any.Any{ - TypeUrl: resource.TypeUrl, - Value: marshaledResource, - }) + marshaledResources = append(marshaledResources, marshaledResource) } return marshaledResources, nil } diff --git a/internal/app/cache/cache_test.go b/internal/app/cache/cache_test.go index 6b12c3f0..a090488c 100644 --- a/internal/app/cache/cache_test.go +++ b/internal/app/cache/cache_test.go @@ -56,28 +56,30 @@ var testDiscoveryResponse = v2.DiscoveryResponse{ } var testResponse = Response{ - raw: testDiscoveryResponse, - marshaledResources: []*any.Any{ - { - Value: []byte("\x12\x04test"), - }, + Raw: testDiscoveryResponse, + MarshaledResources: [][]byte{ + []byte("\x12\x04test"), }, } +var testResource = Resource{ + Resp: &testResponse, +} + func TestAddRequestAndFetch(t *testing.T) { cache, err := NewCache(1, testOnEvict, time.Second*60) assert.NoError(t, err) - response, err := cache.Fetch(testKeyA) + resource, err := cache.Fetch(testKeyA) assert.EqualError(t, err, "no value found for key: key_A") - assert.Nil(t, response) + assert.Nil(t, resource) err = cache.AddRequest(testKeyA, testRequest) assert.NoError(t, err) - response, err = cache.Fetch(testKeyA) + resource, err = cache.Fetch(testKeyA) assert.NoError(t, err) - assert.Nil(t, response) + assert.Nil(t, resource.Resp) } func TestSetResponseAndFetch(t *testing.T) { @@ -85,17 +87,17 @@ func TestSetResponseAndFetch(t *testing.T) { assert.NoError(t, err) // Simulate cache miss and setting of new response. - response, err := cache.Fetch(testKeyA) + resource, err := cache.Fetch(testKeyA) assert.EqualError(t, err, "no value found for key: key_A") - assert.Nil(t, response) + assert.Nil(t, resource) requests, err := cache.SetResponse(testKeyA, testDiscoveryResponse) assert.NoError(t, err) assert.Nil(t, requests) - response, err = cache.Fetch(testKeyA) + resource, err = cache.Fetch(testKeyA) assert.NoError(t, err) - assert.Equal(t, testResponse, *response) + assert.Equal(t, testResponse, *resource.Resp) } func TestAddRequestAndSetResponse(t *testing.T) { @@ -114,9 +116,9 @@ func TestAddRequestAndSetResponse(t *testing.T) { assert.Equal(t, testRequest, *requests[0]) assert.Equal(t, testRequest, *requests[1]) - response, err := cache.Fetch(testKeyA) + resource, err := cache.Fetch(testKeyA) assert.NoError(t, err) - assert.Equal(t, testResponse, *response) + assert.Equal(t, testResponse, *resource.Resp) } func TestMaxEntries(t *testing.T) { @@ -126,9 +128,9 @@ func TestMaxEntries(t *testing.T) { _, err = cache.SetResponse(testKeyA, testDiscoveryResponse) assert.NoError(t, err) - response, err := cache.Fetch(testKeyA) + resource, err := cache.Fetch(testKeyA) assert.NoError(t, err) - assert.Equal(t, testResponse, *response) + assert.Equal(t, testResponse, *resource.Resp) assert.PanicsWithValue(t, panicValues{ key: testKeyA, @@ -138,13 +140,13 @@ func TestMaxEntries(t *testing.T) { assert.NoError(t, err) }) - response, err = cache.Fetch(testKeyA) + resource, err = cache.Fetch(testKeyA) assert.EqualError(t, err, "no value found for key: key_A") - assert.Nil(t, response) + assert.Nil(t, resource) - response, err = cache.Fetch(testKeyB) + resource, err = cache.Fetch(testKeyB) assert.NoError(t, err) - assert.Nil(t, response) + assert.Nil(t, resource.Resp) } func TestTTL_Enabled(t *testing.T) { @@ -154,23 +156,23 @@ func TestTTL_Enabled(t *testing.T) { _, err = cache.SetResponse(testKeyA, testDiscoveryResponse) assert.NoError(t, err) - response, err := cache.Fetch(testKeyA) + resource, err := cache.Fetch(testKeyA) assert.NoError(t, err) - assert.Equal(t, testResponse, *response) + assert.Equal(t, testResponse, *resource.Resp) time.Sleep(time.Millisecond * 10) assert.PanicsWithValue(t, panicValues{ key: testKeyA, reason: "testOnEvict called", }, func() { - response, err = cache.Fetch(testKeyA) + resource, err = cache.Fetch(testKeyA) assert.NoError(t, err) - assert.Nil(t, response) + assert.Nil(t, resource) }) - response, err = cache.Fetch(testKeyA) + resource, err = cache.Fetch(testKeyA) assert.EqualError(t, err, "no value found for key: key_A") - assert.Nil(t, response) + assert.Nil(t, resource) } func TestTTL_Disabled(t *testing.T) { @@ -181,13 +183,13 @@ func TestTTL_Disabled(t *testing.T) { _, err = cache.SetResponse(testKeyA, testDiscoveryResponse) assert.NoError(t, err) - response, err := cache.Fetch(testKeyA) + resource, err := cache.Fetch(testKeyA) assert.NoError(t, err) - assert.Equal(t, testResponse, *response) + assert.Equal(t, testResponse, *resource.Resp) - gomega.Consistently(func() (*Response, error) { + gomega.Consistently(func() (*Resource, error) { return cache.Fetch(testKeyA) - }).Should(gomega.Equal(&testResponse)) + }).Should(gomega.Equal(&testResource)) } func TestTTL_Negative(t *testing.T) { @@ -200,14 +202,14 @@ func TestIsExpired(t *testing.T) { var resource Resource // The expiration time is 0, meaning TTL is disabled, so the resource is not considered expired. - resource.expirationTime = time.Time{} + resource.ExpirationTime = time.Time{} assert.False(t, resource.isExpired(time.Now())) - resource.expirationTime = time.Now() + resource.ExpirationTime = time.Now() assert.False(t, resource.isExpired(time.Time{})) - resource.expirationTime = time.Now() - assert.True(t, resource.isExpired(resource.expirationTime.Add(1))) + resource.ExpirationTime = time.Now() + assert.True(t, resource.isExpired(resource.ExpirationTime.Add(1))) } func TestGetExpirationTime(t *testing.T) {