Skip to content
This repository has been archived by the owner on Nov 9, 2020. It is now read-only.

Adds support for whitespace chars in Datastore names on Windows #1745

Merged
merged 2 commits into from
Aug 17, 2017

Conversation

venilnoronha
Copy link
Contributor

@venilnoronha venilnoronha commented Aug 11, 2017

This PR adds handling to correctly map volumes with whitespace characters in its name. Testing was done manually i.e. a volume was created with docker volume create --driver=vsphere "newvolume@local   .3 -   0", and attached with docker run -it -v "newvolume@local   .3 -   0:C:/mountpoint/" microsoft/nanoserver powershell, and the following log was observed.

time="2017-08-11T17:26:39Z" level=info msg="Successfully retrieved mounts" map=map[newvolume@local   .3 -   0:4]

See #1724 for original issue.

This commit adds handling to correctly map volumes with whitespace
characters in its name.
Copy link
Contributor

@msterin msterin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the fix is OK but please do add automated test. We have volume name checking automation in unit tests, so just add a line or two there with volumes with spaces.

@venilnoronha
Copy link
Contributor Author

venilnoronha commented Aug 11, 2017

@msterin addressed your comment in 50c5d67.

Copy link
Contributor

@pshahzeb pshahzeb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Please update the specifics of manual testing. Through workflows like docker volume create and docker run.
Always convincing to see the command outputs.

@venilnoronha
Copy link
Contributor Author

@pshahzeb thanks! I've updated the test details.

Copy link
Contributor

@govint govint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks ok, although could add test logs for problem cases and show how the fix is working fine to handle those.

@venilnoronha venilnoronha changed the title Adds support for whitespace chars in volume names on Windows Adds support for whitespace chars in Datastore names on Windows Aug 14, 2017
@venilnoronha venilnoronha merged commit 933e463 into vmware-archive:master Aug 17, 2017
shuklanirdesh82 pushed a commit that referenced this pull request Aug 18, 2017
This commit adds handling to correctly map volumes with whitespace characters in its name.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants