Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore instance group deletion error when in-use #1063

Merged
merged 1 commit into from
Aug 2, 2017

Conversation

nicksardo
Copy link
Contributor

When the last ingress is deleted, the controller needs to gracefully fail when the instance groups are being used by ILBs.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 2, 2017
@k8s-reviewable
Copy link

This change is Reviewable

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 44.004% when pulling 27e42e9 on nicksardo:fix-ilb into 504bf92 on kubernetes:master.

@@ -130,11 +130,15 @@ func (i *Instances) DeleteInstanceGroup(name string) error {
}
for _, zone := range zones {
if err := i.cloud.DeleteInstanceGroup(name, zone); err != nil {
if !utils.IsHTTPErrorCode(err, http.StatusNotFound) {
if utils.IsNotFoundError(err) {
glog.V(3).Infof("Instance group %v in zone %v did not exist", name, zone)
Copy link
Contributor

Choose a reason for hiding this comment

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

Not in this PR, but we should standardize our logs. At some places we use ("instance group %v/%v", zone, name) and at some places we use ("instance group %v in zone %v", name, zone)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. Thanks

@nikhiljindal
Copy link
Contributor

lgtm

@nicksardo nicksardo merged commit 8f89899 into kubernetes:master Aug 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants