Add GetClosestVolumeIDFromTargetPath API to the Volume API Group

[mauriciopoppe]

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespaces from that line:

/kind api-change
/kind feature

What this PR does / why we need it:
Adds the new Volume API, GetClosestVolumeIDFromTargetPath, it allows getting the closest volume from a target path handling more scenarios as described below.

The algorithm is:

• given a target path do the following for a max of 256 iterations (1)
  • if it's a symlink and has the form Volume{ then its a volume, return it
  • if it's a symlink and points to a location that exists then follow it and go to 1
  • if it's not a symlink then move up one level in the hierarchy and go to 1
  • if after moving one level up we get to the same location then we're at the root, return the volume corresponding to the boot disk

In the following example all the calls to GetClosestVolumeIDFromTargetPath are with paths inside /mnt/disks:

/               (Volume{A}) # boot disk
/foo/bar
/mnt/volume/  (symlink -> Volume{B})
  shared-disk/
     foo/bar/shared0/
     shared1/
/mnt/single0/ (symlink -> Volume{C})  # standalone disk
/mnt/disks/ (discovery directory)
  shared0/ (symlink -> /mnt/volume/shared-disk/foo/bar/shared0) # returns Volume{B}
  shared1/ (symlink -> /mnt/volume/shared-disk/shared1) # returns Volume{B}
  single0/ (symlink -> /mnt/single0) # returns Volume{C}
  volume0/ (symlink -> Volume{D}) # returns Volume{D}
  boot0/   (symlink -> /foo/bar) # returns Volume{A}

Does this PR introduce a user-facing change?:

Add Volume API GetClosestVolumeIDFromTargetPath to find the closest Volume from a given path.

/hold

Depends on #188

/cc @jingxu97 @ddebroy @msau42

[ddebroy]

Recommend tightening the prefix check for Volume. I think the Volume prefix check in k/k should also be tightened with a regex.

[ddebroy]

I feel the Volume prefix check can be tightened with a regex to exactly match Volume{...} format. If a disk layout has symlink chains with links named VolumeData1 and VolumeData2 that are actually links to Volume{A} and Volume{B}, the HasPrefix logic would return incorrect results.

[ddebroy]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ddebroy, mauriciopoppe

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [ddebroy]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[mauriciopoppe]

/test pull-kubernetes-csi-csi-proxy-integration

[mauriciopoppe]

/test pull-kubernetes-csi-csi-proxy-integration

[mauriciopoppe]

/test pull-kubernetes-csi-csi-proxy-integration

[mauriciopoppe]

/hold cancel

pull-kubernetes-csi-csi-proxy-integration passed, @jingxu97 it's now ready for review

[jingxu97]

not sure it is the case, sometimes the error is empty, but command actually return some error messages, so targetb will be some strings. In this case just want to make sure the logic can handle it.

[mauriciopoppe]

true, cmd.Output returns the combined output, places where this is called ensure that the path sent is a symlink however another way to test it would be by analyzing stderr apart from stdout.

The existing GetVolumeIDFromTargetPath doesn't do a separate analysis either, it's using the combined output and relying on err for errors

[jingxu97]

I just wondering whether it is possible to make sure it return a valid path or a volume id, instead of some long string

[mauriciopoppe]

I think that if the target doesn't exist it'll return an error that's propagated, so at this point we know that it's a path or a volumeId, for anything in stderr I'm not checking though, I'll only analyze the output of stdout and error if stderr has contents

[jingxu97]

should we check prefix just in case before adding it?

[jingxu97]

/lgtm