Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regional instance #6570

Merged
merged 12 commits into from
Mar 14, 2024
Merged
27 changes: 19 additions & 8 deletions cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@ type AutoscalingGceClient interface {
FetchMigTargetSize(GceRef) (int64, error)
FetchMigBasename(GceRef) (string, error)
FetchMigInstances(GceRef) ([]GceInstance, 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)
Expand Down Expand Up @@ -580,26 +580,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()
}
Expand Down
23 changes: 15 additions & 8 deletions cluster-autoscaler/cloudprovider/gce/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Edwinhr716 marked this conversation as resolved.
Show resolved Hide resolved
}

// GceCache is used for caching cluster resources state.
//
// It is needed to:
Expand Down Expand Up @@ -67,7 +74,7 @@ type GceCache struct {
migTargetSizeCache map[GceRef]int64
migBaseNameCache map[GceRef]string
listManagedInstancesResultsCache map[GceRef]string
instanceTemplateNameCache map[GceRef]string
instanceTemplateNameCache map[GceRef]InstanceTemplateName
instanceTemplatesCache map[GceRef]*gce.InstanceTemplate
kubeEnvCache map[GceRef]KubeEnv
}
Expand All @@ -85,7 +92,7 @@ func NewGceCache() *GceCache {
migTargetSizeCache: map[GceRef]int64{},
migBaseNameCache: map[GceRef]string{},
listManagedInstancesResultsCache: map[GceRef]string{},
instanceTemplateNameCache: map[GceRef]string{},
instanceTemplateNameCache: map[GceRef]InstanceTemplateName{},
instanceTemplatesCache: map[GceRef]*gce.InstanceTemplate{},
kubeEnvCache: map[GceRef]KubeEnv{},
}
Expand Down Expand Up @@ -334,23 +341,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
Expand All @@ -370,7 +377,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
Expand Down
2 changes: 1 addition & 1 deletion cluster-autoscaler/cloudprovider/gce/gce_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ func newTestGceManager(t *testing.T, testServerURL string, regional bool) *gceMa
{"us-central1-f", "n1-standard-1"}: {Name: "n1-standard-1", CPU: 1, Memory: 1},
},
migTargetSizeCache: map[GceRef]int64{},
instanceTemplateNameCache: map[GceRef]string{},
instanceTemplateNameCache: map[GceRef]InstanceTemplateName{},
instanceTemplatesCache: map[GceRef]*gce.InstanceTemplate{},
kubeEnvCache: map[GceRef]KubeEnv{},
migBaseNameCache: map[GceRef]string{},
Expand Down
5 changes: 5 additions & 0 deletions cluster-autoscaler/cloudprovider/gce/gce_url.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
31 changes: 31 additions & 0 deletions cluster-autoscaler/cloudprovider/gce/gce_url_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
}
39 changes: 22 additions & 17 deletions cluster-autoscaler/cloudprovider/gce/mig_info_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,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)
Edwinhr716 marked this conversation as resolved.
Show resolved Hide resolved
// GetMigKubeEnv returns kube-env for given MIG ref
Expand Down Expand Up @@ -243,44 +243,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
}
Expand All @@ -289,13 +289,13 @@ func (c *cachingMigInfoProvider) GetMigInstanceTemplate(migRef GceRef) (*gce.Ins
}

func (c *cachingMigInfoProvider) GetMigKubeEnv(migRef GceRef) (KubeEnv, error) {
templateName, err := c.GetMigInstanceTemplateName(migRef)
instanceTemplateName, err := c.GetMigInstanceTemplateName(migRef)
if err != nil {
return KubeEnv{}, err
}

kubeEnv, kubeEnvFound := c.cache.GetMigKubeEnv(migRef)
if kubeEnvFound && kubeEnv.templateName == templateName {
if kubeEnvFound && kubeEnv.templateName == instanceTemplateName.Name {
return kubeEnv, nil
}

Expand Down Expand Up @@ -363,7 +363,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})
}
}
}
}
Expand Down
Loading
Loading