Skip to content

Commit

Permalink
Merge pull request kubernetes#6473 from scotch-bonnet/master
Browse files Browse the repository at this point in the history 
Introduce GceInstance that extends cloudprovider.Instance with NumericId
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Jan 30, 2024
2 parents a2f8902 + 0673eb0 commit cf171a7
Show file tree
Hide file tree
Showing 9 changed files with 236 additions and 144 deletions.
29 changes: 19 additions & 10 deletions cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,12 @@ var (
}
)

// GceInstance extends cloudprovider.Instance with GCE specific numeric id.
type GceInstance struct {
cloudprovider.Instance
NumericId uint64
}

// AutoscalingGceClient is used for communicating with GCE API.
type AutoscalingGceClient interface {
// reading resources
Expand All @@ -96,7 +102,7 @@ type AutoscalingGceClient interface {
FetchAllMigs(zone string) ([]*gce.InstanceGroupManager, error)
FetchMigTargetSize(GceRef) (int64, error)
FetchMigBasename(GceRef) (string, error)
FetchMigInstances(GceRef) ([]cloudprovider.Instance, error)
FetchMigInstances(GceRef) ([]GceInstance, error)
FetchMigTemplateName(migRef GceRef) (string, error)
FetchMigTemplate(migRef GceRef, templateName string) (*gce.InstanceTemplate, error)
FetchMigsWithName(zone string, filter *regexp.Regexp) ([]string, error)
Expand Down Expand Up @@ -306,7 +312,7 @@ func (client *autoscalingGceClientV1) DeleteInstances(migRef GceRef, instances [
return client.waitForOp(op, migRef.Project, migRef.Zone, true)
}

func (client *autoscalingGceClientV1) FetchMigInstances(migRef GceRef) ([]cloudprovider.Instance, error) {
func (client *autoscalingGceClientV1) FetchMigInstances(migRef GceRef) ([]GceInstance, error) {
registerRequest("instance_group_managers", "list_managed_instances")
b := newInstanceListBuilder(migRef)
err := client.gceService.InstanceGroupManagers.ListManagedInstances(migRef.Project, migRef.Zone, migRef.Name).Pages(context.Background(), b.loadPage)
Expand All @@ -321,7 +327,7 @@ type instanceListBuilder struct {
migRef GceRef
errorCodeCounts map[string]int
errorLoggingQuota *klogx.Quota
infos []cloudprovider.Instance
infos []GceInstance
}

func newInstanceListBuilder(migRef GceRef) *instanceListBuilder {
Expand All @@ -334,7 +340,7 @@ func newInstanceListBuilder(migRef GceRef) *instanceListBuilder {

func (i *instanceListBuilder) loadPage(page *gce.InstanceGroupManagersListManagedInstancesResponse) error {
if i.infos == nil {
i.infos = make([]cloudprovider.Instance, 0, len(page.ManagedInstances))
i.infos = make([]GceInstance, 0, len(page.ManagedInstances))
}
for _, gceInstance := range page.ManagedInstances {
ref, err := ParseInstanceUrlRef(gceInstance.Instance)
Expand All @@ -348,12 +354,15 @@ func (i *instanceListBuilder) loadPage(page *gce.InstanceGroupManagersListManage
return nil
}

func (i *instanceListBuilder) gceInstanceToInstance(ref GceRef, gceInstance *gce.ManagedInstance) cloudprovider.Instance {
instance := cloudprovider.Instance{
Id: ref.ToProviderId(),
Status: &cloudprovider.InstanceStatus{
State: getInstanceState(gceInstance.CurrentAction),
func (i *instanceListBuilder) gceInstanceToInstance(ref GceRef, gceInstance *gce.ManagedInstance) GceInstance {
instance := GceInstance{
Instance: cloudprovider.Instance{
Id: ref.ToProviderId(),
Status: &cloudprovider.InstanceStatus{
State: getInstanceState(gceInstance.CurrentAction),
},
},
NumericId: gceInstance.Id,
}

if instance.Status.State != cloudprovider.InstanceCreating {
Expand Down Expand Up @@ -395,7 +404,7 @@ func (i *instanceListBuilder) gceInstanceToInstance(ref GceRef, gceInstance *gce
return instance
}

func (i *instanceListBuilder) build() []cloudprovider.Instance {
func (i *instanceListBuilder) build() []GceInstance {
klogx.V(4).Over(i.errorLoggingQuota).Infof("Got %v other GCE instances being created with lastAttemptErrors", -i.errorLoggingQuota.Left())
if len(i.errorCodeCounts) > 0 {
klog.Warningf("Spotted following instance creation error codes: %#v", i.errorCodeCounts)
Expand Down
125 changes: 91 additions & 34 deletions cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,20 +237,22 @@ func TestFetchMigInstancesInstanceUrlHandling(t *testing.T) {
name string
lmiResponse gce_api.InstanceGroupManagersListManagedInstancesResponse
lmiPageResponses map[string]gce_api.InstanceGroupManagersListManagedInstancesResponse
wantInstances []cloudprovider.Instance
wantInstances []GceInstance
}{
{
name: "all instances good",
lmiResponse: gce_api.InstanceGroupManagersListManagedInstancesResponse{
ManagedInstances: []*gce_api.ManagedInstance{
{
Id: 2,
Instance: fmt.Sprintf(goodInstanceUrlTempl, 2),
CurrentAction: "CREATING",
LastAttempt: &gce_api.ManagedInstanceLastAttempt{
Errors: &gce_api.ManagedInstanceLastAttemptErrors{},
},
},
{
Id: 42,
Instance: fmt.Sprintf(goodInstanceUrlTempl, 42),
CurrentAction: "CREATING",
LastAttempt: &gce_api.ManagedInstanceLastAttempt{
Expand All @@ -259,14 +261,20 @@ func TestFetchMigInstancesInstanceUrlHandling(t *testing.T) {
},
},
},
wantInstances: []cloudprovider.Instance{
wantInstances: []GceInstance{
{
Id: "gce://myprojid/myzone/myinst_2",
Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating},
Instance: cloudprovider.Instance{
Id: "gce://myprojid/myzone/myinst_2",
Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating},
},
NumericId: 2,
},
{
Id: "gce://myprojid/myzone/myinst_42",
Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating},
Instance: cloudprovider.Instance{
Id: "gce://myprojid/myzone/myinst_42",
Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating},
},
NumericId: 42,
},
},
},
Expand All @@ -275,13 +283,15 @@ func TestFetchMigInstancesInstanceUrlHandling(t *testing.T) {
lmiResponse: gce_api.InstanceGroupManagersListManagedInstancesResponse{
ManagedInstances: []*gce_api.ManagedInstance{
{
Id: 2,
Instance: fmt.Sprintf(goodInstanceUrlTempl, 2),
CurrentAction: "CREATING",
LastAttempt: &gce_api.ManagedInstanceLastAttempt{
Errors: &gce_api.ManagedInstanceLastAttemptErrors{},
},
},
{
Id: 42,
Instance: fmt.Sprintf(goodInstanceUrlTempl, 42),
CurrentAction: "CREATING",
LastAttempt: &gce_api.ManagedInstanceLastAttempt{
Expand All @@ -295,13 +305,15 @@ func TestFetchMigInstancesInstanceUrlHandling(t *testing.T) {
"foo": {
ManagedInstances: []*gce_api.ManagedInstance{
{
Id: 123,
Instance: fmt.Sprintf(goodInstanceUrlTempl, 123),
CurrentAction: "CREATING",
LastAttempt: &gce_api.ManagedInstanceLastAttempt{
Errors: &gce_api.ManagedInstanceLastAttemptErrors{},
},
},
{
Id: 456,
Instance: fmt.Sprintf(goodInstanceUrlTempl, 456),
CurrentAction: "CREATING",
LastAttempt: &gce_api.ManagedInstanceLastAttempt{
Expand All @@ -311,22 +323,34 @@ func TestFetchMigInstancesInstanceUrlHandling(t *testing.T) {
},
},
},
wantInstances: []cloudprovider.Instance{
wantInstances: []GceInstance{
{
Id: "gce://myprojid/myzone/myinst_2",
Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating},
Instance: cloudprovider.Instance{
Id: "gce://myprojid/myzone/myinst_2",
Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating},
},
NumericId: 2,
},
{
Id: "gce://myprojid/myzone/myinst_42",
Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating},
Instance: cloudprovider.Instance{
Id: "gce://myprojid/myzone/myinst_42",
Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating},
},
NumericId: 42,
},
{
Id: "gce://myprojid/myzone/myinst_123",
Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating},
Instance: cloudprovider.Instance{
Id: "gce://myprojid/myzone/myinst_123",
Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating},
},
NumericId: 123,
},
{
Id: "gce://myprojid/myzone/myinst_456",
Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating},
Instance: cloudprovider.Instance{
Id: "gce://myprojid/myzone/myinst_456",
Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating},
},
NumericId: 456,
},
},
},
Expand All @@ -335,13 +359,15 @@ func TestFetchMigInstancesInstanceUrlHandling(t *testing.T) {
lmiResponse: gce_api.InstanceGroupManagersListManagedInstancesResponse{
ManagedInstances: []*gce_api.ManagedInstance{
{
Id: 2,
Instance: fmt.Sprintf(goodInstanceUrlTempl, 2),
CurrentAction: "CREATING",
LastAttempt: &gce_api.ManagedInstanceLastAttempt{
Errors: &gce_api.ManagedInstanceLastAttemptErrors{},
},
},
{
Id: 42,
Instance: fmt.Sprintf(goodInstanceUrlTempl, 42),
CurrentAction: "CREATING",
LastAttempt: &gce_api.ManagedInstanceLastAttempt{
Expand All @@ -355,13 +381,15 @@ func TestFetchMigInstancesInstanceUrlHandling(t *testing.T) {
"foo": {
ManagedInstances: []*gce_api.ManagedInstance{
{
Id: 123,
Instance: fmt.Sprintf(goodInstanceUrlTempl, 123),
CurrentAction: "CREATING",
LastAttempt: &gce_api.ManagedInstanceLastAttempt{
Errors: &gce_api.ManagedInstanceLastAttemptErrors{},
},
},
{
Id: 456,
Instance: fmt.Sprintf(goodInstanceUrlTempl, 456),
CurrentAction: "CREATING",
LastAttempt: &gce_api.ManagedInstanceLastAttempt{
Expand All @@ -374,13 +402,15 @@ func TestFetchMigInstancesInstanceUrlHandling(t *testing.T) {
"bar": {
ManagedInstances: []*gce_api.ManagedInstance{
{
Id: 789,
Instance: fmt.Sprintf(goodInstanceUrlTempl, 789),
CurrentAction: "CREATING",
LastAttempt: &gce_api.ManagedInstanceLastAttempt{
Errors: &gce_api.ManagedInstanceLastAttemptErrors{},
},
},
{
Id: 666,
Instance: fmt.Sprintf(goodInstanceUrlTempl, 666),
CurrentAction: "CREATING",
LastAttempt: &gce_api.ManagedInstanceLastAttempt{
Expand All @@ -390,30 +420,48 @@ func TestFetchMigInstancesInstanceUrlHandling(t *testing.T) {
},
},
},
wantInstances: []cloudprovider.Instance{
wantInstances: []GceInstance{
{
Id: "gce://myprojid/myzone/myinst_2",
Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating},
Instance: cloudprovider.Instance{
Id: "gce://myprojid/myzone/myinst_2",
Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating},
},
NumericId: 2,
},
{
Id: "gce://myprojid/myzone/myinst_42",
Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating},
Instance: cloudprovider.Instance{
Id: "gce://myprojid/myzone/myinst_42",
Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating},
},
NumericId: 42,
},
{
Id: "gce://myprojid/myzone/myinst_123",
Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating},
Instance: cloudprovider.Instance{
Id: "gce://myprojid/myzone/myinst_123",
Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating},
},
NumericId: 123,
},
{
Id: "gce://myprojid/myzone/myinst_456",
Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating},
Instance: cloudprovider.Instance{
Id: "gce://myprojid/myzone/myinst_456",
Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating},
},
NumericId: 456,
},
{
Id: "gce://myprojid/myzone/myinst_789",
Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating},
Instance: cloudprovider.Instance{
Id: "gce://myprojid/myzone/myinst_789",
Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating},
},
NumericId: 789,
},
{
Id: "gce://myprojid/myzone/myinst_666",
Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating},
Instance: cloudprovider.Instance{
Id: "gce://myprojid/myzone/myinst_666",
Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating},
},
NumericId: 666,
},
},
},
Expand All @@ -422,13 +470,15 @@ func TestFetchMigInstancesInstanceUrlHandling(t *testing.T) {
lmiResponse: gce_api.InstanceGroupManagersListManagedInstancesResponse{
ManagedInstances: []*gce_api.ManagedInstance{
{
Id: 99999,
Instance: badInstanceUrl,
CurrentAction: "CREATING",
LastAttempt: &gce_api.ManagedInstanceLastAttempt{
Errors: &gce_api.ManagedInstanceLastAttemptErrors{},
},
},
{
Id: 42,
Instance: fmt.Sprintf(goodInstanceUrlTempl, 42),
CurrentAction: "CREATING",
LastAttempt: &gce_api.ManagedInstanceLastAttempt{
Expand All @@ -437,10 +487,13 @@ func TestFetchMigInstancesInstanceUrlHandling(t *testing.T) {
},
},
},
wantInstances: []cloudprovider.Instance{
wantInstances: []GceInstance{
{
Id: "gce://myprojid/myzone/myinst_42",
Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating},
Instance: cloudprovider.Instance{
Id: "gce://myprojid/myzone/myinst_42",
Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating},
},
NumericId: 42,
},
},
},
Expand All @@ -456,6 +509,7 @@ func TestFetchMigInstancesInstanceUrlHandling(t *testing.T) {
},
},
{
Id: 42,
Instance: fmt.Sprintf(goodInstanceUrlTempl, 42),
CurrentAction: "CREATING",
LastAttempt: &gce_api.ManagedInstanceLastAttempt{
Expand All @@ -464,10 +518,13 @@ func TestFetchMigInstancesInstanceUrlHandling(t *testing.T) {
},
},
},
wantInstances: []cloudprovider.Instance{
wantInstances: []GceInstance{
{
Id: "gce://myprojid/myzone/myinst_42",
Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating},
Instance: cloudprovider.Instance{
Id: "gce://myprojid/myzone/myinst_42",
Status: &cloudprovider.InstanceStatus{State: cloudprovider.InstanceCreating},
},
NumericId: 42,
},
},
},
Expand Down
Loading

0 comments on commit cf171a7

Please sign in to comment.