Skip to content

Adding synchronization around peerAdd and peerDelete to prevent network connectivity issue#2356

Merged
mavenugo merged 1 commit intomoby:masterfrom
pradipd:RS1-Synch
Apr 6, 2019
Merged

Adding synchronization around peerAdd and peerDelete to prevent network connectivity issue#2356
mavenugo merged 1 commit intomoby:masterfrom
pradipd:RS1-Synch

Conversation

@pradipd
Copy link
Contributor

@pradipd pradipd commented Apr 5, 2019

When multiple networks are present in a Swarm Cluster, multiple peerAdd or peerDelete calls are an issue for different remote endpoints. These threads are updating the remote endpoint to HNS parallelly. In 2016 HNS code base, we don't have synchronization around remoteEndpoint addition and deletion. So serializing the peerAdd and peerDelete calls from docker network driver.

@GordonTheTurtle
Copy link

Please sign your commits following these rules:
https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "RS1-Synch" git@github.com:pradipd/libnetwork.git somewhere
$ cd somewhere
$ git commit --amend -s --no-edit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

@pradipd
Copy link
Contributor Author

pradipd commented Apr 5, 2019

FYI to docker team: @mavenugo @danil @selansen
FYI to windows team: @madhanrm @nwoodmsft @dineshgovindasamy

Copy link

@dineshgovindasamy dineshgovindasamy left a comment

Choose a reason for hiding this comment

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

LGTM

…rk connectivity issue

Signed-off-by: Pradip Dhara <pradipd@microsoft.com>
}

func endpointRequest(method, path, request string) (*hcsshim.HNSEndpoint, error) {
if windowsBuild == 14393 {
Copy link
Contributor

Choose a reason for hiding this comment

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

@pradipd is this a fixed version for WS 2016 server or will this change for every patch ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed version. We (windows) should really have a constant for this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks. We see it as a hcsshim constant osversion.RS5. Thanks.

Copy link
Contributor

@mavenugo mavenugo left a comment

Choose a reason for hiding this comment

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

LGTM

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.

5 participants