Add check for CNI plugins before tearing down pod network by sameersaeed · Pull Request #10744 · containerd/containerd

sameersaeed · 2024-09-27T20:13:34Z

Currently, locking behavior occurs if a user creates a sandbox pod and then tries to remove / delete it without having any CNI plugins initialized on their system.

Sample flow

pod-config.json:, per crictl docs

{
  "metadata": {
    "name": "nginx-sandbox",
    "namespace": "default",
    "attempt": 1,
    "uid": "65fb5845de97d8a4"
  },
  "log_directory": "/tmp",
  "linux": {
  }
}

Running sandbox pod - receive CNI plugin error:

root@3cd5ffda91af:~# crictl runp pod-config.json 
WARN[0000] runtime connect using default endpoints: [unix:///run/containerd/containerd.sock unix:///run/crio/crio.sock unix:///var/run/cri-dockerd.sock]. As the default settings are now deprecated, you should set the endpoint instead. 
E0922 01:07:22.941363     315 log.go:32] "RunPodSandbox from runtime service failed" err="rpc error: code = Unknown desc = failed to setup network for sandbox \"c4c57d078fc28ae83f4fd22167a23aa62e5ca77a09e9e1e62de72bc0135031ef\": cni plugin not initialized"
FATA[0003] run pod sandbox: rpc error: code = Unknown desc = failed to setup network for sandbox "c4c57d078fc28ae83f4fd22167a23aa62e5ca77a09e9e1e62de72bc0135031ef": cni plugin not initialized

Pod shows up as NotReady:

root@3cd5ffda91af:~# crictl pods
WARN[0000] runtime connect using default endpoints: [unix:///run/containerd/containerd.sock unix:///run/crio/crio.sock unix:///var/run/cri-dockerd.sock]. As the default settings are now deprecated, you should set the endpoint instead. 
POD ID              CREATED             STATE               NAME                NAMESPACE           ATTEMPT             RUNTIME
c4c57d078fc28       270 years ago       NotReady            nginx-sandbox       default             1                   (default)

Locking behavior - cannot stop / remove pod due to CNI plugin error:

root@3cd5ffda91af:~# crictl stopp c4
WARN[0000] runtime connect using default endpoints: [unix:///run/containerd/containerd.sock unix:///run/crio/crio.sock unix:///var/run/cri-dockerd.sock]. As the default settings are now deprecated, you should set the endpoint instead. 
E0922 01:07:43.024652     335 log.go:32] "StopPodSandbox from runtime service failed" err="rpc error: code = Unknown desc = failed to destroy network for sandbox \"c4c57d078fc28ae83f4fd22167a23aa62e5ca77a09e9e1e62de72bc0135031ef\": cni plugin not initialized" podSandboxID="c4"
FATA[0000] stopping the pod sandbox "c4": rpc error: code = Unknown desc = failed to destroy network for sandbox "c4c57d078fc28ae83f4fd22167a23aa62e5ca77a09e9e1e62de72bc0135031ef": cni plugin not initialized

root@3cd5ffda91af:~# crictl rmp c4
WARN[0000] runtime connect using default endpoints: [unix:///run/containerd/containerd.sock unix:///run/crio/crio.sock unix:///var/run/cri-dockerd.sock]. As the default settings are now deprecated, you should set the endpoint instead. 
E0922 01:07:48.578086     345 log.go:32] "RemovePodSandbox from runtime service failed" err="rpc error: code = Unknown desc = failed to forcibly stop sandbox \"c4c57d078fc28ae83f4fd22167a23aa62e5ca77a09e9e1e62de72bc0135031ef\": failed to destroy network for sandbox \"c4c57d078fc28ae83f4fd22167a23aa62e5ca77a09e9e1e62de72bc0135031ef\": cni plugin not initialized" podSandboxID="c4"
removing the pod sandbox "c4": rpc error: code = Unknown desc = failed to forcibly stop sandbox "c4c57d078fc28ae83f4fd22167a23aa62e5ca77a09e9e1e62de72bc0135031ef": failed to destroy network for sandbox "c4c57d078fc28ae83f4fd22167a23aa62e5ca77a09e9e1e62de72bc0135031ef": cni plugin not initialized

Changes made

Current code:

containerd/internal/cri/server/sandbox_stop.go

Lines 114 to 116 in db97449

    
           if err := c.teardownPodNetwork(ctx, sandbox); err != nil { 
        
           	return fmt.Errorf("failed to destroy network for sandbox %q: %w", id, err) 
        
           }

Added a minor change to the above sandbox_stop.go code so that the pod network teardown will only be performed if CNIResult returns a valid value:

if sandbox.CNIResult != nil {
	if err := c.teardownPodNetwork(ctx, sandbox); err != nil {
		return fmt.Errorf("failed to destroy network for sandbox %q: %w", id, err)
	}	
}

This avoids the locking behavior, so the pods are able to be stopped and / or removed as expected:

root@03195a9aecb5:/src# crictl rmp c4
DEBU[0000] get runtime connection                       
Stopped sandbox c4

root@03195a9aecb5:/src# crictl rmp c4
DEBU[0000] get runtime connection                       
Removed sandbox c4

root@03195a9aecb5:/src# crictl pods
DEBU[0000] get runtime connection                       
DEBU[0000] ListPodSandboxRequest: &ListPodSandboxRequest{Filter:&PodSandboxFilter{Id:,State:nil,LabelSelector:map[string]string{},},} 
DEBU[0000] ListPodSandboxResponse: []                   
POD ID              CREATED             STATE               NAME                NAMESPACE           ATTEMPT             RUNTIME
``

Signed-off-by: Sameer <sameer.saeed@live.ca>

k8s-ci-robot · 2024-09-27T20:13:44Z

Hi @sameersaeed. Thanks for your PR.

I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

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.

mxpv · 2024-09-27T21:40:25Z

/ok-to-test

mikebrow

LGTM

mikebrow · 2024-09-30T12:50:44Z

/test pull-containerd-k8s-e2e-ec2

mikebrow · 2024-09-30T13:22:22Z

Additionally, the following needs to be fixed in a follow up PR as the defer'd tear down will also fail and "block" the netns removal https://github.com/containerd/containerd/blob/release/1.6/pkg/cri/server/sandbox_run.go#L312. @fuweid FYI.. probably should not be deferring tear down before the network bring up, or if we do should ignore teardown failure when setup fails. Same for 1.6/7

MrHohn · 2025-07-21T18:15:17Z

Hi folks, posting here for more visibility - I just filed a bug on #12130 that is closely relevant to this change. Would appreciate your inputs there, thanks!

This is a follow up of both containerd#10744 and containerd#10839, where teardownPodNetwork may be skipped if setup networks failed in the first place. In the situation illusrated on containerd#12130, for chained CNI pattern the skipping logic may be problematic and leads to network resource leakage (e.g. Pod IP). This PR attempts to make the sandbox network handling more robust by conditionally ignoring the teardown network when error is related to CNI initialization, while allowing teardown retry to happen when it makes sense.

This is a follow up of both containerd#10744 and containerd#10839, where teardownPodNetwork may be skipped if setup networks failed in the first place. In the situation illustrated on containerd#12130, for chained CNI pattern the skipping logic may be problematic and can lead to network resource leakage (e.g. Pod IP). This PR attempts to make the sandbox network handling more robust by conditionally ignoring the teardown network when error is related to CNI initialization, while allowing teardown retry to happen when it makes sense.

This is a follow up of both containerd#10744 and containerd#10839, where teardownPodNetwork may be skipped if setup networks failed in the first place. In the situation illustrated on containerd#12130, for chained CNI pattern the skipping logic may be problematic and can lead to network resource leakage (e.g. Pod IP). This PR attempts to make the sandbox network handling more robust by conditionally ignoring the teardown network when error is related to CNI initialization, while allowing teardown retry to happen when it makes sense. Signed-off-by: Zihong Zheng <zihongz@google.com>

This is a follow up of both containerd#10744 and containerd#10839, where teardownPodNetwork may be skipped if setup networks failed in the first place. In the situation illustrated on containerd#12130, for chained CNI pattern the skipping logic may be problematic and can lead to network resource leakage (e.g. Pod IP). This PR attempts to make the sandbox network handling more robust by conditionally skipping the teardown network only when error is related to CNI initialization, while allowing teardown retry to happen if setup already allocated network resoruce partially. Signed-off-by: Zihong Zheng <zihongz@google.com>

Add check for CNI plugins before tearing down pod network

b7b6b32

Signed-off-by: Sameer <sameer.saeed@live.ca>

k8s-ci-robot added size/XS needs-ok-to-test labels Sep 27, 2024

dosubot bot added area/cri Container Runtime Interface (CRI) kind/bug labels Sep 27, 2024

k8s-ci-robot added ok-to-test and removed needs-ok-to-test labels Sep 27, 2024

mxpv approved these changes Sep 29, 2024

View reviewed changes

mikebrow approved these changes Sep 30, 2024

View reviewed changes

mikebrow added the cherry-pick/1.7.x Change to be cherry picked to release/1.7 branch label Sep 30, 2024

mikebrow added the cherry-pick/1.6.x label Sep 30, 2024

mikebrow mentioned this pull request Sep 30, 2024

Unable to remove sandbox when cni plugin not initialized #8774

Closed

mikebrow added this pull request to the merge queue Sep 30, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 30, 2024

mxpv added this pull request to the merge queue Sep 30, 2024

Merged via the queue into containerd:main with commit 03db11c Sep 30, 2024

This was referenced Oct 3, 2024

[release/1.6] Add check for CNI plugins before tearing down pod network #10766

Merged

[release/1.7] Add check for CNI plugins before tearing down pod network #10767

Merged

Adding regression tests for sandbox pods when no CNI plugins are initialized #10768

Open

nabeelmohamed mentioned this pull request Oct 4, 2024

Add regression tests for sandbox pods when no CNI plugins are initialized #10771

Open

mikebrow mentioned this pull request Oct 4, 2024

[release/1.7] Handle teardown failure to avoid blocking cleanup #10770

Open

austinvazquez added cherry-picked/1.6.x PR commits are cherry-picked into release/1.6 branch cherry-picked/1.7.x PR commits are cherry-picked into release/1.7 branch and removed cherry-pick/1.6.x cherry-pick/1.7.x Change to be cherry picked to release/1.7 branch labels Nov 20, 2024

mikebrow mentioned this pull request Mar 20, 2025

cri:fix containerd panic when can't find sandbox extension #11576

Merged

MrHohn mentioned this pull request Jul 21, 2025

IP Address Leak with Chained CNI Plugins When Teardown Fails #12130

Open

MrHohn mentioned this pull request Jul 21, 2025

Handle pod network teardown more carefully when setup failed. #12132

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add check for CNI plugins before tearing down pod network#10744

Add check for CNI plugins before tearing down pod network#10744
mxpv merged 1 commit intocontainerd:mainfrom
sameersaeed:sandbox-cni-plugins

sameersaeed commented Sep 27, 2024

Uh oh!

k8s-ci-robot commented Sep 27, 2024

Uh oh!

mxpv commented Sep 27, 2024

Uh oh!

mikebrow left a comment

Uh oh!

mikebrow commented Sep 30, 2024

Uh oh!

mikebrow commented Sep 30, 2024 •

edited

Loading

Uh oh!

Uh oh!

MrHohn commented Jul 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

	if err := c.teardownPodNetwork(ctx, sandbox); err != nil {
	return fmt.Errorf("failed to destroy network for sandbox %q: %w", id, err)
	}

Conversation

sameersaeed commented Sep 27, 2024

Sample flow

Changes made

Uh oh!

k8s-ci-robot commented Sep 27, 2024

Uh oh!

mxpv commented Sep 27, 2024

Uh oh!

mikebrow left a comment

Choose a reason for hiding this comment

Uh oh!

mikebrow commented Sep 30, 2024

Uh oh!

mikebrow commented Sep 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

MrHohn commented Jul 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mikebrow commented Sep 30, 2024 •

edited

Loading