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

Implement support for GCE health check with HTTP Host Header in GLBC #393

Merged

Conversation

itamaro
Copy link
Contributor

@itamaro itamaro commented Mar 7, 2017

Fixes issue #388 (for me, at least :-) )

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

This change is Reviewable

@coveralls
Copy link

Coverage Status

Coverage increased (+0.07%) to 46.199% when pulling c4dbc87 on itamaro:glbc-healthcheck-with-host-header into edb9b00 on kubernetes:master.

@nicksardo
Copy link
Contributor

nicksardo commented Mar 7, 2017

I'll look at this after 0.9.2 goes out. The flagset fix was small and harmless enough to include. Thanks.

Edit: See issue for comments

@nicksardo
Copy link
Contributor

@itamaro Could you rebase this and fix it up? We were considering to use the node-level health check implemented in K8s 1.7.0, but have decided against that path. Thanks!

@itamaro itamaro force-pushed the glbc-healthcheck-with-host-header branch from c4dbc87 to b6c1693 Compare June 16, 2017 12:55
@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 46.678% when pulling b6c1693 on itamaro:glbc-healthcheck-with-host-header into c6b5335 on kubernetes:master.

@itamaro
Copy link
Contributor Author

itamaro commented Jun 16, 2017

@itamaro Could you rebase this and fix it up?

pushed an attempt. it seems much has changed since my initial attempt, so I'm not sure this is correct (and can't test it at the moment).

@nicksardo
Copy link
Contributor

Thanks. Needs this line to use the correct host: https://github.com/kubernetes/ingress/blob/ca5f402322097b7fc079effaa2597eadae65734d/controllers/gce/backends/backends.go#L463 as you did in the previous commit.

I can make a new PR if you're too busy for this - didn't want to steal your work.

@itamaro itamaro force-pushed the glbc-healthcheck-with-host-header branch from b6c1693 to 488dd07 Compare June 20, 2017 11:14
@itamaro
Copy link
Contributor Author

itamaro commented Jun 20, 2017

something like the update I've pushed?

feel free to modify it, you should have write permissions :-)

@coveralls
Copy link

Coverage Status

Coverage increased (+0.005%) to 44.592% when pulling 488dd07 on itamaro:glbc-healthcheck-with-host-header into ca5f402 on kubernetes:master.

@nicksardo
Copy link
Contributor

/lgtm

Tested this and it worked fine. Thanks @itamaro!

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 20, 2017
@nicksardo nicksardo merged commit 103d082 into kubernetes:master Jun 20, 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. lgtm "Looks good to me", indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants