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 bug in backend syncer where backend service was being created without health check #431

Merged
merged 1 commit into from
Aug 14, 2018

Conversation

rramkumar1
Copy link
Contributor

Introduced by #424. Looks like we can't create a BackendService without a health check. Fixed the BackendPool.Create implementation to now take in a health check link.

/assign @bowei

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 14, 2018
@bowei
Copy link
Member

bowei commented Aug 14, 2018

Did this get detected in our e2e tests?

@bowei
Copy link
Member

bowei commented Aug 14, 2018

I remember you described the new flow as updating the health check later. Is this going to be updating the healthcheck an additional time?

@rramkumar1 rramkumar1 changed the title Fix bug in syncer where backend was being created without health check Fix bug in backend syncer where backend service was being created without health check Aug 14, 2018
@rramkumar1
Copy link
Contributor Author

rramkumar1 commented Aug 14, 2018

Yeah, e2e's detected this.

See: https://k8s-gubernator.appspot.com/build/kubernetes-jenkins/logs/ci-ingress-gce-e2e/2694

I'm not sure why the test hasn't timed out yet, but if you look at the logs, you can see ingresses failing to acquire IP's. Further investigation of GLBC logs clearly indicated the issue.

@rramkumar1
Copy link
Contributor Author

rramkumar1 commented Aug 14, 2018

No this will go back to the previous behavior where we create the backend service w/ a HC link and then do an "ensure" on the health check link. The ensure of the health check link right after a create will obviously do nothing since the health check link should be up-to-date already.

When a backend is updated, the health check could change. In any case, we should still only sync a health check once per backend sync.

@bowei
Copy link
Member

bowei commented Aug 14, 2018

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 14, 2018
@bowei bowei merged commit ec7aa4d into kubernetes:master Aug 14, 2018
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. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants