Merge pull request #4032 from ddysher/remove-ip-cache

Remove ip cache, use node status cache instead.

pkg/master/ip_cache.go

@@ -1,70 +0,0 @@
-/*
-Copyright 2014 Google Inc. All rights reserved.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package master
-
-import (
-	"time"
-
-	"github.com/GoogleCloudPlatform/kubernetes/pkg/cloudprovider"
-	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
-
-	"github.com/golang/glog"
-)
-
-// NewIPCache makes a new ip caching layer, which will get IP addresses from cp,
-// and use clock for deciding when to re-get an IP address.
-// Thread-safe.
-//
-// TODO: when we switch to go1.4, this class would be a good candidate for something
-// that could be produced from a template and a type via `go generate`.
-func NewIPCache(cp cloudprovider.Interface, clock util.Clock, ttl time.Duration) *ipCache {
-	return &ipCache{
-		cache: util.NewTimeCache(
-			clock,
-			ttl,
-			func(host string) util.T {
-				return getInstanceIPFromCloud(cp, host)
-			},
-		),
-	}
-}
-
-type ipCache struct {
-	cache util.TimeCache
-}
-
-// GetInstanceIP returns the IP address of host, from the cache
-// if possible, otherwise it asks the cloud provider.
-func (c *ipCache) GetInstanceIP(host string) string {
-	return c.cache.Get(host).(string)
-}
-
-func getInstanceIPFromCloud(cloud cloudprovider.Interface, host string) string {
-	if cloud == nil {
-		return ""
-	}
-	instances, ok := cloud.Instances()
-	if instances == nil || !ok {
-		return ""
-	}
-	addr, err := instances.IPAddress(host)
-	if err != nil {
-		glog.Errorf("Error getting instance IP for %q: %v", host, err)
-		return ""
-	}
-	return addr.String()
-}

pkg/master/ip_cache_test.go

@@ -1,59 +0,0 @@
-/*
-Copyright 2014 Google Inc. All rights reserved.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package master
-
-import (
-	"testing"
-	"time"
-
-	fake_cloud "github.com/GoogleCloudPlatform/kubernetes/pkg/cloudprovider/fake"
-	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
-)
-
-func TestCacheExpire(t *testing.T) {
-	fakeCloud := &fake_cloud.FakeCloud{}
-	clock := &util.FakeClock{time.Now()}
-
-	c := NewIPCache(fakeCloud, clock, 60*time.Second)
-
-	_ = c.GetInstanceIP("foo")
-	// This call should hit the cache, so we expect no additional calls to the cloud
-	_ = c.GetInstanceIP("foo")
-	// Advance the clock, this call should miss the cache, so expect one more call.
-	clock.Time = clock.Time.Add(61 * time.Second)
-	_ = c.GetInstanceIP("foo")
-
-	if len(fakeCloud.Calls) != 2 || fakeCloud.Calls[1] != "ip-address" || fakeCloud.Calls[0] != "ip-address" {
-		t.Errorf("Unexpected calls: %+v", fakeCloud.Calls)
-	}
-}
-
-func TestCacheNotExpire(t *testing.T) {
-	fakeCloud := &fake_cloud.FakeCloud{}
-	clock := &util.FakeClock{time.Now()}
-
-	c := NewIPCache(fakeCloud, clock, 60*time.Second)
-
-	_ = c.GetInstanceIP("foo")
-	// This call should hit the cache, so we expect no additional calls to the cloud
-	clock.Time = clock.Time.Add(60 * time.Second)
-	_ = c.GetInstanceIP("foo")
-
-	if len(fakeCloud.Calls) != 1 || fakeCloud.Calls[0] != "ip-address" {
-		t.Errorf("Unexpected calls: %+v", fakeCloud.Calls)
-	}
-}

pkg/master/master.go

@@ -141,7 +141,6 @@ type Master struct {
 	admissionControl      admission.Interface
 	masterCount           int
 	v1beta3               bool
-	nodeIPCache           IPGetter
 
 	publicIP             net.IP
 	publicReadOnlyPort   int
@@ -295,7 +294,6 @@ func New(c *Config) *Master {
 		authorizer:            c.Authorizer,
 		admissionControl:      c.AdmissionControl,
 		v1beta3:               c.EnableV1Beta3,
-		nodeIPCache:           NewIPCache(c.Cloud, util.RealClock{}, c.CacheTimeout),
 
 		cacheTimeout: c.CacheTimeout,
 
@@ -374,7 +372,6 @@ func (m *Master) init(c *Config) {
 
 	nodeRESTStorage := minion.NewREST(m.minionRegistry)
 	podCache := NewPodCache(
-		m.nodeIPCache,
 		c.KubeletClient,
 		RESTStorageToNodes(nodeRESTStorage).Nodes(),
 		m.podRegistry,

pkg/master/pod_cache.go

@@ -28,14 +28,9 @@ import (
 	"github.com/golang/glog"
 )
 
-type IPGetter interface {
-	GetInstanceIP(host string) (ip string)
-}
-
 // PodCache contains both a cache of container information, as well as the mechanism for keeping
 // that cache up to date.
 type PodCache struct {
-	ipCache       IPGetter
 	containerInfo client.PodInfoGetter
 	pods          pod.Registry
 	// For confirming existance of a node
@@ -57,9 +52,8 @@ type objKey struct {
 // NewPodCache returns a new PodCache which watches container information
 // registered in the given PodRegistry.
 // TODO(lavalamp): pods should be a client.PodInterface.
-func NewPodCache(ipCache IPGetter, info client.PodInfoGetter, nodes client.NodeInterface, pods pod.Registry) *PodCache {
+func NewPodCache(info client.PodInfoGetter, nodes client.NodeInterface, pods pod.Registry) *PodCache {
 	return &PodCache{
-		ipCache:       ipCache,
 		containerInfo: info,
 		pods:          pods,
 		nodes:         nodes,
@@ -187,7 +181,7 @@ func (p *PodCache) computePodStatus(pod *api.Pod) (api.PodStatus, error) {
 	}
 
 	result, err := p.containerInfo.GetPodStatus(pod.Status.Host, pod.Namespace, pod.Name)
-	newStatus.HostIP = p.ipCache.GetInstanceIP(pod.Status.Host)
+	newStatus.HostIP = nodeStatus.HostIP
 
 	if err != nil {
 		newStatus.Phase = api.PodUnknown

pkg/master/pod_cache_test.go

@@ -74,7 +74,7 @@ func (f *FakePodInfoGetter) GetPodStatus(host, namespace, name string) (api.PodS
 }
 
 func TestPodCacheGetDifferentNamespace(t *testing.T) {
-	cache := NewPodCache(nil, nil, nil, nil)
+	cache := NewPodCache(nil, nil, nil)
 
 	expectedDefault := api.PodStatus{
 		Info: api.PodInfo{
@@ -108,7 +108,7 @@ func TestPodCacheGetDifferentNamespace(t *testing.T) {
 }
 
 func TestPodCacheGet(t *testing.T) {
-	cache := NewPodCache(nil, nil, nil, nil)
+	cache := NewPodCache(nil, nil, nil)
 
 	expected := api.PodStatus{
 		Info: api.PodInfo{
@@ -156,21 +156,14 @@ func TestPodCacheDelete(t *testing.T) {
 	if err != client.ErrPodInfoNotAvailable {
 		t.Errorf("Unexpected error: %v, expecting: %v", err, client.ErrPodInfoNotAvailable)
 	}
-
 }
 
 func TestPodCacheGetMissing(t *testing.T) {
 	pod1 := makePod(api.NamespaceDefault, "foo", "machine", "bar")
 	config := podCacheTestConfig{
-		ipFunc: func(host string) string {
-			if host == "machine" {
-				return "1.2.3.5"
-			}
-			return ""
-		},
 		kubeletContainerInfo: api.PodStatus{
 			Info: api.PodInfo{"bar": api.ContainerStatus{}}},
-		nodes: []api.Node{*makeHealthyNode("machine")},
+		nodes: []api.Node{*makeHealthyNode("machine", "1.2.3.5")},
 		pod:   pod1,
 	}
 	cache := config.Construct()
@@ -195,14 +188,7 @@ func TestPodCacheGetMissing(t *testing.T) {
 	}
 }
 
-type fakeIPCache func(string) string
-
-func (f fakeIPCache) GetInstanceIP(host string) (ip string) {
-	return f(host)
-}
-
 type podCacheTestConfig struct {
-	ipFunc               func(string) string // Construct will set a default if nil
 	nodes                []api.Node
 	pods                 []api.Pod
 	pod                  *api.Pod
@@ -216,11 +202,6 @@ type podCacheTestConfig struct {
 }
 
 func (c *podCacheTestConfig) Construct() *PodCache {
-	if c.ipFunc == nil {
-		c.ipFunc = func(host string) string {
-			return "ip of " + host
-		}
-	}
 	c.fakePodInfo = &FakePodInfoGetter{
 		data: api.PodStatusResult{
 			Status: c.kubeletContainerInfo,
@@ -235,7 +216,6 @@ func (c *podCacheTestConfig) Construct() *PodCache {
 	c.fakePods.Pod = c.pod
 	c.fakePods.Err = c.err
 	return NewPodCache(
-		fakeIPCache(c.ipFunc),
 		c.fakePodInfo,
 		c.fakeNodes.Nodes(),
 		c.fakePods,
@@ -253,12 +233,15 @@ func makePod(namespace, name, host string, containers ...string) *api.Pod {
 	return pod
 }
 
-func makeHealthyNode(name string) *api.Node {
+func makeHealthyNode(name string, ip string) *api.Node {
 	return &api.Node{
 		ObjectMeta: api.ObjectMeta{Name: name},
-		Status: api.NodeStatus{Conditions: []api.NodeCondition{
+		Status: api.NodeStatus{
-			{Kind: api.NodeReady, Status: api.ConditionFull},
+			HostIP: ip,
-		}},
+			Conditions: []api.NodeCondition{
+				{Kind: api.NodeReady, Status: api.ConditionFull},
+			},
+		},
 	}
 }
 
@@ -275,15 +258,9 @@ func TestPodUpdateAllContainers(t *testing.T) {
 	pod1 := makePod(api.NamespaceDefault, "foo", "machine", "bar")
 	pod2 := makePod(api.NamespaceDefault, "baz", "machine", "qux")
 	config := podCacheTestConfig{
-		ipFunc: func(host string) string {
-			if host == "machine" {
-				return "1.2.3.5"
-			}
-			return ""
-		},
 		kubeletContainerInfo: api.PodStatus{
 			Info: api.PodInfo{"bar": api.ContainerStatus{}}},
-		nodes: []api.Node{*makeHealthyNode("machine")},
+		nodes: []api.Node{*makeHealthyNode("machine", "1.2.3.5")},
 		pods:  []api.Pod{*pod1, *pod2},
 	}
 	cache := config.Construct()
@@ -326,7 +303,7 @@ func TestFillPodStatusNoHost(t *testing.T) {
 	pod := makePod(api.NamespaceDefault, "foo", "", "bar")
 	config := podCacheTestConfig{
 		kubeletContainerInfo: api.PodStatus{},
-		nodes:                []api.Node{*makeHealthyNode("machine")},
+		nodes:                []api.Node{*makeHealthyNode("machine", "")},
 		pods:                 []api.Pod{*pod},
 	}
 	cache := config.Construct()
@@ -382,7 +359,7 @@ func TestFillPodStatus(t *testing.T) {
 				},
 			},
 		},
-		nodes: []api.Node{*makeHealthyNode("machine")},
+		nodes: []api.Node{*makeHealthyNode("machine", "ip of machine")},
 		pods:  []api.Pod{*pod},
 	}
 	cache := config.Construct()
@@ -409,7 +386,7 @@ func TestFillPodInfoNoData(t *testing.T) {
 				leaky.PodInfraContainerName: {},
 			},
 		},
-		nodes: []api.Node{*makeHealthyNode("machine")},
+		nodes: []api.Node{*makeHealthyNode("machine", "ip of machine")},
 		pods:  []api.Pod{*pod},
 	}
 	cache := config.Construct()