moby / swarmkit

A toolkit for orchestrating distributed systems at any scale. It includes primitives for node discovery, raft-based consensus, task scheduling and more.
Apache License 2.0
3.36k stars 615 forks source link

allocator: Consolidate port update logic #2143

Open aaronlehmann opened 7 years ago

aaronlehmann commented 7 years ago

There are a number of overlapping places where the ports are compared between s.Endpoint and s.Spec.Endpoint, and possibly updated:

I find these different functions confusing, and we've had bugs before when they behaved inconsistently or weren't correct. It's especially problematic because the host-published-port-specific functions have to avoid interfering with ingress ports.

I think there should be a single function that reconciles s.Endpoint.Ports with s.Endpoint.Spec.Ports. If any ingress ports change, these should be deallocated/allocated as necessary. If nothing at all changes, we should skip updating the service in the store.

I think this will be easier to maintain than separate paths for general port allocation and only updating host-published ports.

cc @aboch @yongtang @abhinandanpb

aaronlehmann commented 7 years ago

cc @ijc25

aaronlehmann commented 7 years ago

Note that portsAllocatedInHostPublishMode was renamed to hostPublishPortsNeedUpdate.