In 28.0.0, Docker appended to the FORWARD chain - breaking other
applications that had appended their own rules that needed to execute
after Docker's rules.
Move most of Docker's rules out of the filter-FORWARD chain into a
new DOCKER-FORWARD chain, so that Docker can append to DOCKER-FORWARD
without affecting the order of rules in the FORWARD chain.
After daemon startup inserts jumps to DOCKER-USER and DOCKER-FORWARD,
the bridge driver will not touch the FORWARD chain again. DOCKER-INGRESS
is still added to the FORWARD chain, if used, as it was in 27.x and
earlier.
Signed-off-by: Rob Murray <rob.murray@docker.com>
On a freshly rebooted Linux host that's configured to use nftables
with the iptables front-end, "make test-unit" fails "TestUserChains"
on the first run - it's ok in subsequent runs.
The unit tests run in moby's dev container.
The first test in TestUserChain runs with ip6tables disabled, so the
bridge driver doesn't try to load the ip6_tables module. Then, because
the module isn't loaded (it wasn't needed by the daemon running on the
nftables host when it started), the test fails because it can't check
what's in the ip6tables filter chain.
The next test in TestUserChain does the same thing, but with ip6tables
enabled. So the module gets loaded by the bridge driver, and everything
works normally after that.
The dev container used to try to load the module on startup, but that
was removed in commit 2af19b6b ("Don't try to modprobe ip6_tables in
the moby dev container"), as part of a change to give the daemon a way
to load modules itself.
Rather that put back the dev container's code to load ip6_tables on
startup (which would mean the daemon's module-loading code not getting
to run on nftables/firewalld hosts) ...
Run the tests in TestUserChains in a different order, with iptables
enabled in the first test will make it happen to work. At least for
now.
It's not ideal, but we'll be switching to nftables soon, so the issue
will go away.
Signed-off-by: Rob Murray <rob.murray@docker.com>
It no longer needs to be moved to the top of the filter-FORWARD
chain after creating a new bridge network. But, it does need to
be rearranged after setting up Swarm ingress.
Similarly, the jump to DOCKER-INGRESS no longer needs to be
shuffled back to the top of the FORWARD chain after adding a
new network.
Signed-off-by: Rob Murray <rob.murray@docker.com>
libnetwork/endpoint_info.go:467:3: The copy of the 'for' variable "r" can be deleted (Go 1.22+) (copyloopvar)
r := r
^
libnetwork/firewall_linux_test.go:52:3: The copy of the 'for' variable "tc" can be deleted (Go 1.22+) (copyloopvar)
tc := tc
^
libnetwork/libnetwork_internal_test.go:403:3: The copy of the 'for' variable "tc" can be deleted (Go 1.22+) (copyloopvar)
tc := tc
^
libnetwork/libnetwork_linux_test.go:2045:3: The copy of the 'for' variable "i" can be deleted (Go 1.22+) (copyloopvar)
i := i
^
libnetwork/libnetwork_linux_test.go:2165:3: The copy of the 'for' variable "tc" can be deleted (Go 1.22+) (copyloopvar)
tc := tc
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This option fn was defining a custom directory, file name and bucket
name for boltdb. Users can only change data-dir through `OptionDataDir`.
Better reuse that function instead, that'll make refactorings easier.
It won't set a custom bucket name or file name as `OptionBoltdbWithRandomDBFile`
was doing, but that's not needed since every test will use a different
temp dir anyway.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
The bbolt library wants exclusive access to the boltdb file and uses
file locking to assure that is the case. The controller and each network
driver that needs persistent storage instantiates its own unique
datastore instance, backed by the same boltdb file. The boltdb kvstore
implementation works around multiple access to the same boltdb file by
aggressively closing the boltdb file between each transaction. This is
very inefficient. Have the controller pass its datastore instance into
the drivers and enable the PersistConnection option to disable closing
the boltdb between transactions.
Set data-dir in unit tests which instantiate libnetwork controllers so
they don't hang trying to lock the default boltdb database file.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Don't fail early if we can still test more, and be slightly more strict
in what error we're looking for.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The test already creates instances for each ip-version, so let's
re-use them. While changing, also use assert.Check to not fail early.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
New() allows for driver-options to be passed using the config.OptionDriverConfig.
Update the test to not manually mutate the controller's configuration after
creating.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This reverts commit 2d397beb00.
moby#44706 and moby#44805 were both merged, and both refactored the same
file. The combination broke the build, and was not detected in CI as
only the combination of the two, applied to the same parent commit,
caused the failure.
moby#44706 should be carried forward, based on the current master, in
order to resolve this conflict.
Signed-off-by: Bjorn Neergaard <bneergaard@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>
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>
