Previously, with the patch from #43146, it was possible for a
network configured with a single ingress or load balancer on a
distribution which does not have the `ip_vs` kernel module loaded
by default to try to apply sysctls which did not exist yet, and
subsequently dynamically load the module as part of ipvs/netlink.go.
This module is vendored, and not a great place to try to tie back
into core libnetwork functionality, so also ensure that the sysctls
(which are idempotent) are called after ingress/lb creation once
`ipvs` has been initialized.
Signed-off-by: Ryan Barry <rbarry@mirantis.com>
While looking at this code, I noticed that we were wasting quite some resources
by first constructing a string, only to split it again (with `strings.Fields()`)
into a string slice.
Some conversions were also happening multiple times (int to string, IP-address to
string, etc.)
Setting up networking is known to be costing a considerable amount of time when
starting containers, and while this may only be a small part of that, it doesn't
hurt to save some resources (and readability of the code isn't significantly
impacted).
For example, benchmarking the `redirector()` code before/after:
BenchmarkParseOld-4 137646 8398 ns/op 4192 B/op 75 allocs/op
BenchmarkParseNew-4 629395 1762 ns/op 2362 B/op 24 allocs/op
Average over 10 runs:
benchstat old.txt new.txt
name old time/op new time/op delta
Parse-4 8.43µs ± 2% 1.79µs ± 3% -78.76% (p=0.000 n=9+8)
name old alloc/op new alloc/op delta
Parse-4 4.19kB ± 0% 2.36kB ± 0% -43.65% (p=0.000 n=10+10)
name old allocs/op new allocs/op delta
Parse-4 75.0 ± 0% 24.0 ± 0% -68.00% (p=0.000 n=10+10)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The io/ioutil package has been deprecated in Go 1.16. This commit
replaces the existing io/ioutil functions with their new definitions in
io and os packages.
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
These were purposefully ignored before but this goes ahead and "fixes"
most of them.
Note that none of the things gosec flagged are problematic, just
quieting the linter here.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
After moving libnetwork to this repo, we need to update all the import
paths for libnetwork to point to docker/docker/libnetwork instead of
docker/libnetwork.
This change implements that.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
The ipvs package was moved to a separate repo.
The ipvs package is a fairly generic set of helpers for managing IPVS.
The ipvs package is used by docker swarm and kubernetes.
Because we want to merge libnetwork back into the moby/moby codebase
while also not creating more dependencies for other projects on
moby/moby itself, it was decided that the best path for ipvs is to live
on it's own since there are no other ties to libnetwork.
Ref: https://github.com/moby/libnetwork/issues/2522
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Allow DSR to be a configurable option through a generic option to the
overlay driver. On the one hand this approach makes sense insofar as
only overlay networks can currently perform load balancing. On the
other hand, this approach has several issues. First, should we create
another type of swarm scope network, this will prevent it working.
Second, the service core code is separate from the driver code and the
driver code can't influence the core data structures. So the driver
code can't set this option itself. Therefore, implementing in this way
requires some hack code to test for this option in
controller.NewNetwork.
A more correct approach would be to make this a generic option for any
network. Then the driver could ignore, reject or be unaware of the option
depending on the chosen model. This would require changes to:
* libnetwork - naturally
* the docker API - to carry the option
* swarmkit - to propagate the option
* the docker CLI - to support the option
* moby - to translate the API option into a libnetwork option
Given the urgency of requests to address this issue, this approach will
be saved for a future iteration.
Signed-off-by: Chris Telfer <ctelfer@docker.com>
Modify the loadbalancing for east-west traffic to use direct routing
rather than NAT and update tasks to use direct service return under
linux. This avoids hiding the source address of the sender and improves
the performance in single-client/single-server tests.
Signed-off-by: Chris Telfer <ctelfer@docker.com>
The previous code used string slices to limit the length of certain
fields like endpoint or sandbox IDs. This assumes that these strings
are at least as long as the slice length. Unfortunately, some sandbox
IDs can be smaller than 7 characters. This fix addresses this issue
by systematically converting format string calls that were taking
fixed-slice arguments to use a precision specifier in the string format
itself. From the golang fmt package documentation:
For strings, byte slices and byte arrays, however, precision limits
the length of the input to be formatted (not the size of the output),
truncating if necessary. Normally it is measured in runes, but for
these types when formatted with the %x or %X format it is measured
in bytes.
This nicely fits the desired behavior: it will limit the number of
runes considered for string interpolation to the precision value.
Signed-off-by: Chris Telfer <ctelfer@docker.com>
Add debug and error logs to notify when a load balancing sandbox
is not found. This can occur in normal operation during removal.
Signed-off-by: Chris Telfer <ctelfer@docker.com>
This is the heart of the scalability change for services in libnetwork.
The present routing mesh adds load-balancing rules for a network to
every container connected to the network. This newer approach creates a
load-balancing endpoint per network per node. For every service on a
network, libnetwork assigns the VIP of the service to the endpoint's
interface as an alias. This endpoint must have a unique IP address in
order to route return traffic to it. Traffic destined for a service's
VIP arrives at the load-balancing endpoint on the VIP and from there,
Linux load balances it among backend destinations while SNATing said
traffic to the endpoint's unique IP address.
The net result of this scheme is that each node in a swarm need only
have one set of load balancing state per service instead of one per
container on the node. This scheme is very similar to how services
currently operate on Windows nodes in libnetwork. It (as with Windows
nodes) costs the use of extra IP addresses in a network (one per node)
and an extra network hop in the stack, although, always in the stack
local to the container.
In order to prevent existing deployments from suddenly failing if they
failed to allocate sufficient address space to include per-node
load-balancing endpoint IP addresses, this patch preserves the existing
functionality and activates the new functionality on a per-network
basis depending on whether the network has a load-balancing endpoint.
Eventually, moby should always set this option when creating new
networks and should only omit it for networks created as part of a swarm
that are not marked to use endpoint load balancing.
This patch also normalizes the code to treat "load" and "balancer"
as two separate words from the perspectives of variable/function naming.
This means that the 'b' in "balancer" must be capitalized.
Signed-off-by: Chris Telfer <ctelfer@docker.com>
This was passing extra information and adding confusion about the
purpose of the load balancing structure.
Signed-off-by: Chris Telfer <ctelfer@docker.com>
Make sure that iptables operations on ingress
are serialized.
Before 2 racing routines trying to create the ingress chain
were allowed and one was failing reporting the chain as
already existing.
The lock guarantees that this condition does not happen anymore
Signed-off-by: Flavio Crisciani <flavio.crisciani@docker.com>
This patch attempts to allow endpoints to complete servicing connections
while being removed from a service. The change adds a flag to the
endpoint.deleteServiceInfoFromCluster() method to indicate whether this
removal should fully remove connectivity through the load balancer
to the endpoint or should just disable directing further connections to
the endpoint. If the flag is 'false', then the load balancer assigns
a weight of 0 to the endpoint but does not remove it as a linux load
balancing destination. It does remove the endpoint as a docker load
balancing endpoint but tracks it in a special map of "disabled-but-not-
destroyed" load balancing endpoints. This allows traffic to continue
flowing, at least under Linux. If the flag is 'true', then the code
removes the endpoint entirely as a load balancing destination.
The sandbox.DisableService() method invokes deleteServiceInfoFromCluster()
with the flag sent to 'false', while the endpoint.sbLeave() method invokes
it with the flag set to 'true' to complete the removal on endpoint
finalization. Renaming the endpoint invokes deleteServiceInfoFromCluster()
with the flag set to 'true' because renaming attempts to completely
remove and then re-add each endpoint service entry.
The controller.rmServiceBinding() method, which carries out the operation,
similarly gets a new flag for whether to fully remove the endpoint. If
the flag is false, it does the job of moving the endpoint from the
load balancing set to the 'disabled' set. It then removes or
de-weights the entry in the OS load balancing table via
network.rmLBBackend(). It removes the service entirely via said method
ONLY IF there are no more live or disabled load balancing endpoints.
Similarly network.addLBBackend() requires slight tweaking to properly
manage the disabled set.
Finally, this change requires propagating the status of disabled
service endpoints via the networkDB. Accordingly, the patch includes
both code to generate and handle service update messages. It also
augments the service structure with a ServiceDisabled boolean to convey
whether an endpoint should ultimately be removed or just disabled.
This, naturally, required a rebuild of the protocol buffer code as well.
Signed-off-by: Chris Telfer <ctelfer@docker.com>
* Correct SetMatrix documentation
The SetMatrix is a generic data structure, so the description
should not be tight to any specific use
Signed-off-by: Flavio Crisciani <flavio.crisciani@docker.com>
* Service Discovery reuse name and serviceBindings deletion
- Added logic to handle name reuse from different services
- Moved the deletion from the serviceBindings map at the end
of the rmServiceBindings body to avoid race with new services
Signed-off-by: Flavio Crisciani <flavio.crisciani@docker.com>
* Avoid race on network cleanup
Use the locker to avoid the race between the network
deletion and new endpoints being created
Signed-off-by: Flavio Crisciani <flavio.crisciani@docker.com>
* CleanupServiceBindings to clean the SD records
Allow the cleanupServicebindings to take care of the service discovery
cleanup. Also avoid to trigger the cleanup for each endpoint from an SD
point of view
LB and SD will be separated in the future
Signed-off-by: Flavio Crisciani <flavio.crisciani@docker.com>
* Addressed comments
Signed-off-by: Flavio Crisciani <flavio.crisciani@docker.com>
* NetworkDB deleteEntry has to happen
If there is an error locally guarantee that the delete entry
on network DB is still honored
Signed-off-by: Flavio Crisciani <flavio.crisciani@docker.com>
changed the ipMap to SetMatrix to allow transient states
Compacted the addSvc and deleteSvc into a one single method
Updated the datastructure for backends to allow storing all the information needed
to cleanup properly during the cleanupServiceBindings
Removed the enable/disable Service logic that was racing with sbLeave/sbJoin logic
Add some debug logs to track further race conditions
Signed-off-by: Flavio Crisciani <flavio.crisciani@docker.com>
- do not error on duplicate service removal
- give some context to service logs,
this would help debugging related issues
Signed-off-by: Alessandro Boch <aboch@docker.com>
- Do not relay on software flags to decide when to create the
virtual service. Instead query the kernel for presence.
So that it cannot happen that a real server creation
fails because the virtual server is missing.
Signed-off-by: Alessandro Boch <aboch@docker.com>
Ping on VIP has been behaving inconsistently depending on if a task
for a service is local or remote.
With this fix, the ICMP echo-request packets to service VIP are replied
to by the NAT rule to self
Signed-off-by: Madhu Venugopal <madhu@docker.com>
1. Base work was done by msabansal and nwoodmsft
from : https://github.com/msabansal/docker/tree/overlay
2. reorganized under drivers/windows/overlay and rebased to
libnetwork master
3. Porting overlay common fixes to windows driver
* 46f525c
* ba8714e
* 6368406
4. Windows Service Discovery changes for swarm-mode
5. renaming default windows ipam drivers as "windows"
Signed-off-by: Madhu Venugopal <madhu@docker.com>
Signed-off-by: msabansal <sabansal@microsoft.com>
Signed-off-by: nwoodmsft <Nicholas.Wood@microsoft.com>
Currently there is an instance of controller and service lock being
obtained in different order which causes the AB/BA deadlock. Do not ever
wrap controller lock around service lock.
Signed-off-by: Jana Radhakrishnan <mrjana@docker.com>
With port redirect in the ingress path happening before ipvs in the
ingess sandbox, there is a chance of 5-tuple collision in the ipvs
connection table for two entirely different services have different
PublishedPorts but the same TargetPort. To disambiguate the ipvs
connection table, delay the port redirect from PublishedPort to
TargetPort until after the loadbalancing has happened in ipvs. To be
specific, perform the redirect after the packet enters the real backend
container namespace.
Signed-off-by: Jana Radhakrishnan <mrjana@docker.com>
Currently, a reference counting scheme is used to reference count all
individual port configs that need to be plumbed in the ingress to make
sure that in situations where a service with the same set of port
configs is getting added or removed doesn't accidentally remove the port
config plumbing if the add/remove notifications come out of order. This
same reference counting scheme is also used for plumbing the port-based
marking rules. But marking rules should not be plumbed based on that
because marks are always different for different instantiations of the
same service. So fixed the code to plumb port-based mark rules based on
the complete set of port configs, while plumbing pure port rules and
proxies based on a filter set of port configs based on the reference
count.
Signed-off-by: Jana Radhakrishnan <mrjana@docker.com>
This also allows pubslied services to be accessible from containers on bridge
networks on the host
Signed-off-by: Santhosh Manohar <santhosh@docker.com>
When leaving the entire gossip cluster or when leaving a network
specific gossip cluster, we may not have had a chance to cleanup service
bindings by way of gossip updates due to premature closure of gossip
channel. Make sure to cleanup all service bindings since we are not
participating in the cluster any more.
Signed-off-by: Jana Radhakrishnan <mrjana@docker.com>
The SNAT rules added for LB egress is broader and breaks load balancing
if the service is connected to multiple networks. Make it conditional
based on the subnet to which the network belongs so that the right SNAT
rule gets matched when egressing the corresponding network.
Signed-off-by: Jana Radhakrishnan <mrjana@docker.com>
Make service loadbalancing to work from within one of the containers of
the service. Currently this only works when the loadbalancer selects the
current container. If another container of the same service is chosen,
the connection times out. This fix adds a SNAT rule to change the source
IP to the containers primary IP so that responses can be routed back to
this container.
Signed-off-by: Jana Radhakrishnan <mrjana@docker.com>
Ingress loadbalancer is only required to be plumbed in ingress sandboxes
of nodes which are the only mechanism to get traffix outside the cluster
to tasks. Since the tasks are part of ingress network, these
loadbalancers were getting added in all tasks which are exposing ports
which is totally unnecessary resource usage. This PR avoids that.
Signed-off-by: Jana Radhakrishnan <mrjana@docker.com>
