Skip to content

Cluster-coherence preflight guard (#95) - #107

Open
e-300 wants to merge 2 commits into
opencost:mainfrom
e-300:env-coherence-guard
Open

Cluster-coherence preflight guard (#95)#107
e-300 wants to merge 2 commits into
opencost:mainfrom
e-300:env-coherence-guard

Conversation

@e-300

@e-300 e-300 commented Jun 17, 2026

Copy link
Copy Markdown
Member

Description

Closes #95 -> pkg/prometheus/client.go silently falls back to the demo Prometheus when the prometheus url is unset, and pkg/env/env.go falls back to localhost when OPENCOST_URL is unset.

The smoke tests (#74) check each endpoint is reachable, but not that both endpoints observe the same cluster.

This PR adds a preflight guard that runs before the rest of the suite. It:

  • fails fast if either PROMETHEUS_URL or OPENCOST_URL is unset (refusing the
    silent fallback),
  • queries both endpoints over one shared, hour-aligned historical window —
    kube_pod_container_status_running from Prometheus and /allocation
    aggregated by pod — and compares their running-pod sets,
  • filters synthetic entries (__idle__, *-unmounted-pvcs) and requires
    cluster-wide overlap of at least 50% (same-cluster is ~90%+, different-cluster
    is ~0%, so normal churn can't cross the line),
  • on mismatch, t.Fatals with both URLs and sample pods from each side.

It reuses the shared-timestamp, churn-tolerance, and retry from the pod-count test (#94).

Changes

  • test/integration/preflight/cluster_coherence_test.go (new) — the guard
    itself: env-var fail-fast, shared window, pod-set comparison, fresh-cluster
    fallback, and the failure diagnostics. Reads the env vars raw rather than
    through the existing helpers, since those helpers are what hide the missing
    value. Production code (client.go/env.go) is intentionally left unchanged
    so other tests and local demo runs keep their fallbacks.
  • test/integration/preflight/test.bats (new) — runs the guard with
    go test -count=1 (always re-queries live targets, never a cached result).
    Tagged # bats test_tags=preflight so the main suite can exclude it.
  • .github/workflows/integration-testing.yaml (modified) — runs the guard
    as its own step before the main suite, so an incoherent environment aborts the
    run early. The main suite still runs recursively (bats -r test/integration)
    and uses --filter-tags '!preflight' to skip the guard, so new top-level test
    dirs are still picked up automatically.

Testing

Ran the full pipeline locally against the demo target:

  • go vet ./... and go build ./... — clean.
  • Preflight step — passes (overlap 1.000, 263 vs 263 pods).
  • Main suite — 46/46 pass; tag filter excludes only the guard
    (47 tests unfiltered, 46 filtered).
  • Fail-fast: with PROMETHEUS_URL unset, the guard t.Fatals before any query.
  • Mismatch: local OpenCost vs demo Prometheus fails with overlap 0.000 (11 vs
    263) and the full URL + sample-pod diagnostic — the exact bug the ticket
    describes.

@e-300
e-300 requested a review from a team as a code owner June 17, 2026 18:01
Copilot AI review requested due to automatic review settings June 17, 2026 18:01

Copilot AI 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.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR adds a preflight cluster-coherence guard that verifies OpenCost and Prometheus observe the same cluster before running integration tests. It compares running-pod sets from both sources using Jaccard similarity, failing fast if the overlap is below a threshold (0.50), which prevents downstream tests from producing meaningless results against mismatched targets.

Changes:

  • Added a Go test (cluster_coherence_test.go) that queries both sources for running pods over a shared historical window and compares them via Jaccard overlap.
  • Added a bats wrapper (test.bats) tagged preflight so the main test step can exclude it via --filter-tags.
  • Updated the CI workflow to run the preflight guard as a dedicated step before the main integration tests, using tag-based exclusion to avoid double-running.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
test/integration/preflight/test.bats Bats wrapper that invokes the Go preflight test with cache-busting (-count=1).
test/integration/preflight/cluster_coherence_test.go Core coherence logic: env-var validation, pod-set fetching from both sources, Jaccard comparison with churn resampling, and fresh-cluster fallback.
.github/workflows/integration-testing.yaml Adds a dedicated preflight step and excludes preflight-tagged tests from the main recursive bats run.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/workflows/integration-testing.yaml Outdated
@Manas23601

Copy link
Copy Markdown
Collaborator

The tests pass so we know the code works. I like the idea of having a fast-fail mechanism, saves us build time. My only suggestion is since you are interested in the number of common pods between prometheus and allocation, I would think about repurposing the https://github.com/opencost/opencost-integration-tests/blob/main/test/integration/query/count/allocation_running_pods_test.go this existing test. It does the exact same thing you are trying to do, calculates the number of pods per namespace (slight difference) and performs a trivial comparison. How about adding the guards and preflight steps to that check instead @ameijer ?

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.

Add environment coherence guard: verify PROMETHEUS_URL and OPENCOST_URL observe the same cluster

3 participants