cri: emit warning for concurrent CreateContainer

We have existing detection for concurrent CreateContainer requests, but
the error message is unclear and there is no warning in containerd logs.
This change adds a warning and clarifies the error message.

Signed-off-by: Samuel Karp <samuelkarp@google.com>

internal/cri/server/container_create.go

@@ -25,6 +25,16 @@ import (
 	"strings"
 	"time"
 
+	"github.com/containerd/log"
+	"github.com/containerd/platforms"
+	"github.com/containerd/typeurl/v2"
+	"github.com/davecgh/go-spew/spew"
+	imagespec "github.com/opencontainers/image-spec/specs-go/v1"
+	runtimespec "github.com/opencontainers/runtime-spec/specs-go"
+	"github.com/opencontainers/selinux/go-selinux"
+	"github.com/opencontainers/selinux/go-selinux/label"
+	runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
+
 	containerd "github.com/containerd/containerd/v2/client"
 	"github.com/containerd/containerd/v2/core/containers"
 	"github.com/containerd/containerd/v2/internal/cri/annotations"
@@ -35,18 +45,10 @@ import (
 	containerstore "github.com/containerd/containerd/v2/internal/cri/store/container"
 	"github.com/containerd/containerd/v2/internal/cri/store/sandbox"
 	"github.com/containerd/containerd/v2/internal/cri/util"
+	"github.com/containerd/containerd/v2/internal/registrar"
 	"github.com/containerd/containerd/v2/pkg/blockio"
 	"github.com/containerd/containerd/v2/pkg/oci"
 	"github.com/containerd/containerd/v2/pkg/tracing"
-	"github.com/containerd/log"
-	"github.com/containerd/platforms"
-	"github.com/containerd/typeurl/v2"
-	"github.com/davecgh/go-spew/spew"
-	imagespec "github.com/opencontainers/image-spec/specs-go/v1"
-	runtimespec "github.com/opencontainers/runtime-spec/specs-go"
-	"github.com/opencontainers/selinux/go-selinux"
-	"github.com/opencontainers/selinux/go-selinux/label"
-	runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
 )
 
 func init() {
@@ -94,6 +96,11 @@ func (c *criService) CreateContainer(ctx context.Context, r *runtime.CreateConta
 	name := makeContainerName(metadata, sandboxMetadata)
 	log.G(ctx).Debugf("Generated id %q for container %q", id, name)
 	if err = c.containerNameIndex.Reserve(name, id); err != nil {
+		var resErr *registrar.ReservedErr
+		if errors.As(err, &resErr) {
+			log.G(ctx).WithError(err).Warn("possible concurrent CreateContainer request")
+			return nil, fmt.Errorf("failed to reserve container name %q; check if another CreateContainer request is in progress: %w", name, err)
+		}
 		return nil, fmt.Errorf("failed to reserve container name %q: %w", name, err)
 	}
 	span.SetAttributes(

internal/registrar/registrar.go

@@ -38,6 +38,24 @@ func NewRegistrar() *Registrar {
 	}
 }
 
+type ReservedErr struct {
+	Field string
+	Name  string
+	Key   string
+}
+
+func (e *ReservedErr) Error() string {
+	switch e.Field {
+	case "name":
+		return fmt.Sprintf("name %q is reserved for %q", e.Name, e.Key)
+	case "key":
+		return fmt.Sprintf("key %q is reserved for %q", e.Key, e.Name)
+	}
+	return fmt.Sprintf("name %q is reserved for %q", e.Name, e.Key)
+}
+
+func (e *ReservedErr) Conflict() {}
+
 // Reserve registers a name<->key mapping, name or key must not
 // be empty.
 // Reserve is idempotent.
@@ -54,14 +72,14 @@ func (r *Registrar) Reserve(name, key string) error {
 
 	if k, exists := r.nameToKey[name]; exists {
 		if k != key {
-			return fmt.Errorf("name %q is reserved for %q", name, k)
+			return &ReservedErr{Field: "name", Name: name, Key: k}
 		}
 		return nil
 	}
 
 	if n, exists := r.keyToName[key]; exists {
 		if n != name {
-			return fmt.Errorf("key %q is reserved for %q", key, n)
+			return &ReservedErr{Field: "key", Name: n, Key: key}
 		}
 		return nil
 	}

internal/registrar/registrar_test.go

@@ -25,6 +25,8 @@ import (
 func TestRegistrar(t *testing.T) {
 	r := NewRegistrar()
 	assert := assertlib.New(t)
+	var err error
+	var resErr *ReservedErr
 
 	t.Logf("should be able to reserve a name<->key mapping")
 	assert.NoError(r.Reserve("test-name-1", "test-id-1"))
@@ -36,8 +38,14 @@ func TestRegistrar(t *testing.T) {
 	assert.NoError(r.Reserve("test-name-1", "test-id-1"))
 
 	t.Logf("should not be able to reserve conflict name<->key mapping")
-	assert.Error(r.Reserve("test-name-1", "test-id-conflict"))
-	assert.Error(r.Reserve("test-name-conflict", "test-id-2"))
+	err = r.Reserve("test-name-1", "test-id-conflict")
+	assert.Error(err)
+	assert.ErrorAs(err, &resErr)
+	assert.ErrorContains(resErr, `name "test-name-1" is reserved for "test-id-1"`)
+	err = r.Reserve("test-name-conflict", "test-id-2")
+	assert.Error(err)
+	assert.ErrorAs(err, &resErr)
+	assert.ErrorContains(resErr, `key "test-id-2" is reserved for "test-name-2"`)
 
 	t.Logf("should be able to release name<->key mapping by key")
 	r.ReleaseByKey("test-id-1")