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

Fix panic in ControllerManager when GCE external loadbalancer healthcheck is nil #52646

Merged
merged 1 commit into from
Sep 20, 2017

Conversation

gmarek
Copy link
Contributor

@gmarek gmarek commented Sep 18, 2017

Fix #52722

We should cherry pick it to 1.7 and 1.6.

cc @nicksardo @abgworrall @wojtek-t @ethernetdan @enisoc

Fix panic in ControllerManager on GCE when it has a problem with creating external loadbalancer healthcheck

@gmarek gmarek added this to the v1.8 milestone Sep 18, 2017
@k8s-ci-robot k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Sep 18, 2017
@k8s-github-robot k8s-github-robot added the do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. label Sep 18, 2017
@gmarek gmarek assigned mwielgus and unassigned lavalamp and zmerlynn Sep 18, 2017
@mwielgus
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 18, 2017
@mwielgus
Copy link
Contributor

/approve no-issue

@FengyunPan
Copy link

/lgtm

@dims
Copy link
Member

dims commented Sep 18, 2017

LGTM 👍

@FengyunPan
Copy link

/assign @lavalamp

@gmarek gmarek added release-note-none Denotes a PR that doesn't merit a release note. release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. release-note-none Denotes a PR that doesn't merit a release note. labels Sep 18, 2017
@gmarek
Copy link
Contributor Author

gmarek commented Sep 18, 2017

@FengyunPan - I have an ownership of cloudprovider directory, so I guess it's missing some milestone thingy (or we implemented freeze by removing approval bot).

@dims
Copy link
Member

dims commented Sep 18, 2017

@gmarek - want to try /approve no-issue explicitly?

@gmarek
Copy link
Contributor Author

gmarek commented Sep 18, 2017

@mwielgus already did it.

@gmarek
Copy link
Contributor Author

gmarek commented Sep 19, 2017

/retest

@dims
Copy link
Member

dims commented Sep 19, 2017

@gmarek the bot is still happy, says "NOT APPROVED", looks like one of the owners have to do the /approve no-issue (https://github.com/kubernetes/kubernetes/blob/master/pkg/cloudprovider/providers/gce/OWNERS)

@gmarek
Copy link
Contributor Author

gmarek commented Sep 19, 2017

I'm an owner of pkg/cloudprovider...

@gmarek
Copy link
Contributor Author

gmarek commented Sep 19, 2017

/approve no-issue

1 similar comment
@piosz
Copy link
Member

piosz commented Sep 19, 2017

/approve no-issue

@k8s-github-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: FengyunPan, gmarek, mwielgus, piosz
We suggest the following additional approver: lavalamp

Assign the PR to them by writing /assign @lavalamp in a comment when ready.

No associated issue. Update pull-request body to add a reference to an issue, or get approval with /approve no-issue

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@gmarek gmarek closed this Sep 19, 2017
@gmarek gmarek reopened this Sep 19, 2017
@gmarek
Copy link
Contributor Author

gmarek commented Sep 19, 2017

@kubernetes/test-infra-maintainers - any ideas why this is not approved yet?

@shyamjvs
Copy link
Member

@gmarek I've reported this issue long time back - kubernetes/test-infra#4245. Few other folks also added their stories :)

@gmarek gmarek added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 19, 2017
@gmarek
Copy link
Contributor Author

gmarek commented Sep 19, 2017

Manually adding approved label. Life's too short for that.

@gmarek
Copy link
Contributor Author

gmarek commented Sep 19, 2017

@kubernetes/test-infra-maintainers - things are broken.

@krzyzacy
Copy link
Member

/retest

@krzyzacy
Copy link
Member

/test pull-kubernetes-e2e-gce-etcd3

@k8s-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 51337, 47080, 52646, 52635, 52666). If you want to cherry-pick this change to another branch, please follow the instructions here..

@k8s-github-robot k8s-github-robot merged commit 177df68 into kubernetes:master Sep 20, 2017
k8s-github-robot pushed a commit that referenced this pull request Sep 20, 2017
…-upstream-release-1.6

Automatic merge from submit-queue.

Automated cherry pick of #52646

Cherry pick of #52646 on release-1.6.

#52646: Fix panic in ControllerManager when GCE external loadbalancer
k8s-github-robot pushed a commit that referenced this pull request Oct 2, 2017
…-upstream-release-1.7

Automatic merge from submit-queue.

Automated cherry pick of #52646

Cherry pick of #52646 on release-1.7.

#52646: Fix panic in ControllerManager when GCE external loadbalancer
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.