Repository navigation
Report crash looping Kelvin and PEM pods as unhealthy data plane pods - #2412
Open
NopeCharon wants to merge 1 commit into
Open
NopeCharon wants to merge 1 commit into
NopeCharon wants to merge 1 commit into
Conversation
The cloud connector reports a sample of unhealthy data plane pods to the cloud, which the UI shows on the cluster details "Pixie pods" tab. Two problems kept crashing pods from ever showing up there: * A pod was only considered unhealthy if its phase was not Running. A pod whose container is in CrashLoopBackOff keeps the Running phase, so a crash looping Kelvin or PEM was treated as healthy. * The loop that adds unhealthy PEMs to the sample broke out as soon as the sample held 10 pods or fewer, so no PEM was ever added. Add isDataPlanePodHealthy(), which requires the Running phase and every container to be ready, and use it for both Kelvin and PEM pods. Fix the inverted check so that unhealthy PEMs fill the sample up to its limit of 10 pods. The number of instrumented nodes now only counts PEMs that are healthy by the same definition. To make getDataPlaneState() testable, K8sVizierInfo now stores the clientset as kubernetes.Interface. Add tests that run it against a fake clientset. Fixes pixie-io#2152 Signed-off-by: NopeCharon <115794254+NopeCharon@users.noreply.github.com>
NopeCharon
requested a deployment
to
pr-actions-approval
October 26, 2026 12:21 — with 
GitHub Actions
Waiting
GitHub Actions
Waiting
This branch is waiting to be deployed
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: Report crash looping Kelvin and PEM pods as unhealthy data plane pods
The cloud connector sends a sample of unhealthy data plane pods to the cloud, and the UI shows them under "Sample of Unhealthy Data Plane Pods" on the cluster's "Pixie pods" tab. As described in the issue, a crash looping Kelvin never shows up there, and neither does any PEM.
K8sVizierInfo.getDataPlaneState()has two problems:Running. A pod whose container is inCrashLoopBackOffkeeps theRunningphase (see the pod status in the issue), so it is treated as healthy.<=10 pods instead of>=10, so it always exits before adding any PEM.This PR adds
isDataPlanePodHealthy(), which requires theRunningphase and every container to be ready, and uses it for both Kelvin and PEM pods. It also fixes the inverted check, so unhealthy PEMs (sorted by name, as before) fill the sample up to the existing limit of 10 pods. Kelvin and PEM have no readiness probes, so a container counts as ready once it is running and has not crashed.Because the same check is used for the PEM count, the reported number of instrumented nodes no longer includes crash looping PEMs.
To test
getDataPlaneState()against a fake clientset,K8sVizierInfonow stores its clientset askubernetes.Interfaceinstead of*kubernetes.Clientset.NewK8sVizierInfo()still creates the same client.Relevant Issues: Fixes #2152
Type of change: /kind bug
Test Plan: Added
vzinfo_test.goto//src/vizier/services/cloud_connector/bridge:bridge_test. It coversisDataPlanePodHealthy()and runsgetDataPlaneState()against a fake clientset with healthy pods, a crash looping Kelvin, crash looping and pending PEMs, and more unhealthy PEMs than the sample limit. Onmain, thegetDataPlaneState()cases with crash looping Kelvin, unhealthy PEMs and the sample limit fail. With this change they pass.go test,go vetand golangci-lint v2.3.1 with the repo's.golangci.yamlare clean for the package.Changelog Message: