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.

Sign up for GitHub

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

Jump to bottom

Adding more proxy options and header to nginx load-balancer. #43625

Merged
merged 1 commit into from
Apr 4, 2017
Merged

Adding more proxy options and header to nginx load-balancer. #43625

merged 1 commit into from
Apr 4, 2017

Conversation

mbruzek
Copy link
Contributor

@mbruzek mbruzek commented Mar 24, 2017 •
edited
Loading

What this PR does / why we need it: The kubeapi-load-balancer uses nginx to proxy commands to the kube-apiserver. It currently does not support SPDY and therefore the kubectl exec command is broken.

Which issue this PR fixes :
fixes https://github.com/juju-solutions/bundle-canonical-kubernetes/issues/226
fixes https://github.com/juju-solutions/bundle-canonical-kubernetes/issues/201

Special notes for your reviewer: This only changes the nginx configuration no code change was required.

Release note:

Using http2 in kubeapi-load-balancer to fix kubectl exec uses

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

This change is Reviewable

@k8s-github-robot k8s-github-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Mar 24, 2017
@mbruzek
Copy link
Contributor Author

mbruzek commented Mar 30, 2017

@nikhiljindal now that 1.6 has been released code freeze should be over. Can you have a look at this pull requests and let me know what you think? Thanks!

@ErwanMAS
Copy link

I have this issue .
do we have a workaround ?

@eblazquez
Copy link

Hi, I applied the same changes on the apilb.conf file as they appear on this pull request and it seems that it fix the issue juju-solutions/bundle-canonical-kubernetes#226 indeed.

Thanks a lot @mbruzek !

@nikhiljindal nikhiljindal assigned castrojo and unassigned nikhiljindal Apr 3, 2017
@nikhiljindal
Copy link
Contributor

nikhiljindal commented Apr 3, 2017 •
edited
Loading

Reassigning since I have no idea about this code :)
Picked a random reviewer from cluster/juju/OWNERS

@lazypower
Copy link
Contributor

@castrojo I've given this a look and its +1 LGTM. We've verified this passes the sniff test for kubectl exec, logs, and other HTTP2-enabled commands.

@castrojo
Copy link
Member

castrojo commented Apr 4, 2017

/approve

@lazypower
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 Apr 4, 2017
@k8s-github-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: castrojo, chuckbutler, mbruzek

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

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 12fbc90 into kubernetes:master Apr 4, 2017
@Cryptophobia Cryptophobia mentioned this pull request May 11, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@lazypower lazypower

@castrojo castrojo

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/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
9 participants
@mbruzek @k8s-reviewable @ErwanMAS @eblazquez @nikhiljindal @lazypower @castrojo @k8s-github-robot @k8s-ci-robot