[cri] podSandboxStats should return errdefs.ErrUnavailable without wrapping by dims · Pull Request #9897 · containerd/containerd

dims · 2024-02-28T22:02:41Z

kubelet is calling CRI's ListPodSandboxStats, when this method is looping through sandbox(es), it calls podSandboxStats and expects to get info about sandboxes that are StateReady.

ListPodSandboxStats expects errdefs.ErrUnavailable when sandbox is NOT StateReady. But in the current implementation, the ErrUnavailable is wrapped into another one using fmt.Errorf so the ListPodSandboxStats fails fast without skipping the sandboxes that are not ready.

from: https://prow.k8s.io/view/gs/kubernetes-jenkins/logs/ci-kubernetes-e2e-gci-gce-alpha-enabled-default/1762909002125021184
test: Kubernetes e2e suite: [It] [sig-node] NodeProblemDetector [NodeFeature:NodeProblemDetector] should run without error
log snippet:

STEP: Gather node-problem-detector cpu and memory stats - k8s.io/kubernetes/test/e2e/node/node_problem_detector.go:192 @ 02/28/24 17:19:35.031
I0228 17:19:36.098816 10590 node_problem_detector.go:380] Unexpected error: 
    <*errors.StatusError | 0xc004515040>: 
    an error on the server ("Internal Error: failed to list pod stats: rpc error: code = NotFound desc = failed to decode sandbox container metrics for sandbox \"2c95299f99fa7b8a03e66be60d2a4ddd51bf2501d08d2bcf7f6ba8203f57c6de\": no running task found: task bc6c3f30a0a61641cbe56bdbfb867e195b463f92609505972ee2880dbe4826d8 not found: not found") has prevented the request from succeeding (get nodes bootstrap-e2e-minion-group-1526:10250)
    {
        ErrStatus: 
            code: 500
            details:
              causes:
              - message: 'Internal Error: failed to list pod stats: rpc error: code = NotFound
                  desc = failed to decode sandbox container metrics for sandbox "2c95299f99fa7b8a03e66be60d2a4ddd51bf2501d08d2bcf7f6ba8203f57c6de":
                  no running task found: task bc6c3f30a0a61641cbe56bdbfb867e195b463f92609505972ee2880dbe4826d8
                  not found: not found'
                reason: UnexpectedServerResponse
              kind: nodes
              name: bootstrap-e2e-minion-group-1526:10250
            message: 'an error on the server ("Internal Error: failed to list pod stats: rpc error:
              code = NotFound desc = failed to decode sandbox container metrics for sandbox \"2c95299f99fa7b8a03e66be60d2a4ddd51bf2501d08d2bcf7f6ba8203f57c6de\":
              no running task found: task bc6c3f30a0a61641cbe56bdbfb867e195b463f92609505972ee2880dbe4826d8
              not found: not found") has prevented the request from succeeding (get nodes bootstrap-e2e-minion-group-1526:10250)'
            metadata: {}
            reason: InternalError
            status: Failure,
    }
[FAILED] an error on the server ("Internal Error: failed to list pod stats: rpc error: code = NotFound desc = failed to decode sandbox container metrics for sandbox \"2c95299f99fa7b8a03e66be60d2a4ddd51bf2501d08d2bcf7f6ba8203f57c6de\": no running task found: task bc6c3f30a0a61641cbe56bdbfb867e195b463f92609505972ee2880dbe4826d8 not found: not found") has prevented the request from succeeding (get nodes bootstrap-e2e-minion-group-1526:10250)

…apping Signed-off-by: Davanum Srinivas <[email protected]>

dmcgowan · 2024-02-28T22:30:07Z

The bug here seems to be on the check side rather than generation side. The wrapping is done correctly, somewhere isn't unwrapping though like it should.

dims · 2024-02-28T23:27:01Z

/retest

dims · 2024-02-29T01:49:27Z

/retest

dims · 2024-02-29T02:24:46Z

/retest

dims · 2024-02-29T03:25:29Z

/retest

fuweid · 2024-02-29T12:43:59Z


 	if sandbox.Status.Get().State != sandboxstore.StateReady {
-		return nil, fmt.Errorf("failed to get pod sandbox stats since sandbox container %q is not in ready state: %w", meta.ID, errdefs.ErrUnavailable)
+		log.G(ctx).WithField("podsandboxid", sandbox.ID).Debugf("failed to get pod sandbox stats since sandbox container %q is not in ready state", meta.ID)


is it client side issue? I mean that we use fmt.Errorf(%w) to mark root cause as ErrUnavailable. The client side should unwrap error.

@fuweid not a client side issue. please see where this is called from within containerd. (wrote a long PR text to describe why this is needed)

Sorry, I didn't follow the description.

containerd/internal/cri/server/sandbox_stats_list.go

Lines 39 to 49 in 9a2b855

for _, sandbox := range sandboxes {

sandboxStats, err := c.podSandboxStats(ctx, sandbox)

switch {

case errdefs.IsUnavailable(err):

log.G(ctx).WithField("podsandboxid", sandbox.ID).Debugf("failed to get pod sandbox stats, this is likely a transient error: %v", err)

case err != nil:

errs = append(errs, fmt.Errorf("failed to decode sandbox container metrics for sandbox %q: %w", sandbox.ID, err))

default:

podSandboxStats.Stats = append(podSandboxStats.Stats, sandboxStats)

}

}

Checked the main branch and we did expect err to unavailable.

And the error looks weird because it doesn't have unavailable information. Do you know how to check what containerd version used in CI?

: failed to get pod sandbox stats since sandbox container \"c821a1cfa253eca57db68a1ae85184359d1422c7595982e32f47addeb8d8a78e\" is not in ready state"

f914edf

containerd://1.7.13

@fuweid yes originally i saw it in 1.7.13, but inspecting the code in main i see the issue can be here as well. right?

I'd like to fix it first here and then cherry pick back to 1.7 branch

I don't think so. main doesn't have the issue you mentioned. please provide the log. thanks

For the ErrUnabailable error, we did use errors.Is to check. That's why the issue has been fixed.

Based on your description, the error is like

no running task found

I think we should skip task not found error instead of unavailable.

fuweid

I think it's v1.7 bug instead of main

#8671

dims · 2024-03-01T03:14:07Z

ok i had forgotten the unwrap/compare!! thanks

https://go.dev/play/p/ThFh6qN5ait?v=gotip

true
true

[cri] podSandboxStats should return errdefs.ErrUnavailable without wr…

0c092c9

…apping Signed-off-by: Davanum Srinivas <[email protected]>

k8s-ci-robot added the size/XS label Feb 28, 2024

dims assigned dmcgowan and mikebrow Feb 28, 2024

fuweid reviewed Feb 29, 2024

View reviewed changes

AkihiroSuda approved these changes Feb 29, 2024

View reviewed changes

dims mentioned this pull request Feb 29, 2024

Add a optional presubmit to test k8s master and containerd master together kubernetes/test-infra#32110

Merged

fuweid requested changes Mar 1, 2024

View reviewed changes

dims closed this Mar 1, 2024

fuweid mentioned this pull request Mar 31, 2024

Add IsNotFound case to ListPodSandboxStats #10023

Merged

	for _, sandbox := range sandboxes {
	sandboxStats, err := c.podSandboxStats(ctx, sandbox)
	switch {
	case errdefs.IsUnavailable(err):
	log.G(ctx).WithField("podsandboxid", sandbox.ID).Debugf("failed to get pod sandbox stats, this is likely a transient error: %v", err)
	case err != nil:
	errs = append(errs, fmt.Errorf("failed to decode sandbox container metrics for sandbox %q: %w", sandbox.ID, err))
	default:
	podSandboxStats.Stats = append(podSandboxStats.Stats, sandboxStats)
	}
	}

Conversation

dims commented Feb 28, 2024

Uh oh!

dmcgowan commented Feb 28, 2024

Uh oh!

dims commented Feb 28, 2024

Uh oh!

dims commented Feb 29, 2024

Uh oh!

dims commented Feb 29, 2024

Uh oh!

dims commented Feb 29, 2024

Uh oh!

fuweid Feb 29, 2024

Choose a reason for hiding this comment

Uh oh!

dims Feb 29, 2024 • 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!

fuweid Mar 1, 2024

Choose a reason for hiding this comment

Uh oh!

fuweid Mar 1, 2024

Choose a reason for hiding this comment

Uh oh!

dims Mar 1, 2024

Choose a reason for hiding this comment

Uh oh!

dims Mar 1, 2024

Choose a reason for hiding this comment

Uh oh!

fuweid Mar 1, 2024

Choose a reason for hiding this comment

Uh oh!

fuweid Mar 1, 2024

Choose a reason for hiding this comment

Uh oh!

fuweid left a comment

Choose a reason for hiding this comment

Uh oh!

dims commented Mar 1, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

dims Feb 29, 2024 •

edited

Loading