elasticsearch: readiness probe extension #609

wozniakjan · 2017-08-25T11:52:45Z

Interim readiness probe until multi-role Elasticsearch topology from openshift/openshift-ansible#5001 is introduced.

Proposed changes are based on this document: https://docs.google.com/document/d/1m9-LXr-UThs9WhRfkFRbwf1Lppxa5Opw_PYRY1wuDbY/

List of implemented readiness probe checks:

localhost:9200/ returns OK 200
localhost:9200/.searchguard.${DC_NAME} returns OK 200
localhost:9200/_template/${template} returns OK 200 for each $template in $ES_HOME/index_templates/*.json
localhost:9200/.searchguard.$DC_NAME/_count returns count of 5 documents in the response body

Depends on this PR

Bug 1459430: add optional master discovery by label fabric8io/elasticsearch-cloud-kubernetes#98

wozniakjan · 2017-08-25T12:07:24Z

(3) Elasticsearch label discovery [epic-stability][logging]

elasticsearch/probe/readiness.sh

lukas-vlcek

LGTM, left minor comments.

As for the missing feature defined as:

We need to ensure pod is still accessible by admin or by using admin certs

I am not sure what this meant, we are already using admin certs with curl so it is implicitly fulfilled?

elasticsearch/probe/readiness.sh

jcantrill · 2017-08-25T18:36:17Z

@wozniakjan Regarding 'We need to ensure pod is still accessible by admin or by using admin certs' this is not intended to implemented in the probe but more a note we need to ensure that when a pod is in a status such that it is not ready, that we can still access the member of the node to obtain information (e.g. _cat/indices). I believe the ACL backend registry will allow you to do this assuming you have access to the admin certs

jcantrill

LGTM. We will need to backport this to 3.6. Let's make sure not to forget that PR

elasticsearch/probe/readiness.sh

richm · 2017-08-28T14:16:26Z

elasticsearch/probe/readiness.sh

+  | python -c "import sys, json; print json.load(sys.stdin)['count']")
+
+if [ "$sg_doc_count" != $EXPECTED_SG_DOCUMENT_COUNT ]; then
+  echo "Incorrect SG document count, expected $EXPECTED_SG_DOCUMENT_COUNT [received doc count: ${sg_doc_count}]"


see above comment about errors

jcantrill · 2017-08-28T17:43:05Z

[test]

wozniakjan · 2017-09-06T15:47:21Z

@jcantrill I updated versions of elasticsearch cloud kubernetes plugin and tested again. All seems to be ready for merge and QE tests.

additional checks in readiness probe: 1) localhost:9200/ returns OK 200 2) localhost:9200/.searchguard.${DC_NAME} returns OK 200 3) localhost:9200/_template/${template} returns OK 200 for each template in $ES_HOME/index_templates/*.json 4) localhost:9200/.searchguard.$DC_NAME/_search?size=0 returns count of 5 documents in the response body

wozniakjan · 2017-09-06T15:59:51Z

[test]

openshift-bot · 2017-09-06T16:15:43Z

Evaluated for origin aggregated logging test up to c691f2a

openshift-bot · 2017-09-06T17:18:00Z

Origin Aggregated Logging Test Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin_aggregated_logging/378/) (Base Commit: 0b6cc33) (PR Branch Commit: c691f2a)

wozniakjan · 2017-09-07T07:06:06Z

@jcantrill tests are passing, my local tests also succeeded, do you think we can merge it?

jcantrill · 2017-09-07T15:41:32Z

[merge]

openshift-bot · 2017-09-07T15:47:27Z

Evaluated for origin aggregated logging merge up to c691f2a

openshift-bot · 2017-09-07T16:32:36Z

Origin Aggregated Logging Merge Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_origin_aggregated_logging/158/) (Base Commit: b3418b5) (PR Branch Commit: c691f2a)

portante · 2017-09-07T20:45:09Z

I have some reservations about this probe. This relies on the cluster being operational for the probes to work. If the searchguard index is sharded across all nodes of the cluster, and one of the nodes is not available, then will this probe work?

Fundamentally, readiness probes should not relied on the cluster-wide state of Elasticsearch to determine if one pod is ready to join the service.

wozniakjan · 2017-09-08T10:46:07Z

I have some reservations about this probe. This relies on the cluster being operational for the probes to work. If the searchguard index is sharded across all nodes of the cluster, and one of the nodes is not available, then will this probe work?

Each probe verifies only its own searchguard index. If fewer than half of ES pods aren't available, rest of the service remains operational. Those malfunctioning ES pods will be removed from ES service and the service will loadbalance only across working pods forming a cluster.

Fundamentally, readiness probes should not relied on the cluster-wide state of Elasticsearch to determine if one pod is ready to join the service.

I am afraid there is no way around it. It is ES internal mechanism and a single pod is not ready until it negotiates with a subset of his peers to form an operational cluster according to its configuration.

richm · 2017-09-08T14:20:04Z

How can we test this on a real, live, multi-node cluster, so we can try to reproduce some of these scenarios @portante is describing?

portante · 2017-09-08T14:30:12Z

Each probe verifies only its own searchguard index.

Are not indices in Elasticsearch maintained cluster wide? Don't they have state that is managed across all nodes in the cluster? If that is not the case, share where that is documented to be otherwise.

Assuming it is cluster-wide state, making it part of a pod's readiness probe does not make sense.

There is a way to drop per-ES pod searchguard indices which would make this particular issue go away.

But it still holds a that a readiness probe is per pod, and as such, should not rely on service or ES cluster state to determine its readiness to join the service.

wozniakjan · 2017-09-08T14:34:11Z

@richm locally I tested the searchguard index miss by creating an image that had this commented out and deployed cluster with 3 nodes.

origin-aggregated-logging/elasticsearch/run.sh

Line 116 in ca917ff

es_seed_acl

Then another image with this line back and redeploy 2 out of 3 images. Two images formed an operational cluster, one was still waiting for the probe to succeed and was, therefore, dropped out of the service

wozniakjan requested review from richm, lukas-vlcek, portante, jcantrill and t0ffel August 25, 2017 11:52

wozniakjan mentioned this pull request Aug 25, 2017

elasticsearch: reintroduce readiness probe openshift/openshift-ansible#5209

Merged

wozniakjan added the component/elasticsearch label Aug 25, 2017

t0ffel reviewed Aug 25, 2017

View reviewed changes

elasticsearch/probe/readiness.sh Outdated Show resolved Hide resolved

elasticsearch/probe/readiness.sh Show resolved Hide resolved

wozniakjan force-pushed the es_readiness_probe branch 2 times, most recently from 169cb95 to 0540d24 Compare August 25, 2017 14:31

lukas-vlcek reviewed Aug 25, 2017

View reviewed changes

elasticsearch/probe/readiness.sh Show resolved Hide resolved

jcantrill added the backport/3.6 label Aug 25, 2017

jcantrill approved these changes Aug 25, 2017

View reviewed changes

wozniakjan force-pushed the es_readiness_probe branch from 0540d24 to 615f6d8 Compare August 28, 2017 06:41

wozniakjan mentioned this pull request Aug 28, 2017

Backport 3.6 - elasticsearch: readiness probe extension #612

Merged

wozniakjan changed the title ~~WIP elasticsearch: readiness probe extension~~ elasticsearch: readiness probe extension Aug 28, 2017

richm reviewed Aug 28, 2017

View reviewed changes

wozniakjan force-pushed the es_readiness_probe branch from 615f6d8 to 9e64af7 Compare August 28, 2017 16:17

wozniakjan force-pushed the es_readiness_probe branch from 9e64af7 to f52a737 Compare September 6, 2017 15:45

wozniakjan force-pushed the es_readiness_probe branch from f52a737 to c691f2a Compare September 6, 2017 15:58

lukas-vlcek approved these changes Sep 7, 2017

View reviewed changes

openshift-bot merged commit ca917ff into openshift:master Sep 7, 2017

wozniakjan mentioned this pull request Sep 22, 2017

Fresh install or upgrade of logging stack to v3.6.0 === Unknown Discovery type [kubernetes] openshift/openshift-ansible#5497

Closed

wozniakjan added the backport/completed label Nov 2, 2017

elasticsearch: readiness probe extension #609

elasticsearch: readiness probe extension #609

wozniakjan commented Aug 25, 2017 •

edited

wozniakjan commented Aug 25, 2017

lukas-vlcek left a comment

jcantrill commented Aug 25, 2017

jcantrill left a comment

richm Aug 28, 2017

jcantrill commented Aug 28, 2017

wozniakjan commented Sep 6, 2017 •

edited

wozniakjan commented Sep 6, 2017

openshift-bot commented Sep 6, 2017

openshift-bot commented Sep 6, 2017

wozniakjan commented Sep 7, 2017

jcantrill commented Sep 7, 2017

openshift-bot commented Sep 7, 2017

openshift-bot commented Sep 7, 2017 •

edited

portante commented Sep 7, 2017

wozniakjan commented Sep 8, 2017

richm commented Sep 8, 2017

portante commented Sep 8, 2017

wozniakjan commented Sep 8, 2017 •

edited

elasticsearch: readiness probe extension #609

elasticsearch: readiness probe extension #609

Conversation

wozniakjan commented Aug 25, 2017 • edited

wozniakjan commented Aug 25, 2017

lukas-vlcek left a comment

Choose a reason for hiding this comment

jcantrill commented Aug 25, 2017

jcantrill left a comment

Choose a reason for hiding this comment

richm Aug 28, 2017

Choose a reason for hiding this comment

jcantrill commented Aug 28, 2017

wozniakjan commented Sep 6, 2017 • edited

wozniakjan commented Sep 6, 2017

openshift-bot commented Sep 6, 2017

openshift-bot commented Sep 6, 2017

wozniakjan commented Sep 7, 2017

jcantrill commented Sep 7, 2017

openshift-bot commented Sep 7, 2017

openshift-bot commented Sep 7, 2017 • edited

portante commented Sep 7, 2017

wozniakjan commented Sep 8, 2017

richm commented Sep 8, 2017

portante commented Sep 8, 2017

wozniakjan commented Sep 8, 2017 • edited

wozniakjan commented Aug 25, 2017 •

edited

wozniakjan commented Sep 6, 2017 •

edited

openshift-bot commented Sep 7, 2017 •

edited

wozniakjan commented Sep 8, 2017 •

edited