Merge pull request #12137 from vrothberg/fix-11735

pod/container create: resolve conflicts of generated names

3 files changed, 47 insertions, 26 deletions

diff --git a/libpod/container_internal.go b/libpod/container_internal.go
index 19b48e14b..fbc2c1f38 100644
--- a/libpod/container_internal.go
+++ b/libpod/container_internal.go
@@ -496,9 +496,27 @@ func (c *Container) setupStorage(ctx context.Context) error {
 
 	c.setupStorageMapping(&options.IDMappingOptions, &c.config.IDMappings)
 
-	containerInfo, err := c.runtime.storageService.CreateContainerStorage(ctx, c.runtime.imageContext, c.config.RootfsImageName, c.config.RootfsImageID, c.config.Name, c.config.ID, options)
-	if err != nil {
-		return errors.Wrapf(err, "error creating container storage")
+	// Unless the user has specified a name, use a randomly generated one.
+	// Note that name conflicts may occur (see #11735), so we need to loop.
+	generateName := c.config.Name == ""
+	var containerInfo ContainerInfo
+	var containerInfoErr error
+	for {
+		if generateName {
+			name, err := c.runtime.generateName()
+			if err != nil {
+				return err
+			}
+			c.config.Name = name
+		}
+		containerInfo, containerInfoErr = c.runtime.storageService.CreateContainerStorage(ctx, c.runtime.imageContext, c.config.RootfsImageName, c.config.RootfsImageID, c.config.Name, c.config.ID, options)
+
+		if !generateName || errors.Cause(containerInfoErr) != storage.ErrDuplicateName {
+			break
+		}
+	}
+	if containerInfoErr != nil {
+		return errors.Wrapf(containerInfoErr, "error creating container storage")
 	}
 
 	// only reconfig IDMappings if layer was mounted from storage
diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go
index 0a7db33f1..114bf9315 100644
--- a/libpod/runtime_ctr.go
+++ b/libpod/runtime_ctr.go
@@ -326,15 +326,6 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Contai
 		}
 	}
 
-	if ctr.config.Name == "" {
-		name, err := r.generateName()
-		if err != nil {
-			return nil, err
-		}
-
-		ctr.config.Name = name
-	}
-
 	// Check CGroup parent sanity, and set it if it was not set.
 	// Only if we're actually configuring CGroups.
 	if !ctr.config.NoCgroups {
diff --git a/libpod/runtime_pod_linux.go b/libpod/runtime_pod_linux.go
index 7d7fef4d1..15050ef48 100644
--- a/libpod/runtime_pod_linux.go
+++ b/libpod/runtime_pod_linux.go
@@ -43,18 +43,6 @@ func (r *Runtime) NewPod(ctx context.Context, p specgen.PodSpecGenerator, option
 		}
 	}
 
-	if pod.config.Name == "" {
-		name, err := r.generateName()
-		if err != nil {
-			return nil, err
-		}
-		pod.config.Name = name
-	}
-
-	if p.InfraContainerSpec != nil && p.InfraContainerSpec.Hostname == "" {
-		p.InfraContainerSpec.Hostname = pod.config.Name
-	}
-
 	// Allocate a lock for the pod
 	lock, err := r.lockManager.AllocateLock()
 	if err != nil {
@@ -131,9 +119,33 @@ func (r *Runtime) NewPod(ctx context.Context, p specgen.PodSpecGenerator, option
 		logrus.Infof("Pod has an infra container, but shares no namespaces")
 	}
 
-	if err := r.state.AddPod(pod); err != nil {
-		return nil, errors.Wrapf(err, "error adding pod to state")
+	// Unless the user has specified a name, use a randomly generated one.
+	// Note that name conflicts may occur (see #11735), so we need to loop.
+	generateName := pod.config.Name == ""
+	var addPodErr error
+	for {
+		if generateName {
+			name, err := r.generateName()
+			if err != nil {
+				return nil, err
+			}
+			pod.config.Name = name
+		}
+
+		if p.InfraContainerSpec != nil && p.InfraContainerSpec.Hostname == "" {
+			p.InfraContainerSpec.Hostname = pod.config.Name
+		}
+		if addPodErr = r.state.AddPod(pod); addPodErr == nil {
+			return pod, nil
+		}
+		if !generateName || (errors.Cause(addPodErr) != define.ErrPodExists && errors.Cause(addPodErr) != define.ErrCtrExists) {
+			break
+		}
+	}
+	if addPodErr != nil {
+		return nil, errors.Wrapf(addPodErr, "error adding pod to state")
 	}
+
 	return pod, nil
 }