github/containerd
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 2 Wiki Activity

cri: Fix sandbox_mode "shim"

Browse Source 
This is a partial revert of "cri/sbserver: Use platform instead of GOOS
for userns detection".

While what that commit did is 100% the right thing to do, when the
sandbox_mode is "shim" all controller.XXX() calls are RPCs and the
controller.Create() call initializes the controller. Therefore, things
like "getSandboxController()" don't work in the case of "shim"
sandbox_mode until after the controller.Create().

Due to this asymmetry and the lack of tests for shim mode, we didn't
catch it before.

This patch just reverts that commit so that the Create() and
getSandboxController() calls remain where they were, and just relies on
the config Linux section as a hack to detect if the pod sandbox will use
user namespaces or not.

Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
This commit is contained in:
Rodrigo Campos 2023-08-18 14:39:13 +02:00
parent 0f5dea3cc0
commit d09f7cbe00
1 changed files with 14 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
pkg/cri/sbserver/sandbox_run.go
View File

@ -143,18 +143,16 @@ func (c *criService) RunPodSandbox(ctx context.Context, r *runtime.RunPodSandbox
} }
}() }()
controller, err := c.getSandboxController(sandbox.Config, sandbox.RuntimeHandler) // XXX: What we really want here is to call controller.Platform() and then check
if err != nil { // platform.OS, but that is only populated after controller.Create() and that needs to be
return nil, fmt.Errorf("failed to get sandbox controller: %w", err) // done later (uses sandbox.NSPath that we will set just _after_ this).
} // So, lets check for the Linux section on the config, if that is populated, we assume the
platform, err := controller.Platform(ctx, sandbox.ID) // platform is linux.
if err != nil { // This is a hack, we should improve the controller interface to return the platform
return nil, fmt.Errorf("failed to query sandbox platform: %w", err) // earlier. But should work fine for this specific use.
}
userNsEnabled := false userNsEnabled := false
if platform.OS == "linux" { if linux := config.GetLinux(); linux != nil {
usernsOpts := config.GetLinux().GetSecurityContext().GetNamespaceOptions().GetUsernsOptions() usernsOpts := linux.GetSecurityContext().GetNamespaceOptions().GetUsernsOptions()
if usernsOpts != nil && usernsOpts.GetMode() == runtime.NamespaceMode_POD { if usernsOpts != nil && usernsOpts.GetMode() == runtime.NamespaceMode_POD {
userNsEnabled = true userNsEnabled = true
} }
@ -241,6 +239,11 @@ func (c *criService) RunPodSandbox(ctx context.Context, r *runtime.RunPodSandbox
return nil, fmt.Errorf("unable to save sandbox %q to store: %w", id, err) return nil, fmt.Errorf("unable to save sandbox %q to store: %w", id, err)
} }
controller, err := c.getSandboxController(config, r.GetRuntimeHandler())
if err != nil {
return nil, fmt.Errorf("failed to get sandbox controller: %w", err)
}
// Save sandbox metadata to store // Save sandbox metadata to store
if sandboxInfo, err = c.client.SandboxStore().Update(ctx, sandboxInfo, "extensions"); err != nil { if sandboxInfo, err = c.client.SandboxStore().Update(ctx, sandboxInfo, "extensions"); err != nil {
return nil, fmt.Errorf("unable to update extensions for sandbox %q: %w", id, err) return nil, fmt.Errorf("unable to update extensions for sandbox %q: %w", id, err)