separated url parser to its own function, created unit test for the f…

…unction
kubernetes · Mar 13, 2024 · 5345def · 5345def
1 parent cd3ab2c
commit 5345def
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 4 deletions.
diff --git a/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client.go b/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client.go
@@ -595,7 +595,7 @@ func (client *autoscalingGceClientV1) FetchMigTemplateName(migRef GceRef) (Insta
 	if err != nil {
 		return InstanceTemplateName{}, err
 	}
-	regional, err := regexp.MatchString("(/projects/.*[A-Za-z0-9]+.*/regions/)", templateUrl.String())
+	regional, err := IsInstanceTemplateRegional(templateUrl.String())
 	if err != nil {
 		return InstanceTemplateName{}, err
 	}

diff --git a/cluster-autoscaler/cloudprovider/gce/gce_url.go b/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 verifies if the instance template is regional or global
+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)

diff --git a/cluster-autoscaler/cloudprovider/gce/gce_url_test.go b/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)
+		})
+	}
+}
diff --git a/cluster-autoscaler/cloudprovider/gce/mig_info_provider.go b/cluster-autoscaler/cloudprovider/gce/mig_info_provider.go
@@ -21,7 +21,6 @@ import (
 	"fmt"
 	"net/url"
 	"path"
-	"regexp"
 	"strings"
 	"sync"
 	"time"
@@ -364,8 +363,12 @@ func (c *cachingMigInfoProvider) fillMigInfoCache() error {
 				templateUrl, err := url.Parse(zoneMig.InstanceTemplate)
 				if err == nil {
 					_, templateName := path.Split(templateUrl.EscapedPath())
-					regional, _ := regexp.MatchString("(/projects/.*[A-Za-z0-9]+.*/regions/)", templateUrl.String())
-					c.cache.SetMigInstanceTemplateName(zoneMigRef, InstanceTemplateName{templateName, regional})
+					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})
+					}
 				}
 			}
 		}