Skip to content

Commit

Permalink
CRI: Fix no CNI info for pod sandbox on restart
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
dcantah committed Dec 20, 2022
1 parent 8f7cfbd commit 3ee6dd5
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 1 deletion.
15 changes: 15 additions & 0 deletions integration/restart_test.go
Expand Up @@ -179,6 +179,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
}
}
Expand Down
11 changes: 10 additions & 1 deletion pkg/cri/server/sandbox_run.go
Expand Up @@ -280,7 +280,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)
}

Expand Down Expand Up @@ -310,6 +311,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)
}

Expand Down

0 comments on commit 3ee6dd5

Please sign in to comment.