Skip to content

fix(verification): pin checks to the run's cluster and stop scoring a broken pass - #79

Open
geojaz wants to merge 2 commits into
kubernetes-sigs:mainfrom
geojaz:ehole/pin-checks-to-run-cluster
Open

fix(verification): pin checks to the run's cluster and stop scoring a broken pass#79
geojaz wants to merge 2 commits into
kubernetes-sigs:mainfrom
geojaz:ehole/pin-checks-to-run-cluster

Conversation

@geojaz

@geojaz geojaz commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Two related correctness problems in how a run's checks reach the cluster and how a broken run gets scored.

Checks were resolving their cluster through ambient kubeconfig context rather than the cluster the run actually provisioned. On a workstation with more than one cluster in kubeconfig, or when two runs overlap, a check could observe the wrong cluster and report a confident result about something it never touched. This threads the run's cluster identity through the context and the provider layer so every check is pinned to the cluster the run owns.

Separately, a run that failed to produce a usable result could still reach the judge and be scored as a pass. That turns an infrastructure failure into a silent correctness signal, which is the worst direction for the error to go. A broken run now stops before scoring instead.

Touches the context and provider plumbing, the verification base, and the pod_healthy, resource_property, and scaling_complete verifiers. Unit tests cover the pinning behavior and the withheld-score path.

… broken pass with the judge

Two defects that together let a run whose verification never executed report a
perfect score.

First, verifiers carried no cluster identity. BaseVerifier.kubeconfig defaulted
to None and nothing ever set it, so every check inherited the ambient
kubeconfig's current-context, which is a mutable global that any run can
rewrite. A run was observed getting an authoritative "namespace not found" for a
namespace the agent had just written to, because the check queried a different
cluster. ClusterInfo now carries the resolved context (kind-<cluster> for kind,
the existing gke_<project>_<location>_<cluster> for GCP) and the harness pins
both kubeconfig and context onto every verifier leaf, recursing through compound
nodes, before either the chaos scenario or the post-run pass runs. kubectl gains
a --context passthrough.

Second, an all-errored pass was scored by the LLM judge. rollup drops error
entries from both numerator and denominator, so when every objective errors the
correctness signal is never emitted, and the composite's preference chain fell
through to the judged OutcomeValidity. Two runs reported c=1.000 that way with
all 11 entries errored. The composite is now withheld when deterministic
verification was declared but produced no correctness, leaving outcomeScore null
and VerificationCoverage as the diagnostic. The judged fallback still applies
unchanged to tasks that declare no verification_spec, which is what it was for.

Signed-off-by: Eric Hole <ehole@onixnet.com>
@kubernetes-prow

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: geojaz
Once this PR has been reviewed and has the lgtm label, please assign janetkuo for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubernetes-prow
kubernetes-prow Bot requested a review from janetkuo August 7, 2026 13:08
@kubernetes-prow kubernetes-prow Bot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 7, 2026
@kubernetes-prow

Copy link
Copy Markdown

Hi @geojaz. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@kubernetes-prow kubernetes-prow Bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Aug 7, 2026
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@geojaz, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 57 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: d6101147-341c-43fb-afe7-a85176eeb98f

📥 Commits

Reviewing files that changed from the base of the PR and between 4670d76 and 77a23d1.

📒 Files selected for processing (13)
  • devops_bench/core/context.py
  • devops_bench/evalharness/default.py
  • devops_bench/k8s/kubectl.py
  • devops_bench/metrics/pipeline.py
  • devops_bench/providers/gcp.py
  • devops_bench/providers/kind.py
  • devops_bench/verification/base.py
  • devops_bench/verification/verifiers/pod_healthy.py
  • devops_bench/verification/verifiers/resource_property.py
  • devops_bench/verification/verifiers/scaling_complete.py
  • tests/unit/k8s/test_k8s_kubectl.py
  • tests/unit/metrics/test_metrics_pipeline.py
  • tests/unit/verification/test_scaling_complete.py

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@kubernetes-prow kubernetes-prow Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 7, 2026
@geojaz

geojaz commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

Noting a follow-up that is deliberately not in this PR, so the gap does not read as an oversight.

This pins every verifier that exists on main today. Two more, http_probe and external_http_probe, are still in flight in #55 and #54, and they are not pinned here because their source files do not exist on this branch yet.

The commit that pins them is written and tested. It needs #54, #55 and this PR all present at the same time, since it patches both verifier files and reads the context field this PR adds to BaseVerifier. No single branch has that combination today, so it is parked until they merge. I will open it as a small follow-up PR once the last of the three lands.

I did try splitting it by file onto #54 and #55 so nothing would be left waiting. That does not work: neither of those branches has the context field, so each half fails its own tests. The dependency is real rather than cosmetic, which is why it is sequenced instead of forced.

@janetkuo janetkuo added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Aug 11, 2026
The previous implementation appended context args after any "--" separator
in the argv list. For "kubectl run" commands, everything after "--" is
interpreted as the container's command, so "--context <name>" was received
by the probe container rather than kubectl, causing every ephemeral http_probe
pod to exit 2 with an "unknown flag" error.

The fix detects a "--" separator and inserts context args immediately before
it, so kubectl sees --context while the container command is unaffected. When
no separator is present the args are appended as before. The blast radius was
any kubectl.run() call that used the context option together with a container
command argument list.

The append is latent here and becomes a live bug when combined with run_pod-style argv that contains a "--" separator.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants