Skip to content

Commit ffe9680

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #6570 from Edwinhr716/regional-instance
Regional instance
2 parents 586137e + 771e932 commit ffe9680

File tree

7 files changed

+146
-74
lines changed

7 files changed

+146
-74
lines changed

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

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,8 @@ type AutoscalingGceClient interface {
102102
FetchMigTargetSize(GceRef) (int64, error)
103103
FetchMigBasename(GceRef) (string, error)
104104
FetchMigInstances(GceRef) ([]GceInstance, error)
105-
FetchMigTemplateName(migRef GceRef) (string, error)
106-
FetchMigTemplate(migRef GceRef, templateName string) (*gce.InstanceTemplate, error)
105+
FetchMigTemplateName(migRef GceRef) (InstanceTemplateName, error)
106+
FetchMigTemplate(migRef GceRef, templateName string, regional bool) (*gce.InstanceTemplate, error)
107107
FetchMigsWithName(zone string, filter *regexp.Regexp) ([]string, error)
108108
FetchZones(region string) ([]string, error)
109109
FetchAvailableCpuPlatforms() (map[string][]string, error)
@@ -585,26 +585,37 @@ func (client *autoscalingGceClientV1) FetchAvailableCpuPlatforms() (map[string][
585585
return availableCpuPlatforms, nil
586586
}
587587

588-
func (client *autoscalingGceClientV1) FetchMigTemplateName(migRef GceRef) (string, error) {
588+
func (client *autoscalingGceClientV1) FetchMigTemplateName(migRef GceRef) (InstanceTemplateName, error) {
589589
registerRequest("instance_group_managers", "get")
590590
igm, err := client.gceService.InstanceGroupManagers.Get(migRef.Project, migRef.Zone, migRef.Name).Do()
591591
if err != nil {
592592
if err, ok := err.(*googleapi.Error); ok {
593593
if err.Code == http.StatusNotFound {
594-
return "", errors.NewAutoscalerError(errors.NodeGroupDoesNotExistError, "%s", err.Error())
594+
return InstanceTemplateName{}, errors.NewAutoscalerError(errors.NodeGroupDoesNotExistError, "%s", err.Error())
595595
}
596596
}
597-
return "", err
597+
return InstanceTemplateName{}, err
598598
}
599599
templateUrl, err := url.Parse(igm.InstanceTemplate)
600600
if err != nil {
601-
return "", err
601+
return InstanceTemplateName{}, err
602602
}
603+
regional, err := IsInstanceTemplateRegional(templateUrl.String())
604+
if err != nil {
605+
return InstanceTemplateName{}, err
606+
}
607+
603608
_, templateName := path.Split(templateUrl.EscapedPath())
604-
return templateName, nil
609+
return InstanceTemplateName{templateName, regional}, nil
605610
}
606611

607-
func (client *autoscalingGceClientV1) FetchMigTemplate(migRef GceRef, templateName string) (*gce.InstanceTemplate, error) {
612+
func (client *autoscalingGceClientV1) FetchMigTemplate(migRef GceRef, templateName string, regional bool) (*gce.InstanceTemplate, error) {
613+
if regional {
614+
zoneHyphenIndex := strings.LastIndex(migRef.Zone, "-")
615+
region := migRef.Zone[:zoneHyphenIndex]
616+
registerRequest("region_instance_templates", "get")
617+
return client.gceService.RegionInstanceTemplates.Get(migRef.Project, region, templateName).Do()
618+
}
608619
registerRequest("instance_templates", "get")
609620
return client.gceService.InstanceTemplates.Get(migRef.Project, templateName).Do()
610621
}

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

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,13 @@ type MachineTypeKey struct {
3333
MachineTypeName string
3434
}
3535

36+
// InstanceTemplateName is used to store the name, and
37+
// whether or not the instance template is regional
38+
type InstanceTemplateName struct {
39+
Name string
40+
Regional bool
41+
}
42+
3643
// GceCache is used for caching cluster resources state.
3744
//
3845
// It is needed to:
@@ -67,7 +74,7 @@ type GceCache struct {
6774
migTargetSizeCache map[GceRef]int64
6875
migBaseNameCache map[GceRef]string
6976
listManagedInstancesResultsCache map[GceRef]string
70-
instanceTemplateNameCache map[GceRef]string
77+
instanceTemplateNameCache map[GceRef]InstanceTemplateName
7178
instanceTemplatesCache map[GceRef]*gce.InstanceTemplate
7279
kubeEnvCache map[GceRef]KubeEnv
7380
}
@@ -85,7 +92,7 @@ func NewGceCache() *GceCache {
8592
migTargetSizeCache: map[GceRef]int64{},
8693
migBaseNameCache: map[GceRef]string{},
8794
listManagedInstancesResultsCache: map[GceRef]string{},
88-
instanceTemplateNameCache: map[GceRef]string{},
95+
instanceTemplateNameCache: map[GceRef]InstanceTemplateName{},
8996
instanceTemplatesCache: map[GceRef]*gce.InstanceTemplate{},
9097
kubeEnvCache: map[GceRef]KubeEnv{},
9198
}
@@ -334,23 +341,23 @@ func (gc *GceCache) InvalidateAllMigTargetSizes() {
334341
}
335342

336343
// GetMigInstanceTemplateName returns the cached instance template ref for a mig GceRef
337-
func (gc *GceCache) GetMigInstanceTemplateName(ref GceRef) (string, bool) {
344+
func (gc *GceCache) GetMigInstanceTemplateName(ref GceRef) (InstanceTemplateName, bool) {
338345
gc.cacheMutex.Lock()
339346
defer gc.cacheMutex.Unlock()
340347

341-
templateName, found := gc.instanceTemplateNameCache[ref]
348+
instanceTemplateName, found := gc.instanceTemplateNameCache[ref]
342349
if found {
343350
klog.V(5).Infof("Instance template names cache hit for %s", ref)
344351
}
345-
return templateName, found
352+
return instanceTemplateName, found
346353
}
347354

348355
// SetMigInstanceTemplateName sets instance template ref for a mig GceRef
349-
func (gc *GceCache) SetMigInstanceTemplateName(ref GceRef, templateName string) {
356+
func (gc *GceCache) SetMigInstanceTemplateName(ref GceRef, instanceTemplateName InstanceTemplateName) {
350357
gc.cacheMutex.Lock()
351358
defer gc.cacheMutex.Unlock()
352359

353-
gc.instanceTemplateNameCache[ref] = templateName
360+
gc.instanceTemplateNameCache[ref] = instanceTemplateName
354361
}
355362

356363
// InvalidateMigInstanceTemplateName clears the instance template ref cache for a mig GceRef
@@ -370,7 +377,7 @@ func (gc *GceCache) InvalidateAllMigInstanceTemplateNames() {
370377
defer gc.cacheMutex.Unlock()
371378

372379
klog.V(5).Infof("Instance template names cache invalidated")
373-
gc.instanceTemplateNameCache = map[GceRef]string{}
380+
gc.instanceTemplateNameCache = map[GceRef]InstanceTemplateName{}
374381
}
375382

376383
// GetMigInstanceTemplate returns the cached gce.InstanceTemplate for a mig GceRef

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,7 @@ func newTestGceManager(t *testing.T, testServerURL string, regional bool) *gceMa
343343
{"us-central1-f", "n1-standard-1"}: {Name: "n1-standard-1", CPU: 1, Memory: 1},
344344
},
345345
migTargetSizeCache: map[GceRef]int64{},
346-
instanceTemplateNameCache: map[GceRef]string{},
346+
instanceTemplateNameCache: map[GceRef]InstanceTemplateName{},
347347
instanceTemplatesCache: map[GceRef]*gce.InstanceTemplate{},
348348
kubeEnvCache: map[GceRef]KubeEnv{},
349349
migBaseNameCache: map[GceRef]string{},

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,11 @@ func GenerateMigUrl(domainUrl string, ref GceRef) string {
7878
return fmt.Sprintf(migUrlTemplate, ref.Project, ref.Zone, ref.Name)
7979
}
8080

81+
// IsInstanceTemplateRegional determines whether or not an instance template is regional based on the url
82+
func IsInstanceTemplateRegional(templateUrl string) (bool, error) {
83+
return regexp.MatchString("(/projects/.*[A-Za-z0-9]+.*/regions/)", templateUrl)
84+
}
85+
8186
func parseGceUrl(url, expectedResource string) (project string, zone string, name string, err error) {
8287
reg := regexp.MustCompile(fmt.Sprintf("https://.*/projects/.*/zones/.*/%s/.*", expectedResource))
8388
errMsg := fmt.Errorf("wrong url: expected format <url>/projects/<project-id>/zones/<zone>/%s/<name>, got %s", expectedResource, url)

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,3 +263,34 @@ func TestParseMigUrl(t *testing.T) {
263263
})
264264
}
265265
}
266+
267+
func TestIsInstanceTemplateRegional(t *testing.T) {
268+
tests := []struct {
269+
name string
270+
templateUrl string
271+
expectRegional bool
272+
wantErr error
273+
}{
274+
{
275+
name: "Has regional instance url",
276+
templateUrl: "https://www.googleapis.com/compute/v1/projects/test-project/regions/us-central1/instanceTemplates/instance-template",
277+
expectRegional: true,
278+
},
279+
{
280+
name: "Has global instance url",
281+
templateUrl: "https://www.googleapis.com/compute/v1/projects/test-project/global/instanceTemplates/instance-template",
282+
expectRegional: false,
283+
},
284+
}
285+
286+
for _, tt := range tests {
287+
t.Run(tt.name, func(t *testing.T) {
288+
regional, err := IsInstanceTemplateRegional(tt.templateUrl)
289+
assert.Equal(t, tt.wantErr, err)
290+
if tt.wantErr != nil {
291+
return
292+
}
293+
assert.Equal(t, tt.expectRegional, regional)
294+
})
295+
}
296+
}

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

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ type MigInfoProvider interface {
4343
// GetMigBasename returns basename for given MIG ref
4444
GetMigBasename(migRef GceRef) (string, error)
4545
// GetMigInstanceTemplateName returns instance template name for given MIG ref
46-
GetMigInstanceTemplateName(migRef GceRef) (string, error)
46+
GetMigInstanceTemplateName(migRef GceRef) (InstanceTemplateName, error)
4747
// GetMigInstanceTemplate returns instance template for given MIG ref
4848
GetMigInstanceTemplate(migRef GceRef) (*gce.InstanceTemplate, error)
4949
// GetMigKubeEnv returns kube-env for given MIG ref
@@ -243,44 +243,44 @@ func (c *cachingMigInfoProvider) GetMigBasename(migRef GceRef) (string, error) {
243243
return basename, nil
244244
}
245245

246-
func (c *cachingMigInfoProvider) GetMigInstanceTemplateName(migRef GceRef) (string, error) {
246+
func (c *cachingMigInfoProvider) GetMigInstanceTemplateName(migRef GceRef) (InstanceTemplateName, error) {
247247
c.migInfoMutex.Lock()
248248
defer c.migInfoMutex.Unlock()
249249

250-
templateName, found := c.cache.GetMigInstanceTemplateName(migRef)
250+
instanceTemplateName, found := c.cache.GetMigInstanceTemplateName(migRef)
251251
if found {
252-
return templateName, nil
252+
return instanceTemplateName, nil
253253
}
254254

255255
err := c.fillMigInfoCache()
256-
templateName, found = c.cache.GetMigInstanceTemplateName(migRef)
256+
instanceTemplateName, found = c.cache.GetMigInstanceTemplateName(migRef)
257257
if err == nil && found {
258-
return templateName, nil
258+
return instanceTemplateName, nil
259259
}
260260

261261
// fallback to querying for single mig
262-
templateName, err = c.gceClient.FetchMigTemplateName(migRef)
262+
instanceTemplateName, err = c.gceClient.FetchMigTemplateName(migRef)
263263
if err != nil {
264264
c.migLister.HandleMigIssue(migRef, err)
265-
return "", err
265+
return InstanceTemplateName{}, err
266266
}
267-
c.cache.SetMigInstanceTemplateName(migRef, templateName)
268-
return templateName, nil
267+
c.cache.SetMigInstanceTemplateName(migRef, instanceTemplateName)
268+
return instanceTemplateName, nil
269269
}
270270

271271
func (c *cachingMigInfoProvider) GetMigInstanceTemplate(migRef GceRef) (*gce.InstanceTemplate, error) {
272-
templateName, err := c.GetMigInstanceTemplateName(migRef)
272+
instanceTemplateName, err := c.GetMigInstanceTemplateName(migRef)
273273
if err != nil {
274274
return nil, err
275275
}
276276

277277
template, found := c.cache.GetMigInstanceTemplate(migRef)
278-
if found && template.Name == templateName {
278+
if found && template.Name == instanceTemplateName.Name {
279279
return template, nil
280280
}
281281

282-
klog.V(2).Infof("Instance template of mig %v changed to %v", migRef.Name, templateName)
283-
template, err = c.gceClient.FetchMigTemplate(migRef, templateName)
282+
klog.V(2).Infof("Instance template of mig %v changed to %v", migRef.Name, instanceTemplateName.Name)
283+
template, err = c.gceClient.FetchMigTemplate(migRef, instanceTemplateName.Name, instanceTemplateName.Regional)
284284
if err != nil {
285285
return nil, err
286286
}
@@ -289,13 +289,13 @@ func (c *cachingMigInfoProvider) GetMigInstanceTemplate(migRef GceRef) (*gce.Ins
289289
}
290290

291291
func (c *cachingMigInfoProvider) GetMigKubeEnv(migRef GceRef) (KubeEnv, error) {
292-
templateName, err := c.GetMigInstanceTemplateName(migRef)
292+
instanceTemplateName, err := c.GetMigInstanceTemplateName(migRef)
293293
if err != nil {
294294
return KubeEnv{}, err
295295
}
296296

297297
kubeEnv, kubeEnvFound := c.cache.GetMigKubeEnv(migRef)
298-
if kubeEnvFound && kubeEnv.templateName == templateName {
298+
if kubeEnvFound && kubeEnv.templateName == instanceTemplateName.Name {
299299
return kubeEnv, nil
300300
}
301301

@@ -363,7 +363,12 @@ func (c *cachingMigInfoProvider) fillMigInfoCache() error {
363363
templateUrl, err := url.Parse(zoneMig.InstanceTemplate)
364364
if err == nil {
365365
_, templateName := path.Split(templateUrl.EscapedPath())
366-
c.cache.SetMigInstanceTemplateName(zoneMigRef, templateName)
366+
regional, err := IsInstanceTemplateRegional(templateUrl.String())
367+
if err != nil {
368+
klog.Errorf("Error parsing instance template url: %v; err=%v ", templateUrl.String(), err)
369+
} else {
370+
c.cache.SetMigInstanceTemplateName(zoneMigRef, InstanceTemplateName{templateName, regional})
371+
}
367372
}
368373
}
369374
}

0 commit comments

Comments
 (0)