feat: add KSM HCP E2E test with rendered config support - #6672
Open
Jan-Hendrik Boll (janboll) wants to merge 3 commits into
Open
feat: add KSM HCP E2E test with rendered config support#6672Jan-Hendrik Boll (janboll) wants to merge 3 commits into
Jan-Hendrik Boll (janboll) wants to merge 3 commits into
Conversation
openshift-ci
Bot
requested review from
Mike Gahagan (mgahagan73) and
Mariusz Mazur (mmazur)
August 25, 2026 09:10
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: janboll The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Copilot started reviewing on behalf of
Jan-Hendrik Boll (janboll)
August 25, 2026 09:10
View session
Contributor
There was a problem hiding this comment.
Pull request overview
This PR enhances the E2E test framework to consume a rendered config via RENDERED_CONFIG, factors Prometheus/config helpers into shared test/util/ packages, and adds an E2E test that validates KSM (kube_node_info) metrics are ingested into Azure Monitor for a happy-path HCP cluster.
Changes:
- Introduces shared Prometheus utilities (
LookupPrometheusEndpoint,QueryRange) and rendered-config helpers undertest/util/. - Wires rendered config loading into the E2E framework via a lazy
tc.RenderedConfig()accessor backed byRENDERED_CONFIG. - Adds a new E2E test for KSM metrics ingestion and updates existing Kusto/must-gather verifiers to read cluster/region from rendered config rather than hardcoding.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/util/verifiers/must_gather_cli.go | Removes hardcoded Kusto location and threads Kusto cluster/region through verifier execution. |
| test/util/verifiers/kusto.go | Threads Kusto cluster/region into must-gather log verification config (removes hardcoded defaults). |
| test/util/testconfig/config.go | Renames package to testconfig to match its folder and intended shared usage. |
| test/util/prometheus/prometheus.go | Adds shared Azure Monitor Prometheus endpoint lookup + query_range helper types/functions. |
| test/util/framework/per_test_framework.go | Exposes rendered config via tc.RenderedConfig() on the per-test context. |
| test/util/framework/per_invocation_framework.go | Implements lazy, once-per-invocation loading of rendered config from RENDERED_CONFIG. |
| test/e2e/kusto_logs_present.go | Updates Kusto logs + must-gather CLI verifications to load Kusto settings from rendered config. |
| test/e2e/ksm_hcp_metrics.go | Adds new E2E spec validating kube_node_info KSM metrics appear in Azure Monitor. |
| test/cmd/aro-hcp-tests/gather-observability/workspace.go | Switches Prometheus endpoint lookup to the shared prom util package. |
| test/cmd/aro-hcp-tests/gather-observability/promql.go | Reuses shared Prometheus response/result types via type aliases; removes duplicated implementations. |
| test/cmd/aro-hcp-tests/gather-observability/options.go | Switches to shared rendered-config + Prometheus query helpers (removes internal testutil dependency). |
| test/cmd/aro-hcp-tests/custom-link-tools/options.go | Switches rendered-config helpers to shared testconfig package. |
| nonlocal-e2e-specs.txt | Adds the new KSM metrics E2E spec to the nonlocal spec list. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+127
to
+129
| for _, tc := range testCases { | ||
| logger.Info("Running must-gather CLI test case", "name", tc.name) | ||
| if err := runTestCase(ctx, binary, tc); err != nil { | ||
| if err := runTestCase(ctx, binary, tc, v.kustoCluster, v.kustoRegion); err != nil { |
Comment on lines
+54
to
+69
| // LookupPrometheusEndpoint retrieves the Prometheus query endpoint for an | ||
| // Azure Monitor workspace using the ARM SDK. | ||
| func LookupPrometheusEndpoint(ctx context.Context, cred azcore.TokenCredential, subscriptionID, resourceGroup, workspaceName string) (string, error) { | ||
| client, err := armmonitor.NewAzureMonitorWorkspacesClient(subscriptionID, cred, nil) | ||
| if err != nil { | ||
| return "", fmt.Errorf("failed to create monitor workspaces client: %w", err) | ||
| } | ||
| resp, err := client.Get(ctx, resourceGroup, workspaceName, nil) | ||
| if err != nil { | ||
| return "", fmt.Errorf("failed to get workspace %s: %w", workspaceName, err) | ||
| } | ||
| if resp.Properties == nil || resp.Properties.Metrics == nil || resp.Properties.Metrics.PrometheusQueryEndpoint == nil { | ||
| return "", fmt.Errorf("workspace %s has no Prometheus query endpoint", workspaceName) | ||
| } | ||
| return *resp.Properties.Metrics.PrometheusQueryEndpoint, nil | ||
| } |
… utilities Extract Prometheus query utilities and config loading helpers from internal packages into shared locations (test/util/prometheus/ and test/util/testconfig/) so E2E tests can access them. Wire rendered config into the E2E framework via RENDERED_CONFIG env var with lazy-loaded access through tc.RenderedConfig(). Add a new E2E test verifying that kube-state-metrics (KSM) metrics deployed by the ksmhcp controller are present in Azure Monitor for running HCP clusters by querying kube_node_info. Also remove hardcoded kusto cluster/region values from the must-gather verifiers, reading them from rendered config instead. Ref: AROSLSRE-1869 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Jan-Hendrik Boll (janboll)
force-pushed
the
ksm-test
branch
from
August 25, 2026 10:18
22dc550 to
a095cb3
Compare
Copilot started reviewing on behalf of
Jan-Hendrik Boll (janboll)
August 25, 2026 10:18
View session
Collaborator
Author
|
/retest-required |
Provides a sane default for RENDERED_CONFIG based on DEPLOY_ENV and LOCATION so local E2E runs can pick up the rendered config without requiring the variable to be set explicitly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Jan-Hendrik Boll (janboll)
force-pushed
the
ksm-test
branch
from
August 25, 2026 13:30
a095cb3 to
522f453
Compare
Copilot started reviewing on behalf of
Jan-Hendrik Boll (janboll)
August 25, 2026 13:31
View session
Copilot started reviewing on behalf of
Jan-Hendrik Boll (janboll)
August 25, 2026 15:02
View session
Comment on lines
+42
to
+46
| regionRG := os.Getenv("REGION_RG") | ||
| Expect(regionRG).NotTo(BeEmpty(), "REGION_RG environment variable must be set") | ||
|
|
||
| hcpWorkspaceName := os.Getenv("HCP_WORKSPACE_NAME") | ||
| Expect(hcpWorkspaceName).NotTo(BeEmpty(), "HCP_WORKSPACE_NAME environment variable must be set") |
Comment on lines
+57
to
+60
| kustoName := os.Getenv("KUSTO_NAME") | ||
| Expect(kustoName).NotTo(BeEmpty(), "KUSTO_NAME environment variable must be set") | ||
| kustoRegion := os.Getenv("KUSTO_REGION") | ||
| Expect(kustoRegion).NotTo(BeEmpty(), "KUSTO_REGION environment variable must be set") |
Replace the RENDERED_CONFIG-based config loading approach with direct environment variables (REGION_RG, HCP_WORKSPACE_NAME, KUSTO_NAME, KUSTO_REGION) defined in test/Env.mk and resolved via templatize. This removes the rendered config infrastructure from the E2E framework (RenderedConfig(), getRenderedConfig(), renderedConfigPath) and instead has tests read values directly from env vars, matching the existing pattern for LOCATION, SVC_CLUSTER, etc. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Jan-Hendrik Boll (janboll)
force-pushed
the
ksm-test
branch
from
August 25, 2026 15:10
f29a02a to
7a93dd5
Compare
Copilot started reviewing on behalf of
Jan-Hendrik Boll (janboll)
August 25, 2026 15:10
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (1)
test/e2e/ksm_hcp_metrics.go:47
- PR description says the E2E framework is wired to use rendered config via a RENDERED_CONFIG env var with a lazy tc.RenderedConfig() accessor and shared test/util/testconfig helpers, but the current changes don’t introduce those helpers/accessor and this new test reads REGION_RG and HCP_WORKSPACE_NAME directly from environment variables. Grep shows no references to RENDERED_CONFIG or a test/util/testconfig package in the repo, so either the implementation is missing or the PR description/CI requirement needs to be updated to match the actual configuration mechanism.
tc := framework.NewTestContext()
regionRG := os.Getenv("REGION_RG")
Expect(regionRG).NotTo(BeEmpty(), "REGION_RG environment variable must be set")
hcpWorkspaceName := os.Getenv("HCP_WORKSPACE_NAME")
Expect(hcpWorkspaceName).NotTo(BeEmpty(), "HCP_WORKSPACE_NAME environment variable must be set")
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.
Summary
LookupPrometheusEndpoint,QueryRange) and config loading helpers (LoadRenderedConfig,ConfigGetString,ConfigGetInt) frominternalpackages into sharedtest/util/prometheus/andtest/util/testconfig/packagesRENDERED_CONFIGenv var with lazytc.RenderedConfig()accessorkube_node_infoKSM metrics reach Azure Monitor for the happy-path HCP clustergather-observabilityandcustom-link-toolsCLI commands to use shared packages directly (deleteinternal/testutil)CI Requirement
The Prow job running E2E tests needs
RENDERED_CONFIGset to the rendered config YAML path. Without it, the KSM test and kusto logs test will fail with a clear error.Test plan
go test ./cmd/aro-hcp-tests/...)go build -tags E2Etests ./e2e/...)make lint— 0 issues)nonlocal-e2e-specs.txtregeneratedRENDERED_CONFIGsetRef: AROSLSRE-1869
🤖 Generated with Claude Code