Skip to content

Commit

Permalink
Return Resource on cache Fetch
Browse files Browse the repository at this point in the history 
* 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 <jyuen@lyft.com>
  • Loading branch information
@jessicayuen
jessicayuen committed Apr 8, 2020
1 parent 199a63c commit 124d83e
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 66 deletions.
58 changes: 27 additions & 31 deletions internal/app/cache/cache.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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.
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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) {
Expand All @@ -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
Expand All @@ -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 {
Expand All @@ -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
Expand All @@ -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 {
Expand All @@ -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
}
72 changes: 37 additions & 35 deletions internal/app/cache/cache_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,46 +56,48 @@ 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) {
cache, err := NewCache(1, testOnEvict, time.Second*60)
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) {
Expand All @@ -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) {
Expand All @@ -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,
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand Down

0 comments on commit 124d83e

Please sign in to comment.