This adds an etcd health check endpoint to kube-apiserver #49412

bjhaid · 2017-07-21T20:32:29Z

addressing #48215.

What this PR does / why we need it:
This ensures kube-apiserver /healthz endpoint fails whenever connectivity cannot be established to etcd, also ensures the etcd preflight checks works with unix sockets

Which issue this PR fixes: fixes #48215

Special notes for your reviewer:
This PR does not use the etcd client directly as the client object is wrapped behind the storage interface and not exposed directly for use, so I decided to reuse what's being done in the preflight. So this will only check fail for connectivity and not etcd auth related problems. I did not write tests for the endpoint because I couldn't find examples that I could follow for writing tests for healthz related endpoints, I'll be willing to write those tests if someone can point me at a relevant one.

Release note:

Add etcd connectivity endpoint to healthz

@deads2k please help review, thanks!

k8s-ci-robot · 2017-07-21T20:32:31Z

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://github.com/kubernetes/kubernetes/wiki/CLA-FAQ to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
If you have done the above and are still having issues with the CLA being reported as unsigned, please email the CNCF helpdesk: helpdesk@rt.linuxfoundation.org

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

k8s-ci-robot · 2017-07-21T20:32:36Z

Hi @bjhaid. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

bjhaid · 2017-07-21T20:38:29Z

/assign @deads2k

deads2k · 2017-07-24T16:59:19Z

staging/src/k8s.io/apiserver/pkg/server/options/etcd.go

@@ -20,6 +20,9 @@ import (
 	"fmt"

 	"github.com/spf13/pflag"
+	"net/http"
+	"k8s.io/apiserver/pkg/server/healthz"
+	"k8s.io/kubernetes/cmd/kube-apiserver/app/preflight"


This dependency won't be allowed. You'll need to move the etcd check code to the k8s.io/apiserver staging section somewhere.

Is the reverse allowed, that is adding a k8s.io/apiserver/... dependency to k8s.io/kubernetes/cmd/kube-apiserver/app/preflight? So I don't end up duplicating code

Is the reverse allowed, that is adding a k8s.io/apiserver/... dependency to k8s.io/kubernetes/cmd/kube-apiserver/app/preflight

yes the reverse is allowed

Can I just move preflight and it's test in to staging and fix the references?

Can I just move preflight and it's test in to staging and fix the references?

Yes. This seems more like apiserver/pkg/storage/etcd3/healthz

deads2k · 2017-07-24T17:00:05Z

@k8s-bot ok to test

deads2k · 2017-07-24T17:00:45Z

This is looking good.

deads2k · 2017-07-26T13:29:14Z

looks like something is going wrong.

bjhaid · 2017-07-26T14:45:07Z

@deads2k

looks like something is going wrong.

I spoke to folks in #sig-testing and they told me to run make coverage and one of the many things I saw is the not allowed dependency, if I move things around maybe that will help

bjhaid · 2017-07-30T04:22:47Z

/test pull-kubernetes-bazel

bjhaid · 2017-07-31T17:24:01Z

/test pull-kubernetes-bazel

bjhaid · 2017-07-31T18:39:24Z

/test pull-kubernetes-bazel

bjhaid · 2017-07-31T19:59:02Z

/test Submit Queue

bjhaid · 2017-07-31T21:38:22Z

@deads2k needs a lgtm..

deads2k · 2017-08-01T17:53:11Z

staging/src/k8s.io/apiserver/pkg/server/options/etcd.go

@@ -20,6 +20,9 @@ import (
 	"fmt"

 	"github.com/spf13/pflag"
+	"k8s.io/apiserver/pkg/server/healthz"
+	"k8s.io/apiserver/pkg/storage/etcd3/preflight"
+	"net/http"


nit: this goes in first section of imports since its a stdlib

deads2k · 2017-08-01T17:53:43Z

staging/src/k8s.io/apiserver/pkg/server/options/etcd.go

@@ -20,6 +20,9 @@ import (
 	"fmt"

 	"github.com/spf13/pflag"
+	"k8s.io/apiserver/pkg/server/healthz"


nit: want an empty line before these. we normally put a gap between dependencies and in-project deps

deads2k · 2017-08-01T18:02:07Z

/lgtm

addressing kubernetes#48215.

bjhaid · 2017-08-01T22:21:28Z

/test pull-kubernetes-unit

bjhaid · 2017-08-01T22:23:03Z

/test pull-kubernetes-verify

bjhaid · 2017-08-02T01:51:20Z

/test pull-kubernetes-e2e-kops-aws

bjhaid · 2017-08-02T01:51:30Z

/test pull-kubernetes-e2e-gce-etcd3

k8s-ci-robot · 2017-08-02T01:51:34Z

@bjhaid: you can't request testing unless you are a kubernetes member.

In response to this:

/test pull-kubernetes-e2e-kops-aws

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

k8s-ci-robot · 2017-08-02T01:51:43Z

@bjhaid: you can't request testing unless you are a kubernetes member.

In response to this:

/test pull-kubernetes-e2e-gce-etcd3

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

bjhaid · 2017-08-02T01:53:20Z

/test pull-kubernetes-e2e-kops-aws

k8s-ci-robot · 2017-08-02T01:53:34Z

@bjhaid: you can't request testing unless you are a kubernetes member.

In response to this:

/test pull-kubernetes-e2e-kops-aws

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

fejta · 2017-08-02T02:07:25Z

@deads2k we retired the @k8s-bot ok to test command, please start using /ok-to-test (we no longer recognize the ok to test message)

/ok-to-test

deads2k · 2017-08-02T12:03:35Z

/lgtm

k8s-github-robot · 2017-08-02T12:03:45Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bjhaid, deads2k

Associated issue: 48215

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

~~cmd/kube-apiserver/OWNERS~~ [deads2k]
~~staging/src/k8s.io/apiserver/OWNERS~~ [deads2k]

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 · 2017-08-03T00:06:00Z

Automatic merge from submit-queue (batch tested with PRs 49989, 49806, 49649, 49412, 49512)

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Jul 21, 2017

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jul 21, 2017

k8s-github-robot assigned nikhiljindal and liggitt Jul 21, 2017

k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Jul 21, 2017

k8s-ci-robot assigned deads2k Jul 21, 2017

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Jul 21, 2017

deads2k reviewed Jul 24, 2017

View reviewed changes

k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jul 24, 2017

bjhaid force-pushed the etcd_healthz_endpoint branch 2 times, most recently from 89ee978 to 9e62dc9 Compare July 25, 2017 22:26

bjhaid changed the title ~~This add and etcd health check endpoint to kube-apiserver~~ This add an etcd health check endpoint to kube-apiserver Jul 26, 2017

bjhaid force-pushed the etcd_healthz_endpoint branch from 9e62dc9 to 9f77bb5 Compare July 29, 2017 20:43

bjhaid changed the title ~~This add an etcd health check endpoint to kube-apiserver~~ This adds an etcd health check endpoint to kube-apiserver Jul 29, 2017

bjhaid force-pushed the etcd_healthz_endpoint branch 4 times, most recently from 0630735 to bbed16e Compare July 30, 2017 03:36

kubernetes deleted a comment from bjhaid Aug 1, 2017

deads2k reviewed Aug 1, 2017

View reviewed changes

bjhaid force-pushed the etcd_healthz_endpoint branch from bbed16e to 419f60a Compare August 1, 2017 17:58

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 1, 2017

k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 1, 2017

This adds an etcd health check endpoint to kube-apiserver

47d748c

addressing kubernetes#48215.

bjhaid force-pushed the etcd_healthz_endpoint branch from 419f60a to 47d748c Compare August 1, 2017 21:37

k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 1, 2017

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 2, 2017

k8s-github-robot merged commit 22af024 into kubernetes:master Aug 3, 2017

aaronlevy mentioned this pull request Oct 18, 2017

Figure out how we can re-enable healthchecks on the kube-apiserver kubernetes-retired/bootkube#495

Closed

dlipovetsky mentioned this pull request Aug 14, 2018

After etcd recovery, kube-apiserver reports TLS errors until it is restarted platform9/cctl#98

Closed

bjhaid deleted the etcd_healthz_endpoint branch May 11, 2019 02:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

This adds an etcd health check endpoint to kube-apiserver #49412

This adds an etcd health check endpoint to kube-apiserver #49412

bjhaid commented Jul 21, 2017

k8s-ci-robot commented Jul 21, 2017

k8s-ci-robot commented Jul 21, 2017

bjhaid commented Jul 21, 2017

deads2k Jul 24, 2017

bjhaid Jul 25, 2017

deads2k Jul 26, 2017

bjhaid Jul 26, 2017

deads2k Jul 26, 2017

deads2k commented Jul 24, 2017

deads2k commented Jul 24, 2017

deads2k commented Jul 26, 2017

bjhaid commented Jul 26, 2017

bjhaid commented Jul 30, 2017

bjhaid commented Jul 31, 2017

bjhaid commented Jul 31, 2017

bjhaid commented Jul 31, 2017

bjhaid commented Jul 31, 2017

deads2k Aug 1, 2017

deads2k Aug 1, 2017

deads2k commented Aug 1, 2017

bjhaid commented Aug 1, 2017

bjhaid commented Aug 1, 2017

bjhaid commented Aug 2, 2017

bjhaid commented Aug 2, 2017

k8s-ci-robot commented Aug 2, 2017

k8s-ci-robot commented Aug 2, 2017

bjhaid commented Aug 2, 2017

k8s-ci-robot commented Aug 2, 2017

fejta commented Aug 2, 2017

deads2k commented Aug 2, 2017

k8s-github-robot commented Aug 2, 2017

k8s-github-robot commented Aug 3, 2017

This adds an etcd health check endpoint to kube-apiserver #49412

This adds an etcd health check endpoint to kube-apiserver #49412

Conversation

bjhaid commented Jul 21, 2017

k8s-ci-robot commented Jul 21, 2017

k8s-ci-robot commented Jul 21, 2017

bjhaid commented Jul 21, 2017

deads2k Jul 24, 2017

Choose a reason for hiding this comment

bjhaid Jul 25, 2017

Choose a reason for hiding this comment

deads2k Jul 26, 2017

Choose a reason for hiding this comment

bjhaid Jul 26, 2017

Choose a reason for hiding this comment

deads2k Jul 26, 2017

Choose a reason for hiding this comment

deads2k commented Jul 24, 2017

deads2k commented Jul 24, 2017

deads2k commented Jul 26, 2017

bjhaid commented Jul 26, 2017

bjhaid commented Jul 30, 2017

bjhaid commented Jul 31, 2017

bjhaid commented Jul 31, 2017

bjhaid commented Jul 31, 2017

bjhaid commented Jul 31, 2017

deads2k Aug 1, 2017

Choose a reason for hiding this comment

deads2k Aug 1, 2017

Choose a reason for hiding this comment

deads2k commented Aug 1, 2017

bjhaid commented Aug 1, 2017

bjhaid commented Aug 1, 2017

bjhaid commented Aug 2, 2017

bjhaid commented Aug 2, 2017

k8s-ci-robot commented Aug 2, 2017

k8s-ci-robot commented Aug 2, 2017

bjhaid commented Aug 2, 2017

k8s-ci-robot commented Aug 2, 2017

fejta commented Aug 2, 2017

deads2k commented Aug 2, 2017

k8s-github-robot commented Aug 2, 2017

k8s-github-robot commented Aug 3, 2017