From 6da4e40b22ce2beb3cbb88dcdb8c7ede279e5b14 Mon Sep 17 00:00:00 2001 From: Sascha Grunert Date: Tue, 30 Jul 2024 11:47:22 +0200 Subject: [PATCH] Make `StopContainer` RPC idempotent Similar to container removal, the stop of a container should be a noop if the container has not been found. Found during: https://github.com/kubernetes-sigs/cri-tools/pull/1536 Signed-off-by: Sascha Grunert (cherry picked from commit a97b11898a195251e0852c94bce6a0189df51ba5) Signed-off-by: Sascha Grunert --- pkg/cri/sbserver/container_stop.go | 9 ++++++++- pkg/cri/server/container_stop.go | 9 ++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/pkg/cri/sbserver/container_stop.go b/pkg/cri/sbserver/container_stop.go index f317f0518173..c59a9f53c48a 100644 --- a/pkg/cri/sbserver/container_stop.go +++ b/pkg/cri/sbserver/container_stop.go @@ -40,7 +40,14 @@ func (c *criService) StopContainer(ctx context.Context, r *runtime.StopContainer // Get container config from container store. container, err := c.containerStore.Get(r.GetContainerId()) if err != nil { - return nil, fmt.Errorf("an error occurred when try to find container %q: %w", r.GetContainerId(), err) + if !errdefs.IsNotFound(err) { + return nil, fmt.Errorf("an error occurred when try to find container %q: %w", r.GetContainerId(), err) + } + + // The StopContainer RPC is idempotent, and must not return an error if + // the container has already been stopped. Ref: + // https://github.com/kubernetes/cri-api/blob/c20fa40/pkg/apis/runtime/v1/api.proto#L67-L68 + return &runtime.StopContainerResponse{}, nil } if err := c.stopContainer(ctx, container, time.Duration(r.GetTimeout())*time.Second); err != nil { diff --git a/pkg/cri/server/container_stop.go b/pkg/cri/server/container_stop.go index 96b390989488..72a1b63779eb 100644 --- a/pkg/cri/server/container_stop.go +++ b/pkg/cri/server/container_stop.go @@ -40,7 +40,14 @@ func (c *criService) StopContainer(ctx context.Context, r *runtime.StopContainer // Get container config from container store. container, err := c.containerStore.Get(r.GetContainerId()) if err != nil { - return nil, fmt.Errorf("an error occurred when try to find container %q: %w", r.GetContainerId(), err) + if !errdefs.IsNotFound(err) { + return nil, fmt.Errorf("an error occurred when try to find container %q: %w", r.GetContainerId(), err) + } + + // The StopContainer RPC is idempotent, and must not return an error if + // the container has already been stopped. Ref: + // https://github.com/kubernetes/cri-api/blob/c20fa40/pkg/apis/runtime/v1/api.proto#L67-L68 + return &runtime.StopContainerResponse{}, nil } if err := c.stopContainer(ctx, container, time.Duration(r.GetTimeout())*time.Second); err != nil {