fix: add retry logic to OpenSearch setup scripts to handle non-JSON responses#64
Conversation
📝 WalkthroughWalkthroughVersion bumps for two Helm charts (observability-logs-opensearch to 0.3.11, observability-tracing-openobserve to 0.2.1) reflected in README and Chart.yaml files. Two setup scripts enhanced with JSON validation and curl error handling for improved cluster readiness polling. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
observability-tracing-opensearch/init/setup-opensearch.sh (1)
31-31: Suppress stdout from JSON validation check.Same issue as in the logs-opensearch script:
jq .prints parsed JSON to stdout when successful.Proposed fix
- if echo "$clusterHealth" | jq . 2>/dev/null; then + if echo "$clusterHealth" | jq . >/dev/null 2>&1; then🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@observability-tracing-opensearch/init/setup-opensearch.sh` at line 31, The JSON validation check using `jq .` on the variable `clusterHealth` currently emits the parsed JSON to stdout; update the conditional that evaluates `if echo "$clusterHealth" | jq . 2>/dev/null; then` so that successful jq output is suppressed (e.g., use jq's exit-only mode or redirect stdout to /dev/null) while preserving stderr redirection; modify the `echo "$clusterHealth" | jq ...` invocation used in that if-condition (the `clusterHealth` check) to avoid printing the parsed JSON on success.observability-logs-opensearch/init/setup-opensearch.sh (1)
34-34: Suppress stdout from JSON validation check.
jq .prints the parsed JSON to stdout when successful, polluting the logs. Redirect both stdout and stderr to/dev/null.Proposed fix
- if echo "$clusterHealth" | jq . 2>/dev/null; then + if echo "$clusterHealth" | jq . >/dev/null 2>&1; then🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@observability-logs-opensearch/init/setup-opensearch.sh` at line 34, The JSON validation check using the if condition that pipes echo "$clusterHealth" into jq (the if echo "$clusterHealth" | jq . invocation) is currently leaving parsed JSON on stdout; suppress both stdout and stderr from jq by redirecting its output to /dev/null in that condition so the check only returns the exit status without polluting logs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@observability-logs-opensearch/init/setup-opensearch.sh`:
- Line 34: The JSON validation check using the if condition that pipes echo
"$clusterHealth" into jq (the if echo "$clusterHealth" | jq . invocation) is
currently leaving parsed JSON on stdout; suppress both stdout and stderr from jq
by redirecting its output to /dev/null in that condition so the check only
returns the exit status without polluting logs.
In `@observability-tracing-opensearch/init/setup-opensearch.sh`:
- Line 31: The JSON validation check using `jq .` on the variable
`clusterHealth` currently emits the parsed JSON to stdout; update the
conditional that evaluates `if echo "$clusterHealth" | jq . 2>/dev/null; then`
so that successful jq output is suppressed (e.g., use jq's exit-only mode or
redirect stdout to /dev/null) while preserving stderr redirection; modify the
`echo "$clusterHealth" | jq ...` invocation used in that if-condition (the
`clusterHealth` check) to avoid printing the parsed JSON on success.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 620b312b-90ba-40d4-9bd9-0ac07af48f85
📒 Files selected for processing (6)
observability-logs-opensearch/README.mdobservability-logs-opensearch/helm/Chart.yamlobservability-logs-opensearch/init/setup-opensearch.shobservability-tracing-openobserve/README.mdobservability-tracing-openobserve/helm/Chart.yamlobservability-tracing-opensearch/init/setup-opensearch.sh
…esponses Signed-off-by: Nilushan Costa <nilushan@wso2.com>
6ff8a0a to
e6ed3ce
Compare
Purpose
openchoreo/openchoreo#2887
The OpenSearch init scripts in observability-logs-opensearch and observability-tracing-opensearch exit prematurely when OpenSearch is still starting up. The health check endpoint returns a non-JSON response during startup, causing jq parsing to fail with a non-zero exit code. Due to set -euo pipefail, this immediately terminates the script, bypassing the retry logic entirely.
Goals
Ensure the init scripts gracefully handle non-JSON responses from the OpenSearch health check endpoint and continue retrying until OpenSearch is fully up or the maximum retry attempts are exhausted.
Approach
Summary by CodeRabbit
Chores
Bug Fixes