Merge pull request kubernetes#1735 from vivekbagade/feature/cacheTarg…

…etSize

Added target size cache to gceCache for GKEMig

cluster-autoscaler/cloudprovider/gce/cache.go

@@ -62,13 +62,14 @@ type MachineTypeKey struct {
 // RegenerateInstancesCache is required to sync it with registered migs.
 type GceCache struct {
 	// Cache content.
-	migs            []*MigInformation
-	instancesCache  map[GceRef]Mig
-	resourceLimiter *cloudprovider.ResourceLimiter
-	machinesCache   map[MachineTypeKey]*gce.MachineType
+	migs               []*MigInformation
+	instancesCache     map[GceRef]Mig
+	resourceLimiter    *cloudprovider.ResourceLimiter
+	machinesCache      map[MachineTypeKey]*gce.MachineType
+	migTargetSizeCache map[GceRef]int64
 	// Locks. Rules of locking:
 	// - migsMutex protects only migs.
-	// - cacheMutex protects instancesCache, resourceLimiter and machinesCache.
+	// - cacheMutex protects instancesCache, resourceLimiter, machinesCache and migTargetSizeCache.
 	// - if both locks are needed, cacheMutex must be obtained before migsMutex.
 	cacheMutex sync.Mutex
 	migsMutex  sync.Mutex
@@ -79,10 +80,11 @@ type GceCache struct {
 // NewGceCache creates empty GceCache.
 func NewGceCache(gceService AutoscalingGceClient) GceCache {
 	return GceCache{
-		migs:           []*MigInformation{},
-		instancesCache: map[GceRef]Mig{},
-		machinesCache:  map[MachineTypeKey]*gce.MachineType{},
-		GceService:     gceService,
+		migs:               []*MigInformation{},
+		instancesCache:     map[GceRef]Mig{},
+		machinesCache:      map[MachineTypeKey]*gce.MachineType{},
+		GceService:         gceService,
+		migTargetSizeCache: map[GceRef]int64{},
 	}
 }
 
@@ -244,6 +246,46 @@ func (gc *GceCache) GetResourceLimiter() (*cloudprovider.ResourceLimiter, error)
 	return gc.resourceLimiter, nil
 }
 
+// GetMigTargetSize returns the cached targetSize for a GceRef
+func (gc *GceCache) GetMigTargetSize(ref GceRef) (int64, bool) {
+	gc.cacheMutex.Lock()
+	defer gc.cacheMutex.Unlock()
+
+	size, found := gc.migTargetSizeCache[ref]
+	if found {
+		klog.V(5).Infof("target size cache hit for %s", ref)
+	}
+	return size, found
+}
+
+// SetMigTargetSize sets targetSize for a GceRef
+func (gc *GceCache) SetMigTargetSize(ref GceRef, size int64) {
+	gc.cacheMutex.Lock()
+	defer gc.cacheMutex.Unlock()
+
+	gc.migTargetSizeCache[ref] = size
+}
+
+// InvalidateTargetSizeCache clears the target size cache
+func (gc *GceCache) InvalidateTargetSizeCache() {
+	gc.cacheMutex.Lock()
+	defer gc.cacheMutex.Unlock()
+
+	klog.V(5).Infof("target size cache invalidated")
+	gc.migTargetSizeCache = map[GceRef]int64{}
+}
+
+// InvalidateTargetSizeCacheForMig clears the target size cache
+func (gc *GceCache) InvalidateTargetSizeCacheForMig(ref GceRef) {
+	gc.cacheMutex.Lock()
+	defer gc.cacheMutex.Unlock()
+
+	if _, found := gc.migTargetSizeCache[ref]; found {
+		klog.V(5).Infof("target size cache invalidated for %s", ref)
+		delete(gc.migTargetSizeCache, ref)
+	}
+}
+
 // GetMachineFromCache retrieves machine type from cache under lock.
 func (gc *GceCache) GetMachineFromCache(machineType string, zone string) *gce.MachineType {
 	gc.cacheMutex.Lock()

cluster-autoscaler/cloudprovider/gce/gce_manager.go

@@ -206,16 +206,21 @@ func (m *gceManagerImpl) registerMig(mig Mig) bool {
 
 // GetMigSize gets MIG size.
 func (m *gceManagerImpl) GetMigSize(mig Mig) (int64, error) {
+	if migSize, found := m.cache.GetMigTargetSize(mig.GceRef()); found {
+		return migSize, nil
+	}
 	targetSize, err := m.GceService.FetchMigTargetSize(mig.GceRef())
 	if err != nil {
 		return -1, err
 	}
+	m.cache.SetMigTargetSize(mig.GceRef(), targetSize)
 	return targetSize, nil
 }
 
 // SetMigSize sets MIG size.
 func (m *gceManagerImpl) SetMigSize(mig Mig, size int64) error {
 	klog.V(0).Infof("Setting mig size %s to %d", mig.Id(), size)
+	m.cache.InvalidateTargetSizeCacheForMig(mig.GceRef())
 	return m.GceService.ResizeMig(mig.GceRef(), size)
 }
 
@@ -237,7 +242,7 @@ func (m *gceManagerImpl) DeleteInstances(instances []*GceRef) error {
 			return fmt.Errorf("cannot delete instances which don't belong to the same MIG.")
 		}
 	}
-
+	m.cache.InvalidateTargetSizeCacheForMig(commonMig.GceRef())
 	return m.GceService.DeleteInstances(commonMig.GceRef(), instances)
 }
 
@@ -258,6 +263,7 @@ func (m *gceManagerImpl) GetMigNodes(mig Mig) ([]cloudprovider.Instance, error)
 
 // Refresh triggers refresh of cached resources.
 func (m *gceManagerImpl) Refresh() error {
+	m.cache.InvalidateTargetSizeCache()
 	if m.lastRefresh.Add(refreshInterval).After(time.Now()) {
 		return nil
 	}

cluster-autoscaler/cloudprovider/gce/gce_manager_test.go

@@ -74,6 +74,31 @@ const instanceGroupManagerResponseTemplate = `{
 }
 `
 
+const instanceGroupManagerTargetSize4ResponseTemplate = `{
+  "kind": "compute#instanceGroupManager",
+  "id": "3213213219",
+  "creationTimestamp": "2017-09-15T04:47:24.687-07:00",
+  "name": "%s",
+  "zone": "https://www.googleapis.com/compute/v1/projects/project1/zones/%s",
+  "instanceTemplate": "https://www.googleapis.com/compute/v1/projects/project1/global/instanceTemplates/%s",
+  "instanceGroup": "https://www.googleapis.com/compute/v1/projects/project1/zones/%s/instanceGroups/%s",
+  "baseInstanceName": "gke-cluster-1-default-pool-f23aac-grp",
+  "fingerprint": "kfdsuH",
+  "currentActions": {
+    "none": 3,
+    "creating": 0,
+    "creatingWithoutRetries": 0,
+    "recreating": 0,
+    "deleting": 0,
+    "abandoning": 0,
+    "restarting": 0,
+    "refreshing": 0
+  },
+  "targetSize": 4,
+  "selfLink": "https://www.googleapis.com/compute/v1/projects/project1/zones/%s/instanceGroupManagers/%s"
+}
+`
+
 const instanceTemplate = `
 {
  "kind": "compute#instanceTemplate",
@@ -252,6 +277,7 @@ func newTestGceManager(t *testing.T, testServerURL string, regional bool) *gceMa
 				{"us-central1-c", "n1-standard-1"}: {GuestCpus: 1, MemoryMb: 1},
 				{"us-central1-f", "n1-standard-1"}: {GuestCpus: 1, MemoryMb: 1},
 			},
+			migTargetSizeCache: map[GceRef]int64{},
 		},
 		GceService:           gceService,
 		projectId:            projectId,
@@ -392,7 +418,7 @@ func TestDeleteInstances(t *testing.T) {
 	mock.AssertExpectationsForObjects(t, server)
 }
 
-func TestGetMigSize(t *testing.T) {
+func TestGetMigSizeWithCache(t *testing.T) {
 	server := NewHttpServerMock()
 	defer server.Close()
 	g := newTestGceManager(t, server.URL, false)
@@ -412,6 +438,44 @@ func TestGetMigSize(t *testing.T) {
 	size, err := g.GetMigSize(mig)
 	assert.NoError(t, err)
 	assert.Equal(t, int64(3), size)
+
+	size, err = g.GetMigSize(mig)
+	assert.Equal(t, int64(3), size)
+	mock.AssertExpectationsForObjects(t, server)
+}
+
+func TestGetAndSetMigSizeWithCache(t *testing.T) {
+	server := NewHttpServerMock()
+	defer server.Close()
+	g := newTestGceManager(t, server.URL, false)
+
+	server.On("handle", "/project1/zones/us-central1-b/instanceGroupManagers/extra-pool-323233232").Return(instanceGroupManagerResponseTemplate).Once()
+	server.On("handle", "/project1/zones/us-central1-b/instanceGroupManagers/extra-pool-323233232/resize").Return(setMigSizeResponse).Once()
+	server.On("handle", "/project1/zones/us-central1-b/operations/operation-1505739408819-5597646964339-eb839c88-28805931").Return(setMigSizeOperationResponse).Once()
+	server.On("handle", "/project1/zones/us-central1-b/instanceGroupManagers/extra-pool-323233232").Return(instanceGroupManagerTargetSize4ResponseTemplate).Once()
+
+	mig := &gceMig{
+		gceRef: GceRef{
+			Project: projectId,
+			Zone:    zoneB,
+			Name:    "extra-pool-323233232",
+		},
+		gceManager: g,
+		minSize:    0,
+		maxSize:    1000,
+	}
+	size, err := g.GetMigSize(mig)
+	assert.NoError(t, err)
+	assert.Equal(t, int64(3), size)
+
+	err = g.SetMigSize(mig, 4)
+	assert.NoError(t, err)
+
+	size, err = g.GetMigSize(mig)
+	assert.Equal(t, int64(4), size)
+
+	size, err = g.GetMigSize(mig)
+	assert.Equal(t, int64(4), size)
 	mock.AssertExpectationsForObjects(t, server)
 }