Move healthz check to secure ports #6446
Conversation
k8s-ci-robot
added
kind/feature
k8s-ci-robot
added
the
size/XS
floryut
force-pushed
the
change_scheduler_manager_healthz
branch
from
f6b2e44 to
a1d732f
Compare
floryut
force-pushed
the
change_scheduler_manager_healthz
branch
from
a1d732f to
ecdb710
Compare
|
/lgtm |
k8s-ci-robot
added
the
lgtm
| @@ -111,8 +113,6 @@ | |||
| uri: | |||
| url: "{{ kube_apiserver_endpoint }}/healthz" | |||
| validate_certs: no | |||
| client_cert: "{{ kube_apiserver_client_cert }}" | |||
| client_key: "{{ kube_apiserver_client_key }}" | |||
| register: result | |||
There was a problem hiding this comment.
I can keep them but as the validate certs is set to 'no' they are unused
| @@ -93,15 +93,17 @@ | |||
|
|
|||
| - name: Master | wait for kube-scheduler | |||
| uri: | |||
| url: http://localhost:10251/healthz | |||
| url: https://localhost:10259/healthz | |||
| validate_certs: no | |||
| register: scheduler_result | |||
There was a problem hiding this comment.
I assume we can use kube_apiserver_client_cert/kube_apiserver_client_key to validate both scheduler and controller manager endpoints, because they point to {{ kube_cert_dir }}/ca.crt by default but I'm wondering if that's a good solution to rely on a defaults that people can change.
There was a problem hiding this comment.
I tried to validate the url using certs but seems maybe the ca is needed or another issue as I wasn't able to query using the correct certs (even the API Server check doesn't work without the 'validate certs' set to no
Regarding the default you mean the ports? If so indeed but as we don't have a kubespray variable for that we don't have to worry I'd say.. Maybe we can add a variable and use it here with 10259 as the default
|
I could see that we move to |
@Miouge1 you mean this one ? https://github.com/kubernetes-sigs/kubespray/pull/6435/files I indeed tried to apply some certs but never manage to got a valid curl with ansible uri module.. and I saw that even the API check below fail without |
|
Considering that this is currently http, it's still better than nothing :) /approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: floryut, Miouge1 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
* 'master' of https://github.com/kubernetes-sigs/kubespray: Documentation for Ingress (kubernetes-sigs#6378) Fix ansible-lint E301 for commands fetching data (kubernetes-sigs#6465) Fix shellcheck url (kubernetes-sigs#6462) Fix ansible-lint E305 (kubernetes-sigs#6459) Fix ansible-lint E404 (kubernetes-sigs#6417) Update README.md and openstack.md (kubernetes-sigs#6455) Add noqa and disable .ansible-lint global exclusions (kubernetes-sigs#6410) Move healthz check to secure ports (kubernetes-sigs#6446) Update multus version & crio conf (kubernetes-sigs#6444) Fix remove etcd broken with etcdctl_api 3 (kubernetes-sigs#6448) update cinder csi manifests (kubernetes-sigs#6434) Update docker package to 19.03.12 (kubernetes-sigs#6439) * add proxy_env definition to remove_node.yml resolving kubernetes-sigs#6430 (kubernetes-sigs#6431)
What type of PR is this?
/kind feature
What this PR does / why we need it:
Insecure port were deprecated and are now removed by default (since 1.17.9, 1.18.6 etc..)
Which issue(s) this PR fixes:
None
Special notes for your reviewer:
None
Does this PR introduce a user-facing change?: