From f8b5990ada6fdec0d53117b5d40b8b33961933ae Mon Sep 17 00:00:00 2001 From: Maria Oparka Date: Mon, 1 Jul 2024 16:39:25 +0200 Subject: [PATCH] Add InstanceTemplate field to GceInstance --- .../gce/autoscaling_gce_client.go | 25 +++--- .../gce/autoscaling_gce_client_test.go | 82 +++++++++++++++---- .../cloudprovider/gce/gce_url.go | 17 ++++ .../cloudprovider/gce/gce_url_test.go | 36 ++++++++ 4 files changed, 132 insertions(+), 28 deletions(-) diff --git a/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client.go b/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client.go index dc5c43b402fa..4dde60940700 100644 --- a/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client.go +++ b/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client.go @@ -20,7 +20,6 @@ import ( "context" "fmt" "net/http" - "net/url" "path" "regexp" "strings" @@ -96,8 +95,9 @@ var ( // GceInstance extends cloudprovider.Instance with GCE specific numeric id. type GceInstance struct { cloudprovider.Instance - NumericId uint64 - Igm GceRef + NumericId uint64 + Igm GceRef + InstanceTemplateName string } // AutoscalingGceClient is used for communicating with GCE API. @@ -481,6 +481,13 @@ func (i *instanceListBuilder) gceInstanceToInstance(ref GceRef, gceInstance *gce NumericId: gceInstance.Id, } + if gceInstance.Version != nil { + instanceTemplate, err := InstanceTemplateNameFromUrl(gceInstance.Version.InstanceTemplate) + if err == nil { + instance.InstanceTemplateName = instanceTemplate.Name + } + } + if instance.Status.State != cloudprovider.InstanceCreating { return instance } @@ -725,17 +732,7 @@ func (client *autoscalingGceClientV1) FetchMigTemplateName(migRef GceRef) (Insta } return InstanceTemplateName{}, err } - templateUrl, err := url.Parse(igm.InstanceTemplate) - if err != nil { - return InstanceTemplateName{}, err - } - regional, err := IsInstanceTemplateRegional(templateUrl.String()) - if err != nil { - return InstanceTemplateName{}, err - } - - _, templateName := path.Split(templateUrl.EscapedPath()) - return InstanceTemplateName{templateName, regional}, nil + return InstanceTemplateNameFromUrl(igm.InstanceTemplate) } func (client *autoscalingGceClientV1) FetchMigTemplate(migRef GceRef, templateName string, regional bool) (*gce.InstanceTemplate, error) { diff --git a/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client_test.go b/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client_test.go index 1f69573b8a37..b5e829ef2b7d 100644 --- a/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client_test.go +++ b/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client_test.go @@ -245,6 +245,10 @@ func TestErrors(t *testing.T) { func TestFetchMigInstancesInstanceUrlHandling(t *testing.T) { const goodInstanceUrlTempl = "https://content.googleapis.com/compute/v1/projects/myprojid/zones/myzone/instances/myinst_%d" const badInstanceUrl = "https://badurl.com/compute/v1/projects3/myprojid/zones/myzone/instances/myinst" + + const instanceTemplateNameTempl = "my_inst_templ%d" + const instanceTemplateUrlTempl = "https://content.googleapis.com/compute/v1/projects/myprojid/global/instanceTemplates/my_inst_templ%d" + server := test_util.NewHttpServerMock() defer server.Close() g := newTestAutoscalingGceClient(t, "project1", server.URL, "") @@ -266,6 +270,9 @@ func TestFetchMigInstancesInstanceUrlHandling(t *testing.T) { LastAttempt: &gce_api.ManagedInstanceLastAttempt{ Errors: &gce_api.ManagedInstanceLastAttemptErrors{}, }, + Version: &gce_api.ManagedInstanceVersion{ + InstanceTemplate: fmt.Sprintf(instanceTemplateUrlTempl, 2), + }, }, { Id: 42, @@ -274,6 +281,9 @@ func TestFetchMigInstancesInstanceUrlHandling(t *testing.T) { LastAttempt: &gce_api.ManagedInstanceLastAttempt{ Errors: &gce_api.ManagedInstanceLastAttemptErrors{}, }, + Version: &gce_api.ManagedInstanceVersion{ + InstanceTemplate: fmt.Sprintf(instanceTemplateUrlTempl, 42), + }, }, }, }, @@ -283,14 +293,16 @@ func TestFetchMigInstancesInstanceUrlHandling(t *testing.T) { Id: "gce://myprojid/myzone/myinst_2", Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating}, }, - NumericId: 2, + NumericId: 2, + InstanceTemplateName: fmt.Sprintf(instanceTemplateNameTempl, 2), }, { Instance: cloudprovider.Instance{ Id: "gce://myprojid/myzone/myinst_42", Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating}, }, - NumericId: 42, + NumericId: 42, + InstanceTemplateName: fmt.Sprintf(instanceTemplateNameTempl, 42), }, }, }, @@ -305,6 +317,9 @@ func TestFetchMigInstancesInstanceUrlHandling(t *testing.T) { LastAttempt: &gce_api.ManagedInstanceLastAttempt{ Errors: &gce_api.ManagedInstanceLastAttemptErrors{}, }, + Version: &gce_api.ManagedInstanceVersion{ + InstanceTemplate: fmt.Sprintf(instanceTemplateUrlTempl, 2), + }, }, { Id: 42, @@ -313,6 +328,9 @@ func TestFetchMigInstancesInstanceUrlHandling(t *testing.T) { LastAttempt: &gce_api.ManagedInstanceLastAttempt{ Errors: &gce_api.ManagedInstanceLastAttemptErrors{}, }, + Version: &gce_api.ManagedInstanceVersion{ + InstanceTemplate: fmt.Sprintf(instanceTemplateUrlTempl, 42), + }, }, }, NextPageToken: "foo", @@ -327,6 +345,9 @@ func TestFetchMigInstancesInstanceUrlHandling(t *testing.T) { LastAttempt: &gce_api.ManagedInstanceLastAttempt{ Errors: &gce_api.ManagedInstanceLastAttemptErrors{}, }, + Version: &gce_api.ManagedInstanceVersion{ + InstanceTemplate: fmt.Sprintf(instanceTemplateUrlTempl, 127), + }, }, { Id: 456, @@ -335,6 +356,9 @@ func TestFetchMigInstancesInstanceUrlHandling(t *testing.T) { LastAttempt: &gce_api.ManagedInstanceLastAttempt{ Errors: &gce_api.ManagedInstanceLastAttemptErrors{}, }, + Version: &gce_api.ManagedInstanceVersion{ + InstanceTemplate: fmt.Sprintf(instanceTemplateUrlTempl, 17), + }, }, }, }, @@ -345,28 +369,32 @@ func TestFetchMigInstancesInstanceUrlHandling(t *testing.T) { Id: "gce://myprojid/myzone/myinst_2", Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating}, }, - NumericId: 2, + NumericId: 2, + InstanceTemplateName: fmt.Sprintf(instanceTemplateNameTempl, 2), }, { Instance: cloudprovider.Instance{ Id: "gce://myprojid/myzone/myinst_42", Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating}, }, - NumericId: 42, + NumericId: 42, + InstanceTemplateName: fmt.Sprintf(instanceTemplateNameTempl, 42), }, { Instance: cloudprovider.Instance{ Id: "gce://myprojid/myzone/myinst_123", Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating}, }, - NumericId: 123, + NumericId: 123, + InstanceTemplateName: fmt.Sprintf(instanceTemplateNameTempl, 127), }, { Instance: cloudprovider.Instance{ Id: "gce://myprojid/myzone/myinst_456", Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating}, }, - NumericId: 456, + NumericId: 456, + InstanceTemplateName: fmt.Sprintf(instanceTemplateNameTempl, 17), }, }, }, @@ -381,6 +409,9 @@ func TestFetchMigInstancesInstanceUrlHandling(t *testing.T) { LastAttempt: &gce_api.ManagedInstanceLastAttempt{ Errors: &gce_api.ManagedInstanceLastAttemptErrors{}, }, + Version: &gce_api.ManagedInstanceVersion{ + InstanceTemplate: fmt.Sprintf(instanceTemplateUrlTempl, 17), + }, }, { Id: 42, @@ -389,6 +420,9 @@ func TestFetchMigInstancesInstanceUrlHandling(t *testing.T) { LastAttempt: &gce_api.ManagedInstanceLastAttempt{ Errors: &gce_api.ManagedInstanceLastAttemptErrors{}, }, + Version: &gce_api.ManagedInstanceVersion{ + InstanceTemplate: fmt.Sprintf(instanceTemplateUrlTempl, 17), + }, }, }, NextPageToken: "foo", @@ -403,6 +437,9 @@ func TestFetchMigInstancesInstanceUrlHandling(t *testing.T) { LastAttempt: &gce_api.ManagedInstanceLastAttempt{ Errors: &gce_api.ManagedInstanceLastAttemptErrors{}, }, + Version: &gce_api.ManagedInstanceVersion{ + InstanceTemplate: fmt.Sprintf(instanceTemplateUrlTempl, 17), + }, }, { Id: 456, @@ -411,6 +448,9 @@ func TestFetchMigInstancesInstanceUrlHandling(t *testing.T) { LastAttempt: &gce_api.ManagedInstanceLastAttempt{ Errors: &gce_api.ManagedInstanceLastAttemptErrors{}, }, + Version: &gce_api.ManagedInstanceVersion{ + InstanceTemplate: fmt.Sprintf(instanceTemplateUrlTempl, 17), + }, }, }, NextPageToken: "bar", @@ -424,6 +464,9 @@ func TestFetchMigInstancesInstanceUrlHandling(t *testing.T) { LastAttempt: &gce_api.ManagedInstanceLastAttempt{ Errors: &gce_api.ManagedInstanceLastAttemptErrors{}, }, + Version: &gce_api.ManagedInstanceVersion{ + InstanceTemplate: fmt.Sprintf(instanceTemplateUrlTempl, 17), + }, }, { Id: 666, @@ -432,6 +475,9 @@ func TestFetchMigInstancesInstanceUrlHandling(t *testing.T) { LastAttempt: &gce_api.ManagedInstanceLastAttempt{ Errors: &gce_api.ManagedInstanceLastAttemptErrors{}, }, + Version: &gce_api.ManagedInstanceVersion{ + InstanceTemplate: fmt.Sprintf(instanceTemplateUrlTempl, 127), + }, }, }, }, @@ -442,42 +488,48 @@ func TestFetchMigInstancesInstanceUrlHandling(t *testing.T) { Id: "gce://myprojid/myzone/myinst_2", Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating}, }, - NumericId: 2, + NumericId: 2, + InstanceTemplateName: fmt.Sprintf(instanceTemplateNameTempl, 17), }, { Instance: cloudprovider.Instance{ Id: "gce://myprojid/myzone/myinst_42", Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating}, }, - NumericId: 42, + NumericId: 42, + InstanceTemplateName: fmt.Sprintf(instanceTemplateNameTempl, 17), }, { Instance: cloudprovider.Instance{ Id: "gce://myprojid/myzone/myinst_123", Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating}, }, - NumericId: 123, + NumericId: 123, + InstanceTemplateName: fmt.Sprintf(instanceTemplateNameTempl, 17), }, { Instance: cloudprovider.Instance{ Id: "gce://myprojid/myzone/myinst_456", Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating}, }, - NumericId: 456, + NumericId: 456, + InstanceTemplateName: fmt.Sprintf(instanceTemplateNameTempl, 17), }, { Instance: cloudprovider.Instance{ Id: "gce://myprojid/myzone/myinst_789", Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating}, }, - NumericId: 789, + NumericId: 789, + InstanceTemplateName: fmt.Sprintf(instanceTemplateNameTempl, 17), }, { Instance: cloudprovider.Instance{ Id: "gce://myprojid/myzone/myinst_666", Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating}, }, - NumericId: 666, + NumericId: 666, + InstanceTemplateName: fmt.Sprintf(instanceTemplateNameTempl, 127), }, }, }, @@ -509,7 +561,8 @@ func TestFetchMigInstancesInstanceUrlHandling(t *testing.T) { Id: "gce://myprojid/myzone/myinst_42", Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating}, }, - NumericId: 42, + NumericId: 42, + InstanceTemplateName: "", }, }, }, @@ -540,7 +593,8 @@ func TestFetchMigInstancesInstanceUrlHandling(t *testing.T) { Id: "gce://myprojid/myzone/myinst_42", Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating}, }, - NumericId: 42, + NumericId: 42, + InstanceTemplateName: "", }, }, }, diff --git a/cluster-autoscaler/cloudprovider/gce/gce_url.go b/cluster-autoscaler/cloudprovider/gce/gce_url.go index fbdeda5583c5..e8011560a456 100644 --- a/cluster-autoscaler/cloudprovider/gce/gce_url.go +++ b/cluster-autoscaler/cloudprovider/gce/gce_url.go @@ -18,6 +18,8 @@ package gce import ( "fmt" + "net/url" + "path" "regexp" ) @@ -98,6 +100,21 @@ func IsInstanceTemplateRegional(templateUrl string) (bool, error) { return regexp.MatchString("(/projects/.*[A-Za-z0-9]+.*/regions/)", templateUrl) } +// InstanceTemplateNameFromUrl retrieves name of the Instance Template from the url. +func InstanceTemplateNameFromUrl(instanceTemplateLink string) (InstanceTemplateName, error) { + templateUrl, err := url.Parse(instanceTemplateLink) + if err != nil { + return InstanceTemplateName{}, err + } + regional, err := IsInstanceTemplateRegional(templateUrl.String()) + if err != nil { + return InstanceTemplateName{}, err + } + + _, templateName := path.Split(templateUrl.EscapedPath()) + return InstanceTemplateName{templateName, regional}, nil +} + func parseGceUrl(prefix, url, expectedResource string) (project string, zone string, name string, err error) { reg := regexp.MustCompile(fmt.Sprintf("%sprojects/.*/zones/.*/%s/.*", prefix, expectedResource)) errMsg := fmt.Errorf("wrong url: expected format %sprojects//zones//%s/, got %s", prefix, expectedResource, url) diff --git a/cluster-autoscaler/cloudprovider/gce/gce_url_test.go b/cluster-autoscaler/cloudprovider/gce/gce_url_test.go index 56e3152f8fed..68170866d81e 100644 --- a/cluster-autoscaler/cloudprovider/gce/gce_url_test.go +++ b/cluster-autoscaler/cloudprovider/gce/gce_url_test.go @@ -337,3 +337,39 @@ func TestIsInstanceTemplateRegional(t *testing.T) { }) } } + +func TestInstanceTemplateNameFromUrl(t *testing.T) { + tests := []struct { + name string + templateUrl string + expectInstanceTemplateName InstanceTemplateName + wantErr error + }{ + { + name: "Has regional instance url", + templateUrl: "https://www.googleapis.com/compute/v1/projects/test-project/regions/us-central1/instanceTemplates/instance-template", + expectInstanceTemplateName: InstanceTemplateName{"instance-template", true}, + }, + { + name: "Has global instance url", + templateUrl: "https://www.googleapis.com/compute/v1/projects/test-project/global/instanceTemplates/instance-template", + expectInstanceTemplateName: InstanceTemplateName{"instance-template", false}, + }, + { + name: "Empty url", + templateUrl: "", + expectInstanceTemplateName: InstanceTemplateName{"", false}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + itName, err := InstanceTemplateNameFromUrl(tt.templateUrl) + assert.Equal(t, tt.wantErr, err) + if tt.wantErr != nil { + return + } + assert.Equal(t, tt.expectInstanceTemplateName, itName) + }) + } +}