fix(verification): pin checks to the run's cluster and stop scoring a broken pass - #79
fix(verification): pin checks to the run's cluster and stop scoring a broken pass#79geojaz wants to merge 2 commits into
Conversation
… 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>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: geojaz The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
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 Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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. |
|
Warning Review limit reached
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 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (13)
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. Comment |
|
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, 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 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 |
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.
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, andscaling_completeverifiers. Unit tests cover the pinning behavior and the withheld-score path.