From a4d5c3e5cb98186a30675e1a9c7560502b13daff Mon Sep 17 00:00:00 2001 From: Maksym Pavlenko Date: Wed, 14 Dec 2022 18:22:52 -0800 Subject: [PATCH] Support sandboxed shims shutdown Signed-off-by: Maksym Pavlenko --- cmd/ctr/commands/sandboxes/sandboxes.go | 2 +- pkg/cri/sbserver/podsandbox/sandbox_delete.go | 6 +++--- pkg/cri/sbserver/sandbox_remove.go | 2 +- runtime/v2/runc/pause/sandbox.go | 4 ++++ sandbox.go | 17 +++++++++++------ sandbox/controller.go | 4 ++-- sandbox/proxy/controller.go | 4 ++-- services/sandbox/controller_local.go | 14 ++++++++++++-- services/sandbox/controller_service.go | 6 +++--- 9 files changed, 39 insertions(+), 20 deletions(-) diff --git a/cmd/ctr/commands/sandboxes/sandboxes.go b/cmd/ctr/commands/sandboxes/sandboxes.go index ff0db8d8c..8b3eaa14a 100644 --- a/cmd/ctr/commands/sandboxes/sandboxes.go +++ b/cmd/ctr/commands/sandboxes/sandboxes.go @@ -178,7 +178,7 @@ var removeCommand = cli.Command{ } } - err = sandbox.Delete(ctx) + err = sandbox.Shutdown(ctx) if err != nil { log.G(ctx).WithError(err).Errorf("failed to shutdown sandbox %s", id) continue diff --git a/pkg/cri/sbserver/podsandbox/sandbox_delete.go b/pkg/cri/sbserver/podsandbox/sandbox_delete.go index 73b67e58f..4efb38e67 100644 --- a/pkg/cri/sbserver/podsandbox/sandbox_delete.go +++ b/pkg/cri/sbserver/podsandbox/sandbox_delete.go @@ -26,7 +26,7 @@ import ( "github.com/containerd/containerd/log" ) -func (c *Controller) Delete(ctx context.Context, sandboxID string) (*api.ControllerDeleteResponse, error) { +func (c *Controller) Shutdown(ctx context.Context, sandboxID string) (*api.ControllerShutdownResponse, error) { sandbox, err := c.sandboxStore.Get(sandboxID) if err != nil { if !errdefs.IsNotFound(err) { @@ -34,7 +34,7 @@ func (c *Controller) Delete(ctx context.Context, sandboxID string) (*api.Control } // 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.ControllerDeleteResponse{}, nil + return &api.ControllerShutdownResponse{}, nil } // Cleanup the sandbox root directories. @@ -58,5 +58,5 @@ func (c *Controller) Delete(ctx context.Context, sandboxID string) (*api.Control } } - return &api.ControllerDeleteResponse{}, nil + return &api.ControllerShutdownResponse{}, nil } diff --git a/pkg/cri/sbserver/sandbox_remove.go b/pkg/cri/sbserver/sandbox_remove.go index 59e139904..41d006fb5 100644 --- a/pkg/cri/sbserver/sandbox_remove.go +++ b/pkg/cri/sbserver/sandbox_remove.go @@ -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.Delete(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) } diff --git a/runtime/v2/runc/pause/sandbox.go b/runtime/v2/runc/pause/sandbox.go index f5277dbb1..e9f00e4c6 100644 --- a/runtime/v2/runc/pause/sandbox.go +++ b/runtime/v2/runc/pause/sandbox.go @@ -92,3 +92,7 @@ func (p *pauseService) SandboxStatus(ctx context.Context, req *api.SandboxStatus func (p *pauseService) PingSandbox(ctx context.Context, req *api.PingRequest) (*api.PingResponse, error) { return &api.PingResponse{}, nil } + +func (p *pauseService) ShutdownSandbox(ctx context.Context, request *api.ShutdownSandboxRequest) (*api.ShutdownSandboxResponse, error) { + return &api.ShutdownSandboxResponse{}, nil +} diff --git a/sandbox.go b/sandbox.go index b1934464c..9644dd783 100644 --- a/sandbox.go +++ b/sandbox.go @@ -46,8 +46,8 @@ type Sandbox interface { Stop(ctx context.Context) error // Wait blocks until sandbox process exits. Wait(ctx context.Context) (<-chan ExitStatus, error) - // Delete removes sandbox from the metadata store. - Delete(ctx context.Context) error + // Shutdown removes sandbox from the metadata store and shutdowns shim instance. + Shutdown(ctx context.Context) error } type sandboxClient struct { @@ -121,11 +121,16 @@ func (s *sandboxClient) Stop(ctx context.Context) error { return nil } -func (s *sandboxClient) Delete(ctx context.Context) error { - if _, err := s.client.SandboxController().Delete(ctx, s.ID()); err != nil { - return err +func (s *sandboxClient) Shutdown(ctx context.Context) error { + if _, err := s.client.SandboxController().Shutdown(ctx, s.ID()); err != nil { + return fmt.Errorf("failed to shutdown sandbox: %w", err) } - return s.client.SandboxStore().Delete(ctx, s.ID()) + + if err := s.client.SandboxStore().Delete(ctx, s.ID()); err != nil { + return fmt.Errorf("failed to delete sandbox from store: %w", err) + } + + return nil } // NewSandbox creates new sandbox client diff --git a/sandbox/controller.go b/sandbox/controller.go index 979a61dee..e11089b99 100644 --- a/sandbox/controller.go +++ b/sandbox/controller.go @@ -37,6 +37,6 @@ type Controller interface { // Status will query sandbox process status. It is heavier than Ping call and must be used whenever you need to // gather metadata about current sandbox state (status, uptime, resource use, etc). Status(ctx context.Context, sandboxID string, verbose bool) (*sandbox.ControllerStatusResponse, error) - // Delete deletes and cleans all tasks and sandbox instance. - Delete(ctx context.Context, sandboxID string) (*sandbox.ControllerDeleteResponse, error) + // Shutdown deletes and cleans all tasks and sandbox instance. + Shutdown(ctx context.Context, sandboxID string) (*sandbox.ControllerShutdownResponse, error) } diff --git a/sandbox/proxy/controller.go b/sandbox/proxy/controller.go index 586b66023..97d1f8e53 100644 --- a/sandbox/proxy/controller.go +++ b/sandbox/proxy/controller.go @@ -63,8 +63,8 @@ func (s *remoteSandboxController) Stop(ctx context.Context, sandboxID string) (* return resp, nil } -func (s *remoteSandboxController) Delete(ctx context.Context, sandboxID string) (*api.ControllerDeleteResponse, error) { - resp, err := s.client.Delete(ctx, &api.ControllerDeleteRequest{SandboxID: sandboxID}) +func (s *remoteSandboxController) Shutdown(ctx context.Context, sandboxID string) (*api.ControllerShutdownResponse, error) { + resp, err := s.client.Shutdown(ctx, &api.ControllerShutdownRequest{SandboxID: sandboxID}) if err != nil { return nil, errdefs.FromGRPC(err) } diff --git a/services/sandbox/controller_local.go b/services/sandbox/controller_local.go index 9302f28cc..3a33aa5a9 100644 --- a/services/sandbox/controller_local.go +++ b/services/sandbox/controller_local.go @@ -154,12 +154,22 @@ func (c *controllerLocal) Stop(ctx context.Context, in *api.ControllerStopReques return &api.ControllerStopResponse{}, nil } -func (c *controllerLocal) Delete(ctx context.Context, in *api.ControllerDeleteRequest, opts ...grpc.CallOption) (*api.ControllerDeleteResponse, error) { +func (c *controllerLocal) Shutdown(ctx context.Context, in *api.ControllerShutdownRequest, opts ...grpc.CallOption) (*api.ControllerShutdownResponse, error) { + svc, err := c.getSandbox(ctx, in.SandboxID) + if err != nil { + return nil, err + } + + _, err = svc.ShutdownSandbox(ctx, &runtimeAPI.ShutdownSandboxRequest{SandboxID: in.SandboxID}) + if err != nil { + return nil, errdefs.ToGRPC(fmt.Errorf("failed to shutdown sandbox: %w", err)) + } + if err := c.shims.Delete(ctx, in.SandboxID); err != nil { return nil, errdefs.ToGRPC(fmt.Errorf("failed to delete sandbox shim: %w", err)) } - return &api.ControllerDeleteResponse{}, nil + return &api.ControllerShutdownResponse{}, nil } func (c *controllerLocal) Wait(ctx context.Context, in *api.ControllerWaitRequest, opts ...grpc.CallOption) (*api.ControllerWaitResponse, error) { diff --git a/services/sandbox/controller_service.go b/services/sandbox/controller_service.go index ba9d77908..339082eaf 100644 --- a/services/sandbox/controller_service.go +++ b/services/sandbox/controller_service.go @@ -94,7 +94,7 @@ func (s *controllerService) Status(ctx context.Context, req *api.ControllerStatu return s.local.Status(ctx, req) } -func (s *controllerService) Delete(ctx context.Context, req *api.ControllerDeleteRequest) (*api.ControllerDeleteResponse, error) { - log.G(ctx).WithField("req", req).Debug("delete sandbox") - return s.local.Delete(ctx, req) +func (s *controllerService) Shutdown(ctx context.Context, req *api.ControllerShutdownRequest) (*api.ControllerShutdownResponse, error) { + log.G(ctx).WithField("req", req).Debug("shutdown sandbox") + return s.local.Shutdown(ctx, req) }