| From f16447e2d495d77c03c6644e78877cd3596ef523 Mon Sep 17 00:00:00 2001 |
| From: Danny Canter <danny@dcantah.dev> |
| Date: Tue, 20 Dec 2022 02:04:34 -0800 |
| Subject: [PATCH] CRI: Fix no CNI info for pod sandbox on restart |
| |
| Due to when we were updating the pod sandboxes underlying container |
| object, the pointer to the sandbox would have the right info, but |
| the on-disk representation of the data was behind. This would cause |
| the data returned from loading any sandboxes after a restart to have |
| no CNI result or IP information for the pod. |
| |
| This change does an additional update to the on-disk container info |
| right after we invoke the CNI plugin so the metadata for the CNI result |
| and other networking information is properly flushed to disk. |
| |
| Signed-off-by: Danny Canter <danny@dcantah.dev> |
| (cherry picked from commit 3ee6dd5c1bca441d1ec4988cbaebadbfbcfde525) |
| Signed-off-by: Danny Canter <danny@dcantah.dev> |
| --- |
| integration/restart_test.go | 15 +++++++++++++++ |
| pkg/cri/server/sandbox_run.go | 11 ++++++++++- |
| 2 files changed, 25 insertions(+), 1 deletion(-) |
| |
| diff --git a/integration/restart_test.go b/integration/restart_test.go |
| index 9cb660252bb..767752ca01a 100644 |
| --- a/integration/restart_test.go |
| +++ b/integration/restart_test.go |
| @@ -191,6 +191,21 @@ func TestContainerdRestart(t *testing.T) { |
| if s.id == loaded.Id { |
| t.Logf("Checking sandbox state for '%s'", s.name) |
| assert.Equal(t, s.state, loaded.State) |
| + |
| + // See https://github.com/containerd/containerd/issues/7843 for details. |
| + // Test that CNI result and sandbox IPs are still present after restart. |
| + if loaded.State == runtime.PodSandboxState_SANDBOX_READY { |
| + status, info, err := SandboxInfo(loaded.Id) |
| + require.NoError(t, err) |
| + |
| + // Check that the NetNS didn't close on us, that we still have |
| + // the CNI result, and that we still have the IP we were given |
| + // for this pod. |
| + require.False(t, info.NetNSClosed) |
| + require.NotNil(t, info.CNIResult) |
| + require.NotNil(t, status.Network) |
| + require.NotEmpty(t, status.Network.Ip) |
| + } |
| break |
| } |
| } |
| diff --git a/pkg/cri/server/sandbox_run.go b/pkg/cri/server/sandbox_run.go |
| index 18657a51d25..08234402289 100644 |
| --- a/pkg/cri/server/sandbox_run.go |
| +++ b/pkg/cri/server/sandbox_run.go |
| @@ -295,7 +295,8 @@ func (c *criService) RunPodSandbox(ctx context.Context, r *runtime.RunPodSandbox |
| // Update spec of the container |
| containerd.UpdateContainerOpts(containerd.WithSpec(spec)), |
| // Update sandbox metadata to include NetNS info |
| - containerd.UpdateContainerOpts(containerd.WithContainerExtension(sandboxMetadataExtension, &sandbox.Metadata))); err != nil { |
| + containerd.UpdateContainerOpts(containerd.WithContainerExtension(sandboxMetadataExtension, &sandbox.Metadata)), |
| + ); err != nil { |
| return nil, fmt.Errorf("failed to update the network namespace for the sandbox container %q: %w", id, err) |
| } |
| |
| @@ -325,6 +326,14 @@ func (c *criService) RunPodSandbox(ctx context.Context, r *runtime.RunPodSandbox |
| return nil, fmt.Errorf("failed to setup network for sandbox %q: %w", id, err) |
| } |
| |
| + // Update metadata here to save CNI result and pod IPs to disk. |
| + if err := container.Update(ctx, |
| + // Update sandbox metadata to include NetNS info |
| + containerd.UpdateContainerOpts(containerd.WithContainerExtension(sandboxMetadataExtension, &sandbox.Metadata)), |
| + ); err != nil { |
| + return nil, fmt.Errorf("failed to update the network namespace for the sandbox container %q: %w", id, err) |
| + } |
| + |
| sandboxCreateNetworkTimer.UpdateSince(netStart) |
| } |
| |