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.

Sign up for GitHub

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 Zookeeper Cluster Readiness Check in init container #102

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Fix Zookeeper Cluster Readiness Check in init container #102

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1741dc5
Use best practice method to check for Zookeeper readiness
gregoryg Oct 23, 2023
24f1973
Merge branch 'develop' of github.com:gregoryg/helm-charts into zookee…
gregoryg Oct 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 15 additions & 17 deletions charts/stardog/templates/statefulset.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,23 +68,21 @@ spec:
- /bin/sh
- -c
- |
{{ if .Values.zookeeper.enabled }}
while :
do
echo "Checking for ZK followers"
ZK_MNTR=$(echo mntr | nc {{ .Release.Name }}-zookeeper-headless.{{ include "stardog.namespace" . }} 2181)
ZK_FOLLOWERS=$(echo "${ZK_MNTR}" | grep zk_synced_followers | awk '{print $2}')
echo "Currently ${ZK_FOLLOWERS} ZK followers"
if [[ "${ZK_FOLLOWERS}" -gt "1" ]]; then
echo "ZK has two sync'd followers (with the leader that makes 3)"
exit 0
fi
sleep 1
done
{{ else }}
echo "Using existing zookeeper"
{{ end }}
{{ end }}
{{- if .Values.zookeeper.enabled }}
while :
do
echo "Checking for ZK readiness"
ZK_OK=$(echo ruok | nc {{ .Release.Name }}-zookeeper-headless.{{ .Release.Namespace }} 2181)
Copy link
Member

Choose a reason for hiding this comment

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

ruok just responds imok if the ZooKeeper server is up and bound to the port [1]. That is fine for a healthcheck on a ZK server alone, but it is not sufficient for Stardog's purposes. Stardog needs to know that ZooKeeper is up and ready (i.e. all ZK instances are participating in the quorum).

My understanding of the k8s service is that it will cycle through the servers so while it may hit a follower which doesn't have the information Stardog needs, it would loop until it finds a server that does.

Are you finding that it never tries new servers and this loops endlessly? Or just that it tries and fails a few times before eventually succeeding?

  1. https://zookeeper.apache.org/doc/r3.8.3/zookeeperAdmin.html#sc_zkCommands

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it never tries new server - this happens very often - the result is that perhaps one of 3 stardog pods comes up while the others never do. Even hours later, the same follower is used. After helm uninstall followed by another helm install, you may get a different result. It's entirely random selection. Some other method needs to be implemented. This is so common, we must be missing something in our tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

BTW, are you saying that the liveness check is inadequate because a Zookeeper cluster can report it's in an ok state even though there is no leader? I wouldn't think that would be possible.

Copy link
Member

Choose a reason for hiding this comment

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

No pods will come up if ZooKeeper hasn't reported ready. Stardog won't move on to deploying until ZK reports ready as determined by this check iirc (though it's been awhile since I looked closely). I suspect something else is wrong with your deploys unless you've modified the charts to deploy Stardog even if this check hasn't passed.

Yes, a liveness check just determines if the server is alive, which isn't the same as if it's ready. For ZK, that is this ruok command, similar to how Stardog has /admin/alive for an alive check for the individual server and /admin/healthcheck for a ready check for the cluster. The alive check just says the server is up. The ready check says it's up and actually ready (i.e. a member of the cluster).

I don't think ZK has something akin to a ready check for an ensemble, hence the need to check something else.

if [[ "${ZK_OK}" == "imok" ]]; then
echo "ZK reports ready and in good state"
exit 0
fi
sleep 1
done
{{- else }}
echo "Using existing zookeeper"
{{- end }}
{{- end }}
containers:
- name: {{ include "stardog.fullname" . }}
imagePullPolicy: {{ .Values.image.pullPolicy }}
Expand Down