Skip to content

Commit e4b015c

Browse files
nicknaginicknagi
committed
Handle Multiple Projects For MIG Cache
1 parent 45d513d commit e4b015c

File tree

4 files changed

+73
-20
lines changed

4 files changed

+73
-20
lines changed

‎cluster-autoscaler/cloudprovider/gce/gce_manager.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ func CreateGceManager(configReader io.Reader, discoveryOpts cloudprovider.NodeGr
188188
cache: cache,
189189
GceService: gceService,
190190
migLister: migLister,
191-
migInfoProvider: NewCachingMigInfoProvider(cache, migLister, gceService, projectId, concurrentGceRefreshes, migInstancesMinRefreshWaitTime, bulkGceMigInstancesListingEnabled),
191+
migInfoProvider: NewCachingMigInfoProvider(cache, migLister, gceService, projectId, concurrentGceRefreshes, migInstancesMinRefreshWaitTime, bulkGceMigInstancesListingEnabled, false),
192192
location: location,
193193
regional: regional,
194194
projectId: projectId,

‎cluster-autoscaler/cloudprovider/gce/gce_manager_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ const listInstanceGroupManagerResponsePartTemplate = `
253253
}
254254
}
255255
],
256-
"instanceGroup": "https://www.googleapis.com/compute/v1/projects/lukaszos-gke-dev2/zones/%v/instanceGroups/%v",
256+
"instanceGroup": "https://www.googleapis.com/compute/v1/projects/project1/zones/%v/instanceGroups/%v",
257257
"baseInstanceName": "%s",
258258
"fingerprint": "ASJwTpesjDI=",
259259
"currentActions": {
@@ -271,7 +271,7 @@ const listInstanceGroupManagerResponsePartTemplate = `
271271
"isStable": true
272272
},
273273
"targetSize": %v,
274-
"selfLink": "https://www.googleapis.com/compute/v1/projects/lukaszos-gke-dev2/zones/us-west1-b/instanceGroupManagers/gke-blah-default-pool-67b773a0-grp",
274+
"selfLink": "https://www.googleapis.com/compute/v1/projects/project1/zones/us-west1-b/instanceGroupManagers/gke-blah-default-pool-67b773a0-grp",
275275
"updatePolicy": {
276276
"type": "OPPORTUNISTIC",
277277
"minimalAction": "REPLACE",
@@ -354,7 +354,7 @@ func newTestGceManager(t *testing.T, testServerURL string, regional bool) *gceMa
354354
manager := &gceManagerImpl{
355355
cache: cache,
356356
migLister: migLister,
357-
migInfoProvider: NewCachingMigInfoProvider(cache, migLister, gceService, projectId, 1, 0*time.Second, false),
357+
migInfoProvider: NewCachingMigInfoProvider(cache, migLister, gceService, projectId, 1, 0*time.Second, false, false),
358358
GceService: gceService,
359359
projectId: projectId,
360360
regional: regional,

‎cluster-autoscaler/cloudprovider/gce/mig_info_provider.go‎

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"fmt"
2222
"net/url"
2323
"path"
24+
"regexp"
2425
"strings"
2526
"sync"
2627
"time"
@@ -62,6 +63,11 @@ type timeProvider interface {
6263
Now() time.Time
6364
}
6465

66+
var (
67+
// Compile a regular expression to find the text between "projects/" and the next "/".
68+
migProjectSelfLinkRe = regexp.MustCompile(`projects/([^/]+)`)
69+
)
70+
6571
type cachingMigInfoProvider struct {
6672
migInfoMutex sync.Mutex
6773
cache *GceCache
@@ -73,6 +79,7 @@ type cachingMigInfoProvider struct {
7379
migInstancesMinRefreshWaitTime time.Duration
7480
timeProvider timeProvider
7581
bulkGceMigInstancesListingEnabled bool
82+
multiProjectCachingEnabled bool
7683
}
7784

7885
type realTime struct{}
@@ -82,7 +89,7 @@ func (r *realTime) Now() time.Time {
8289
}
8390

8491
// NewCachingMigInfoProvider creates an instance of caching MigInfoProvider
85-
func NewCachingMigInfoProvider(cache *GceCache, migLister MigLister, gceClient AutoscalingGceClient, projectId string, concurrentGceRefreshes int, migInstancesMinRefreshWaitTime time.Duration, bulkGceMigInstancesListingEnabled bool) MigInfoProvider {
92+
func NewCachingMigInfoProvider(cache *GceCache, migLister MigLister, gceClient AutoscalingGceClient, projectId string, concurrentGceRefreshes int, migInstancesMinRefreshWaitTime time.Duration, bulkGceMigInstancesListingEnabled bool, multiProjectCachingEnabled bool) MigInfoProvider {
8693
return &cachingMigInfoProvider{
8794
cache: cache,
8895
migLister: migLister,
@@ -92,6 +99,7 @@ func NewCachingMigInfoProvider(cache *GceCache, migLister MigLister, gceClient A
9299
migInstancesMinRefreshWaitTime: migInstancesMinRefreshWaitTime,
93100
timeProvider: &realTime{},
94101
bulkGceMigInstancesListingEnabled: bulkGceMigInstancesListingEnabled,
102+
multiProjectCachingEnabled: multiProjectCachingEnabled,
95103
}
96104
}
97105

@@ -479,8 +487,19 @@ func (c *cachingMigInfoProvider) fillMigInfoCache() error {
479487

480488
for idx, zone := range zones {
481489
for _, zoneMig := range migs[idx] {
490+
projectId := c.projectId
491+
if c.multiProjectCachingEnabled {
492+
var err error
493+
projectId, err = extractProjectWithRegex(zoneMig.SelfLink)
494+
if err != nil {
495+
// At this point we assume its the default project but this could eventually lead to a cache miss
496+
// if the project information is incorrect.
497+
projectId = c.projectId
498+
klog.Errorf("Unable to extract projectID from MIG self link: %s, err: %v", zoneMig.SelfLink, err)
499+
}
500+
}
482501
zoneMigRef := GceRef{
483-
c.projectId,
502+
projectId,
484503
zone,
485504
zoneMig.Name,
486505
}
@@ -508,6 +527,19 @@ func (c *cachingMigInfoProvider) fillMigInfoCache() error {
508527
return nil
509528
}
510529

530+
// extractProjectWithRegex uses a regular expression to find and return the project name
531+
// from the selfLink of a MIG.
532+
func extractProjectWithRegex(selflink string) (string, error) {
533+
// FindStringSubmatch returns an array with the full match and all captured groups.
534+
// matches[0] will be the full matched string (e.g., "/projects/some-project").
535+
// matches[1] will be the content of the first capturing group (e.g., "some-project").
536+
matches := migProjectSelfLinkRe.FindStringSubmatch(selflink)
537+
if len(matches) < 2 {
538+
return "", fmt.Errorf("could not find project name in self link: %s", selflink)
539+
}
540+
return matches[1], nil
541+
}
542+
511543
func (c *cachingMigInfoProvider) getRegisteredMigRefs() map[GceRef]bool {
512544
migRefs := make(map[GceRef]bool)
513545
for _, mig := range c.migLister.GetMigs() {

‎cluster-autoscaler/cloudprovider/gce/mig_info_provider_test.go‎

Lines changed: 35 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ func TestFillMigInstances(t *testing.T) {
264264
fetchMigInstances: fetchMigInstancesWithCounter(newInstances, callCounter),
265265
}
266266

267-
provider, ok := NewCachingMigInfoProvider(tc.cache, NewMigLister(tc.cache), client, mig.GceRef().Project, 1, time.Hour, false).(*cachingMigInfoProvider)
267+
provider, ok := NewCachingMigInfoProvider(tc.cache, NewMigLister(tc.cache), client, mig.GceRef().Project, 1, time.Hour, false, false).(*cachingMigInfoProvider)
268268
assert.True(t, ok)
269269
provider.timeProvider = &fakeTime{now: timeNow}
270270

@@ -409,7 +409,7 @@ func TestMigInfoProviderGetMigForInstance(t *testing.T) {
409409
fetchMigs: fetchMigsConst(nil),
410410
}
411411
migLister := NewMigLister(tc.cache)
412-
provider := NewCachingMigInfoProvider(tc.cache, migLister, client, mig.GceRef().Project, 1, 0*time.Second, false)
412+
provider := NewCachingMigInfoProvider(tc.cache, migLister, client, mig.GceRef().Project, 1, 0*time.Second, false, false)
413413

414414
mig, err := provider.GetMigForInstance(instanceRef)
415415

@@ -492,7 +492,7 @@ func TestGetMigInstances(t *testing.T) {
492492
fetchMigInstances: tc.fetchMigInstances,
493493
}
494494
migLister := NewMigLister(tc.cache)
495-
provider, ok := NewCachingMigInfoProvider(tc.cache, migLister, client, mig.GceRef().Project, 1, 0*time.Second, false).(*cachingMigInfoProvider)
495+
provider, ok := NewCachingMigInfoProvider(tc.cache, migLister, client, mig.GceRef().Project, 1, 0*time.Second, false, false).(*cachingMigInfoProvider)
496496
assert.True(t, ok)
497497
provider.timeProvider = &fakeTime{now: newRefreshTime}
498498

@@ -759,7 +759,7 @@ func TestRegenerateMigInstancesCache(t *testing.T) {
759759
fetchAllInstances: tc.fetchAllInstances,
760760
}
761761
migLister := NewMigLister(tc.cache)
762-
provider := NewCachingMigInfoProvider(tc.cache, migLister, client, tc.projectId, 1, 0*time.Second, tc.bulkGceMigInstancesListingEnabled)
762+
provider := NewCachingMigInfoProvider(tc.cache, migLister, client, tc.projectId, 1, 0*time.Second, tc.bulkGceMigInstancesListingEnabled, false)
763763
err := provider.RegenerateMigInstancesCache()
764764

765765
assert.Equal(t, tc.expectedErr, err)
@@ -787,11 +787,19 @@ func TestGetMigTargetSize(t *testing.T) {
787787
Zone: mig.GceRef().Zone,
788788
Name: mig.GceRef().Name,
789789
TargetSize: targetSize,
790+
SelfLink: fmt.Sprintf("projects/%s/zones/%s/instanceGroups/%s", mig.GceRef().Project, mig.GceRef().Zone, mig.GceRef().Name),
791+
}
792+
instanceGroupManager1 := &gce.InstanceGroupManager{
793+
Zone: mig1.GceRef().Zone,
794+
Name: mig1.GceRef().Name,
795+
TargetSize: targetSize,
796+
SelfLink: fmt.Sprintf("projects/%s/zones/%s/instanceGroups/%s", mig1.GceRef().Project, mig1.GceRef().Zone, mig1.GceRef().Name),
790797
}
791798

792799
testCases := []struct {
793800
name string
794801
cache *GceCache
802+
migQuery *gceMig
795803
fetchMigs func(string) ([]*gce.InstanceGroupManager, error)
796804
fetchMigTargetSize func(GceRef) (int64, error)
797805
expectedTargetSize int64
@@ -804,40 +812,53 @@ func TestGetMigTargetSize(t *testing.T) {
804812
migTargetSizeCache: map[GceRef]int64{mig.GceRef(): targetSize},
805813
},
806814
expectedTargetSize: targetSize,
815+
migQuery: mig,
807816
},
808817
{
809818
name: "target size from cache fill",
810819
cache: emptyCache(),
811820
fetchMigs: fetchMigsConst([]*gce.InstanceGroupManager{instanceGroupManager}),
812821
expectedTargetSize: targetSize,
822+
migQuery: mig,
823+
},
824+
{
825+
name: "target size from cache fill, multiple MIG projects",
826+
cache: emptyCache(),
827+
fetchMigs: fetchMigsConst([]*gce.InstanceGroupManager{instanceGroupManager, instanceGroupManager1}),
828+
expectedTargetSize: targetSize,
829+
migQuery: mig1,
813830
},
814831
{
815832
name: "cache fill without mig, fallback success",
816833
cache: emptyCache(),
817834
fetchMigs: fetchMigsConst([]*gce.InstanceGroupManager{}),
818835
fetchMigTargetSize: fetchMigTargetSizeConst(targetSize),
819836
expectedTargetSize: targetSize,
837+
migQuery: mig,
820838
},
821839
{
822840
name: "cache fill failure, fallback success",
823841
cache: emptyCache(),
824842
fetchMigs: fetchMigsFail,
825843
fetchMigTargetSize: fetchMigTargetSizeConst(targetSize),
826844
expectedTargetSize: targetSize,
845+
migQuery: mig,
827846
},
828847
{
829848
name: "cache fill without mig, fallback failure",
830849
cache: emptyCache(),
831850
fetchMigs: fetchMigsConst([]*gce.InstanceGroupManager{}),
832851
fetchMigTargetSize: fetchMigTargetSizeFail,
833852
expectedErr: errFetchMigTargetSize,
853+
migQuery: mig,
834854
},
835855
{
836856
name: "cache fill failure, fallback failure",
837857
cache: emptyCache(),
838858
fetchMigs: fetchMigsFail,
839859
fetchMigTargetSize: fetchMigTargetSizeFail,
840860
expectedErr: errFetchMigTargetSize,
861+
migQuery: mig,
841862
},
842863
}
843864

@@ -848,10 +869,10 @@ func TestGetMigTargetSize(t *testing.T) {
848869
fetchMigTargetSize: tc.fetchMigTargetSize,
849870
}
850871
migLister := NewMigLister(tc.cache)
851-
provider := NewCachingMigInfoProvider(tc.cache, migLister, client, mig.GceRef().Project, 1, 0*time.Second, false)
872+
provider := NewCachingMigInfoProvider(tc.cache, migLister, client, mig.GceRef().Project, 1, 0*time.Second, false, true)
852873

853-
targetSize, err := provider.GetMigTargetSize(mig.GceRef())
854-
cachedTargetSize, found := tc.cache.GetMigTargetSize(mig.GceRef())
874+
targetSize, err := provider.GetMigTargetSize(tc.migQuery.GceRef())
875+
cachedTargetSize, found := tc.cache.GetMigTargetSize(tc.migQuery.GceRef())
855876

856877
assert.Equal(t, tc.expectedErr, err)
857878
assert.Equal(t, tc.expectedErr == nil, found)
@@ -930,7 +951,7 @@ func TestGetMigBasename(t *testing.T) {
930951
fetchMigBasename: tc.fetchMigBasename,
931952
}
932953
migLister := NewMigLister(tc.cache)
933-
provider := NewCachingMigInfoProvider(tc.cache, migLister, client, mig.GceRef().Project, 1, 0*time.Second, false)
954+
provider := NewCachingMigInfoProvider(tc.cache, migLister, client, mig.GceRef().Project, 1, 0*time.Second, false, false)
934955

935956
basename, err := provider.GetMigBasename(mig.GceRef())
936957
cachedBasename, found := tc.cache.GetMigBasename(mig.GceRef())
@@ -1011,7 +1032,7 @@ func TestGetListManagedInstancesResults(t *testing.T) {
10111032
fetchListManagedInstancesResults: tc.fetchResults,
10121033
}
10131034
migLister := NewMigLister(tc.cache)
1014-
provider := NewCachingMigInfoProvider(tc.cache, migLister, client, mig.GceRef().Project, 1, 0*time.Second, false)
1035+
provider := NewCachingMigInfoProvider(tc.cache, migLister, client, mig.GceRef().Project, 1, 0*time.Second, false, false)
10151036

10161037
results, err := provider.GetListManagedInstancesResults(mig.GceRef())
10171038
cachedResults, found := tc.cache.GetListManagedInstancesResults(mig.GceRef())
@@ -1106,7 +1127,7 @@ func TestGetMigInstanceTemplateName(t *testing.T) {
11061127
fetchMigTemplateName: tc.fetchMigTemplateName,
11071128
}
11081129
migLister := NewMigLister(tc.cache)
1109-
provider := NewCachingMigInfoProvider(tc.cache, migLister, client, mig.GceRef().Project, 1, 0*time.Second, false)
1130+
provider := NewCachingMigInfoProvider(tc.cache, migLister, client, mig.GceRef().Project, 1, 0*time.Second, false, false)
11101131

11111132
instanceTemplateName, err := provider.GetMigInstanceTemplateName(mig.GceRef())
11121133
cachedInstanceTemplateName, found := tc.cache.GetMigInstanceTemplateName(mig.GceRef())
@@ -1212,7 +1233,7 @@ func TestGetMigInstanceTemplate(t *testing.T) {
12121233
fetchMigTemplate: tc.fetchMigTemplate,
12131234
}
12141235
migLister := NewMigLister(tc.cache)
1215-
provider := NewCachingMigInfoProvider(tc.cache, migLister, client, mig.GceRef().Project, 1, 0*time.Second, false)
1236+
provider := NewCachingMigInfoProvider(tc.cache, migLister, client, mig.GceRef().Project, 1, 0*time.Second, false, false)
12161237

12171238
template, err := provider.GetMigInstanceTemplate(mig.GceRef())
12181239
cachedTemplate, found := tc.cache.GetMigInstanceTemplate(mig.GceRef())
@@ -1418,7 +1439,7 @@ func TestGetMigInstanceKubeEnv(t *testing.T) {
14181439
fetchMigTemplate: tc.fetchMigTemplate,
14191440
}
14201441
migLister := NewMigLister(tc.cache)
1421-
provider := NewCachingMigInfoProvider(tc.cache, migLister, client, mig.GceRef().Project, 1, 0*time.Second, false)
1442+
provider := NewCachingMigInfoProvider(tc.cache, migLister, client, mig.GceRef().Project, 1, 0*time.Second, false, false)
14221443

14231444
kubeEnv, err := provider.GetMigKubeEnv(mig.GceRef())
14241445
cachedKubeEnv, found := tc.cache.GetMigKubeEnv(mig.GceRef())
@@ -1513,7 +1534,7 @@ func TestGetMigMachineType(t *testing.T) {
15131534
fetchMachineType: tc.fetchMachineType,
15141535
}
15151536
migLister := NewMigLister(cache)
1516-
provider := NewCachingMigInfoProvider(cache, migLister, client, mig.GceRef().Project, 1, 0*time.Second, false)
1537+
provider := NewCachingMigInfoProvider(cache, migLister, client, mig.GceRef().Project, 1, 0*time.Second, false, false)
15171538
machine, err := provider.GetMigMachineType(mig.GceRef())
15181539
if tc.expectError {
15191540
assert.Error(t, err)
@@ -1916,7 +1937,7 @@ func (f *fakeTime) Now() time.Time {
19161937

19171938
func emptyCache() *GceCache {
19181939
return &GceCache{
1919-
migs: map[GceRef]Mig{mig.GceRef(): mig},
1940+
migs: map[GceRef]Mig{mig.GceRef(): mig, mig1.GceRef(): mig1},
19201941
instances: make(map[GceRef][]GceInstance),
19211942
instancesUpdateTime: make(map[GceRef]time.Time),
19221943
migTargetSizeCache: make(map[GceRef]int64),

0 commit comments

Comments
 (0)