-
Notifications
You must be signed in to change notification settings - Fork 5
Add kuttl tests for lightspeed-app-server /metrics endpoint #22
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| --- | ||
| apiVersion: kuttl.dev/v1beta1 | ||
| kind: TestAssert | ||
| commands: | ||
| - script: | | ||
| #!/bin/bash | ||
| set -euo pipefail | ||
|
|
||
| NAMESPACE="openstack-lightspeed" | ||
| CONTAINER="lightspeed-service-api" | ||
| METRICS_PORT=8443 | ||
| METRICS_PATH="/metrics" | ||
| SA_NAME="lightspeed-metrics-test" | ||
|
|
||
| echo "Getting bearer token..." | ||
| TOKEN="$(oc create token "$SA_NAME" -n "$NAMESPACE")" | ||
|
|
||
| echo "Waiting for lightspeed-stack-deployment pod to be ready..." | ||
| oc wait --for=condition=Ready pod \ | ||
| -l app.kubernetes.io/name=openstack-lightspeed-app-server \ | ||
| -n "$NAMESPACE" \ | ||
| --timeout=120s | ||
|
|
||
| echo "Getting pod name..." | ||
| POD_NAME="$(oc get pods \ | ||
| -l app.kubernetes.io/name=openstack-lightspeed-app-server \ | ||
| -n "$NAMESPACE" \ | ||
| -o jsonpath='{.items[0].metadata.name}')" | ||
|
|
||
| if [ -z "$POD_NAME" ]; then | ||
| echo "ERROR: No pod found for lightspeed-app-server" | ||
| exit 1 | ||
| fi | ||
|
|
||
| echo "Fetching metrics from pod $POD_NAME (container: $CONTAINER)..." | ||
| METRICS_OUTPUT="$(oc exec -n "$NAMESPACE" "$POD_NAME" \ | ||
| -c "$CONTAINER" -- \ | ||
| curl -sk -H "Authorization: Bearer ${TOKEN}" \ | ||
| "https://localhost:${METRICS_PORT}${METRICS_PATH}")" | ||
|
|
||
| if [ -z "$METRICS_OUTPUT" ]; then | ||
| echo "ERROR: Empty response from /metrics endpoint" | ||
| exit 1 | ||
| fi | ||
|
|
||
| echo "Validating response is valid Prometheus exposition format..." | ||
| if ! echo "$METRICS_OUTPUT" | grep -q "^# HELP "; then | ||
| echo "ERROR: Response does not contain Prometheus HELP declarations" | ||
| echo "Response (first 500 chars):" | ||
| echo "$METRICS_OUTPUT" | head -c 500 | ||
| exit 1 | ||
| fi | ||
|
|
||
| if ! echo "$METRICS_OUTPUT" | grep -q "^# TYPE "; then | ||
| echo "ERROR: Response does not contain Prometheus TYPE declarations" | ||
| echo "Response (first 500 chars):" | ||
| echo "$METRICS_OUTPUT" | head -c 500 | ||
| exit 1 | ||
| fi | ||
|
|
||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, if we are OK with the kuttl tests running these type of tests, I think we should also check for at least one
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added a check for the presence of the |
||
| if ! echo "$METRICS_OUTPUT" | grep -q "^# TYPE ls_llm_calls_total "; then | ||
| echo "ERROR: Missing expected lightspeed metric: ls_llm_calls_total" | ||
| echo "Response (first 500 chars):" | ||
| echo "$METRICS_OUTPUT" | head -c 500 | ||
| exit 1 | ||
| fi | ||
|
|
||
| echo "Metrics endpoint is reachable and returns valid Prometheus format" | ||
| timeout: 180 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| ../../common/openstack-lightspeed-instance/assert-metrics-endpoint.yaml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would to hear @lpiwowar and/or @malingatembo opinion on using kuttl tests this ^
It seems like it fits more into the new test framework than kuttl. Because this is asserting some behavior from our API endpoint rather than just asserting the state of the cluster (which I believe kuttl is better designed for).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (blocking): I agree with this. If we did not have the foundation for the test framework laid out by @malingatembo, then I would consider this as an exception, maybe. But since the foundation is laid out, I think we should start adding tests there. It will force us to polish the test framework, and it will make the usage of it much easier later since the relevant tests will already be there and we won't be forced to migrate them there.