VXLAN UDP Port configuration support #38102
Conversation
b0bacf0 to
15e8444
Compare
612afd7 to
a0bf6c2
Compare
Codecov Report
@@ Coverage Diff @@
## master #38102 +/- ##
==========================================
+ Coverage 36.1% 36.1% +<.01%
==========================================
Files 610 610
Lines 45234 45247 +13
==========================================
+ Hits 16333 16338 +5
- Misses 26661 26673 +12
+ Partials 2240 2236 -4 |
5ddd374 to
4139842
Compare
|
@thaJeztah PTAL. |
237b73c to
828d8b8
Compare
|
Below Failures doesn't seem related to my PR 03:53:17 curl: (18) transfer closed with 5073312 bytes remaining to read Experimental: Z |
|
Will look into it
…On Wed, Nov 21, 2018 at 7:18 PM Tibor Vass ***@***.***> wrote:
@selansen <https://github.com/selansen> vendor.conf conflict!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#38102 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AKEKn0gDpA5tPe_6eJ0OUxrDuoIaYyJ2ks5uxe1LgaJpZM4YAqNb>
.
|
|
@thaJeztah @tiborvass PTAL |
internal/test/daemon/swarm.go
Outdated
There was a problem hiding this comment.
Didn't notice this change really, why was this needed? If no port is specified, won't SwarmKit / libnetwork still set the default automatically? (if not, that would be a problem, because older clients won't set this value).
There was a problem hiding this comment.
This is for testing new option DataPathPort. You are right, if port is not specified , libnetwork will use default value. if no port is specified , here the value ill be 0. also this test is going to run only on API version 40 and above.
There was a problem hiding this comment.
But this is setting the port now by default, so even if no port is specified in the test, a port is send in the request.
Point is that this utility is used by at least four tests (TestAPISwarmJoinToken, TestUpdateSwarmAddExternalCA, TestAPISwarmPromoteDemote, TestAPISwarmForceNewCluster), and because it's called by StartAndSwarmInit (which is called by AddDaemon and NewSwarm) is used by 130+ other tests, so all of those tests will now change, correct?
There was a problem hiding this comment.
got it. Sending 0 is more or less equal to setting no value. But I got your point. I will add if check to make sure we add DataPort option only if it is set.
There was a problem hiding this comment.
Hm, so I think my confusion here is that we're overriding the initRequest with a daemon configuration; looking a bit further;
So, this PR adds a daemon option DataPathPort , so that the port gets automatically set when calling swarm.NewSwarm();
moby/integration/internal/swarm/service.go
Lines 50 to 61 in 5d82d77
I think the issue lies there;
- Unlike (e.g.)
storageDriver,DataPathPortis not a daemon configuration - The
DataPathPortmust be given when callingSwarmInit
So the problem is that swarm.NewSwarm() is doing to much for this test; possible resolutions for that;
- Add a
swarmops ...func(*swarm.InitRequest)toswarm.NewSwarm()(so that the options to initialize the swarm can be passed) - Don't use
swarm.NewSwarm()in this test, and first create the daemon, start it (d.StartNode()), then call `
d := daemon.New(t)
d.StartNode(t)
d.SwarmInit(t, swarm.InitRequest{DataPathPort: 12345})There was a problem hiding this comment.
Looks like we've taken the same approach for DefaultAddrPool, which also is not a daemon option (but something to set on swarm init).
We should fix that, but let's do that in a follow-up
8606902 to
9486762
Compare
This commit brings Swarmkit and Libnetwork library changes Signed-off-by: selansen <elango.siva@docker.com>
This commit contains changes to configure DataPathPort option. By default we use 4789 port number. But this commit will allow user to configure port number during swarm init. DataPathPort can't be modified after swarm init. Signed-off-by: selansen <elango.siva@docker.com>
|
Failures unrelated to this PR: FAIL: docker_cli_swarm_test.go:1128: DockerSwarmSuite.TestSwarmLockUnlockCluster @tiborvass , @crosbymichael PTAL. |
|
All green; ignoring the codecov/patch one |
|
Thanks @thaJeztah @tiborvass . Let me update CLI PR n ping you soon. |
|
@selansen first of all, thank you for your work. I am faced with this problem and I just pulled myself a "nightly" which already shows your work. One maybe strange question on my part ist, what about the docker swarm join command? I am not able to specify the vxlan port here. Does that bother? Will my vxlan magiacally get the right port? |
|
@sgutermann you don't need to pass vxlan port during swarm join. swarm init is the only place you will have to pass vxlan port number. During swarm join event , swarm manager will propagate vxlan information to the joining node. It's all taken care so user doesn't need to remember each time when they invoke swarm join. |
Signed-off-by: selansen elango.siva@docker.com
- What I did
- How I did it
- How to verify it
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)