Handle error scenarios gracefully in NodeUnpublishVolume

[swamibluedata]

This fixes

#158

Issue unmount if and only if the target folder is present and is a valid mount point.

Issue Remove call if and only if the target folder exists

If the mount point doesn't exist, report success, which will allow kubelet to continue with pod termination sequence

Signed-off-by: Swami Viswanathan <[email protected]>

[swamibluedata]

Addressed a PR comment from the closed PR

#160

[azdagron]

Hey @swamibluedata! Thanks for this contribution! If you could fix the DCO on your commit, that would be great. You can click on the "details" link to the right of the failed action.

In your case it looks like the Signed-off-by is set to one email, but the author of the commit is another. You may need to do a git commit --amend -s --reset-author to fix the commit before force pushing.

[azdagron]

Looks like at least one unit test is also failing and may need to be updated based on the new behavior.

The failing integration test is unrelated (the v1.21.14 test is failing across the board at the moment).

[swamibluedata]

Hey @swamibluedata! Thanks for this contribution! If you could fix the DCO on your commit, that would be great. You can click on the "details" link to the right of the failed action.

In your case it looks like the Signed-off-by is set to one email, but the author of the commit is another. You may need to do a git commit --amend -s --reset-author to fix the commit before force pushing.

Got it and will need to address the test case as well.

[swamibluedata]

Looks like at least one unit test is also failing and may need to be updated based on the new behavior.

The failing integration test is unrelated (the v1.21.14 test is failing across the board at the moment).

Both the new "unable to verify mount point" error and an existing error "unable to unmount" are going to be tricky to test.

For "unable to verify mount point", i need to make enumerateMounts fail
For "unable to mount", i can run a process that opens a file on that mountpoint which will prevent unmount

Thinking

[azdagron]

The unit tests for the driver package replace the various functions with those where they can inject different conditions. That's probably good enough for the unit tests. At some point we'll wire up more test cases into the integration test, which is running in a kind cluster, but for now I think unit test coverage give us the same test fidelity we have now.

[swamibluedata]

Hey @swamibluedata! Thanks for this contribution! If you could fix the DCO on your commit, that would be great. You can click on the "details" link to the right of the failed action.

In your case it looks like the Signed-off-by is set to one email, but the author of the commit is another. You may need to do a git commit --amend -s --reset-author to fix the commit before force pushing.

Updated the commit, still complaining :(

37bb8fa

[azdagron]

Looks like the last thing to fix is a linting error. You need to run goimports on one of the files you edited (setting up your editor to do this automatically when saving is usually the easiest path).

[swamibluedata]

@azdagron anything i need to address

[azdagron]

Thanks, @swamibluedata! Good to go.