These were only used in a single location, and in a rather bad way;
replace them with strings.Cut() which should be all we need for this.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- config collided with import
- cap collided with a built-in
- c collided with the "controller" receiver
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This code was forked from libcontainer (now runc) in
fb6dd9766e
From the description of this code:
> THIS CODE DOES NOT COMMUNICATE WITH KERNEL VIA RTNETLINK INTERFACE
> IT IS HERE FOR BACKWARDS COMPATIBILITY WITH OLDER LINUX KERNELS
> WHICH SHIP WITH OLDER NOT ENTIRELY FUNCTIONAL VERSION OF NETLINK
That comment was added as part of a refactor in;
4fe2c7a4db
Digging deeper into the code, it describes:
> This is more backward-compatible than netlink.NetworkSetMaster and
> works on RHEL 6.
That comment (and code) moved around a few times;
- moved into the libcontainer pkg: 6158ccad97
- moved within the networkdriver pkg: 4cdcea2047
- moved into the networkdriver pkg: 90494600d3
Ultimately leading to 7a94cdf8ed, which implemented
this:
> create the bridge device with ioctl
>
> On RHEL 6, creation of a bridge device with netlink fails. Use the more
> backward-compatible ioctl instead. This fixes networking on RHEL 6.
So from that information, it looks indeed to support RHEL 6, and Ubuntu 12.04
which are both EOL, and we haven't supported for a long time, so probably time
to remove this.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This commit allows to remove dependency on the mutable version armon/go-radix.
The go-immutable-radix package is better maintained.
It is likely that a bit more memory will be used when using the
immutable version, though discarded nodes are being reused in a pool.
These changes happen when networks are added/removed or nodes come and
go in a cluster, so we are still talking about a relatively low
frequency event.
The major changes compared to the old radix are when modifying (insert
or delete) a tree, and those are pretty self-contained: we replace the
entire immutable tree under a lock.
Signed-off-by: Tibor Vass <teabee89@gmail.com>
We only need the content here, not the checksum, so simplifying the code by
just using os.ReadFile().
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
We only need the content here, not the checksum, so simplifying the code by
just using os.ReadFile().
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Some of these options required parsing the resolv.conf file, but the function
could return an error further down; this patch moves the parsing closer to
where their results are used (which may not happen if we're encountering an
error before).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
We only need the content here, not the checksum, so simplifying the code by
just using os.ReadFile().
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The existing code was always parsing the host's resolv.conf to read
the nameservers, searchdomain and options, but those options were
only needed if these options were not configured on the sandbox.
This patch reverses the logic to only parse the resolv.conf if
no options are present in the sandbox configuration.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
We only need the content here, not the checksum, so simplifying the
code by just using os.ReadFile().
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Now that this function is only ever called from contexts where a
*testing.T value is available, several improvements can be made to it.
Refactor it to be a test helper function so that callers do not need to
check and fail the test themselves. Leverage t.TempDir() so that the
temporary file is cleaned up automatically. Change it to return a single
config.Option to get better ergonomics at call sites.
Signed-off-by: Cory Snider <csnider@mirantis.com>
TestCreateParallel, which was ostensibly added as a regression test for
race conditions inside the bridge driver, contains a race condition. The
getIPv4Data() calls race the network configuration and so will sometimes
see the existing address assignments return IP address ranges which do
not conflict with them. While normally a good thing, the test asserts
that exactly one of the 100 networks is successfully created. Pass the
same IPAM data when attempting to create every network to ensure that
the address ranges conflict.
Signed-off-by: Cory Snider <csnider@mirantis.com>
addresses() would incorrectly return all IP addresses assigned to any
interface in the network namespace if exists() is false. This went
unnoticed as the unit test covering this case tested the method inside a
clean new network namespace, which had no interfaces brought up and
therefore no IP addresses assigned. Modifying
testutils.SetupTestOSContext() to bring up the loopback interface 'lo'
resulted in the loopback addresses 127.0.0.1 and [::1] being assigned to
the loopback interface, causing addresses() to return the loopback
addresses and TestAddressesEmptyInterface to start failing. Fix the
implementation of addresses() so that it only ever returns addresses for
the bridge interface.
Signed-off-by: Cory Snider <csnider@mirantis.com>
portallocator.PortAllocator holds persistent state on port allocations
in the network namespace. It normal operation it is used as a singleton,
which is a problem in unit tests as every test runs in a clean network
namespace. The singleton "default" PortAllocator remembers all the port
allocations made in other tests---in other network namespaces---and
can result in spurious test failures. Refactor the bridge driver so that
tests can construct driver instances with unique PortAllocators.
Signed-off-by: Cory Snider <csnider@mirantis.com>
The global netlink handle ns.NlHandle() is indirectly cached for the
life of the process by the netutils.CheckRouteOverlaps() function. This
caching behaviour is a problem for the libnetwork unit tests as the
global netlink handle changes every time testutils.SetupTestOSContext()
is called, i.e. at the start of nearly every test case. Route overlaps
can be checked for in the wrong network namespace, causing spurious test
failures e.g. when running the same test twice in a row with -count=2.
Stop the netlink handle from being cached by shadowing the package-scope
variable with a function-scoped one.
Signed-off-by: Cory Snider <csnider@mirantis.com>
TestParallel has been written in an unusual style which relies on the
testing package's intra-test parallelism feature and lots of global
state to test one thing using three cooperating parallel tests. It is
complicated to reason about and quite brittle. For example, the command
go test -run TestParallel1 ./libnetwork
would deadlock, waiting until the test timeout for TestParallel2 and
TestParallel3 to run. And the test would be skipped if the
'-test.parallel' flag was less than three, either explicitly or
implicitly (default: GOMAXPROCS).
Overhaul TestParallel to address the aforementioned deficiencies and
get rid of mutable global state.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Reusing the same "OS context" (read: network namespace) and
NetworkController across multiple tests risks tests interfering with
each other, or worse: _depending on_ other tests to set up
preconditions. Construct a new controller for every test which needs
one, and run every test which mutates or inspects the host environment
in a fresh OS context.
The only outlier is runParallelTests. It is the only remaining test
implementation which references the "global" package-scoped controller,
so the global controller instance is effectively private to that one
test.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Sharing a single NetworkController instance between multiple tests
makes it possible for tests to interfere with each other. As a first
step towards giving each test its own private controller instance, make
explicit which controller createTestNetwork() creates the test network
on.
Signed-off-by: Cory Snider <csnider@mirantis.com>
There are a handful of libnetwork tests which need to have multiple
concurrent goroutines inside the same test OS context (network
namespace), including some platform-agnostic tests. Provide test
utilities for spawning goroutines inside an OS context and
setting/restoring an existing goroutine's OS context to abstract away
platform differences and so that each test does not need to reinvent the
wheel.
Signed-off-by: Cory Snider <csnider@mirantis.com>
The SetupTestOSContext calls were made conditional in
https://github.com/moby/libnetwork/pull/148 to work around limitations
in runtime.LockOSThread() which existed before Go 1.10. This workaround
is no longer necessary now that runtime.UnlockOSThread() needs to be
called an equal number of times before the goroutine is unlocked from
the OS thread.
Unfortunately some tests break when SetupTestOSContext is not skipped.
(Evidently this code path has not been exercised in a long time.) A
newly-created network namespace is very barebones: it contains a
loopback interface in the down state and little else. Even pinging
localhost does not work inside of a brand new namespace. Set the
loopback interface to up during namespace setup so that tests which
need to use the loopback interface can do so.
Signed-off-by: Cory Snider <csnider@mirantis.com>
If the GenericData option is missing from the map, the bridge driver
would skip configuration. At first glance this seems fine: the driver
defaults are to not configure anything. But it also skips over
initializing the persistent storage, which is configured through other
option keys in the map. Fix this oversight by removing the early return.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Each call to datastore.DefaultScopes() would modify and return the same
map. Consequently, some of the config for every NetworkController in the
process would be mutated each time one is constructed or reconfigured.
This behaviour is unexpected, unintended, and undesirable. Stop
accidentally sharing configuration by changing DefaultScopes() to return
a distinct map on each call.
Signed-off-by: Cory Snider <csnider@mirantis.com>
(*networkNamespace).InvokeFunc() cleaned up the state of the locked
thread by setting its network namespace to the netns of the goroutine
which called InvokeFunc(). If InvokeFunc() was to be called after the
caller had modified its thread's network namespace, InvokeFunc() would
incorrectly "restore" the state of its goroutine thread to the wrong
namespace, violating the invariant that unlocked threads are fungible.
Change the implementation to restore the thread's netns to the netns
that particular thread had before InvokeFunc() modified it.
Signed-off-by: Cory Snider <csnider@mirantis.com>
When running inside a container, testns == origns. Consequently, closing
testns causes the deferred netns.Set(origns) call to fail. Stop closing
the aliased original namespace handle.
Signed-off-by: Cory Snider <csnider@mirantis.com>
TestResolvConfHost and TestParallel both depended on a network named
"testhost" already existing in the libnetwork controller. Only TestHost
created that network, so the aforementioned tests would fail unless run
after TestHost. Fix those tests so they can be run in any order.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Aside from unconditionally unlocking the OS thread even if restoring the
thread's network namespace fails, func (*networkNamespace).InvokeFunc()
correctly implements invoking a function inside a network namespace.
This is far from obvious, however. func InitOSContext() does much of the
heavy lifting but in a bizarre fashion: it restores the initial network
namespace before it is changed in the first place, and the cleanup
function it returns does not restore the network namespace at all! The
InvokeFunc() implementation has to restore the network namespace
explicitly by deferring a call to ns.SetNamespace().
func InitOSContext() is a leaky abstraction taped to a footgun. On the
one hand, it defensively resets the current thread's network namespace,
which has the potential to fix up the thread state if other buggy code
had failed to maintain the invariant that an OS thread must be locked to
a goroutine unless it is interchangeable with a "clean" thread as
spawned by the Go runtime. On the other hand, it _facilitates_ writing
buggy code which fails to maintain the aforementioned invariant because
the cleanup function it returns unlocks the thread from the goroutine
unconditionally while neglecting to restore the thread's network
namespace! It is quite scary to need a function which fixes up threads'
network namespaces after the fact as an arbitrary number of goroutines
could have been scheduled onto a "dirty" thread and run non-libnetwork
code before the thread's namespace is fixed up. Any number of
(not-so-)subtle misbehaviours could result if an unfortunate goroutine
is scheduled onto a "dirty" thread. The whole repository has been
audited to ensure that the aforementioned invariant is never violated,
making after-the-fact fixing up of thread network namespaces redundant.
Make InitOSContext() a no-op on Linux and inline the thread-locking into
the function (singular) which previously relied on it to do so.
func ns.SetNamespace() is of similarly dubious utility. It intermixes
capturing the initial network namespace and restoring the thread's
network namespace, which could result in threads getting put into the
wrong network namespace if the wrong thread is the first to call it.
Delete it entirely; functions which need to manipulate a thread's
network namespace are better served by being explicit about capturing
and restoring the thread's namespace.
Rewrite InvokeFunc() to invoke the closure inside a goroutine to enable
a graceful and safe recovery if the thread's network namespace could not
be restored. Avoid any potential race conditions due to changing the
main thread's network namespace by preventing the aforementioned
goroutines from being eligible to be scheduled onto the main thread.
Signed-off-by: Cory Snider <csnider@mirantis.com>
func (*network) watchMiss() correctly locks its goroutine to an OS
thread before changing the thread's network namespace, but neglects to
restore the thread's network namespace before unlocking. Fix this
oversight by unlocking iff the thread's network namespace is
successfully restored.
Prevent the watchMiss goroutine from being locked to the main thread to
avoid the issues which would arise if such a situation was to occur.
Signed-off-by: Cory Snider <csnider@mirantis.com>
The parallel tests were unconditionally unlocking the test case
goroutine from the OS thread, irrespective of whether the thread's
network namespace was successfully restored. This was not a problem in
practice as the unpaired calls to runtime.LockOSThread() peppered
through the test case would have prevented the goroutine from being
unlocked. Unlock the goroutine from the thread iff the thread's network
namespace is successfully restored.
Signed-off-by: Cory Snider <csnider@mirantis.com>
testutils.SetupTestOSContext() sets the calling thread's network
namespace but neglected to restore it on teardown. This was not a
problem in practice as it called runtime.LockOSThread() twice but
runtime.UnlockOSThread() only once, so the tampered threads would be
terminated by the runtime when the test case returned and replaced with
a clean thread. Correct the utility so it restores the thread's network
namespace during teardown and unlocks the goroutine from the thread on
success.
Remove unnecessary runtime.LockOSThread() calls peppering test cases
which leverage testutils.SetupTestOSContext().
Signed-off-by: Cory Snider <csnider@mirantis.com>
On Linux, when (os/exec.Cmd).SysProcAttr.Pdeathsig is set, the signal
will be sent to the process when the OS thread on which cmd.Start() was
executed dies. The runtime terminates an OS thread when a goroutine
exits after being wired to the thread with runtime.LockOSThread(). If
other goroutines are allowed to be scheduled onto a thread which called
cmd.Start(), an unrelated goroutine could cause the thread to be
terminated and prematurely signal the command. See
https://github.com/golang/go/issues/27505 for more information.
Prevent started subprocesses with Pdeathsig from getting signaled
prematurely by wiring the starting goroutine to the OS thread until the
subprocess has exited. No other goroutines can be scheduled onto a
locked thread so it will remain alive until unlocked or the daemon
process exits.
Signed-off-by: Cory Snider <csnider@mirantis.com>
libnetwork/etchosts/etchosts_test.go:167:54: empty-lines: extra empty line at the end of a block (revive)
libnetwork/osl/route_linux.go:185:74: empty-lines: extra empty line at the start of a block (revive)
libnetwork/osl/sandbox_linux_test.go:323:36: empty-lines: extra empty line at the start of a block (revive)
libnetwork/bitseq/sequence.go:412:48: empty-lines: extra empty line at the start of a block (revive)
libnetwork/datastore/datastore_test.go:67:46: empty-lines: extra empty line at the end of a block (revive)
libnetwork/datastore/mock_store.go:34:60: empty-lines: extra empty line at the end of a block (revive)
libnetwork/iptables/firewalld.go:202:44: empty-lines: extra empty line at the end of a block (revive)
libnetwork/iptables/firewalld_test.go:76:36: empty-lines: extra empty line at the end of a block (revive)
libnetwork/iptables/iptables.go:256:67: empty-lines: extra empty line at the end of a block (revive)
libnetwork/iptables/iptables.go:303:128: empty-lines: extra empty line at the start of a block (revive)
libnetwork/networkdb/cluster.go:183:72: empty-lines: extra empty line at the end of a block (revive)
libnetwork/ipams/null/null_test.go:44:38: empty-lines: extra empty line at the end of a block (revive)
libnetwork/drivers/macvlan/macvlan_store.go:45:52: empty-lines: extra empty line at the end of a block (revive)
libnetwork/ipam/allocator_test.go:1058:39: empty-lines: extra empty line at the start of a block (revive)
libnetwork/drivers/bridge/port_mapping.go:88:111: empty-lines: extra empty line at the end of a block (revive)
libnetwork/drivers/bridge/link.go:26:90: empty-lines: extra empty line at the end of a block (revive)
libnetwork/drivers/bridge/setup_ipv6_test.go:17:34: empty-lines: extra empty line at the end of a block (revive)
libnetwork/drivers/bridge/setup_ip_tables.go:392:4: empty-lines: extra empty line at the start of a block (revive)
libnetwork/drivers/bridge/bridge.go:804:50: empty-lines: extra empty line at the start of a block (revive)
libnetwork/drivers/overlay/ov_serf.go:183:29: empty-lines: extra empty line at the start of a block (revive)
libnetwork/drivers/overlay/ov_utils.go:81:64: empty-lines: extra empty line at the end of a block (revive)
libnetwork/drivers/overlay/peerdb.go:172:67: empty-lines: extra empty line at the start of a block (revive)
libnetwork/drivers/overlay/peerdb.go:209:67: empty-lines: extra empty line at the start of a block (revive)
libnetwork/drivers/overlay/peerdb.go:344:89: empty-lines: extra empty line at the start of a block (revive)
libnetwork/drivers/overlay/peerdb.go:436:63: empty-lines: extra empty line at the start of a block (revive)
libnetwork/drivers/overlay/overlay.go:183:36: empty-lines: extra empty line at the start of a block (revive)
libnetwork/drivers/overlay/encryption.go:69:28: empty-lines: extra empty line at the end of a block (revive)
libnetwork/drivers/overlay/ov_network.go:563:81: empty-lines: extra empty line at the start of a block (revive)
libnetwork/default_gateway.go:32:43: empty-lines: extra empty line at the start of a block (revive)
libnetwork/errors_test.go:9:40: empty-lines: extra empty line at the start of a block (revive)
libnetwork/service_common.go:184:64: empty-lines: extra empty line at the end of a block (revive)
libnetwork/endpoint.go:161:55: empty-lines: extra empty line at the end of a block (revive)
libnetwork/store.go:320:33: empty-lines: extra empty line at the end of a block (revive)
libnetwork/store_linux_test.go:11:38: empty-lines: extra empty line at the end of a block (revive)
libnetwork/sandbox.go:571:36: empty-lines: extra empty line at the start of a block (revive)
libnetwork/service_common.go:317:246: empty-lines: extra empty line at the start of a block (revive)
libnetwork/endpoint.go:550:17: empty-lines: extra empty line at the end of a block (revive)
libnetwork/sandbox_dns_unix.go:213:106: empty-lines: extra empty line at the start of a block (revive)
libnetwork/controller.go:676:85: empty-lines: extra empty line at the end of a block (revive)
libnetwork/agent.go:876:60: empty-lines: extra empty line at the end of a block (revive)
libnetwork/resolver.go:324:69: empty-lines: extra empty line at the end of a block (revive)
libnetwork/network.go:1153:92: empty-lines: extra empty line at the end of a block (revive)
libnetwork/network.go:1955:67: empty-lines: extra empty line at the start of a block (revive)
libnetwork/network.go:2235:9: empty-lines: extra empty line at the start of a block (revive)
libnetwork/libnetwork_internal_test.go:336:26: empty-lines: extra empty line at the start of a block (revive)
libnetwork/resolver_test.go:76:35: empty-lines: extra empty line at the end of a block (revive)
libnetwork/libnetwork_test.go:303:38: empty-lines: extra empty line at the end of a block (revive)
libnetwork/libnetwork_test.go:985:46: empty-lines: extra empty line at the end of a block (revive)
libnetwork/ipam/allocator_test.go:1263:37: empty-lines: extra empty line at the start of a block (revive)
libnetwork/errors_test.go:9:40: empty-lines: extra empty line at the end of a block (revive)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
It was unclear what the distinction was between these configuration
structs, so merging them to simplify.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This was used for testing purposes when libnetwork was in a separate repo, using
the dnet utility, which was removed in 7266a956a8.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Libnetwork configuration files were only used as part of integration tests using
the dnet utility, which was removed in 7266a956a8
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Remove the "deadcode", "structcheck", and "varcheck" linters, as they are
deprecated:
WARN [runner] The linter 'deadcode' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter. Replaced by unused.
WARN [runner] The linter 'structcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter. Replaced by unused.
WARN [runner] The linter 'varcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter. Replaced by unused.
WARN [linters context] structcheck is disabled because of generics. You can track the evolution of the generics support by following the https://github.com/golangci/golangci-lint/issues/2649.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These functions were used in 63a7ccdd23, which was
part of Docker v1.5.0 and v1.6.0, but removed in Docker v1.7.0 when the network
stack was replaced with libnetwork in d18919e304.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
After discussing in the maintainers meeting, we concluded that Slowloris attacks
are not a real risk other than potentially having some additional goroutines
lingering around, so setting a long timeout to satisfy the linter, and to at
least have "some" timeout.
libnetwork/diagnostic/server.go:96:10: G112: Potential Slowloris Attack because ReadHeaderTimeout is not configured in the http.Server (gosec)
srv := &http.Server{
Addr: net.JoinHostPort(ip, strconv.Itoa(port)),
Handler: s,
}
api/server/server.go:60:10: G112: Potential Slowloris Attack because ReadHeaderTimeout is not configured in the http.Server (gosec)
srv: &http.Server{
Addr: addr,
},
daemon/metrics_unix.go:34:13: G114: Use of net/http serve function that has no support for setting timeouts (gosec)
if err := http.Serve(l, mux); err != nil && !strings.Contains(err.Error(), "use of closed network connection") {
^
cmd/dockerd/metrics.go:27:13: G114: Use of net/http serve function that has no support for setting timeouts (gosec)
if err := http.Serve(l, mux); err != nil && !strings.Contains(err.Error(), "use of closed network connection") {
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The linter falsely detects this as using "math/rand":
libnetwork/networkdb/cluster.go:721:14: G404: Use of weak random number generator (math/rand instead of crypto/rand) (gosec)
val, err := rand.Int(rand.Reader, big.NewInt(int64(n)))
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
In network node change test, the expected behavior is focused on how many nodes
left in networkDB, besides timing issues, things would also go tricky for a
leave-then-join sequence, if the check (counting the nodes) happened before the
first "leave" event, then the testcase actually miss its target and report PASS
without verifying its final result; if the check happened after the 'leave' event,
but before the 'join' event, the test would report FAIL unnecessary;
This code change would check both the db changes and the node count, it would
report PASS only when networkdb has indeed changed and the node count is expected.
Signed-off-by: David Wang <00107082@163.com>
The correct formatting for machine-readable comments is;
//<some alphanumeric identifier>:<options>[,<option>...][ // comment]
Which basically means:
- MUST NOT have a space before `<identifier>` (e.g. `nolint`)
- Identified MUST be alphanumeric
- MUST be followed by a colon
- MUST be followed by at least one `<option>`
- Optionally additional `<options>` (comma-separated)
- Optionally followed by a comment
Any other format will not be considered a machine-readable comment by `gofmt`,
and thus formatted as a regular comment. Note that this also means that a
`//nolint` (without anything after it) is considered invalid, same for `//#nosec`
(starts with a `#`).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Older versions of Go don't format comments, so committing this as
a separate commit, so that we can already make these changes before
we upgrade to Go 1.19.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
libnetwork/firewall_linux.go:11:21: var-declaration: should drop = nil from declaration of var ctrl; it is the zero value (revive)
ctrl *controller = nil
^
distribution/pull_v2_test.go:213:4: S1038: should use t.Fatalf(...) instead of t.Fatal(fmt.Sprintf(...)) (gosimple)
t.Fatal(fmt.Sprintf("expected formatPlatform to show windows platform with a version, but got '%s'", result))
^
integration-cli/docker_cli_build_test.go:5951:3: S1038: should use c.Skipf(...) instead of c.Skip(fmt.Sprintf(...)) (gosimple)
c.Skip(fmt.Sprintf("Bug fixed in 18.06 or higher.Skipping it for %s", testEnv.DaemonInfo.ServerVersion))
^
integration-cli/docker_cli_daemon_test.go:240:3: S1038: should use c.Skipf(...) instead of c.Skip(fmt.Sprintf(...)) (gosimple)
c.Skip(fmt.Sprintf("New base device size (%v) must be greater than (%s)", units.HumanSize(float64(newBasesizeBytes)), units.HumanSize(float64(oldBasesizeBytes))))
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Inlining the string makes the code more grep'able; renaming the
const to "driverName" to reflect the remaining uses of it.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Inlining the string makes the code more grep'able; renaming the
const to "driverName" to reflect the remaining uses of it.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This was effectively a constructor, but through some indirection; make it a
regular function, which is a bit more idiomatic.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This was effectively a constructor, but through some indirection; make it a
regular function, which is a bit more idiomatic.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
bump netlink to 1.2.1
change usages of netlink handle .Delete() to Close()
remove superfluous replace in vendor.mod
make requires of github.com/Azure/go-ansiterm direct
Signed-off-by: Martin Braun <braun@neuroforge.de>
iptables package has a function `detectIptables()` called to initialize
some local variables. Since v20.10.0, it first looks for iptables bin,
then ip6tables and finally it checks what iptables flags are available
(including -C). It early exits when ip6tables isn't available, and
doesn't execute the last check.
To remove port mappings (eg. when a container stops/dies), Docker
first checks if those NAT rules exist and then deletes them. However, in
the particular case where there's no ip6tables bin available, iptables
`-C` flag is considered unavailable and thus it looks for NAT rules by
using some substring matching. This substring matching then fails
because `iptables -t nat -S POSTROUTING` dumps rules in a slighly format
than what's expected.
For instance, here's what `iptables -t nat -S POSTROUTING` dumps:
```
-A POSTROUTING -s 172.18.0.2/32 -d 172.18.0.2/32 -p tcp -m tcp --dport 9999 -j MASQUERADE
```
And here's what Docker looks for:
```
POSTROUTING -p tcp -s 172.18.0.2 -d 172.18.0.2 --dport 9999 -j MASQUERADE
```
Because of that, those rules are considered non-existant by Docker and
thus never deleted. To fix that, this change reorders the code in
`detectIptables()`.
Fixes#42127.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
Since commit 2c4a868f64, Docker doesn't
use the value of net.ipv4.ip_local_port_range when choosing an ephemeral
port. This change reverts back to the previous behavior.
Fixes#43054.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
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>
relates to #35082, moby/libnetwork#2491
Previously, values for expire_quiescent_template, conn_reuse_mode,
and expire_nodest_conn were set only system-wide. Also apply them
for new lb_* and ingress_sbox sandboxes, so they are appropriately
propagated
Signed-off-by: Ryan Barry <rbarry@mirantis.com>
strings.ReplaceAll(s, old, new) is a wrapper function for
strings.Replace(s, old, new, -1). But strings.ReplaceAll is more
readable and removes the hardcoded -1.
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
Some packages were using `logrus.Fatal()` in init functions (which logs the error,
and (by default) calls `os.Exit(1)` after logging).
Given that logrus formatting and outputs have not yet been configured during the
initialization stage, it does not provide much benefits over a plain `panic()`.
This patch replaces some instances of `logrus.Fatal()` with `panic()`, which has
the added benefits of not introducing logrus as a dependency in some of these
packages, and also produces a stacktrace, which could help locating the problem
in the unlikely event an `init()` fails.
Before this change, an error would look like:
$ dockerd
FATA[0000] something bad happened
After this change, the same error looks like:
$ dockerd
panic: something bad happened
goroutine 1 [running]:
github.com/docker/docker/daemon/logger/awslogs.init.0()
/go/src/github.com/docker/docker/daemon/logger/awslogs/cloudwatchlogs.go:128 +0x89
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
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>
Make the call to cleanupServiceBindings during network deletion
conditional on Windows (where it is required), thereby providing a
performance improvement to network cleanup on Linux.
Signed-off-by: Trapier Marshall <tmarshall@mirantis.com>
Removal of PolicyLists from Windows VFP must be performed prior to
removing the HNS network. Otherwise PolicyList removal fails with
HNS error "network not found".
Signed-off-by: Trapier Marshall <tmarshall@mirantis.com>
There is a race condition between the local proxy and iptables rule
setting. When we have a lot of UDP traffic, the kernel will create
conntrack entries to the local proxy and will ignore the iptables
rules set after that.
Related to PR #32505. Fix#8795.
Signed-off-by: Vincent Bernat <vincent@bernat.ch>
Operations performed on overlay network sandboxes are handled by
dispatching operations send through a channel. This allows for
asynchronous operations to be performed which, since they are
not called from within another function, are able to operate in
an idempotent manner with a known/measurable starting state from
which an identical series of iterative actions can be performed.
However, it was possible in some cases for an operation dispatched
from this channel to write a message back to the channel in the
case of joining a network when a sufficient volume of sandboxes
were operated on.
A goroutine which is simultaneously reading and writing to an
unbuffered channel can deadlock if it sends a message to a channel
then waits for it to be consumed and completed, since the only
available goroutine is more or less "talking to itself". In order
to break this deadlock, in the observed race, a goroutine is now
created to send the message to the channel.
Signed-off-by: Martin Dojcak <martin.dojcak@lablabs.io>
Signed-off-by: Ryan Barry <rbarry@mirantis.com>
Windows Server 2016 (RS1) reached end of support, and Docker Desktop requires
Windows 10 V19H2 (version 1909, build 18363) as a minimum.
This patch makes Windows Server RS5 / ltsc2019 (build 17763) the minimum version
to run the daemon, and removes some hacks for older versions of Windows.
There is one check remaining that checks for Windows RS3 for a workaround
on older versions, but recent changes in Windows seemed to have regressed
on the same issue, so I kept that code for now to check if we may need that
workaround (again);
085c6a98d5/daemon/graphdriver/windows/windows.go (L319-L341)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Since moby/libnetwork#2635 has been merged, allocatePortsInternal()
checks if IPv6 is enabled by calling IsV6Listenable(). This function
calls `net.Listen("tcp6", "[::1]:0")` and returns false when
net.Listen() fails.
TestPortMappingV6Config() starts by setting up a new net ns to run into
it. The loopback interface is not bring up in this net ns, thus
net.Listen() fails and IsV6Listenable() returns false. This change takes
care of bringing loopback iface up right after moving to the new net ns.
This test has been reported has flaky on s390x in #42468. For some
reason, this test seems to be consistently green on the CI (on amd64
arch) and when running `hack/test/unit` locally. However it consistently
fails when running `TESTFLAGS='-shuffle on' hack/test/unit` locally.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
This was originally in docker/libnetwork#2624, which has been closed since the
code was moved here.
When creating a new network, IPAM's address allocator attempts to reserve the
network and broadcast addresses on IPv4 networks of all sizes. For RFC 3021
point-to-point networks (IPv4 /31s), this consumes both available addresses and
renders any attempt to allocate an address from the block unsuccessful.
This change prevents those reservations from taking place on IPv4 networks having
two or fewer addresses (i.e., /31s and /32s) while retaining the existing behavior
for larger IPv4 blocks and all IPv6 blocks.
In case you're wondering why anyone would allocate /31s: I work for a network
service provider. We use a lot of point-to-point networks. This cuts our
address space utilization for those by 50%, which makes ARIN happy.
This patch modifies the network allocator to recognize when an network is too
small for network and broadcast addresses and skip those reservations.
There are additional unit tests to make sure the functions involved behave as expected.
Try these out:
* `docker network create --driver bridge --subnet 10.200.1.0/31 --ip-range 10.200.1.0/31 test-31`
* `docker network create --driver bridge --subnet 10.200.1.0/32 --ip-range 10.200.1.0/32 test-32`
My installation has been running this patch in production with /31s since March.
Signed-off-by: Mark Feit <mfeit@internet2.edu>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
commit b1a3fe4934 changed how the error was
returned (which is now wrapped), causing the test to fail:
=== RUN TestInvalidRemoteDriver
libnetwork_test.go:1289: Did not fail with expected error. Actual error: Plugin does not implement the requested driver: plugin="invalid-network-driver", requested implementation="NetworkDriver"
--- FAIL: TestInvalidRemoteDriver (0.01s)
Changing the test to use errors.Is()
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- don't pass the query's quetion.name separately, as we're already
passing the query itself.
- remove a "fallthrough" in favor of combining the cases in the switch
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- Make sure all log messages have the `[resolver]` prefix
- Use `logrus.WithError()` consistently
- Improve information included in some logs
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This was added in b3c883bb2f, but resulted
in a panic if the embedded DNS had to handle an unsupported query-type,
such as ANY.
This patch adds a debug log for this case (to better describe how it's
handled.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- fix incorrectly formatted GoDoc and comments
- rename a variable that collided with the `cap` built-in
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>
The IsLocalhost utility was not used, which only leaves the IsIPv4Localhost
utility.
Go's "net" package provides a `IsLoopBack()` check, but it checks for both
IPv4 and IPv6 loopback interfaces. We likely should also do IPv6 here, but
that's better left for a separate change, so instead, I replicated the IPv4
bits from Go's net.IP.IsLoopback().
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This allows using the package without having to import the "types" package,
and without having to consume github.com/ishidawataru/sctp.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Perhaps the testutils package in the past had an `init()` function to set up
specific things, but it no longer has. so these imports were doing nothing.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
It is not directly related to signal-handling, so can well live
in its own package.
Also added a variant that doesn't take a directory to write files
to, for easier consumption / better match to how it's used.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
`github.com/hashicorp/memberlist` update caused `TestNetworkDBCRUDTableEntries`
to occasionally fail, because the test would try to check whether an entry
write is propagated to all nodes, but it would not wait for all nodes to
be available before performing the write.
It could be that the failure is caused simply by improved performance of
the dependency - it could also be that some connectivity guarantee the
test depended on is not provided by the dependency anymore.
The same fix is applied to `TestNetworkDBNodeJoinLeaveIteration` due to
same issue.
Signed-off-by: Roman Volosatovs <roman.volosatovs@docker.com>
This allows the rejoin intervals to be chosen according to the context
within which the component is used, and, in particular, this allows
lower intervals to be used within TestNetworkDBIslands test.
Signed-off-by: Roman Volosatovs <roman.volosatovs@docker.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>
This is another one of those tools to mimic the docker network cli.
It is not needed anymore, along with an old fork of the docker flag
packages which was a fork of the go flag package.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Since this command is part of the official distribution and even
required for tests, let's move this up to the main cmd's.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
This was used for testing purposes when libnetwork was in a separate
repo.
Now that it is integrated we no longer need it since dockerd and docker
cli provide the same function.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
We had some hosts with quite a bit of cycling containers that ocassionally causes docker daemons to lock up.
Most prominently `docker run` commands do not respond and nothing happens anymore.
Looking at the stack trace the following is at least likely sometimes a cause to that:
Two goroutines g0 and g1 can race against each other:
* (g0) 1. getSvcRecords is called and calls (*network).Lock()
--> Network is locked.
* (g1) 2. processEndpointDelete is called, and calls (*controller).Lock()
--> Controller is locked
* (g1) 3. processEndpointDelete tries (*network).ID() which calls (*network).Lock().
* (g0) 4. getSvcRecords calls (*controller).Lock().
3./4. are deadlocked against each other since the other goroutine holds the lock they need.
References b5dc370370/network.go
Signed-off-by: Steffen Butzer <steffen.butzer@outlook.com>
Both getDynamicPortRange() and sanitizePortRange() could produce
and error, and the error message was currently discarded, silently
falling back to using the default port range.
This patch:
- Moves the fallback message from getDynamicPortRange() to getDefaultPortRange(),
which is where the actual fallback occurs.
- Logs the fallback message and the error that causes the fallback.
The message/error is currently printed at the INFO level, but could be raised
to a WARN, depending on what kind of situations can cause the error.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The second (sandbox) argument was unused, and it was only
used in a single location, so we may as well inline the
check.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This argument was used to detect conflicts, but was later removed in
1c73b1c99c14d7f048a2318a3caf589865c76fad.
However, it was never removed, and we were still getting a list
of all networks, without using the results.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
