CRI: Fix no CNI info for pod sandbox on restart

dcantah · dcantah · commit 23c2a863e338 · 2022-12-20T14:56:41.000-08:00
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 3ee6dd5) Signed-off-by: Danny Canter <danny@dcantah.dev>
diff --git a/integration/restart_test.go b/integration/restart_test.go
@@ -160,6 +160,21 @@ func TestContainerdRestart(t *testing.T) {
 		for _, loaded := range loadedSandboxes {
 			if s.id == loaded.Id {
 				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
@@ -18,6 +18,7 @@ package server
 
 import (
 	"encoding/json"
+	"fmt"
 	"math"
 	"path/filepath"
 	goruntime "runtime"
@@ -286,8 +287,9 @@ 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 {
-			return nil, errors.Wrapf(err, "failed to update the network namespace for the sandbox container %q", id)
+			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)
 		}
 
 		// Define this defer to teardownPodNetwork prior to the setupPodNetwork function call.
@@ -315,6 +317,14 @@ func (c *criService) RunPodSandbox(ctx context.Context, r *runtime.RunPodSandbox
 		if err := c.setupPodNetwork(ctx, &sandbox); err != nil {
 			return nil, errors.Wrapf(err, "failed to setup network for sandbox %q", id)
 		}
+
+		// 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)
+		}
 	}
 
 	// Create sandbox task in containerd.
