kubelet: enhance exec probe logging with pod and container context #132744

xigang · 2025-07-05T14:23:27Z

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

This improves debugging capabilities by providing more context in exec probe error messages, making it easier to identify which specific pod and container encountered issues during probe execution.

Which issue(s) this PR is related to:

#132714

Special notes for your reviewer:

Does this PR introduce a user-facing change?

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-ci-robot · 2025-07-05T14:23:36Z

This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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.

xigang · 2025-07-05T14:24:09Z

/ok-to-test

xigang · 2025-07-05T14:31:26Z

/cc @bart0sh @SergeyKanzhelev @dims
Please take a look. Thanks!

xigang · 2025-07-07T11:18:13Z

/cc @pacoxu
PTAL. Thanks!

pacoxu · 2025-07-08T03:48:27Z

/cc @tzneal @NoicFank @bart0sh
/assign @tallclair

xigang · 2025-07-09T15:01:11Z

@bart0sh Would you have some time to review this PR? Thank you!

pkg/kubelet/prober/prober.go

bart0sh · 2025-07-10T11:41:40Z

pkg/kubelet/prober/prober.go

@@ -152,7 +152,7 @@ func (pb *prober) runProbe(ctx context.Context, probeType probeType, p *v1.Probe
 	case p.Exec != nil:
 		klog.V(4).InfoS("Exec-Probe runProbe", "pod", klog.KObj(pod), "containerName", container.Name, "execCommand", p.Exec.Command)
 		command := kubecontainer.ExpandContainerCommandOnlyStatic(p.Exec.Command, container.Env)
-		return pb.exec.Probe(pb.newExecInContainer(ctx, container, containerID, command, timeout))
+		return pb.exec.Probe(pb.newExecInContainer(ctx, pod, container, containerID, command, timeout))


Why is this only implemented for exec probes?

For other Probes, when they fail, they return an error and the upper-level caller logs it with the podName and containerName. However, when an exec Probe succeeds, it writes the command output to the data stream. If eic.writer.Write(data) returns an error, the resulting log doesn't include the podName or containerName.

kubernetes/pkg/kubelet/prober/prober.go

Lines 267 to 276 in 6ecec84

func (eic *execInContainer) Start() error {

data, err := eic.run()

if eic.writer != nil {

// only record the write error, do not cover the command run error

if p, err := eic.writer.Write(data); err != nil {

klog.ErrorS(err, "Unable to write all bytes from execInContainer", "expectedBytes", len(data), "actualBytes", p)

}

}

return err

}

Errors from other Probes are consistently logged at this point.

kubernetes/pkg/kubelet/prober/prober.go

Lines 100 to 107 in 6ecec84

result, output, err := pb.runProbeWithRetries(ctx, probeType, probeSpec, pod, status, container, containerID, maxProbeRetries)

if err != nil {

// Handle probe error

klog.V(1).ErrorS(err, "Probe errored", "probeType", probeType, "pod", klog.KObj(pod), "podUID", pod.UID, "containerName", container.Name, "probeResult", result)

pb.recordContainerEvent(pod, &container, v1.EventTypeWarning, events.ContainerUnhealthy, "%s probe errored and resulted in %s state: %s", probeType, result, err)

return results.Failure, err

}

pkg/kubelet/prober/prober.go

aojea · 2025-07-10T11:52:21Z

pkg/kubelet/prober/prober.go

@@ -253,7 +259,7 @@ func (eic *execInContainer) Start() error {
 	if eic.writer != nil {
 		// only record the write error, do not cover the command run error
 		if p, err := eic.writer.Write(data); err != nil {
-			klog.ErrorS(err, "Unable to write all bytes from execInContainer", "expectedBytes", len(data), "actualBytes", p)
+			klog.ErrorS(err, "Unable to write all bytes from execInContainer", "expectedBytes", len(data), "actualBytes", p, "pod", klog.KObj(eic.pod), "containerName", eic.containerName)


you don't have to cascade the pod object here, just get the error in https://github.com/kubernetes/kubernetes/pull/132744/files#r2197504163 and log out there, don't create this dependency

Good suggestion. 👍🏻

@aojea but write error is not returned from the execInContainer::Start(). The probe can be successful with unsuccessful write.

kubernetes/pkg/kubelet/prober/prober.go

Lines 267 to 276 in 6ecec84

func (eic *execInContainer) Start() error {

data, err := eic.run()

if eic.writer != nil {

// only record the write error, do not cover the command run error

if p, err := eic.writer.Write(data); err != nil {

klog.ErrorS(err, "Unable to write all bytes from execInContainer", "expectedBytes", len(data), "actualBytes", p)

}

}

return err

}

you don't have to cascade the pod object here, just get the error in https://github.com/kubernetes/kubernetes/pull/132744/files#r2197504163 and log out there, don't create this dependency

When an exec Probe succeeds but eic.writer.Write(data) fails, the error isn't propagated to the upper-level function runProbeWithRetries, so no error log is printed there. That's why this log should include the podName and containerName.

kubernetes/pkg/kubelet/prober/prober.go

Lines 100 to 107 in 6ecec84

result, output, err := pb.runProbeWithRetries(ctx, probeType, probeSpec, pod, status, container, containerID, maxProbeRetries)

if err != nil {

// Handle probe error

klog.V(1).ErrorS(err, "Probe errored", "probeType", probeType, "pod", klog.KObj(pod), "podUID", pod.UID, "containerName", container.Name, "probeResult", result)

pb.recordContainerEvent(pod, &container, v1.EventTypeWarning, events.ContainerUnhealthy, "%s probe errored and resulted in %s state: %s", probeType, result, err)

return results.Failure, err

}

aojea · 2025-07-10T11:53:19Z

you don't have to modify the existing struct and methods, and check the returned value on the probe execution to log

k8s-ci-robot · 2025-07-10T12:55:56Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

Needs approval from an approver in each of these files:

pkg/kubelet/OWNERS

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

xigang · 2025-07-10T23:49:36Z

@aojea @bart0sh PTAL

zerkms · 2025-07-11T00:11:48Z

Just in case my message in threads does not get enough attention:

you don't have to modify the existing struct and methods, and check the returned value on the probe execution to log

@aojea

kubernetes/pkg/kubelet/prober/prober.go

Lines 267 to 276 in 6ecec84

    
           func (eic *execInContainer) Start() error { 
        
           	data, err := eic.run() 
        
           	if eic.writer != nil { 
        
           		// only record the write error, do not cover the command run error 
        
           		if p, err := eic.writer.Write(data); err != nil { 
        
           			klog.ErrorS(err, "Unable to write all bytes from execInContainer", "expectedBytes", len(data), "actualBytes", p) 
        
           		} 
        
           	} 
        
           	return err 
        
           }

Given that Write error IS NOT returned from the prober - I believe you cannot simply check the returned value on the probe execution: probe can be successful while writer has failed (due to write limiter).

Eg:

cat /dev/random | head -c 15000

☝️ this would be a successful probe with failing writer.

Signed-off-by: xigang <wangxigang2014@gmail.com>

k8s-ci-robot added needs-priority Indicates a PR lacks a `priority/foo` label and requires one. area/kubelet sig/node Categorizes an issue or PR as relevant to SIG Node. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jul 5, 2025

github-project-automation bot moved this to Triage in SIG Node: code and documentation PRs Jul 5, 2025

github-project-automation bot added this to SIG Node: code and documentation PRs Jul 5, 2025

k8s-ci-robot requested review from dims and odinuge July 5, 2025 14:23

k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Jul 5, 2025

k8s-ci-robot requested review from bart0sh and SergeyKanzhelev July 5, 2025 14:31

xigang force-pushed the probe_exec branch from 963c35c to c5a7bf3 Compare July 5, 2025 14:34

xigang mentioned this pull request Jul 5, 2025

Prober limited writer should log what pod/container it comes from. #132714

Open

xigang force-pushed the probe_exec branch from c5a7bf3 to 80d80be Compare July 5, 2025 15:08

k8s-ci-robot requested a review from pacoxu July 7, 2025 11:18

k8s-ci-robot assigned tallclair Jul 8, 2025

k8s-ci-robot requested review from NoicFank and tzneal July 8, 2025 03:48

bart0sh reviewed Jul 10, 2025

View reviewed changes

pkg/kubelet/prober/prober.go Outdated Show resolved Hide resolved

bart0sh reviewed Jul 10, 2025

View reviewed changes

aojea reviewed Jul 10, 2025

View reviewed changes

pkg/kubelet/prober/prober.go Show resolved Hide resolved

aojea reviewed Jul 10, 2025

View reviewed changes

xigang force-pushed the probe_exec branch from 80d80be to e58fa5b Compare July 10, 2025 12:55

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jul 10, 2025

xigang force-pushed the probe_exec branch from e58fa5b to 09ca487 Compare July 10, 2025 12:57

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 10, 2025

xigang force-pushed the probe_exec branch 2 times, most recently from 8d737c1 to 6ecec84 Compare July 10, 2025 14:53

xigang force-pushed the probe_exec branch 2 times, most recently from ddc00a7 to 8a46065 Compare July 11, 2025 02:26

enhance exec probe logging with pod and container context

4f0d3ea

Signed-off-by: xigang <wangxigang2014@gmail.com>

xigang force-pushed the probe_exec branch from 8a46065 to 4f0d3ea Compare July 11, 2025 10:59

	func (eic *execInContainer) Start() error {
	data, err := eic.run()
	if eic.writer != nil {
	// only record the write error, do not cover the command run error
	if p, err := eic.writer.Write(data); err != nil {
	klog.ErrorS(err, "Unable to write all bytes from execInContainer", "expectedBytes", len(data), "actualBytes", p)
	}
	}
	return err
	}

	result, output, err := pb.runProbeWithRetries(ctx, probeType, probeSpec, pod, status, container, containerID, maxProbeRetries)

	if err != nil {
	// Handle probe error
	klog.V(1).ErrorS(err, "Probe errored", "probeType", probeType, "pod", klog.KObj(pod), "podUID", pod.UID, "containerName", container.Name, "probeResult", result)
	pb.recordContainerEvent(pod, &container, v1.EventTypeWarning, events.ContainerUnhealthy, "%s probe errored and resulted in %s state: %s", probeType, result, err)
	return results.Failure, err
	}

kubelet: enhance exec probe logging with pod and container context #132744

Are you sure you want to change the base?

kubelet: enhance exec probe logging with pod and container context #132744

Conversation

xigang commented Jul 5, 2025

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR is related to:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

Uh oh!

k8s-ci-robot commented Jul 5, 2025

Uh oh!

xigang commented Jul 5, 2025

Uh oh!

xigang commented Jul 5, 2025

Uh oh!

xigang commented Jul 7, 2025

Uh oh!

pacoxu commented Jul 8, 2025

Uh oh!

xigang commented Jul 9, 2025

Uh oh!

Uh oh!

bart0sh Jul 10, 2025

Choose a reason for hiding this comment

Uh oh!

xigang Jul 11, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aojea Jul 10, 2025

Choose a reason for hiding this comment

Uh oh!

xigang Jul 10, 2025

Choose a reason for hiding this comment

Uh oh!

zerkms Jul 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xigang Jul 11, 2025

Choose a reason for hiding this comment

Uh oh!

aojea commented Jul 10, 2025

Uh oh!

k8s-ci-robot commented Jul 10, 2025

Uh oh!

xigang commented Jul 10, 2025

Uh oh!

zerkms commented Jul 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

zerkms Jul 11, 2025 •

edited

Loading

zerkms commented Jul 11, 2025 •

edited

Loading