daemon: build ports-related ep options in a dedicated func

The `buildCreateEndpointOptions` does a lot of things to build the list
of `libnetwork.EndpointOption` from the `EndpointSettings` spec. To skip
ports-related options, an early return was put in the middle of that
function body.

Early returns are generally great, but put in the middle of a 150-loc
long function that does a lot, they're just a potential footgun. And I'm
the one who pulled the trigger in 052562f. Since this commit, generic
options won't be applied to endpoints if there's already one with
exposed/published ports. As a consequence, only the first endpoint can
have a user-defined MAC address right now.

Instead of moving up the code line that adds generic options, a better
change IMO is to move ports-related options, and the early-return gating
those options, to a dedicated func to make `buildCreateEndpointOptions`
slightly easier to read and reason about.

There was actually one oddity in the original
`buildCreateEndpointOptions`: the early-return also gates the addition
of `CreateOptionDNS`. These options are Windows-specific; a comment is
added to explain that. But the oddity is really: why are we checking if
an endpoint with exposed / published ports joined this sandbox to decide
whether we want to configure DNS server on the endpoint's adapter? Well,
this early-return was most probably overlooked by the original author
and by reviewers at the time these options were added (in commit d1e0a78)

Let's fix that in a follow-up commit.

Signed-off-by: Albin Kerouanton <albinker@gmail.com>

daemon/network.go

@@ -862,11 +862,43 @@ func buildCreateEndpointOptions(c *container.Container, n *libnetwork.Network, e
 		createOptions = append(createOptions, libnetwork.CreateOptionDisableResolution())
 	}
 
+	opts, err := buildPortsRelatedCreateEndpointOptions(c, n, sb)
+	if err != nil {
+		return nil, err
+	}
+	createOptions = append(createOptions, opts...)
+
+	// On Windows, DNS config is a per-adapter config option whereas on Linux, it's a sandbox-wide parameter; hence why
+	// we're dealing with DNS config both here and in buildSandboxOptions. Following DNS options are only honored by
+	// Windows netdrivers, whereas DNS options in buildSandboxOptions are only honored by Linux netdrivers.
+	//
+	// Now that being said, you might ask: why is this cond checking whether there's already an endpoint with exposed /
+	// published ports tied to the container sandbox? Isn't that logic flawed? Well, probably it is! These DNS options
+	// were added by d1e0a78 at the end of buildCreateEndpointOptions. The fact that it was added *after* an
+	// early-return checking exposed / published ports was most probably overlooked by the original author and
+	// reviewers.
+	// TODO(aker): fix this ^
+	if !n.Internal() && len(getPortMapInfo(sb)) == 0 {
+		if len(c.HostConfig.DNS) > 0 {
+			createOptions = append(createOptions, libnetwork.CreateOptionDNS(c.HostConfig.DNS))
+		} else if len(daemonDNS) > 0 {
+			createOptions = append(createOptions, libnetwork.CreateOptionDNS(daemonDNS))
+		}
+	}
+
+	createOptions = append(createOptions, libnetwork.EndpointOptionGeneric(genericOptions))
+
+	return createOptions, nil
+}
+
+// buildPortsRelatedCreateEndpointOptions returns the appropriate endpoint options to apply config related to port
+// mapping and exposed ports.
+func buildPortsRelatedCreateEndpointOptions(c *container.Container, n *libnetwork.Network, sb *libnetwork.Sandbox) ([]libnetwork.EndpointOption, error) {
 	// Port-mapping rules belong to the container & applicable only to non-internal networks.
 	//
 	// TODO(thaJeztah): Look if we can provide a more minimal function for getPortMapInfo, as it does a lot, and we only need the "length".
 	if n.Internal() || len(getPortMapInfo(sb)) > 0 {
-		return createOptions, nil
+		return nil, nil
 	}
 
 	bindings := make(nat.PortMap)
@@ -927,18 +959,10 @@ func buildCreateEndpointOptions(c *container.Container, n *libnetwork.Network, e
 		}
 	}
 
-	if len(c.HostConfig.DNS) > 0 {
+	return []libnetwork.EndpointOption{
-		createOptions = append(createOptions, libnetwork.CreateOptionDNS(c.HostConfig.DNS))
-	} else if len(daemonDNS) > 0 {
-		createOptions = append(createOptions, libnetwork.CreateOptionDNS(daemonDNS))
-	}
-
-	createOptions = append(createOptions,
 		libnetwork.CreateOptionPortMapping(publishedPorts),
 		libnetwork.CreateOptionExposedPorts(exposedPorts),
-		libnetwork.EndpointOptionGeneric(genericOptions))
+	}, nil
-
-	return createOptions, nil
 }
 
 // getPortMapInfo retrieves the current port-mapping programmed for the given sandbox