Backporting support for Regional Instance Template GCE to 1.29

cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client.go

@@ -97,8 +97,8 @@ type AutoscalingGceClient interface {
 	FetchMigTargetSize(GceRef) (int64, error)
 	FetchMigBasename(GceRef) (string, error)
 	FetchMigInstances(GceRef) ([]cloudprovider.Instance, error)
-	FetchMigTemplateName(migRef GceRef) (string, error)
-	FetchMigTemplate(migRef GceRef, templateName string) (*gce.InstanceTemplate, error)
+	FetchMigTemplateName(migRef GceRef) (InstanceTemplateName, error)
+	FetchMigTemplate(migRef GceRef, templateName string, regional bool) (*gce.InstanceTemplate, error)
 	FetchMigsWithName(zone string, filter *regexp.Regexp) ([]string, error)
 	FetchZones(region string) ([]string, error)
 	FetchAvailableCpuPlatforms() (map[string][]string, error)
@@ -550,26 +550,37 @@ func (client *autoscalingGceClientV1) FetchAvailableCpuPlatforms() (map[string][
 	return availableCpuPlatforms, nil
 }
 
-func (client *autoscalingGceClientV1) FetchMigTemplateName(migRef GceRef) (string, error) {
+func (client *autoscalingGceClientV1) FetchMigTemplateName(migRef GceRef) (InstanceTemplateName, error) {
 	registerRequest("instance_group_managers", "get")
 	igm, err := client.gceService.InstanceGroupManagers.Get(migRef.Project, migRef.Zone, migRef.Name).Do()
 	if err != nil {
 		if err, ok := err.(*googleapi.Error); ok {
 			if err.Code == http.StatusNotFound {
-				return "", errors.NewAutoscalerError(errors.NodeGroupDoesNotExistError, "%s", err.Error())
+				return InstanceTemplateName{}, errors.NewAutoscalerError(errors.NodeGroupDoesNotExistError, "%s", err.Error())
 			}
 		}
-		return "", err
+		return InstanceTemplateName{}, err
 	}
 	templateUrl, err := url.Parse(igm.InstanceTemplate)
 	if err != nil {
-		return "", err
+		return InstanceTemplateName{}, err
+	}
+	regional, err := IsInstanceTemplateRegional(templateUrl.String())
+	if err != nil {
+		return InstanceTemplateName{}, err
 	}
+
 	_, templateName := path.Split(templateUrl.EscapedPath())
-	return templateName, nil
+	return InstanceTemplateName{templateName, regional}, nil
 }
 
-func (client *autoscalingGceClientV1) FetchMigTemplate(migRef GceRef, templateName string) (*gce.InstanceTemplate, error) {
+func (client *autoscalingGceClientV1) FetchMigTemplate(migRef GceRef, templateName string, regional bool) (*gce.InstanceTemplate, error) {
+	if regional {
+		zoneHyphenIndex := strings.LastIndex(migRef.Zone, "-")
+		region := migRef.Zone[:zoneHyphenIndex]
+		registerRequest("region_instance_templates", "get")
+		return client.gceService.RegionInstanceTemplates.Get(migRef.Project, region, templateName).Do()
+	}
 	registerRequest("instance_templates", "get")
 	return client.gceService.InstanceTemplates.Get(migRef.Project, templateName).Do()
 }

cluster-autoscaler/cloudprovider/gce/cache.go

@@ -33,6 +33,13 @@ type MachineTypeKey struct {
 	MachineTypeName string
 }
 
+// InstanceTemplateName is used to store the name, and
+// whether or not the instance template is regional
+type InstanceTemplateName struct {
+	Name     string
+	Regional bool
+}
+
 // GceCache is used for caching cluster resources state.
 //
 // It is needed to:
@@ -56,34 +63,36 @@ type GceCache struct {
 	cacheMutex sync.Mutex
 
 	// Cache content.
-	migs                      map[GceRef]Mig
-	instances                 map[GceRef][]cloudprovider.Instance
-	instancesUpdateTime       map[GceRef]time.Time
-	instancesToMig            map[GceRef]GceRef
-	instancesFromUnknownMig   map[GceRef]bool
-	resourceLimiter           *cloudprovider.ResourceLimiter
-	autoscalingOptionsCache   map[GceRef]map[string]string
-	machinesCache             map[MachineTypeKey]MachineType
-	migTargetSizeCache        map[GceRef]int64
-	migBaseNameCache          map[GceRef]string
-	instanceTemplateNameCache map[GceRef]string
-	instanceTemplatesCache    map[GceRef]*gce.InstanceTemplate
+	migs                             map[GceRef]Mig
+	instances                        map[GceRef][]cloudprovider.Instance
+	instancesUpdateTime              map[GceRef]time.Time
+	instancesToMig                   map[GceRef]GceRef
+	instancesFromUnknownMig          map[GceRef]bool
+	resourceLimiter                  *cloudprovider.ResourceLimiter
+	autoscalingOptionsCache          map[GceRef]map[string]string
+	machinesCache                    map[MachineTypeKey]MachineType
+	migTargetSizeCache               map[GceRef]int64
+	migBaseNameCache                 map[GceRef]string
+	listManagedInstancesResultsCache map[GceRef]string
+	instanceTemplateNameCache        map[GceRef]InstanceTemplateName
+	instanceTemplatesCache           map[GceRef]*gce.InstanceTemplate
 }
 
 // NewGceCache creates empty GceCache.
 func NewGceCache() *GceCache {
 	return &GceCache{
-		migs:                      map[GceRef]Mig{},
-		instances:                 map[GceRef][]cloudprovider.Instance{},
-		instancesUpdateTime:       map[GceRef]time.Time{},
-		instancesToMig:            map[GceRef]GceRef{},
-		instancesFromUnknownMig:   map[GceRef]bool{},
-		autoscalingOptionsCache:   map[GceRef]map[string]string{},
-		machinesCache:             map[MachineTypeKey]MachineType{},
-		migTargetSizeCache:        map[GceRef]int64{},
-		migBaseNameCache:          map[GceRef]string{},
-		instanceTemplateNameCache: map[GceRef]string{},
-		instanceTemplatesCache:    map[GceRef]*gce.InstanceTemplate{},
+		migs:                             map[GceRef]Mig{},
+		instances:                        map[GceRef][]cloudprovider.Instance{},
+		instancesUpdateTime:              map[GceRef]time.Time{},
+		instancesToMig:                   map[GceRef]GceRef{},
+		instancesFromUnknownMig:          map[GceRef]bool{},
+		autoscalingOptionsCache:          map[GceRef]map[string]string{},
+		machinesCache:                    map[MachineTypeKey]MachineType{},
+		migTargetSizeCache:               map[GceRef]int64{},
+		migBaseNameCache:                 map[GceRef]string{},
+		listManagedInstancesResultsCache: map[GceRef]string{},
+		instanceTemplateNameCache:        map[GceRef]InstanceTemplateName{},
+		instanceTemplatesCache:           map[GceRef]*gce.InstanceTemplate{},
 	}
 }
 
@@ -330,23 +339,23 @@ func (gc *GceCache) InvalidateAllMigTargetSizes() {
 }
 
 // GetMigInstanceTemplateName returns the cached instance template ref for a mig GceRef
-func (gc *GceCache) GetMigInstanceTemplateName(ref GceRef) (string, bool) {
+func (gc *GceCache) GetMigInstanceTemplateName(ref GceRef) (InstanceTemplateName, bool) {
 	gc.cacheMutex.Lock()
 	defer gc.cacheMutex.Unlock()
 
-	templateName, found := gc.instanceTemplateNameCache[ref]
+	instanceTemplateName, found := gc.instanceTemplateNameCache[ref]
 	if found {
 		klog.V(5).Infof("Instance template names cache hit for %s", ref)
 	}
-	return templateName, found
+	return instanceTemplateName, found
 }
 
 // SetMigInstanceTemplateName sets instance template ref for a mig GceRef
-func (gc *GceCache) SetMigInstanceTemplateName(ref GceRef, templateName string) {
+func (gc *GceCache) SetMigInstanceTemplateName(ref GceRef, instanceTemplateName InstanceTemplateName) {
 	gc.cacheMutex.Lock()
 	defer gc.cacheMutex.Unlock()
 
-	gc.instanceTemplateNameCache[ref] = templateName
+	gc.instanceTemplateNameCache[ref] = instanceTemplateName
 }
 
 // InvalidateMigInstanceTemplateName clears the instance template ref cache for a mig GceRef
@@ -366,7 +375,7 @@ func (gc *GceCache) InvalidateAllMigInstanceTemplateNames() {
 	defer gc.cacheMutex.Unlock()
 
 	klog.V(5).Infof("Instance template names cache invalidated")
-	gc.instanceTemplateNameCache = map[GceRef]string{}
+	gc.instanceTemplateNameCache = map[GceRef]InstanceTemplateName{}
 }
 
 // GetMigInstanceTemplate returns the cached gce.InstanceTemplate for a mig GceRef

cluster-autoscaler/cloudprovider/gce/gce_manager_test.go

@@ -343,10 +343,11 @@ func newTestGceManager(t *testing.T, testServerURL string, regional bool) *gceMa
 			{"us-central1-c", "n1-standard-1"}: {Name: "n1-standard-1", CPU: 1, Memory: 1},
 			{"us-central1-f", "n1-standard-1"}: {Name: "n1-standard-1", CPU: 1, Memory: 1},
 		},
-		migTargetSizeCache:        map[GceRef]int64{},
-		instanceTemplateNameCache: map[GceRef]string{},
-		instanceTemplatesCache:    map[GceRef]*gce.InstanceTemplate{},
-		migBaseNameCache:          map[GceRef]string{},
+		migTargetSizeCache:               map[GceRef]int64{},
+		instanceTemplateNameCache:        map[GceRef]InstanceTemplateName{},
+		instanceTemplatesCache:           map[GceRef]*gce.InstanceTemplate{},
+		migBaseNameCache:                 map[GceRef]string{},
+		listManagedInstancesResultsCache: map[GceRef]string{},
 	}
 	migLister := NewMigLister(cache)
 	manager := &gceManagerImpl{

cluster-autoscaler/cloudprovider/gce/gce_url.go

@@ -78,6 +78,11 @@ func GenerateMigUrl(domainUrl string, ref GceRef) string {
 	return fmt.Sprintf(migUrlTemplate, ref.Project, ref.Zone, ref.Name)
 }
 
+// IsInstanceTemplateRegional determines whether or not an instance template is regional based on the url
+func IsInstanceTemplateRegional(templateUrl string) (bool, error) {
+	return regexp.MatchString("(/projects/.*[A-Za-z0-9]+.*/regions/)", templateUrl)
+}
+
 func parseGceUrl(url, expectedResource string) (project string, zone string, name string, err error) {
 	reg := regexp.MustCompile(fmt.Sprintf("https://.*/projects/.*/zones/.*/%s/.*", expectedResource))
 	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

@@ -263,3 +263,34 @@ func TestParseMigUrl(t *testing.T) {
 		})
 	}
 }
+
+func TestIsInstanceTemplateRegional(t *testing.T) {
+	tests := []struct {
+		name           string
+		templateUrl    string
+		expectRegional bool
+		wantErr        error
+	}{
+		{
+			name:           "Has regional instance url",
+			templateUrl:    "https://www.googleapis.com/compute/v1/projects/test-project/regions/us-central1/instanceTemplates/instance-template",
+			expectRegional: true,
+		},
+		{
+			name:           "Has global instance url",
+			templateUrl:    "https://www.googleapis.com/compute/v1/projects/test-project/global/instanceTemplates/instance-template",
+			expectRegional: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			regional, err := IsInstanceTemplateRegional(tt.templateUrl)
+			assert.Equal(t, tt.wantErr, err)
+			if tt.wantErr != nil {
+				return
+			}
+			assert.Equal(t, tt.expectRegional, regional)
+		})
+	}
+}

cluster-autoscaler/cloudprovider/gce/mig_info_provider.go

@@ -44,7 +44,7 @@ type MigInfoProvider interface {
 	// GetMigBasename returns basename for given MIG ref
 	GetMigBasename(migRef GceRef) (string, error)
 	// GetMigInstanceTemplateName returns instance template name for given MIG ref
-	GetMigInstanceTemplateName(migRef GceRef) (string, error)
+	GetMigInstanceTemplateName(migRef GceRef) (InstanceTemplateName, error)
 	// GetMigInstanceTemplate returns instance template for given MIG ref
 	GetMigInstanceTemplate(migRef GceRef) (*gce.InstanceTemplate, error)
 	// GetMigMachineType returns machine type used by a MIG.
@@ -240,44 +240,44 @@ func (c *cachingMigInfoProvider) GetMigBasename(migRef GceRef) (string, error) {
 	return basename, nil
 }
 
-func (c *cachingMigInfoProvider) GetMigInstanceTemplateName(migRef GceRef) (string, error) {
+func (c *cachingMigInfoProvider) GetMigInstanceTemplateName(migRef GceRef) (InstanceTemplateName, error) {
 	c.migInfoMutex.Lock()
 	defer c.migInfoMutex.Unlock()
 
-	templateName, found := c.cache.GetMigInstanceTemplateName(migRef)
+	instanceTemplateName, found := c.cache.GetMigInstanceTemplateName(migRef)
 	if found {
-		return templateName, nil
+		return instanceTemplateName, nil
 	}
 
 	err := c.fillMigInfoCache()
-	templateName, found = c.cache.GetMigInstanceTemplateName(migRef)
+	instanceTemplateName, found = c.cache.GetMigInstanceTemplateName(migRef)
 	if err == nil && found {
-		return templateName, nil
+		return instanceTemplateName, nil
 	}
 
 	// fallback to querying for single mig
-	templateName, err = c.gceClient.FetchMigTemplateName(migRef)
+	instanceTemplateName, err = c.gceClient.FetchMigTemplateName(migRef)
 	if err != nil {
 		c.migLister.HandleMigIssue(migRef, err)
-		return "", err
+		return InstanceTemplateName{}, err
 	}
-	c.cache.SetMigInstanceTemplateName(migRef, templateName)
-	return templateName, nil
+	c.cache.SetMigInstanceTemplateName(migRef, instanceTemplateName)
+	return instanceTemplateName, nil
 }
 
 func (c *cachingMigInfoProvider) GetMigInstanceTemplate(migRef GceRef) (*gce.InstanceTemplate, error) {
-	templateName, err := c.GetMigInstanceTemplateName(migRef)
+	instanceTemplateName, err := c.GetMigInstanceTemplateName(migRef)
 	if err != nil {
 		return nil, err
 	}
 
 	template, found := c.cache.GetMigInstanceTemplate(migRef)
-	if found && template.Name == templateName {
+	if found && template.Name == instanceTemplateName.Name {
 		return template, nil
 	}
 
-	klog.V(2).Infof("Instance template of mig %v changed to %v", migRef.Name, templateName)
-	template, err = c.gceClient.FetchMigTemplate(migRef, templateName)
+	klog.V(2).Infof("Instance template of mig %v changed to %v", migRef.Name, instanceTemplateName.Name)
+	template, err = c.gceClient.FetchMigTemplate(migRef, instanceTemplateName.Name, instanceTemplateName.Regional)
 	if err != nil {
 		return nil, err
 	}
@@ -336,7 +336,12 @@ func (c *cachingMigInfoProvider) fillMigInfoCache() error {
 				templateUrl, err := url.Parse(zoneMig.InstanceTemplate)
 				if err == nil {
 					_, templateName := path.Split(templateUrl.EscapedPath())
-					c.cache.SetMigInstanceTemplateName(zoneMigRef, templateName)
+					regional, err := IsInstanceTemplateRegional(templateUrl.String())
+					if err != nil {
+						klog.Errorf("Error parsing instance template url: %v; err=%v ", templateUrl.String(), err)
+					} else {
+						c.cache.SetMigInstanceTemplateName(zoneMigRef, InstanceTemplateName{templateName, regional})
+					}
 				}
 			}
 		}