csi: NodePublish should not create target_path, only its parent dir (#…

…8505)

The NodePublish workflow currently creates the target path and its parent
directory. However, the CSI specification says that the CO shall ensure the
parent directory of the target path exists, and that the SP shall place the
block device or mounted directory at the target path. Much of our testing has
been with CSI plugins that are more forgiving, but our behavior breaks
spec-compliant CSI plugins.

This changeset ensures we only create the parent directory.

client/pluginmanager/csimanager/volume.go

@@ -67,7 +67,11 @@ func (v *volumeManager) stagingDirForVolume(root string, volID string, usage *Us
 	return filepath.Join(root, StagingDirName, volID, usage.ToFS())
 }
 
-func (v *volumeManager) allocDirForVolume(root string, volID, allocID string, usage *UsageOptions) string {
+func (v *volumeManager) allocDirForVolume(root string, volID, allocID string) string {
+	return filepath.Join(root, AllocSpecificDirName, allocID, volID)
+}
+
+func (v *volumeManager) targetForVolume(root string, volID, allocID string, usage *UsageOptions) string {
 	return filepath.Join(root, AllocSpecificDirName, allocID, volID, usage.ToFS())
 }
 
@@ -103,21 +107,22 @@ func (v *volumeManager) ensureStagingDir(vol *structs.CSIVolume, usage *UsageOpt
 // Returns whether the directory is a pre-existing mountpoint, the publish path,
 // and any errors that occurred.
 func (v *volumeManager) ensureAllocDir(vol *structs.CSIVolume, alloc *structs.Allocation, usage *UsageOptions) (string, bool, error) {
-	allocPath := v.allocDirForVolume(v.mountRoot, vol.ID, alloc.ID, usage)
+	allocPath := v.allocDirForVolume(v.mountRoot, vol.ID, alloc.ID)
 
 	// Make the alloc path, owned by the Nomad User
 	if err := os.MkdirAll(allocPath, 0700); err != nil && !os.IsExist(err) {
 		return "", false, fmt.Errorf("failed to create allocation directory for volume (%s): %v", vol.ID, err)
 	}
 
-	// Validate that it is not already a mount point
+	// Validate that the target is not already a mount point
+	targetPath := v.targetForVolume(v.mountRoot, vol.ID, alloc.ID, usage)
 	m := mount.New()
-	isNotMount, err := m.IsNotAMountPoint(allocPath)
+	isNotMount, err := m.IsNotAMountPoint(targetPath)
 	if err != nil {
 		return "", false, fmt.Errorf("mount point detection failed for volume (%s): %v", vol.ID, err)
 	}
 
-	return allocPath, !isNotMount, nil
+	return targetPath, !isNotMount, nil
 }
 
 func volumeCapability(vol *structs.CSIVolume, usage *UsageOptions) (*csi.VolumeCapability, error) {
@@ -192,7 +197,7 @@ func (v *volumeManager) publishVolume(ctx context.Context, vol *structs.CSIVolum
 	if err != nil {
 		return nil, err
 	}
-	pluginTargetPath := v.allocDirForVolume(v.containerMountPoint, vol.ID, alloc.ID, usage)
+	pluginTargetPath := v.targetForVolume(v.containerMountPoint, vol.ID, alloc.ID, usage)
 
 	if isMount {
 		logger.Debug("Re-using existing published volume for allocation")
@@ -296,7 +301,7 @@ func combineErrors(maybeErrs ...error) error {
 }
 
 func (v *volumeManager) unpublishVolume(ctx context.Context, volID, remoteID, allocID string, usage *UsageOptions) error {
-	pluginTargetPath := v.allocDirForVolume(v.containerMountPoint, volID, allocID, usage)
+	pluginTargetPath := v.targetForVolume(v.containerMountPoint, volID, allocID, usage)
 
 	// CSI NodeUnpublishVolume errors for timeout, codes.Unavailable and
 	// codes.ResourceExhausted are retried; all other errors are fatal.
@@ -306,7 +311,7 @@ func (v *volumeManager) unpublishVolume(ctx context.Context, volID, remoteID, al
 		grpc_retry.WithBackoff(grpc_retry.BackoffExponential(100*time.Millisecond)),
 	)
 
-	hostTargetPath := v.allocDirForVolume(v.mountRoot, volID, allocID, usage)
+	hostTargetPath := v.targetForVolume(v.mountRoot, volID, allocID, usage)
 	if _, err := os.Stat(hostTargetPath); os.IsNotExist(err) {
 		if rpcErr != nil && strings.Contains(rpcErr.Error(), "no mount point") {
 			// host target path was already destroyed, nothing to do here.