Skip to content

Commit

Permalink
Merge pull request #6995 from morshielt/gce_instance_template
Browse files Browse the repository at this point in the history 
Add InstanceTemplate field to GceInstance
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot committed Jul 5, 2024
2 parents 02521ed + f8b5990 commit 35359b9
Show file tree
Hide file tree
Showing 4 changed files with 132 additions and 28 deletions.
25 changes: 11 additions & 14 deletions cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"context"
"fmt"
"net/http"
"net/url"
"path"
"regexp"
"strings"
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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) {
Expand Down
82 changes: 68 additions & 14 deletions cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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, "")
Expand All @@ -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,
Expand All @@ -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),
},
},
},
},
Expand All @@ -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),
},
},
},
Expand All @@ -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,
Expand All @@ -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",
Expand All @@ -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,
Expand All @@ -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),
},
},
},
},
Expand All @@ -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),
},
},
},
Expand All @@ -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,
Expand All @@ -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",
Expand All @@ -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,
Expand All @@ -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",
Expand All @@ -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,
Expand All @@ -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),
},
},
},
},
Expand All @@ -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),
},
},
},
Expand Down Expand Up @@ -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: "",
},
},
},
Expand Down Expand Up @@ -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: "",
},
},
},
Expand Down
17 changes: 17 additions & 0 deletions cluster-autoscaler/cloudprovider/gce/gce_url.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ package gce

import (
"fmt"
"net/url"
"path"
"regexp"
)

Expand Down Expand Up @@ -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/<project-id>/zones/<zone>/%s/<name>, got %s", prefix, expectedResource, url)
Expand Down
36 changes: 36 additions & 0 deletions cluster-autoscaler/cloudprovider/gce/gce_url_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
}

0 comments on commit 35359b9

Please sign in to comment.