Make StopContainer RPC idempotent

saschagrunert · saschagrunert · commit 6da4e40b22ce · 2024-07-31T09:38:45.000+02:00
Similar to container removal, the stop of a container should be a noop if the container has not been found. Found during: kubernetes-sigs/cri-tools#1536 Signed-off-by: Sascha Grunert <sgrunert@redhat.com> (cherry picked from commit a97b118) Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
diff --git 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
@@ -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 {
