-
Notifications
You must be signed in to change notification settings - Fork 422
[18.09 backport] Add TC to check dynamic subnet for ingress network #417
[18.09 backport] Add TC to check dynamic subnet for ingress network #417
Conversation
Signed-off-by: Arko Dasgupta <arko.dasgupta@docker.com> (cherry picked from commit 084f5ab) Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (assuming that TestServiceWithDefaultAddressPoolInit is not failing)
[2019-10-23T17:08:36.806Z] integration/network/service_test.go:369:32:warning: undeclared name: ctx (gosimple) |
assert.Equal(t, out.IPAM.Config[0].Subnet, "20.20.1.0/24") | ||
|
||
// Also inspect ingress network and make sure its in the same subnet | ||
out, err = cli.NetworkInspect(ctx, "ingress", types.NetworkInspectOptions{Verbose: true}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[2019-10-23T17:08:36.806Z] integration/network/service_test.go:369:32:warning: undeclared name: ctx (gosimple)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess s/ctx/context.Background()/
or backport moby#38557
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hm.. no, looks like we need moby#39671
.... which is in the other PR 😂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or just commit 56a68c1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, that one is already backported
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hm.. perhaps it was not 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
arf; likely also moby#39332, which is also in the other PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need a fixup
this was included in #401 |
backport of the test from moby#39966, because the related swarmkit changes were included in #346