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 auto-evaluated API access endpoint for bind IP #2086

Merged
merged 1 commit into from
Jan 3, 2018
Merged

Fix auto-evaluated API access endpoint for bind IP #2086

merged 1 commit into from
Jan 3, 2018

Conversation

bogdando
Copy link
Contributor

@bogdando bogdando commented Dec 15, 2017

  • Auto configure API access endpoint with a custom bind IP, if provided.
  • Fix HA docs' http URLs are https in fact, clarify the insecure vs secure
    API access modes as well.

Closes #2051

Signed-off-by: Bogdan Dobrelya [email protected]

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 15, 2017
@bogdando
Copy link
Contributor Author

ci check this

@k8s-ci-robot k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Dec 15, 2017
@bogdando
Copy link
Contributor Author

@mattymo @hardys @jistr PTAL

going to test this locally as well

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Dec 15, 2017
docs/ha-mode.md Outdated
| No ext/int LB | http://lc:p | https://m[0].aip:sp |
| External LB, and internal LB | https://lb:lp | https://lc:nsp |
| No ext/int LB, bind 0.0.0.0 | http://lc:sp | https://m[0].aip:sp |
| No ext/int LB, a custom bind | http://bip:sp | https://m[0].aip:sp |
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you are wondering how these apply to the automagic evaluation, here it is:
(given the 1st number represents rows, and the 2nd number stands for columns of that table):

kube_apiserver_endpoint: |-
  12/32{% if not is_kube_master and loadbalancer_apiserver_localhost -%}
	   https://localhost: nginx_kube_apiserver_port|default(kube_apiserver_port) 
  51{%- elif is_kube_master and not loadbalancer_apiserver_localhost and not loadbalancer_apiserver is defined and kube_apiserver_bind_address != '0.0.0.0' -%}
	   https:// kube_apiserver_bind_address : kube_apiserver_port 
  11/41{%- elif is_kube_master and not loadbalancer_apiserver is defined -%}
	   https://127.0.0.1: kube_apiserver_port  
  {%- else -%}
  21/22/31{%-   if loadbalancer_apiserver is defined and loadbalancer_apiserver.port is defined -%}
	   https:// apiserver_loadbalancer_domain_name|default('lb-apiserver.kubernetes.local') : loadbalancer_apiserver.port|default(kube_apiserver_port) 
  42/52{%-   else -%}
	   https:// first_kube_master : kube_apiserver_port 
  {%-  endif -%}
  {%- endif %}

yeah, it's hard to follow those 10 if..else cases, IKR

@bogdando bogdando closed this Dec 15, 2017
@bogdando bogdando reopened this Dec 15, 2017
@bogdando
Copy link
Contributor Author

Retriggered the CI pipeline

@bogdando bogdando changed the title Fix auto-evaluated API access endpoint for bind IP [WIP] Fix auto-evaluated API access endpoint for bind IP Dec 15, 2017
@bogdando
Copy link
Contributor Author

bogdando commented Dec 15, 2017

I'm not sure the masters should be using the external LB access point when deployed with localhost LB as well, let's consider alternatives

@mattymo
Copy link
Contributor

mattymo commented Dec 15, 2017

My first thought is let masters contact their local apiserver. It's highly unlikely to go down and it reduces latency that might be introduced in going host->lb->host

@jistr
Copy link
Contributor

jistr commented Dec 15, 2017

Yes we just discussed on IRC, maybe we could explore a setup where masters talk to themselves directly, and we only use the external LB for access from hosts which aren't part of the Kubernetes cluster. This way even if the external LB goes down, we would still be able to talk to k8s API within the cluster at least.

@mattymo
Copy link
Contributor

mattymo commented Dec 15, 2017

We should make the logic so that it uses the LB if the LB defined and loadbalancer_apiserver_localhost is false. If loadbalancer_apiserver_localhost is true, nodes don't touch it at all.

The only gray area is should masters use the LB ever or not.

@bogdando bogdando changed the title [WIP] Fix auto-evaluated API access endpoint for bind IP [WIP] Rework internal and external LB endpoints autoeval Dec 15, 2017
@bogdando
Copy link
Contributor Author

@jistr @hardys @mattymo so, it's something like that. I'll postpone this until after my vacation ends on Jan 2 :)
PTAL

@bogdando bogdando closed this Dec 15, 2017
@bogdando bogdando reopened this Dec 15, 2017
@bogdando
Copy link
Contributor Author

bogdando commented Jan 3, 2018

I'll split this into a two parts

Auto configure API access endpoint with a custom bind IP, if provided.
Fix HA docs' http URLs are https in fact, clarify the insecure vs secure
API access modes as well.

Closes: #issues/2051

Signed-off-by: Bogdan Dobrelya <[email protected]>
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 3, 2018
@bogdando bogdando changed the title [WIP] Rework internal and external LB endpoints autoeval Fix auto-evaluated API access endpoint for bind IP Jan 3, 2018
@bogdando bogdando closed this Jan 3, 2018
@bogdando bogdando reopened this Jan 3, 2018
@bogdando bogdando merged commit bac3bf1 into kubernetes-sigs:master Jan 3, 2018
@bogdando bogdando deleted the issues/2051 branch January 3, 2018 16:47
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. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants