test(e2e): TEMPORARY probe for worker CPU usage on node-join failure - #3835
Closed
myasnikovdaniil wants to merge 2 commits into
Closed
test(e2e): TEMPORARY probe for worker CPU usage on node-join failure#3835myasnikovdaniil wants to merge 2 commits into
myasnikovdaniil wants to merge 2 commits into
Conversation
The CFS counters this capture already collects cannot separate the two live explanations of a tenant worker missing the node-Ready budget (#3513). Throttling fires only AT the ceiling, so 853-1151 throttled periods out of ~9000 at quota/period 1.0 says the guest reached its limit sometimes and says nothing about what it got the rest of the time -- and a container held far below its ceiling by competition for host CPU is throttled barely at all. Add container_cpu_usage_seconds_total and container_start_time_seconds to the metric set the capture requests. Everything else is untouched: same endpoint, same auth, same bounds, same three-stage un-piped filter, same truncation handling, same report directory. Both families arrive in the scrape the other five already come in, so the collector's ceiling is the same four bounded reads it was -- one Pod listing plus up to three node reads -- and the diagnostics phase budget sees no change. A second scrape a bounded interval apart was the alternative and is declined on cost and on merit: three more node reads plus the interval, and the capture runs only after the node-Ready deadline has already failed, so the container's whole lifetime is the window and a mean over its uptime is a mean over the stall. The raw total is cumulative and not directly readable, so the tail note divides it by the container's uptime and prints the quotient against the ceiling on the same line. Uptime comes from the sample time each cAdvisor row carries, with this runner's clock as a named fallback. The reading is printed one line per container. The filter narrows by namespace and by the virt-launcher Pod-name prefix and never by container, so the guest-console-log sidecar at 15m lands in the same file as `compute` -- which is how these files were read wrong before. No container predicate is added instead: the filter belongs to the body this capture shares with the network counters, whose rows all carry container="" and would be emptied by one, and narrowing would drop the throttling counters of the other containers this capture returns today and that its tests pin. Refs: #3513 Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
MUST NEVER MERGE. Delete this commit; the one before it is the change worth keeping. Cut the tenant node-Ready budget from 18m to 6m so the kubernetes-* suites fail on purpose and the node-join diagnostics run on every build. Those diagnostics fire only when node-Ready fails, so without this the CPU-usage capture added on the previous commit produces data only when a run happens to hit the flake -- roughly one red run in three, at about three hours a run. A plain hardcoded literal, not a knob or a conditional, so it is obvious in review and a one-line delete to remove. The change site carries a comment block saying the same, including that every node-join failure this branch produces is manufactured by this line and says nothing about node-join itself. Checked rather than assumed: the phase-budget inequality in hack/run-kubernetes-node-join_test.bats is unaffected -- its terms are the 420s phase budget, the 680s largest-collector literal and the crust-gather bound read from source, none of which this touches -- and it still passes (420 + 680 + 390 = 1490 <= 1500). Shortening the wait only leaves more of the 50m op for the phase and the snapshot. No test pins the 18m literal or the failure headline; both appear in prose comments only, and the headline wording is left untouched so this commit stays deletable on its own. Refs: #3513 Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Contributor
Author
|
Did its job, closing. The capture it forced is on run 31872871382 and the reading is in #3513: all four workers sit at 0.926 to 0.955 CPU-seconds per second against a one core ceiling, so the worker is ceiling-bound rather than starved of contended CPU. Nothing here was ever meant to merge. The node-Ready budget cut to 6m was deliberate, to make the node-join capture fire on every run instead of waiting for the flake, and the measurement half of this branch is superseded by #3836, which landed a better two-scrape version on main. |
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
1 participant
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.
Do not merge
This is a throwaway diagnostic PR for #3513. It deliberately makes the tenant Kubernetes suites fail. It exists to produce one artifact and will be closed once it has.
What it measures and why
#3513 has two live hypotheses for why a tenant worker misses the node-Ready budget while its kubelet image trickles in, and the instrument we have cannot tell them apart.
The first is that the guest is ceiling-bound. The virt-launcher
computecontainer runs atlimit 1and the guest has one vCPU, so it cannot go faster no matter what else is idle.The second is that the guest is share-starved. That container carries
request 100m, which is the guest vCPU count divided by the platformcpuAllocationRatioof 10, and the request is what becomescpu.weight. Under contention the kernel divides time by that weight, so the guest can be held near a tenth of a core while its ceiling sits a full core above.The CFS throttle counters already in the artifact cannot decide between them. On nightly 31662393360 they showed 853 to 1151 throttled periods out of about 9000 at a
quota/periodof 1.0, which says the guest reached its ceiling sometimes and says nothing at all about how much CPU it got the rest of the time. Throttling fires only at the ceiling, and a container held far below its ceiling by competition barely throttles.container_cpu_usage_seconds_totalon that container is the series that settles it. Read against wall clock across the capture: usage tracking near 1.0 CPU-seconds per second means ceiling-bound and the CPU request is irrelevant, and usage sitting well below 1.0 while the guest is visibly stalled means share-starved and the request is the lever.Why it forces a failure
The node-join diagnostics only run when node-Ready fails, so a PR that merely adds the series produces nothing unless it happens to catch the flake, which appears in roughly a third of red runs at about three hours each. The second commit therefore cuts the node-Ready budget from 18m to 6m so the capture fires on every run.
That also buys something on a healthy worker rather than only on a sick one. If a worker that would have joined fine shows usage near 1.0 throughout its image pull, the workload is ceiling-bound by nature and the request hypothesis is dead without ever needing to catch the flake.
The two changes are separate commits on purpose. The measurement is worth keeping, the budget cut is not.
Release note