Skip to content

Commit

Permalink
Fix and improve probes (#79)
Browse files Browse the repository at this point in the history
* Fix probes when Qdrant is served over TLS

This also improves the integration test runtime and reliability by not installing a fresh cluster for every test but upgrading it with the new config.

Fixes #77

* Deactivate liveness and startup probes by default

Since these probes restart the pod on failure, they can disrupt the health of the Qdrant cluster unnecessarily and cause more problems then actually help. Having a readinessProbe is usually enough.

* Fix tests

* Try to fix connection issues
  • Loading branch information
bashofmann authored Sep 29, 2023
1 parent 52515f6 commit 264b4cf
Show file tree
Hide file tree
Showing 13 changed files with 126 additions and 95 deletions.
9 changes: 9 additions & 0 deletions charts/qdrant/templates/statefulset.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,9 @@ spec:
httpGet:
path: /livez
port: {{ .targetPort }}
{{- if and $values.config.service $values.config.service.enable_tls }}
scheme: HTTPS
{{- end }}
{{- end }}
initialDelaySeconds: {{ $values.livenessProbe.initialDelaySeconds }}
timeoutSeconds: {{ $values.livenessProbe.timeoutSeconds }}
Expand All @@ -117,6 +120,9 @@ spec:
httpGet:
path: /readyz
port: {{ .targetPort }}
{{- if and $values.config.service $values.config.service.enable_tls }}
scheme: HTTPS
{{- end }}
{{- end }}
initialDelaySeconds: {{ $values.readinessProbe.initialDelaySeconds }}
timeoutSeconds: {{ $values.readinessProbe.timeoutSeconds }}
Expand All @@ -134,6 +140,9 @@ spec:
httpGet:
path: /readyz
port: {{ .targetPort }}
{{- if and $values.config.service $values.config.service.enable_tls }}
scheme: HTTPS
{{- end }}
{{- end }}
initialDelaySeconds: {{ $values.startupProbe.initialDelaySeconds }}
timeoutSeconds: {{ $values.startupProbe.timeoutSeconds }}
Expand Down
86 changes: 53 additions & 33 deletions charts/qdrant/templates/tests/test-db-interaction.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,21 @@ metadata:
spec:
containers:
- name: test-script
image: registry.suse.com/bci/python:3.11
image: registry.suse.com/bci/bci-base:latest
args: ['bash', '/app/entrypoint.sh']
volumeMounts:
- mountPath: /app
name: test-script
{{- if .Values.additionalVolumeMounts }}
{{- toYaml .Values.additionalVolumeMounts | default "" | nindent 8 }}
{{- end}}
volumes:
- name: test-script
configMap:
name: "{{ include "qdrant.fullname" . }}-test-db-interaction"
{{- if .Values.additionalVolumes }}
{{- toYaml .Values.additionalVolumes | default "" | nindent 4 }}
{{- end}}
restartPolicy: Never
serviceAccountName: {{ include "qdrant.fullname" . }}
---
Expand All @@ -35,43 +41,57 @@ data:
entrypoint.sh: |
#!/bin/bash
set -xe
zypper install -y python311-dbm
pip3 install qdrant-client=={{ .Chart.AppVersion }}
python3 /app/test.py
test.py: |
from qdrant_client import QdrantClient
from qdrant_client.http.models import Distance, VectorParams
from qdrant_client.http.models import PointStruct
echo 'connect-timeout = 5' > $HOME/.curlrc
echo 'retry = 5' >> $HOME/.curlrc
if [ -d /mnt/secrets/certs ]; then
cp /mnt/secrets/certs/ca.pem /usr/share/pki/trust/anchors/private-ca.pem
update-ca-certificates
fi
QDRANT_COLLECTION="test_collection"
{{- range .Values.service.ports }}
{{- if eq .name "http" }}
print("Connecting to {{ include "qdrant.fullname" $root }}.{{ $namespace }}:{{ .port }}")
client = QdrantClient("{{ include "qdrant.fullname" $root }}.{{ $namespace }}", port={{ .port }})
echo "Connecting to {{ include "qdrant.fullname" $root }}.{{ $namespace }}:{{ .port }}"
QDRANT_URL="http://{{ include "qdrant.fullname" $root }}.{{ $namespace }}:{{ .port }}"
{{- if and $root.Values.config.service $root.Values.config.service.enable_tls }}
echo "Using https"
QDRANT_URL="https://{{ include "qdrant.fullname" $root }}.{{ $namespace }}:{{ .port }}"
{{- end }}
{{- end }}
{{- end }}
API_KEY_HEADER=""
{{- if .Values.apiKey }}
API_KEY_HEADER="Api-key: {{ .Values.apiKey }}"
{{- end }}
# Delete collection if exists
curl -X DELETE -H "${API_KEY_HEADER}" $QDRANT_URL/collections/${QDRANT_COLLECTION}
client.recreate_collection(
collection_name="test_collection",
vectors_config=VectorParams(size=4, distance=Distance.DOT),
)
# Create collection
curl -X PUT \
-H 'Content-Type: application-json' \
-d '{"vectors":{"size":4,"distance":"Dot"}}' \
-H "${API_KEY_HEADER}" \
$QDRANT_URL/collections/${QDRANT_COLLECTION} --fail-with-body
operation_info = client.upsert(
collection_name="test_collection",
wait=True,
points=[
PointStruct(id=1, vector=[0.05, 0.61, 0.76, 0.74], payload={"city": "Berlin"}),
PointStruct(id=2, vector=[0.19, 0.81, 0.75, 0.11], payload={"city": "London"}),
PointStruct(id=3, vector=[0.36, 0.55, 0.47, 0.94], payload={"city": "Moscow"}),
PointStruct(id=4, vector=[0.18, 0.01, 0.85, 0.80], payload={"city": "New York"}),
PointStruct(id=5, vector=[0.24, 0.18, 0.22, 0.44], payload={"city": "Beijing"}),
PointStruct(id=6, vector=[0.35, 0.08, 0.11, 0.44], payload={"city": "Mumbai"}),
]
)
print(operation_info)
# Insert points
curl -X PUT \
-H 'Content-Type: application-json' \
-d '{"points":[
{"id":1,"vector":[0.05, 0.61, 0.76, 0.74],"payload":{"city":"Berlin"}},
{"id":2,"vector":[0.19, 0.81, 0.75, 0.11],"payload":{"city":"London"}},
{"id":3,"vector":[0.36, 0.55, 0.47, 0.94],"payload":{"city":"Moscow"}},
{"id":4,"vector":[0.18, 0.01, 0.85, 0.80],"payload":{"city":"New York"}},
{"id":5,"vector":[0.24, 0.18, 0.22, 0.44],"payload":{"city":"Beijing"}},
{"id":6,"vector":[0.35, 0.08, 0.11, 0.44],"payload":{"city":"Mumbai"}}
]}' \
-H "${API_KEY_HEADER}" \
$QDRANT_URL/collections/${QDRANT_COLLECTION}/points --fail-with-body
search_result = client.search(
collection_name="test_collection",
query_vector=[0.2, 0.1, 0.9, 0.7],
limit=3
)
print(search_result)
# Run query
curl -X POST \
-H 'Content-Type: application-json' \
-d '{"vector":[0.2, 0.1, 0.9, 0.7],"limit":3}' \
-H "${API_KEY_HEADER}" \
$QDRANT_URL/collections/${QDRANT_COLLECTION}/points/search --fail-with-body
4 changes: 2 additions & 2 deletions charts/qdrant/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ ingress:
# secretName: tls-secret-name

livenessProbe:
enabled: true
enabled: false
initialDelaySeconds: 5
periodSeconds: 5
timeoutSeconds: 1
Expand All @@ -70,7 +70,7 @@ readinessProbe:
successThreshold: 1

startupProbe:
enabled: true
enabled: false
initialDelaySeconds: 10
periodSeconds: 5
timeoutSeconds: 1
Expand Down
14 changes: 3 additions & 11 deletions test/integration/api_key.bats
Original file line number Diff line number Diff line change
@@ -1,20 +1,12 @@
setup_file() {
kubectl create namespace qdrant-helm-integration
kubectl create serviceaccount default -n qdrant-helm-integration || true
helm install qdrant charts/qdrant --set apiKey=foobar -n qdrant-helm-integration --wait
helm upgrade --install qdrant charts/qdrant --set apiKey=foobar -n qdrant-helm-integration --wait
kubectl rollout status statefulset qdrant -n qdrant-helm-integration
}

teardown_file() {
helm uninstall qdrant -n qdrant-helm-integration
kubectl delete serviceaccount default -n qdrant-helm-integration
kubectl delete namespace qdrant-helm-integration
}

@test "api key authentication works" {
run kubectl exec -n default curl -- curl -s http://qdrant.qdrant-helm-integration:6333/collections -H 'api-key: foobar'
run kubectl exec -n default curl -- curl -s http://qdrant.qdrant-helm-integration:6333/collections -H 'api-key: foobar' --fail-with-body
[ $status -eq 0 ]
[ "${output}" != "Invalid api-key" ]
[[ "${output}" =~ .*\"status\":\"ok\".* ]]
}

@test "api key authentication fails with key" {
Expand Down
6 changes: 6 additions & 0 deletions test/integration/assets/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
*.key
*.crt
*.csr
*.pem
*.ext
*.srl
19 changes: 19 additions & 0 deletions test/integration/assets/tls-values.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
additionalVolumes:
- secret:
secretName: test-tls
name: tls

additionalVolumeMounts:
- name: tls
mountPath: /mnt/secrets/certs

config:
service:
enable_tls: true
tls:
cert: "/mnt/secrets/certs/tls.crt"
key: "/mnt/secrets/certs/tls.key"
ca_cert: "/mnt/secrets/certs/ca.pem"
cluster:
p2p:
enable_tls: true
13 changes: 1 addition & 12 deletions test/integration/default_installation.bats
Original file line number Diff line number Diff line change
@@ -1,17 +1,6 @@
setup_file() {
kubectl create namespace qdrant-helm-integration
kubectl create serviceaccount default -n qdrant-helm-integration || true
helm install qdrant charts/qdrant -n qdrant-helm-integration --wait
helm upgrade --install qdrant charts/qdrant -n qdrant-helm-integration --wait
kubectl rollout status statefulset qdrant -n qdrant-helm-integration
# wait a bit to ensure that qdrant is really up, and we don't get connection refused
# errors in Github Actions
sleep 10
}

teardown_file() {
helm uninstall qdrant -n qdrant-helm-integration
kubectl delete serviceaccount default -n qdrant-helm-integration
kubectl delete namespace qdrant-helm-integration
}

@test "helm test - default values" {
Expand Down
24 changes: 24 additions & 0 deletions test/integration/https.bats
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
setup_file() {
rm test/integration/assets/ca.* || true
rm test/integration/assets/tls.* || true
openssl genrsa -des3 -passout pass:insecure -out test/integration/assets/ca.key 2048
openssl req -x509 -passin pass:insecure -new -nodes -key test/integration/assets/ca.key -sha256 -days 1825 -out test/integration/assets/ca.pem -subj "/C=DE/ST=Berlin/L=Berlin/O=Qdrant/OU=Cloud/CN=qdrant"
openssl genrsa -out test/integration/assets/tls.key 2048
openssl req -new -key test/integration/assets/tls.key -out test/integration/assets/tls.csr -subj "/C=DE/ST=Berlin/L=Berlin/O=Qdrant/OU=Cloud/CN=qdrant.qdrant-helm-integration"
openssl x509 -req -passin pass:insecure -in test/integration/assets/tls.csr -CA test/integration/assets/ca.pem -CAkey test/integration/assets/ca.key -CAcreateserial -out test/integration/assets/tls.crt -days 825 -sha256

kubectl -n qdrant-helm-integration create secret generic test-tls --from-file=tls.key="test/integration/assets/tls.key" --from-file=tls.crt="test/integration/assets/tls.crt" --from-file=ca.pem="test/integration/assets/ca.pem"
helm upgrade --install qdrant charts/qdrant -n qdrant-helm-integration --wait -f test/integration/assets/tls-values.yaml
kubectl rollout status statefulset qdrant -n qdrant-helm-integration
}

@test "Connection with https" {
run kubectl exec -n default curl -- curl -k -s https://qdrant.qdrant-helm-integration:6333/collections --fail-with-body
[ $status -eq 0 ]
[[ "${output}" =~ .*\"status\":\"ok\".* ]]
}

@test "helm test - with https" {
run helm test qdrant -n qdrant-helm-integration --logs
[ $status -eq 0 ]
}
14 changes: 3 additions & 11 deletions test/integration/random_api_key.bats
Original file line number Diff line number Diff line change
@@ -1,22 +1,14 @@
setup_file() {
kubectl create namespace qdrant-helm-integration
kubectl create serviceaccount default -n qdrant-helm-integration || true
helm install qdrant charts/qdrant --set apiKey=true -n qdrant-helm-integration --wait
helm upgrade --install qdrant charts/qdrant --set apiKey=true -n qdrant-helm-integration --wait
kubectl rollout status statefulset qdrant -n qdrant-helm-integration
}

teardown_file() {
helm uninstall qdrant -n qdrant-helm-integration
kubectl delete serviceaccount default -n qdrant-helm-integration
kubectl delete namespace qdrant-helm-integration
}

@test "random api key works" {
apiKey=$(kubectl -n qdrant-helm-integration get secret qdrant-apikey -o jsonpath="{.data.api-key}" | base64 --decode)
[ ${#apiKey} -eq 32 ]
run kubectl exec -n default curl -- curl -s http://qdrant.qdrant-helm-integration:6333/collections -H "api-key: ${apiKey}"
run kubectl exec -n default curl -- curl -s http://qdrant.qdrant-helm-integration:6333/collections -H "api-key: ${apiKey}" --fail-with-body
[ $status -eq 0 ]
[ "${output}" != "Invalid api-key" ]
[[ "${output}" =~ .*\"status\":\"ok\".* ]]
}

@test "random api key stays the same after upgrade" {
Expand Down
13 changes: 1 addition & 12 deletions test/integration/security_context.bats
Original file line number Diff line number Diff line change
@@ -1,16 +1,5 @@
setup_file() {
kubectl create namespace qdrant-helm-integration
kubectl create serviceaccount default -n qdrant-helm-integration || true
}

teardown_file() {
helm uninstall qdrant -n qdrant-helm-integration
kubectl delete serviceaccount default -n qdrant-helm-integration
kubectl delete namespace qdrant-helm-integration
}

@test "update without security context to security context corrects file permissions" {
helm install qdrant charts/qdrant --set-json 'podSecurityContext=false,containerSecurityContext=false' -n qdrant-helm-integration --wait
helm upgrade --install qdrant charts/qdrant --set-json 'podSecurityContext=false,containerSecurityContext=false' -n qdrant-helm-integration --wait
kubectl rollout status statefulset qdrant -n qdrant-helm-integration
user=$(kubectl exec qdrant-0 -n qdrant-helm-integration -- id -u)
[ "${user}" = "0" ]
Expand Down
2 changes: 2 additions & 0 deletions test/integration/setup_suite.bash
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ setup_suite() {
kubectl create serviceaccount default -n default
kubectl -n default run curl --image=curlimages/curl --command -- sh -c "sleep 3600"
kubectl wait --for=condition=Ready pod/curl -n default --timeout=300s
kubectl create namespace qdrant-helm-integration
kubectl create serviceaccount default -n qdrant-helm-integration || true
}

teardown_suite() {
Expand Down
13 changes: 1 addition & 12 deletions test/integration/unprivileged_image.bats
Original file line number Diff line number Diff line change
@@ -1,17 +1,6 @@
setup_file() {
kubectl create namespace qdrant-helm-integration
kubectl create serviceaccount default -n qdrant-helm-integration || true
helm install qdrant charts/qdrant -n qdrant-helm-integration --wait --set image.useUnprivilegedImage=true
helm upgrade --install qdrant charts/qdrant -n qdrant-helm-integration --wait --set image.useUnprivilegedImage=true
kubectl rollout status statefulset qdrant -n qdrant-helm-integration
# wait a bit to ensure that qdrant is really up, and we don't get connection refused
# errors in Github Actions
sleep 10
}

teardown_file() {
helm uninstall qdrant -n qdrant-helm-integration
kubectl delete serviceaccount default -n qdrant-helm-integration
kubectl delete namespace qdrant-helm-integration
}

@test "helm test - with unprivileged image" {
Expand Down
4 changes: 2 additions & 2 deletions test/qdrant_probes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func TestDefaultProbesOnStatefulset(t *testing.T) {
return container.Name == "qdrant"
})

require.Equal(t, "/readyz", container.StartupProbe.HTTPGet.Path)
require.Empty(t, container.StartupProbe)
require.Equal(t, "/readyz", container.ReadinessProbe.HTTPGet.Path)
require.Equal(t, "/livez", container.LivenessProbe.HTTPGet.Path)
require.Empty(t, container.LivenessProbe)
}

0 comments on commit 264b4cf

Please sign in to comment.