chore(systest): sync Grafana dashboards in run_systest.sh#10494
Open
basvandijk wants to merge 3 commits into
Open
chore(systest): sync Grafana dashboards in run_systest.sh#10494basvandijk wants to merge 3 commits into
basvandijk wants to merge 3 commits into
Conversation
Move the dfinity-ops/k8s Grafana dashboard checkout out of the `ict` Go tool and into rs/tests/run_systest.sh, gated by the new IC_DASHBOARDS_BRANCH environment variable. Doing the checkout in run_systest.sh makes it work uniformly for both plain testnets (`bazel run //rs/tests/testnets:...`) and colocated tests: the resulting directory is exported as IC_DASHBOARDS_DIR which is already read by the test driver (prometheus_vm.rs) and forwarded into the colocated UVM by colocate_test.rs. A pre-set IC_DASHBOARDS_DIR (e.g. a local clone) is honored and skips the checkout. - run_systest.sh: sparse-checkout the dashboards when IC_DASHBOARDS_BRANCH is set and IC_DASHBOARDS_DIR is not; non-fatal on failure. - ict testnet create: set IC_DASHBOARDS_BRANCH from --k8s-branch instead of checking out the repo itself (dashboards stay on by default). - ict test: drop the dashboard checkout (the driver runs in a sandbox without SSH/network so a runtime checkout cannot work there). - helpers.go: remove the now-unused sparse_checkout helper. - README_NEW.md: document the IC_DASHBOARDS_BRANCH workflow.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR relocates the Grafana dashboard checkout logic from the ict Go CLI into rs/tests/run_systest.sh so dashboards can be provisioned consistently for both plain testnet runs and colocated system tests via IC_DASHBOARDS_DIR.
Changes:
- Add optional, best-effort sparse checkout of
bases/apps/ic-dashboardsfromdfinity-ops/k8sinrun_systest.sh, controlled byIC_DASHBOARDS_BRANCH, exportingIC_DASHBOARDS_DIR. - Update
ict testnet createto setIC_DASHBOARDS_BRANCH(instead of performing the checkout itself) and remove dashboard syncing fromict test. - Remove the now-unused Go helper used for sparse checkouts and document the new workflow in
README_NEW.md.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
rs/tests/run_systest.sh |
Adds gated dashboards sync + exports IC_DASHBOARDS_DIR from a sparse checkout. |
rs/tests/README_NEW.md |
Documents how to enable dashboards via IC_DASHBOARDS_BRANCH / IC_DASHBOARDS_DIR. |
rs/tests/ict/cmd/testnetCreateCmd.go |
Switches to passing IC_DASHBOARDS_BRANCH for run_systest.sh to handle syncing. |
rs/tests/ict/cmd/testCmd.go |
Removes dashboard checkout logic and the associated flag from ict test. |
rs/tests/ict/cmd/helpers.go |
Removes the unused sparse_checkout helper and related imports. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- run_systest.sh: make the previous-checkout cleanup non-fatal so a failed `rm` can't abort the test run under `set -e`. - run_systest.sh: run `git clone` with `GIT_SSH_COMMAND="ssh -oBatchMode=yes"` so a missing SSH key / unknown host key fails fast instead of hanging on an interactive prompt; also clone shallowly (`--depth 1`). - README_NEW.md: clarify that the `ict testnet create` dashboard sync is best-effort and the testnet is still deployed if it fails.
| echo "Syncing Grafana dashboards from k8s branch '$IC_DASHBOARDS_BRANCH' ..." >&2 | ||
| # Don't let a missing SSH key / unknown host key block the run by prompting | ||
| # interactively; fail fast instead (the sync is best-effort, see below). | ||
| if GIT_SSH_COMMAND="ssh -oBatchMode=yes" \ |
Comment on lines
176
to
+180
| env := os.Environ() | ||
| cmd.Println(GREEN + "Will try to sync dashboards from k8s branch: " + cfg.k8sBranch) | ||
| icDashboardsDir, err := sparse_checkout("git@github.com:dfinity-ops/k8s.git", "", []string{"bases/apps/ic-dashboards"}, cfg.k8sBranch) | ||
| if err != nil { | ||
| cmd.PrintErrln(YELLOW + "Failed to sync k8s dashboards. Received the following error: " + err.Error()) | ||
| } else { | ||
| cmd.PrintErrln(GREEN + "Successfully synced dashboards to path " + icDashboardsDir) | ||
| icDashboardsDir = filepath.Join(icDashboardsDir, "bases", "apps", "ic-dashboards") | ||
| cmd.Println(GREEN + "Will use " + icDashboardsDir + " as a root for dashboards") | ||
| env = append(env, "IC_DASHBOARDS_DIR="+icDashboardsDir) | ||
| } | ||
| // run_systest.sh syncs the Grafana dashboards from this k8s branch and exposes | ||
| // them to the test driver via the IC_DASHBOARDS_DIR environment variable. | ||
| cmd.Println(GREEN + "Dashboards will be synced from k8s branch: " + cfg.k8sBranch + NC) | ||
| env = append(env, "IC_DASHBOARDS_BRANCH="+cfg.k8sBranch) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We're deprecating the
icttool. The last remaining use case forictwas that it would automatically checkout the Grafana dashboards from thedfinity-ops/k8srepository.This functionality has been moved into
rs/tests/run_systest.sh, gated by a newIC_DASHBOARDS_BRANCHenvironment variable.run_systest.shis the single chokepoint for both the plain testnet driver and the colocated wrapper, so exportingIC_DASHBOARDS_DIRthere makes dashboards work uniformly with no Rust changes:IC_DASHBOARDS_DIRas before (prometheus_vm.rs).colocate_test.rs) already tarsIC_DASHBOARDS_DIR, ships it to the UVM and re-exports it into the inner docker container.A pre-set
IC_DASHBOARDS_DIR(e.g. a local clone) is honored and skips the checkout.Changes
run_systest.sh: sparse-checkoutbases/apps/ic-dashboardswhenIC_DASHBOARDS_BRANCHis set andIC_DASHBOARDS_DIRis not; non-fatal on failure.ict testnet create: setsIC_DASHBOARDS_BRANCHfrom--k8s-branchinstead of doing the checkout itself (dashboards remain on by default).ict test: drops its dashboard checkout (the driver runs in a sandbox without SSH/network, so a runtime checkout can't work there).helpers.go: removes the now-unusedsparse_checkouthelper.README_NEW.md: documents theIC_DASHBOARDS_BRANCHworkflow.Usage
Testing
bazel build //rs/tests/ict //rs/tests/ict/cmdpasses;gofmtclean;bash -n rs/tests/run_systest.shclean.