Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 2 commits into from
Jul 23, 2020
Merged

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

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a016d06
csi: NodePublish should not create target_path, only its parent dir
tgross Jul 22, 2020
e5bdcda
fix hosttarget path
tgross Jul 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 13 additions & 8 deletions client/pluginmanager/csimanager/volume.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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.
Expand All @@ -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.
Expand Down