From 70da5c783063ee7afefa08e6c3eca5438442ca21 Mon Sep 17 00:00:00 2001 From: Danny Canter Date: Mon, 13 Mar 2023 01:23:42 -0700 Subject: [PATCH] Sandbox: Cleanup shim on Start failure Made a change a bit ago to cleanup the shim on CreateSandbox failures and noted that we should probably do the same on Start as well as nothing gets cleaned up otherwise, and nothing states that a sandbox server/shim should exit itself if Create/Start fail. Ideally this could be hooked up to some subsystem in containerd that'd do it for us, but for now to allow developing prototyping sandbox shims this makes things much friendlier. Signed-off-by: Danny Canter --- plugins/sandbox/controller.go | 37 ++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/plugins/sandbox/controller.go b/plugins/sandbox/controller.go index e85f0739a..7c718577b 100644 --- a/plugins/sandbox/controller.go +++ b/plugins/sandbox/controller.go @@ -83,6 +83,25 @@ type controllerLocal struct { var _ sandbox.Controller = (*controllerLocal)(nil) +func (c *controllerLocal) cleanupShim(ctx context.Context, sandboxID string, svc runtimeAPI.TTRPCSandboxService) { + // Let the shim exit, then we can clean up the bundle after. + if _, sErr := svc.ShutdownSandbox(ctx, &runtimeAPI.ShutdownSandboxRequest{ + SandboxID: sandboxID, + }); sErr != nil { + log.G(ctx).WithError(sErr).WithField("sandboxID", sandboxID). + Error("failed to shutdown sandbox") + } + + ctx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + + dErr := c.shims.Delete(ctx, sandboxID) + if dErr != nil { + log.G(ctx).WithError(dErr).WithField("sandboxID", sandboxID). + Error("failed to delete shim") + } +} + func (c *controllerLocal) Create(ctx context.Context, sandboxID string, opts ...sandbox.CreateOpt) error { var coptions sandbox.CreateOptions for _, opt := range opts { @@ -128,22 +147,7 @@ func (c *controllerLocal) Create(ctx context.Context, sandboxID string, opts ... Options: options, NetnsPath: coptions.NetNSPath, }); err != nil { - // Let the shim exit, then we can clean up the bundle after. - if _, sErr := svc.ShutdownSandbox(ctx, &runtimeAPI.ShutdownSandboxRequest{ - SandboxID: sandboxID, - }); sErr != nil { - log.G(ctx).WithError(sErr).WithField("sandboxID", sandboxID). - Error("failed to shutdown sandbox after failed create") - } - - ctx, cancel := context.WithTimeout(ctx, 5*time.Second) - defer cancel() - - dErr := c.shims.Delete(ctx, sandboxID) - if dErr != nil { - log.G(ctx).WithError(dErr).WithField("sandboxID", sandboxID). - Error("failed to delete shim after failed sandbox create") - } + c.cleanupShim(ctx, sandboxID, svc) return fmt.Errorf("failed to create sandbox %s: %w", sandboxID, errdefs.FromGRPC(err)) } @@ -163,6 +167,7 @@ func (c *controllerLocal) Start(ctx context.Context, sandboxID string) (sandbox. resp, err := svc.StartSandbox(ctx, &runtimeAPI.StartSandboxRequest{SandboxID: sandboxID}) if err != nil { + c.cleanupShim(ctx, sandboxID, svc) return sandbox.ControllerInstance{}, fmt.Errorf("failed to start sandbox %s: %w", sandboxID, errdefs.FromGRPC(err)) }