kubeadm: When etcd is listening on all interfaces, set the etcd probe to use loopback #64670

stealthybox · 2018-06-03T20:03:03Z

What this PR does / why we need it:
When constructing the etcd liveness probe, if the user passes an IPv4 or IPv6 address,
we set the etcdctl liveness probe to use the respective IPv4 or IPv6 loopback address for --endpoints.

The etcd probe is now always formatted with the https:// protocol and square brackets around the IP (required for IPv6 / compatible with IPv4).

::1 is now also included in the etcd serving cert SAN by default.

/kind bug
/area kubeadm
/area etcd
/priority important-soon

/sig cluster-lifecycle
/assign @fabriziopandini

Which issue(s) this PR fixes
Fixes kubernetes/kubeadm#882

Special notes for your reviewer:

root@vagrant:~# /vagrant/bin/882_kubeadm init --config /dev/stdin << EOF |& tail -n5
etcd:
  extraArgs:
    listen-client-urls: https://[::]:2379
EOF
I0603 19:52:15.666594   24743 tlsbootstrap.go:50] [bootstraptoken] configured RBAC rules to allow Node Bootstrap tokens to post CSRs in order for nodes to get long term certificate credentials
I0603 19:52:15.671424   24743 tlsbootstrap.go:72] [bootstraptoken] configured RBAC rules to allow the csrapprover controller automatically approve CSRs from a Node Bootstrap Token
I0603 19:52:15.674607   24743 tlsbootstrap.go:95] [bootstraptoken] configured RBAC rules to allow certificate rotation for all node client certificates in the cluster
I0603 19:52:15.677551   24743 clusterinfo.go:43] [bootstraptoken] creating the "cluster-info" ConfigMap in the "kube-public" namespace
[addons] Applied essential addon: CoreDNS
[addons] Applied essential addon: kube-proxy
root@vagrant:~# cat /etc/kubernetes/manifests/etcd.yaml |grep -C4 listen
spec:
  containers:
  - command:
    - etcd
    - --listen-client-urls=https://[::]:2379
    - --advertise-client-urls=https://127.0.0.1:2379
    - --cert-file=/etc/kubernetes/pki/etcd/server.crt
    - --client-cert-auth=true
    - --data-dir=/var/lib/etcd
root@vagrant:~# cat /etc/kubernetes/manifests/etcd.yaml |grep -C4 etcdctl
      exec:
        command:
        - /bin/sh
        - -ec
        - ETCDCTL_API=3 etcdctl --endpoints=https://[::1]:2379 --cacert=/etc/kubernetes/pki/etcd/ca.crt
          --cert=/etc/kubernetes/pki/etcd/healthcheck-client.crt --key=/etc/kubernetes/pki/etcd/healthcheck-client.key
          get foo
      failureThreshold: 8
      initialDelaySeconds: 15

Release note:

kubeadm now configures the etcd liveness probe correctly when etcd is listening on all interfaces

dixudx · 2018-06-04T07:06:20Z

/lgtm

kad · 2018-06-04T13:19:31Z

cmd/kubeadm/app/util/staticpod/utils.go

@@ -254,6 +254,12 @@ func GetProbeAddress(cfg *kubeadmapi.MasterConfiguration, componentName string)
 				}
 				// Return the IP if the URL contains an address instead of a name.
 				if ip := net.ParseIP(parsedURL.Hostname()); ip != nil {
+					// etcdctl doesn't support auto-converting zero addresses into loopback addresses
+					if ip.Equal(net.IPv4zero) || ip.Equal(net.IPv6zero) {
+						return "127.0.0.1"


if we are in IPv6 configuration, better to use IPv6 loopback.

This is intentional.
IPv6 endpoints don't work /w etcdctl.
I couldn't find an upstream bug -- perhaps we should file one.

this is commented in the line below

So, I tested this locally and using ::1 appears to work for etcdctl v3.1.12.

When testing, I had originally just modified the etcd manifest for a kubectl-based install, but neglected to update the certificate to add the ipv6 loopback address and was seeing this error: Error: grpc: timed out when dialing. Once I updated the etcd server certificate to include ::1 I was able to use etcdctl with --endpoints https://[::1]:2379 without an issue.

@stealthybox We should probably update GetEtcdAltNames() in pki_helpers.go to add the ipv6 loopback address by default as well.

@detiber Ah, that's totally the issue
I really struggled to determine what was wrong since it seems they removed the --debug flag

I'll fix this

@stealthybox I ended up figuring it out by using etcdctl v3.2.x and using the --debug flag 🤣

kad · 2018-06-04T13:20:53Z

cmd/kubeadm/app/util/etcd/etcd_test.go

+    - --data-dir=/var/lib/etcd
+    - --peer-key-file=/etc/kubernetes/pki/etcd/peer.key
+    - --peer-trusted-ca-file=/etc/kubernetes/pki/etcd/ca.crt
+    - --listen-client-urls=https://[::0:0]:2379


https://[::]:2379

This is also intentional.
It forces us to use net.ParseIP() because these are all equivalent:

0:0:0:0:0:0:0:0 ::0:0:0:0:0 ::0:0 ::0 ::

I understand that those are equivalent, but what is the reason behind that particular notation ?

It prevents code that just does a string match on :: or ::0 instead of parsing the IP and doing real equivalence.

If you'd like I can expand the test cases, but I figured that if I just add one, I should pick an edge case.

luxas

Please fix @kad's comments

timothysc

/approve

Please fix comment and @detiber - please weigh in too for lgtm.

stealthybox · 2018-06-04T16:36:21Z

Thanks for the review @kad 👍 -- weighed in on the comments.
Please lmk if we need to clarify more in the code.

detiber · 2018-06-04T18:51:01Z

lgtm, but would like to see the use of the ipv6 loopback in the healthcheck when the ipv6 unspecified address is provided.

dims · 2018-06-04T23:05:59Z

/priority important-soon

Is this priority ok? please alter if you disagree (need one for the bot)

jberkus · 2018-06-05T00:34:23Z

Also needs a kind/ label

/kind bug

cjwagner · 2018-06-05T21:16:13Z

There are multiple priority labels on this PR. The milestone maintainer is expecting exactly 1.

… to use loopback Fixes kubernetes/kubeadm#882

stealthybox · 2018-06-06T00:44:28Z

@kad @detiber @dixudx

added ::1 to the server SAN
now returning ::1 in the ipv6 unspecified case
formatted the probes to always use https://[%s]:%d for ipv6 compatibility
added more tests to cover the v6 parsing cases

PTAL tyvm

k8s-github-robot · 2018-06-06T00:50:25Z

[MILESTONENOTIFIER] Milestone Pull Request Labels Incomplete

@dixudx @fabriziopandini @stealthybox

Action required: This pull request requires label changes.

priority: Must specify exactly one of priority/critical-urgent, priority/important-longterm or priority/important-soon.

Help

dixudx

/lgtm

k8s-ci-robot · 2018-06-06T03:09:31Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dixudx, stealthybox, timothysc

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:

~~cmd/kubeadm/OWNERS~~ [timothysc]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

dixudx · 2018-06-06T03:09:52Z

/retest

k8s-github-robot · 2018-06-06T04:56:33Z

/test all [submit-queue is verifying that this PR is safe to merge]

k8s-github-robot · 2018-06-06T06:22:48Z

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here.

k8s-ci-robot assigned fabriziopandini Jun 3, 2018

k8s-ci-robot requested review from kad and luxas June 3, 2018 20:03

stealthybox mentioned this pull request Jun 3, 2018

kubeadm etcd liveness probe changes. #63408

Closed

k8s-ci-robot assigned dixudx Jun 4, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 4, 2018

kad reviewed Jun 4, 2018

View reviewed changes

luxas approved these changes Jun 4, 2018

View reviewed changes

timothysc added this to the v1.11 milestone Jun 4, 2018

timothysc added status/approved-for-milestone and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Jun 4, 2018

timothysc approved these changes Jun 4, 2018

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 4, 2018

timothysc added priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. and removed priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels Jun 4, 2018

k8s-ci-robot added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Jun 4, 2018

k8s-github-robot added the milestone/incomplete-labels label Jun 4, 2018

timothysc removed the milestone/incomplete-labels label Jun 5, 2018

k8s-github-robot added the milestone/incomplete-labels label Jun 5, 2018

timothysc removed the milestone/incomplete-labels label Jun 5, 2018

k8s-github-robot added the milestone/incomplete-labels label Jun 5, 2018

timothysc removed the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Jun 5, 2018

k8s-github-robot removed the milestone/incomplete-labels label Jun 5, 2018

stealthybox changed the title ~~kubeadm: When etcd is listening on all interfaces, set the etcd probe to use the ipv4 loopback~~ kubeadm: When etcd is listening on all interfaces, set the etcd probe to use loopback Jun 6, 2018

k8s-ci-robot added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Jun 6, 2018

stealthybox force-pushed the feature/kubeadm_882-etcd-zero-probe branch from 83d80c6 to 166b5b9 Compare June 6, 2018 00:31

kubeadm: When etcd is listening on all interfaces, set the etcd probe…

76c04b9

… to use loopback Fixes kubernetes/kubeadm#882

stealthybox force-pushed the feature/kubeadm_882-etcd-zero-probe branch from 166b5b9 to 76c04b9 Compare June 6, 2018 00:34

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 6, 2018

k8s-github-robot added the milestone/incomplete-labels label Jun 6, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 6, 2018

dixudx approved these changes Jun 6, 2018

View reviewed changes

k8s-github-robot merged commit d3a797a into kubernetes:master Jun 6, 2018

stealthybox mentioned this pull request Jun 6, 2018

kubeadm: Cleanup and fix etcd probe tests #64817

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kubeadm: When etcd is listening on all interfaces, set the etcd probe to use loopback #64670

kubeadm: When etcd is listening on all interfaces, set the etcd probe to use loopback #64670

stealthybox commented Jun 3, 2018 •

edited

dixudx commented Jun 4, 2018

kad Jun 4, 2018

stealthybox Jun 4, 2018

stealthybox Jun 4, 2018

detiber Jun 4, 2018

detiber Jun 4, 2018

stealthybox Jun 4, 2018

detiber Jun 5, 2018

kad Jun 4, 2018

stealthybox Jun 4, 2018

kad Jun 4, 2018

stealthybox Jun 4, 2018

luxas left a comment

timothysc left a comment

stealthybox commented Jun 4, 2018

detiber commented Jun 4, 2018

dims commented Jun 4, 2018

jberkus commented Jun 5, 2018

cjwagner commented Jun 5, 2018

stealthybox commented Jun 6, 2018

k8s-github-robot commented Jun 6, 2018

dixudx left a comment

k8s-ci-robot commented Jun 6, 2018

dixudx commented Jun 6, 2018

k8s-github-robot commented Jun 6, 2018

k8s-github-robot commented Jun 6, 2018

kubeadm: When etcd is listening on all interfaces, set the etcd probe to use loopback #64670

kubeadm: When etcd is listening on all interfaces, set the etcd probe to use loopback #64670

Conversation

stealthybox commented Jun 3, 2018 • edited

dixudx commented Jun 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luxas left a comment

Choose a reason for hiding this comment

timothysc left a comment

Choose a reason for hiding this comment

stealthybox commented Jun 4, 2018

detiber commented Jun 4, 2018

dims commented Jun 4, 2018

jberkus commented Jun 5, 2018

cjwagner commented Jun 5, 2018

stealthybox commented Jun 6, 2018

k8s-github-robot commented Jun 6, 2018

dixudx left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Jun 6, 2018

dixudx commented Jun 6, 2018

k8s-github-robot commented Jun 6, 2018

k8s-github-robot commented Jun 6, 2018

stealthybox commented Jun 3, 2018 •

edited