Refactor sandbox controller interface

Update the sandbox controller interface to use local types rather than
using the API types.

Signed-off-by: Derek McGowan <derek@mcg.dev>
This commit is contained in:
Derek McGowan
2023-01-18 21:53:34 -08:00
parent 20de989afc
commit 2717685dad
21 changed files with 378 additions and 355 deletions

View File

@@ -67,14 +67,14 @@ func (c *criService) CreateContainer(ctx context.Context, r *runtime.CreateConta
return nil, fmt.Errorf("failed to get sandbox controller: %w", err)
}
statusResponse, err := controller.Status(ctx, sandbox.ID, false)
cstatus, err := controller.Status(ctx, sandbox.ID, false)
if err != nil {
return nil, fmt.Errorf("failed to get controller status: %w", err)
}
var (
sandboxID = statusResponse.GetID()
sandboxPid = statusResponse.GetPid()
sandboxID = cstatus.SandboxID
sandboxPid = cstatus.Pid
)
// Generate unique id and name for the container and reserve the name.

View File

@@ -26,7 +26,6 @@ import (
"github.com/containerd/containerd"
eventtypes "github.com/containerd/containerd/api/events"
api "github.com/containerd/containerd/api/services/sandbox/v1"
"github.com/containerd/containerd/errdefs"
"github.com/containerd/containerd/oci"
criconfig "github.com/containerd/containerd/pkg/cri/config"
@@ -94,17 +93,17 @@ func (c *Controller) Platform(_ctx context.Context, _sandboxID string) (platform
return platforms.DefaultSpec(), nil
}
func (c *Controller) Wait(ctx context.Context, sandboxID string) (*api.ControllerWaitResponse, error) {
func (c *Controller) Wait(ctx context.Context, sandboxID string) (sandbox.ExitStatus, error) {
status := c.store.Get(sandboxID)
if status == nil {
return nil, fmt.Errorf("failed to get exit channel. %q", sandboxID)
return sandbox.ExitStatus{}, fmt.Errorf("failed to get exit channel. %q", sandboxID)
}
exitStatus, exitedAt, err := c.waitSandboxExit(ctx, sandboxID, status.Waiter)
return &api.ControllerWaitResponse{
return sandbox.ExitStatus{
ExitStatus: exitStatus,
ExitedAt: protobuf.ToTimestamp(exitedAt),
ExitedAt: exitedAt,
}, err
}

View File

@@ -25,7 +25,6 @@ import (
"github.com/containerd/containerd/pkg/cri/store/label"
sandboxstore "github.com/containerd/containerd/pkg/cri/store/sandbox"
ostesting "github.com/containerd/containerd/pkg/os/testing"
"github.com/containerd/containerd/protobuf"
"github.com/stretchr/testify/assert"
)
@@ -83,6 +82,6 @@ func Test_Status(t *testing.T) {
t.Fatal(err)
}
assert.Equal(t, s.Pid, pid)
assert.Equal(t, s.ExitedAt, protobuf.ToTimestamp(exitedAt))
assert.Equal(t, s.ExitedAt, exitedAt)
assert.Equal(t, s.State, sandboxstore.StateReady.String())
}

View File

@@ -23,30 +23,29 @@ import (
runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
"github.com/containerd/containerd"
api "github.com/containerd/containerd/api/services/sandbox/v1"
"github.com/containerd/containerd/errdefs"
"github.com/containerd/containerd/log"
)
func (c *Controller) Shutdown(ctx context.Context, sandboxID string) (*api.ControllerShutdownResponse, error) {
func (c *Controller) Shutdown(ctx context.Context, sandboxID string) error {
sandbox, err := c.sandboxStore.Get(sandboxID)
if err != nil {
if !errdefs.IsNotFound(err) {
return nil, fmt.Errorf("an error occurred when try to find sandbox %q: %w", sandboxID, err)
return fmt.Errorf("an error occurred when try to find sandbox %q: %w", sandboxID, err)
}
// Do not return error if the id doesn't exist.
log.G(ctx).Tracef("Sandbox controller Delete called for sandbox %q that does not exist", sandboxID)
return &api.ControllerShutdownResponse{}, nil
return nil
}
// Cleanup the sandbox root directories.
sandboxRootDir := c.getSandboxRootDir(sandboxID)
if err := ensureRemoveAll(ctx, sandboxRootDir); err != nil {
return nil, fmt.Errorf("failed to remove sandbox root directory %q: %w", sandboxRootDir, err)
return fmt.Errorf("failed to remove sandbox root directory %q: %w", sandboxRootDir, err)
}
volatileSandboxRootDir := c.getVolatileSandboxRootDir(sandboxID)
if err := ensureRemoveAll(ctx, volatileSandboxRootDir); err != nil {
return nil, fmt.Errorf("failed to remove volatile sandbox root directory %q: %w",
return fmt.Errorf("failed to remove volatile sandbox root directory %q: %w",
volatileSandboxRootDir, err)
}
@@ -54,7 +53,7 @@ func (c *Controller) Shutdown(ctx context.Context, sandboxID string) (*api.Contr
if sandbox.Container != nil {
if err := sandbox.Container.Delete(ctx, containerd.WithSnapshotCleanup); err != nil {
if !errdefs.IsNotFound(err) {
return nil, fmt.Errorf("failed to delete sandbox container %q: %w", sandboxID, err)
return fmt.Errorf("failed to delete sandbox container %q: %w", sandboxID, err)
}
log.G(ctx).Tracef("Sandbox controller Delete called for sandbox container %q that does not exist", sandboxID)
}
@@ -65,5 +64,5 @@ func (c *Controller) Shutdown(ctx context.Context, sandboxID string) (*api.Contr
// Send CONTAINER_DELETED event with ContainerId equal to SandboxId.
c.cri.GenerateAndSendContainerEvent(ctx, sandboxID, sandboxID, runtime.ContainerEventType_CONTAINER_DELETED_EVENT)
return &api.ControllerShutdownResponse{}, nil
return nil
}

View File

@@ -29,7 +29,6 @@ import (
runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
"github.com/containerd/containerd"
api "github.com/containerd/containerd/api/services/sandbox/v1"
containerdio "github.com/containerd/containerd/cio"
"github.com/containerd/containerd/errdefs"
"github.com/containerd/containerd/log"
@@ -38,7 +37,7 @@ import (
customopts "github.com/containerd/containerd/pkg/cri/opts"
sandboxstore "github.com/containerd/containerd/pkg/cri/store/sandbox"
ctrdutil "github.com/containerd/containerd/pkg/cri/util"
"github.com/containerd/containerd/protobuf"
"github.com/containerd/containerd/sandbox"
"github.com/containerd/containerd/snapshots"
)
@@ -51,23 +50,22 @@ func init() {
// down the created resources. If an error occurs while tearing down resources, a zero-valued response is returned
// alongside the error. If the teardown was successful, a nil response is returned with the error.
// TODO(samuelkarp) Determine whether this error indication is reasonable to retain once controller.Delete is implemented.
func (c *Controller) Start(ctx context.Context, id string) (resp *api.ControllerStartResponse, retErr error) {
func (c *Controller) Start(ctx context.Context, id string) (cin sandbox.ControllerInstance, retErr error) {
var cleanupErr error
defer func() {
if retErr != nil && cleanupErr != nil {
log.G(ctx).WithField("id", id).WithError(cleanupErr).Errorf("failed to fully teardown sandbox resources after earlier error: %s", retErr)
resp = &api.ControllerStartResponse{}
}
}()
sandboxInfo, err := c.client.SandboxStore().Get(ctx, id)
if err != nil {
return nil, fmt.Errorf("unable to find sandbox with id %q: %w", id, err)
return cin, fmt.Errorf("unable to find sandbox with id %q: %w", id, err)
}
var metadata sandboxstore.Metadata
if err := sandboxInfo.GetExtension(MetadataKey, &metadata); err != nil {
return nil, fmt.Errorf("failed to get sandbox %q metadata: %w", id, err)
return cin, fmt.Errorf("failed to get sandbox %q metadata: %w", id, err)
}
var (
@@ -78,17 +76,17 @@ func (c *Controller) Start(ctx context.Context, id string) (resp *api.Controller
// Ensure sandbox container image snapshot.
image, err := c.cri.EnsureImageExists(ctx, c.config.SandboxImage, config)
if err != nil {
return nil, fmt.Errorf("failed to get sandbox image %q: %w", c.config.SandboxImage, err)
return cin, fmt.Errorf("failed to get sandbox image %q: %w", c.config.SandboxImage, err)
}
containerdImage, err := c.toContainerdImage(ctx, *image)
if err != nil {
return nil, fmt.Errorf("failed to get image from containerd %q: %w", image.ID, err)
return cin, fmt.Errorf("failed to get image from containerd %q: %w", image.ID, err)
}
ociRuntime, err := c.getSandboxRuntime(config, metadata.RuntimeHandler)
if err != nil {
return nil, fmt.Errorf("failed to get sandbox runtime: %w", err)
return cin, fmt.Errorf("failed to get sandbox runtime: %w", err)
}
log.G(ctx).WithField("podsandboxid", id).Debugf("use OCI runtime %+v", ociRuntime)
@@ -100,7 +98,7 @@ func (c *Controller) Start(ctx context.Context, id string) (resp *api.Controller
// it safely.
spec, err := c.sandboxContainerSpec(id, config, &image.ImageSpec.Config, metadata.NetNSPath, ociRuntime.PodAnnotations)
if err != nil {
return nil, fmt.Errorf("failed to generate sandbox container spec: %w", err)
return cin, fmt.Errorf("failed to generate sandbox container spec: %w", err)
}
log.G(ctx).WithField("podsandboxid", id).Debugf("sandbox container spec: %#+v", spew.NewFormatter(spec))
@@ -114,7 +112,7 @@ func (c *Controller) Start(ctx context.Context, id string) (resp *api.Controller
// handle any KVM based runtime
if err := modifyProcessLabel(ociRuntime.Type, spec); err != nil {
return nil, err
return cin, err
}
if config.GetLinux().GetSecurityContext().GetPrivileged() {
@@ -126,7 +124,7 @@ func (c *Controller) Start(ctx context.Context, id string) (resp *api.Controller
// Generate spec options that will be applied to the spec later.
specOpts, err := c.sandboxContainerSpecOpts(config, &image.ImageSpec.Config)
if err != nil {
return nil, fmt.Errorf("failed to generate sandbox container spec options: %w", err)
return cin, fmt.Errorf("failed to generate sandbox container spec options: %w", err)
}
sandboxLabels := buildLabels(config.Labels, image.ImageSpec.Config.Labels, containerKindSandbox)
@@ -143,7 +141,7 @@ func (c *Controller) Start(ctx context.Context, id string) (resp *api.Controller
container, err := c.client.NewContainer(ctx, id, opts...)
if err != nil {
return nil, fmt.Errorf("failed to create containerd container: %w", err)
return cin, fmt.Errorf("failed to create containerd container: %w", err)
}
defer func() {
if retErr != nil && cleanupErr == nil {
@@ -163,7 +161,7 @@ func (c *Controller) Start(ctx context.Context, id string) (resp *api.Controller
// Create sandbox container root directories.
sandboxRootDir := c.getSandboxRootDir(id)
if err := c.os.MkdirAll(sandboxRootDir, 0755); err != nil {
return nil, fmt.Errorf("failed to create sandbox root directory %q: %w",
return cin, fmt.Errorf("failed to create sandbox root directory %q: %w",
sandboxRootDir, err)
}
defer func() {
@@ -178,7 +176,7 @@ func (c *Controller) Start(ctx context.Context, id string) (resp *api.Controller
volatileSandboxRootDir := c.getVolatileSandboxRootDir(id)
if err := c.os.MkdirAll(volatileSandboxRootDir, 0755); err != nil {
return nil, fmt.Errorf("failed to create volatile sandbox root directory %q: %w",
return cin, fmt.Errorf("failed to create volatile sandbox root directory %q: %w",
volatileSandboxRootDir, err)
}
defer func() {
@@ -193,7 +191,7 @@ func (c *Controller) Start(ctx context.Context, id string) (resp *api.Controller
// Setup files required for the sandbox.
if err = c.setupSandboxFiles(id, config); err != nil {
return nil, fmt.Errorf("failed to setup sandbox files: %w", err)
return cin, fmt.Errorf("failed to setup sandbox files: %w", err)
}
defer func() {
if retErr != nil && cleanupErr == nil {
@@ -207,7 +205,7 @@ func (c *Controller) Start(ctx context.Context, id string) (resp *api.Controller
// Update sandbox created timestamp.
info, err := container.Info(ctx)
if err != nil {
return nil, fmt.Errorf("failed to get sandbox container info: %w", err)
return cin, fmt.Errorf("failed to get sandbox container info: %w", err)
}
// Create sandbox task in containerd.
@@ -221,7 +219,7 @@ func (c *Controller) Start(ctx context.Context, id string) (resp *api.Controller
// We don't need stdio for sandbox container.
task, err := container.NewTask(ctx, containerdio.NullIO, taskOpts...)
if err != nil {
return nil, fmt.Errorf("failed to create containerd task: %w", err)
return cin, fmt.Errorf("failed to create containerd task: %w", err)
}
defer func() {
if retErr != nil && cleanupErr == nil {
@@ -238,13 +236,13 @@ func (c *Controller) Start(ctx context.Context, id string) (resp *api.Controller
// wait is a long running background request, no timeout needed.
exitCh, err := task.Wait(ctrdutil.NamespacedContext())
if err != nil {
return nil, fmt.Errorf("failed to wait for sandbox container task: %w", err)
return cin, fmt.Errorf("failed to wait for sandbox container task: %w", err)
}
c.store.Save(id, exitCh)
nric, err := nri.New()
if err != nil {
return nil, fmt.Errorf("unable to create nri client: %w", err)
return cin, fmt.Errorf("unable to create nri client: %w", err)
}
if nric != nil {
nriSB := &nri.Sandbox{
@@ -252,25 +250,23 @@ func (c *Controller) Start(ctx context.Context, id string) (resp *api.Controller
Labels: config.Labels,
}
if _, err := nric.InvokeWithSandbox(ctx, task, v1.Create, nriSB); err != nil {
return nil, fmt.Errorf("nri invoke: %w", err)
return cin, fmt.Errorf("nri invoke: %w", err)
}
}
if err := task.Start(ctx); err != nil {
return nil, fmt.Errorf("failed to start sandbox container task %q: %w", id, err)
return cin, fmt.Errorf("failed to start sandbox container task %q: %w", id, err)
}
// Send CONTAINER_STARTED event with ContainerId equal to SandboxId.
c.cri.GenerateAndSendContainerEvent(ctx, id, id, runtime.ContainerEventType_CONTAINER_STARTED_EVENT)
resp = &api.ControllerStartResponse{
SandboxID: id,
Pid: task.Pid(),
CreatedAt: protobuf.ToTimestamp(info.CreatedAt),
Labels: labels,
}
cin.SandboxID = id
cin.Pid = task.Pid()
cin.CreatedAt = info.CreatedAt
cin.Labels = labels
return resp, nil
return
}
func (c *Controller) Create(ctx context.Context, _id string) error {

View File

@@ -22,11 +22,10 @@ import (
"fmt"
"github.com/containerd/containerd"
api "github.com/containerd/containerd/api/services/sandbox/v1"
"github.com/containerd/containerd/containers"
"github.com/containerd/containerd/errdefs"
sandboxstore "github.com/containerd/containerd/pkg/cri/store/sandbox"
"github.com/containerd/containerd/protobuf"
"github.com/containerd/containerd/sandbox"
"github.com/containerd/go-cni"
"github.com/containerd/typeurl"
runtimespec "github.com/opencontainers/runtime-spec/specs-go"
@@ -55,36 +54,36 @@ type SandboxInfo struct {
Metadata *sandboxstore.Metadata `json:"sandboxMetadata"`
}
func (c *Controller) Status(ctx context.Context, sandboxID string, verbose bool) (*api.ControllerStatusResponse, error) {
sandbox, err := c.sandboxStore.Get(sandboxID)
func (c *Controller) Status(ctx context.Context, sandboxID string, verbose bool) (sandbox.ControllerStatus, error) {
sb, err := c.sandboxStore.Get(sandboxID)
if err != nil {
return nil, fmt.Errorf("an error occurred while trying to find sandbox %q: %w",
return sandbox.ControllerStatus{}, fmt.Errorf("an error occurred while trying to find sandbox %q: %w",
sandboxID, err)
}
status := sandbox.Status.Get()
resp := &api.ControllerStatusResponse{
ID: sandboxID,
status := sb.Status.Get()
cstatus := sandbox.ControllerStatus{
SandboxID: sandboxID,
Pid: status.Pid,
State: status.State.String(),
CreatedAt: protobuf.ToTimestamp(status.CreatedAt),
CreatedAt: status.CreatedAt,
Extra: nil,
}
if !status.ExitedAt.IsZero() {
resp.ExitedAt = protobuf.ToTimestamp(status.ExitedAt)
cstatus.ExitedAt = status.ExitedAt
}
if verbose {
info, err := toCRISandboxInfo(ctx, sandbox)
info, err := toCRISandboxInfo(ctx, sb)
if err != nil {
return nil, err
return sandbox.ControllerStatus{}, err
}
resp.Info = info
cstatus.Info = info
}
return resp, nil
return cstatus, nil
}
// toCRISandboxInfo converts internal container object information to CRI sandbox status response info map.

View File

@@ -25,22 +25,21 @@ import (
"github.com/sirupsen/logrus"
eventtypes "github.com/containerd/containerd/api/events"
api "github.com/containerd/containerd/api/services/sandbox/v1"
"github.com/containerd/containerd/errdefs"
sandboxstore "github.com/containerd/containerd/pkg/cri/store/sandbox"
ctrdutil "github.com/containerd/containerd/pkg/cri/util"
"github.com/containerd/containerd/protobuf"
)
func (c *Controller) Stop(ctx context.Context, sandboxID string) (*api.ControllerStopResponse, error) {
func (c *Controller) Stop(ctx context.Context, sandboxID string) error {
sandbox, err := c.sandboxStore.Get(sandboxID)
if err != nil {
return nil, fmt.Errorf("an error occurred when try to find sandbox %q: %w",
return fmt.Errorf("an error occurred when try to find sandbox %q: %w",
sandboxID, err)
}
if err := c.cleanupSandboxFiles(sandboxID, sandbox.Config); err != nil {
return nil, fmt.Errorf("failed to cleanup sandbox files: %w", err)
return fmt.Errorf("failed to cleanup sandbox files: %w", err)
}
// TODO: The Controller maintains its own Status instead of CRI's sandboxStore.
@@ -48,10 +47,10 @@ func (c *Controller) Stop(ctx context.Context, sandboxID string) (*api.Controlle
state := sandbox.Status.Get().State
if (state == sandboxstore.StateReady || state == sandboxstore.StateUnknown) && sandbox.Container != nil {
if err := c.stopSandboxContainer(ctx, sandbox); err != nil {
return nil, fmt.Errorf("failed to stop sandbox container %q in %q state: %w", sandboxID, state, err)
return fmt.Errorf("failed to stop sandbox container %q in %q state: %w", sandboxID, state, err)
}
}
return &api.ControllerStopResponse{}, nil
return nil
}
// stopSandboxContainer kills the sandbox container.

View File

@@ -86,7 +86,7 @@ func (c *criService) RemovePodSandbox(ctx context.Context, r *runtime.RemovePodS
return nil, fmt.Errorf("failed to get sandbox controller: %w", err)
}
if _, err := controller.Shutdown(ctx, id); err != nil && !errdefs.IsNotFound(err) {
if err := controller.Shutdown(ctx, id); err != nil && !errdefs.IsNotFound(err) {
return nil, fmt.Errorf("failed to delete sandbox %q: %w", id, err)
}

View File

@@ -41,7 +41,6 @@ import (
"github.com/containerd/containerd/pkg/cri/util"
ctrdutil "github.com/containerd/containerd/pkg/cri/util"
"github.com/containerd/containerd/pkg/netns"
"github.com/containerd/containerd/protobuf"
sb "github.com/containerd/containerd/sandbox"
)
@@ -230,10 +229,10 @@ func (c *criService) RunPodSandbox(ctx context.Context, r *runtime.RunPodSandbox
return nil, fmt.Errorf("failed to create sandbox %q: %w", id, err)
}
resp, err := controller.Start(ctx, id)
ctrl, err := controller.Start(ctx, id)
if err != nil {
sandbox.Container, _ = c.client.LoadContainer(ctx, id)
if resp != nil && resp.SandboxID == "" && resp.Pid == 0 && resp.CreatedAt == nil && len(resp.Labels) == 0 {
if ctrl.SandboxID == "" && ctrl.Pid == 0 && ctrl.CreatedAt.IsZero() && len(ctrl.Labels) == 0 {
// if resp is a non-nil zero-value, an error occurred during cleanup
cleanupErr = fmt.Errorf("failed to cleanup sandbox")
}
@@ -249,7 +248,7 @@ func (c *criService) RunPodSandbox(ctx context.Context, r *runtime.RunPodSandbox
sandbox.Container = container
}
labels := resp.GetLabels()
labels := ctrl.Labels
if labels == nil {
labels = map[string]string{}
}
@@ -258,9 +257,9 @@ func (c *criService) RunPodSandbox(ctx context.Context, r *runtime.RunPodSandbox
if err := sandbox.Status.Update(func(status sandboxstore.Status) (sandboxstore.Status, error) {
// Set the pod sandbox as ready after successfully start sandbox container.
status.Pid = resp.Pid
status.Pid = ctrl.Pid
status.State = sandboxstore.StateReady
status.CreatedAt = protobuf.FromTimestamp(resp.CreatedAt)
status.CreatedAt = ctrl.CreatedAt
return status, nil
}); err != nil {
return nil, fmt.Errorf("failed to update sandbox status: %w", err)
@@ -284,7 +283,7 @@ func (c *criService) RunPodSandbox(ctx context.Context, r *runtime.RunPodSandbox
return
}
exitCh <- *containerd.NewExitStatus(resp.ExitStatus, protobuf.FromTimestamp(resp.ExitedAt), nil)
exitCh <- *containerd.NewExitStatus(resp.ExitStatus, resp.ExitedAt, nil)
}()
// start the monitor after adding sandbox into the store, this ensures
@@ -292,7 +291,7 @@ func (c *criService) RunPodSandbox(ctx context.Context, r *runtime.RunPodSandbox
//
// TaskOOM from containerd may come before sandbox is added to store,
// but we don't care about sandbox TaskOOM right now, so it is fine.
c.eventMonitor.startSandboxExitMonitor(context.Background(), id, resp.GetPid(), exitCh)
c.eventMonitor.startSandboxExitMonitor(context.Background(), id, ctrl.Pid, exitCh)
sandboxRuntimeCreateTimer.WithValues(labels["oci_runtime_type"]).UpdateSince(runtimeStart)

View File

@@ -42,12 +42,12 @@ func (c *criService) PodSandboxStatus(ctx context.Context, r *runtime.PodSandbox
return nil, fmt.Errorf("failed to get sandbox controller: %w", err)
}
statusResponse, err := controller.Status(ctx, sandbox.ID, r.GetVerbose())
cstatus, err := controller.Status(ctx, sandbox.ID, r.GetVerbose())
if err != nil {
return nil, fmt.Errorf("failed to query controller status: %w", err)
}
status := toCRISandboxStatus(sandbox.Metadata, statusResponse.State, statusResponse.GetCreatedAt().AsTime(), ip, additionalIPs)
status := toCRISandboxStatus(sandbox.Metadata, cstatus.State, cstatus.CreatedAt, ip, additionalIPs)
if status.GetCreatedAt() == 0 {
// CRI doesn't allow CreatedAt == 0.
sandboxInfo, err := c.client.SandboxStore().Get(ctx, sandbox.ID)
@@ -59,7 +59,7 @@ func (c *criService) PodSandboxStatus(ctx context.Context, r *runtime.PodSandbox
return &runtime.PodSandboxStatusResponse{
Status: status,
Info: statusResponse.GetInfo(),
Info: cstatus.Info,
}, nil
}

View File

@@ -73,7 +73,7 @@ func (c *criService) stopPodSandbox(ctx context.Context, sandbox sandboxstore.Sa
return fmt.Errorf("failed to get sandbox controller: %w", err)
}
if _, err := controller.Stop(ctx, id); err != nil {
if err := controller.Stop(ctx, id); err != nil {
return fmt.Errorf("failed to stop sandbox %q: %w", id, err)
}
}