From a115840e4f350d36924e5893518d7fe7d8383620 Mon Sep 17 00:00:00 2001
From: Brendan Burns <bburns@google.com>
Date: Thu, 2 Oct 2014 21:51:09 -0700
Subject: [PATCH] Add a caching layer, so we don't pound cloud providers for ip
 addresses.

---
 pkg/registry/pod/rest.go      | 42 ++++++++++++++++++++++++++++++++---
 pkg/registry/pod/rest_test.go | 35 +++++++++++++++++++++++++++++
 2 files changed, 74 insertions(+), 3 deletions(-)

diff --git a/pkg/registry/pod/rest.go b/pkg/registry/pod/rest.go
index 4b45b096db2..61b40941fa3 100644
--- a/pkg/registry/pod/rest.go
+++ b/pkg/registry/pod/rest.go
@@ -36,6 +36,23 @@ import (
 	"github.com/golang/glog"
 )
 
+type ipCacheEntry struct {
+	ip         string
+	lastUpdate time.Time
+}
+
+type ipCache map[string]ipCacheEntry
+
+type clock interface {
+	Now() time.Time
+}
+
+type realClock struct{}
+
+func (r realClock) Now() time.Time {
+	return time.Now()
+}
+
 // REST implements the RESTStorage interface in terms of a PodRegistry.
 type REST struct {
 	cloudProvider cloudprovider.Interface
@@ -45,6 +62,8 @@ type REST struct {
 	podPollPeriod time.Duration
 	registry      Registry
 	minions       client.MinionInterface
+	ipCache       ipCache
+	clock         clock
 }
 
 type RESTConfig struct {
@@ -64,6 +83,8 @@ func NewREST(config *RESTConfig) *REST {
 		podPollPeriod: time.Second * 10,
 		registry:      config.Registry,
 		minions:       config.Minions,
+		ipCache:       ipCache{},
+		clock:         realClock{},
 	}
 }
 
@@ -112,7 +133,7 @@ func (rs *REST) Get(ctx api.Context, id string) (runtime.Object, error) {
 		}
 		pod.CurrentState.Status = status
 	}
-	pod.CurrentState.HostIP = getInstanceIP(rs.cloudProvider, pod.CurrentState.Host)
+	pod.CurrentState.HostIP = rs.getInstanceIP(pod.CurrentState.Host)
 	return pod, err
 }
 
@@ -144,7 +165,7 @@ func (rs *REST) List(ctx api.Context, label, field labels.Selector) (runtime.Obj
 				return pod, err
 			}
 			pod.CurrentState.Status = status
-			pod.CurrentState.HostIP = getInstanceIP(rs.cloudProvider, pod.CurrentState.Host)
+			pod.CurrentState.HostIP = rs.getInstanceIP(pod.CurrentState.Host)
 		}
 	}
 	return pods, err
@@ -212,7 +233,22 @@ func (rs *REST) fillPodInfo(pod *api.Pod) {
 	}
 }
 
-func getInstanceIP(cloud cloudprovider.Interface, host string) string {
+func (rs *REST) getInstanceIP(host string) string {
+	data, ok := rs.ipCache[host]
+	now := rs.clock.Now()
+
+	if !ok || now.Sub(data.lastUpdate) > (30*time.Second) {
+		ip := getInstanceIPFromCloud(rs.cloudProvider, host)
+		data = ipCacheEntry{
+			ip:         ip,
+			lastUpdate: now,
+		}
+		rs.ipCache[host] = data
+	}
+	return data.ip
+}
+
+func getInstanceIPFromCloud(cloud cloudprovider.Interface, host string) string {
 	if cloud == nil {
 		return ""
 	}
diff --git a/pkg/registry/pod/rest_test.go b/pkg/registry/pod/rest_test.go
index d6e6f0d7e19..f052670a267 100644
--- a/pkg/registry/pod/rest_test.go
+++ b/pkg/registry/pod/rest_test.go
@@ -163,6 +163,14 @@ func TestListEmptyPodList(t *testing.T) {
 	}
 }
 
+type fakeClock struct {
+	t time.Time
+}
+
+func (f *fakeClock) Now() time.Time {
+	return f.t
+}
+
 func TestListPodList(t *testing.T) {
 	podRegistry := registrytest.NewPodRegistry(nil)
 	podRegistry.Pods = &api.PodList{
@@ -181,6 +189,8 @@ func TestListPodList(t *testing.T) {
 	}
 	storage := REST{
 		registry: podRegistry,
+		ipCache:  ipCache{},
+		clock:    &fakeClock{},
 	}
 	ctx := api.NewContext()
 	podsObj, err := storage.List(ctx, labels.Everything(), labels.Everything())
@@ -222,6 +232,8 @@ func TestListPodListSelection(t *testing.T) {
 	}
 	storage := REST{
 		registry: podRegistry,
+		ipCache:  ipCache{},
+		clock:    &fakeClock{},
 	}
 	ctx := api.NewContext()
 
@@ -311,6 +323,8 @@ func TestGetPod(t *testing.T) {
 	podRegistry.Pod = &api.Pod{JSONBase: api.JSONBase{ID: "foo"}}
 	storage := REST{
 		registry: podRegistry,
+		ipCache:  ipCache{},
+		clock:    &fakeClock{},
 	}
 	ctx := api.NewContext()
 	obj, err := storage.Get(ctx, "foo")
@@ -328,9 +342,14 @@ func TestGetPodCloud(t *testing.T) {
 	fakeCloud := &fake_cloud.FakeCloud{}
 	podRegistry := registrytest.NewPodRegistry(nil)
 	podRegistry.Pod = &api.Pod{JSONBase: api.JSONBase{ID: "foo"}}
+
+	clock := &fakeClock{t: time.Now()}
+
 	storage := REST{
 		registry:      podRegistry,
 		cloudProvider: fakeCloud,
+		ipCache:       ipCache{},
+		clock:         clock,
 	}
 	ctx := api.NewContext()
 	obj, err := storage.Get(ctx, "foo")
@@ -342,9 +361,25 @@ func TestGetPodCloud(t *testing.T) {
 	if e, a := podRegistry.Pod, pod; !reflect.DeepEqual(e, a) {
 		t.Errorf("Unexpected pod. Expected %#v, Got %#v", e, a)
 	}
+
+	// This call should hit the cache, so we expect no additional calls to the cloud
+	obj, err = storage.Get(ctx, "foo")
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
 	if len(fakeCloud.Calls) != 1 || fakeCloud.Calls[0] != "ip-address" {
 		t.Errorf("Unexpected calls: %#v", fakeCloud.Calls)
 	}
+
+	// Advance the clock, this call should miss the cache, so expect one more call.
+	clock.t = clock.t.Add(60 * time.Second)
+	obj, err = storage.Get(ctx, "foo")
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	if len(fakeCloud.Calls) != 2 || fakeCloud.Calls[1] != "ip-address" {
+		t.Errorf("Unexpected calls: %#v", fakeCloud.Calls)
+	}
 }
 
 func TestMakePodStatus(t *testing.T) {