Bump swarmkit to 24fb4cfe8af56803640180c5592bf32da732ced2#40309
Bump swarmkit to 24fb4cfe8af56803640180c5592bf32da732ced2#40309thaJeztah merged 1 commit intomoby:masterfrom
Conversation
|
@dperny there's also #39966, which updates the test (that commit is actually already cherry-picked into release branches, so if it's the same change, I slightly prefer having that change instead of a new variation). However, that PR kept failing on the swarmkit changes; did anything change in swarmkit to fix that (and do we know what was fixed/changed?) |
dperny
force-pushed
the
bump-swarmkit
branch
from
ada4ddf to
ced23b3
Compare
dperny
force-pushed
the
bump-swarmkit
branch
from
ced23b3 to
bfec909
Compare
|
I'm 80% sure the test failure is in |
|
I'm 100% sure the problem is |
|
@dperny moby/swarmkit#2920 was merged, could you
|
dperny
changed the title
|
@thaJeztah I fixed it. Additionally, I hand-copied the test changes from #39966. |
dperny
force-pushed
the
bump-swarmkit
branch
2 times, most recently
from
05e7f2e to
8356566
Compare
Bumps the vendoring of github.com/docker/swarmkit to the above commit, which is the current master at commit time. Most notably, this includes a change making the ingress network respect the default address pool. Because of this change, a change to network integration tests was needed. Signed-off-by: Drew Erny <[email protected]>
|
I think there may be some flakiness, as the last force push succeeded, but the previous on did not. I'm going to force a few more times to verify that it was a transient issue and not a flaky test. |
thaJeztah
left a comment
thaJeztah
left a comment
There was a problem hiding this comment.
LGTM left a comment (not a blocker, but perhaps good to look at for a follow-up)
| poll.WaitOn(t, swarm.NoTasksForService(ctx, client, serviceID2), swarm.ServicePoll) | ||
|
|
||
| err = client.NetworkRemove(context.Background(), overlayID) | ||
| for retry := 0; retry < 5; retry++ { |
There was a problem hiding this comment.
I guess it's ok "for now", but wondering if:
- this should come handy in the
integration/internal/networkpackage - is needed in the actual daemon (as this would likely not be something only occurring in tests)
3 participants
Full diff: docker/[email protected]
changes included:
Notably, because of moby/swarmkit#2890, a change to integration tests is needed. This change has been documented in a comment.
Supercedes #39953
/cc @thaJeztah