Skip to content

Deploy official Istio dashboards in update-istio.sh#734

Merged
awhdesmond merged 3 commits into
mainfrom
awhdesmond/istio-dashboards
Jun 18, 2026
Merged

Deploy official Istio dashboards in update-istio.sh#734
awhdesmond merged 3 commits into
mainfrom
awhdesmond/istio-dashboards

Conversation

@awhdesmond

@awhdesmond awhdesmond commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Deploy official Istio dashboardsbased on the version used in update-istio.sh. I think we can start with these and add custom dashboards when required.

Grafana version required: "pluginVersion": "v11.0.0"

@awhdesmond awhdesmond requested review from Tobias-Pe and ensonic June 17, 2026 08:42
@awhdesmond awhdesmond changed the title Provision Official Istio Dashboard in update-istio.sh Deploy official Istio dashboards in update-istio.sh. Jun 17, 2026
@awhdesmond awhdesmond changed the title Deploy official Istio dashboards in update-istio.sh. Deploy official Istio dashboards in update-istio.sh Jun 17, 2026

@Tobias-Pe Tobias-Pe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks !

Comment thread third_party/istio/update-istio.sh Outdated

for dashboard in "${DASHBOARDS[@]}"; do
json_file="${tmpdir}/${dashboard}"
curl -fsSL "https://raw.githubusercontent.com/istio/istio/${VERSION}/manifests/addons/dashboards/${dashboard}" -o "${json_file}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[CORRECTNESS, PERFORMANCE] Please consider the following updates to improve the dashboard downloads:

  • Verify the exit status of the curl command to ensure the script fails early if a download fails, preventing invalid YAML generation.
  • Optimize the download speed by running them in parallel (using background jobs with & and wait) or using a batched curl invocation to avoid sequential network round-trips.

🤖 AI Reviewer (Jetski) | Traces: correctness: go/traj/bb1b5fdf-6a81-49b3-a3b2-5c871244056f, performance: go/traj/233135f6-adce-4d81-ba66-9607b5ea9999

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

WRT optimiation maybe just a nit or even ignorable imo but the exit code could be relevant

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated script to used batched curl and exit on download failure.

@awhdesmond awhdesmond requested a review from Tobias-Pe June 18, 2026 02:10
@awhdesmond awhdesmond force-pushed the awhdesmond/istio-dashboards branch from 4b61e53 to 9fdf785 Compare June 18, 2026 03:30
Comment thread third_party/istio/update-istio.sh Outdated
)

dashboard_yaml="${tmpdir}/dashboards.yaml"
true > "${dashboard_yaml}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since this is a tempdir, we can probably just use touch "${dashboard_yaml}" as there is no need to reset the file.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated!

@ensonic ensonic left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks!

@awhdesmond awhdesmond force-pushed the awhdesmond/istio-dashboards branch from 038c37b to b30effa Compare June 18, 2026 06:59
@awhdesmond awhdesmond merged commit 85f827b into main Jun 18, 2026
7 checks passed
@awhdesmond awhdesmond deleted the awhdesmond/istio-dashboards branch June 18, 2026 07:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants