Skip to content
This repository was archived by the owner on Apr 17, 2019. It is now read-only.

Conversation

@mwielgus
Copy link
Contributor

@mwielgus mwielgus commented Jan 3, 2017

This is needed to compare the list of nodes created by the cloud provider with the list of nodes registered in Kubernetes.

Ref: #2228 #2229

cc: @jszczepkowski @andrewsykim @piosz @fgrzadkowski

@mwielgus mwielgus requested a review from jszczepkowski January 3, 2017 16:18
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 3, 2017
@k8s-reviewable
Copy link

This change is Reviewable

if len(groups.AutoScalingGroups) < 1 {
return nil, fmt.Errorf("Unable to get first autoscaling.Group for %s", name)
}
return groups.AutoScalingGroups[0], nil
Copy link

@andrewsykim andrewsykim Jan 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return *groups.AutoScalingGroups[0], nil? based on this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it compiles without *.

Copy link

@andrewsykim andrewsykim Jan 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, cause you don't need to deference pointers to access their fields in Go. 👍

return groups.AutoScalingGroups[0], nil
}

// GetMigNodes returns Asg nodes.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/GetMigNodes/GetAsgNodes/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@jszczepkowski
Copy link
Contributor

LGTM (after resolving comments)

@mwielgus mwielgus added the lgtm Indicates that a PR is ready to be merged. label Jan 4, 2017
@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit e5f341a into kubernetes-retired:master Jan 4, 2017
mwielgus pushed a commit to kubernetes/autoscaler that referenced this pull request Apr 18, 2017
…getnodes

Automatic merge from submit-queue

Cluster-autoscaler: add get nodes function to cloud provider interface

This is needed to compare the list of nodes created by the cloud provider with the list of nodes registered in Kubernetes.

Ref: #2228 #2229

cc: @jszczepkowski @andrewsykim @piosz @fgrzadkowski
mwielgus pushed a commit to kubernetes/autoscaler that referenced this pull request Apr 18, 2017
…getnodes

Automatic merge from submit-queue

Cluster-autoscaler: add get nodes function to cloud provider interface

This is needed to compare the list of nodes created by the cloud provider with the list of nodes registered in Kubernetes.

Ref: #2228 #2229

cc: @jszczepkowski @andrewsykim @piosz @fgrzadkowski
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area/autoscaler area/autoscaling cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants