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.

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

Fix port forwarding collisions in parallel run of multi-machine env #226

Merged
merged 2 commits into from
Nov 5, 2015

Conversation

legal90
Copy link
Collaborator

@legal90 legal90 commented Nov 3, 2015

While doing a parallel run of multi-machine environment the race condition could happen:

...
There was an error while command execution. The command and stderr is shown below.

Command: ["/usr/local/bin/prlsrvctl", "net", "set", "Shared", "--nat-tcp-add", "winrm,2200,8977eda8-08ae-46d3-8923-469d8091ad69,5985", "--nat-tcp-add", "winrm-ssl,2201,8977eda8-08ae-46d3-8923-469d8091ad69,5986"]

Stderr: The rule with 'winrm' name already exists!

This PR fixes that by adding adds both class- and process-level locks, so port forwarding will be proceeded subsequently.

Also, I've improved the forward_ports helper by decreasing the number of driver method calls. Now it should work a little bit faster.

cc: @Gray-Wind @racktear @Kasen

It much easier and faster just to avoid the concurrency in this place.
It guarantees that we will not create some port forwarding rules with the same names
Driver methods are slow. Since "forward_ports" action is now parallel-safe,
we can call driver method "read_forwarded_ports" only once per each machine.
@Gray-Wind
Copy link
Contributor

👍

legal90 added a commit that referenced this pull request Nov 5, 2015
Fix port forwarding collisions in parallel run of multi-machine env
@legal90 legal90 merged commit b195868 into Parallels:master Nov 5, 2015
@legal90 legal90 deleted the fix-fp-collisions branch November 5, 2015 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants