Volume create - fast exit when ignore is set and volume exists

Signed-off-by: Ygal Blum <[email protected]>
containers · Oct 1, 2023 · 979c77f · 979c77f
1 parent cc8c28e
commit 979c77f
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 13 deletions.
diff --git a/libpod/runtime_volume_common.go b/libpod/runtime_volume_common.go
@@ -54,12 +54,18 @@ func (r *Runtime) newVolume(ctx context.Context, noCreatePluginVolume bool, opti
 	volume.config.CreatedTime = time.Now()
 
 	// Check if volume with given name exists.
-	if !volume.ignoreIfExists {
-		exists, err := r.state.HasVolume(volume.config.Name)
-		if err != nil {
-			return nil, fmt.Errorf("checking if volume with name %s exists: %w", volume.config.Name, err)
-		}
-		if exists {
+	exists, err := r.state.HasVolume(volume.config.Name)
+	if err != nil {
+		return nil, fmt.Errorf("checking if volume with name %s exists: %w", volume.config.Name, err)
+	}
+	if exists {
+		if volume.ignoreIfExists {
+			existingVolume, err := r.state.Volume(volume.config.Name)
+			if err != nil {
+				return nil, fmt.Errorf("reading volume from state: %w", err)
+			}
+			return existingVolume, nil
+		} else {
 			return nil, fmt.Errorf("volume with name %s already exists: %w", volume.config.Name, define.ErrVolumeExists)
 		}
 	}
@@ -219,13 +225,6 @@ func (r *Runtime) newVolume(ctx context.Context, noCreatePluginVolume bool, opti
 
 	// Add the volume to state
 	if err := r.state.AddVolume(volume); err != nil {
-		if volume.ignoreIfExists && errors.Is(err, define.ErrVolumeExists) {
-			existingVolume, err := r.state.Volume(volume.config.Name)
-			if err != nil {
-				return nil, fmt.Errorf("reading volume from state: %w", err)
-			}
-			return existingVolume, nil
-		}
 		return nil, fmt.Errorf("adding volume to state: %w", err)
 	}
 	defer volume.newVolumeEvent(events.Create)

diff --git a/test/system/160-volumes.bats b/test/system/160-volumes.bats
@@ -574,5 +574,30 @@ EOF
     run_podman volume rm myvol --force
 }
 
+@test "podman volume create --ignore - do not chown" {
+    local user_id=2000
+    local group_id=2000
+    local volume_name=$(random_string)
+
+    # Create a volume and get its mount point
+    run_podman volume create --ignore ${volume_name}
+    run_podman volume inspect --format '{{.Mountpoint}}' $volume_name
+    mountpoint="$output"
+
+    # Run a container with the volume mounted
+    run_podman run --rm --user ${group_id}:${user_id} -v ${volume_name}:/vol $IMAGE
+
+    # Podman chowns the mount point according to the user used in the previous command
+    local original_owner=$(stat --format %g:%u ${mountpoint})
+
+    # Creating an existing volume with ignore should be a noop
+    run_podman volume create --ignore ${volume_name}
+
+    # Verify that the mountpoint was not chowned
+    owner=$(stat --format %g:%u ${mountpoint})
+    is "$owner" "${original_owner}" "The volume was chowned by podman volume create"
+
+    run_podman volume rm $volume_name --force
+}
 
 # vim: filetype=sh