Make sure an error is returned straight away if there's contention on
the underlying db file. This makes sure we don't reintroduce the issue
fixed in d21d088, and it will help detect contention in parallelized
tests if they're badly written. It effectively adds a new error mode to
the daemon, but if anyone faces this error, they should fix their
process manager.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
That function was needlessly complex. Instead of relying on a struct and
a sub-struct, it now just takes two string params: a path and a bucket
name.
Libnetwork config is now initialized with default values. A new struct
is introduced in libnetwork/config to let tests customize the path and
bucket name.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
the resolvconf package is imported in BuildKit, and this is the only
location that used the errdefs package outside of the client.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The internal resolver now uses any namesever found in the host's
/etc/resolv.conf as an external nameserver, and it's accessed
from the host's network namespace.
Before this change, when no external nameservers were found (so
the host had no entries in /etc/resolv.conf) Google's DNS servers
were used as fallbacks, always accessed from the container's
network namespace. If a container's initial set of endpoints had
IPv6 enabled, the IPv6 nameservers were included.
Now we have IPv6-only networks, a similar exception would be
needed for Google's IPv4 nameservers... don't include them if
there are no IPv4 endpoints.
However, only the initial set of endpoints was considered. As
networks are connected/disconnected, IPv4 or IPv6 connectivity
may be lost.
Unlike nameservers read from the host's /etc/resolv.conf, there
is no way to tell which fallback nameservers (v4/v6) might work
from the host's namespace. So, using the host's namespace isn't
a good solution.
Since we want to get away from using fallback nameservers anyway,
this change removes them.
If a host has no /etc/resolv.conf entries, but a container does
need to use DNS, it'll need to be configured with servers via
'--dns'.
Signed-off-by: Rob Murray <rob.murray@docker.com>
The internal resolver needs to know whether to make requets
to external DNS servers from the container's network namespace
or the host's.
The original rule was that requests were always made from the
container's namespace, unless the nameserver was on a localhost
address on the host. IPv6 nameservers were left in the container's
/etc/resolv.conf.
Commit 4e8d9a4 modified that so that IPv6 nameservers were also
used as external nameservers. The internal resolver accessed
them from the host namespace if the container's initial set of
endpoints were IPv4-only, or the nameserver address contained
a zone-id, (or the nameserver was on the IPv6 loopback address).
That would break if initial IPv6 endpoints were disconnected from
the container, leaving it with no IPv6 address.
Once IPv6-only networks are allowed, another exception would need
to be made for IPv4 nameservers (they'd need to be accessed from
the host's namespace).
Instead of doing that ... this change simplifies things, if a
nameserver address is read from the host's /etc/resolv.conf, it'll
work in the host's namespace. So, the rule is now simply that
nameservers read from the host's resolv.conf are accessed from the
host's namespace. DNS servers added as overrides ('--dns') are
accessed from the container's namespace (as before).
Signed-off-by: Rob Murray <rob.murray@docker.com>
This reverts commit d365702dbd.
Because buildkit doesn't run an internal resolver, and it bases its
/etc/resolv.conf on the host's ... when buildkit is run in a container
that has 'nameserver 127.0.0.11', its build containers will use Google's
DNS servers as a fallback (unless the build container uses host
networking).
Before, when the 127.0.0.11 resolver was not used for the default network,
the buildkit container would have inherited a site-local nameserver. So,
the build containers it created would also have inherited that DNS
server - and they'd be able to resolve site-local hostnames.
By replacing the site-local nameserver with Google's, we broke access
to local DNS and its hostnames.
Signed-off-by: Rob Murray <rob.murray@docker.com>
Don't fall-back to Google's DNS servers in a network that has an
internal resolver.
Now the default bridge uses the internal resolver, the only reason a
network started by the daemon should end up without any upstream
servers is if the host's resolv.conf doesn't list any. In this case,
the '--dns' option can be used to explicitly configure nameservers
for a container if necessary.
(Note that buildkit's containers do not have an internal resolver, so
they will still set up Google's nameservers if the host has no
resolvers that can be used in the container's namespace.)
Signed-off-by: Rob Murray <rob.murray@docker.com>
The previous allocator was subnetting address pools eagerly
when the daemon started, and would then just iterate over that
list whenever RequestPool was called. This was leading to high
memory usage whenever IPv6 pools were configured with a target
subnet size too different from the pools prefix size.
For instance: pool = fd00::/8, target size = /64 -- 2 ^ (64-8)
subnets would be generated upfront. This would take approx.
9 * 10^18 bits -- way too much for any human computer in 2024.
Another noteworthy issue, the previous implementation was allocating
a subnet, and then in another layer was checking whether the
allocation was conflicting with some 'reserved networks'. If so,
the allocation would be retried, etc... To make it worse, 'reserved
networks' would be recomputed on every iteration. This is totally
ineffective as there could be 'reserved networks' that fully overlap
a given address pool (or many!).
To fix this issue, a new field `Exclude` is added to `RequestPool`.
It's up to each driver to take it into account. Since we don't know
whether this retry loop is useful for some remote IPAM driver, it's
reimplemented bug-for-bug directly in the remote driver.
The new allocator uses a linear-search algorithm. It takes advantage
of all lists (predefined pools, allocated subnets and reserved
networks) being sorted and logically combines 'allocated' and
'reserved' through a 'double cursor' to iterate on both lists at the
same time while preserving the total order. At the same time, it
iterates over 'predefined' pools and looks for the first empty space
that would be a good fit.
Currently, the size of the allocated subnet is still dictated by
each 'predefined' pools. We should consider hardcoding that size
instead, and let users specify what subnet size they want. This
wasn't possible before as the subnets were generated upfront. This
new allocator should be able to deal with this easily.
The method used for static allocation has been updated to make sure
the ascending order of 'allocated' is preserved. It's bug-for-bug
compatible with the previous implementation.
One consequence of this new algorithm is that we don't keep track
of where the last allocation happened, we just allocate the first
free subnet we find.
Before:
- Allocate: 10.0.1.0/24, 10.0.2.0/24 ; Deallocate: 10.0.1.0/24 ;
Allocate 10.0.3.0/24.
Now, the 3rd allocation would yield 10.0.1.0/24 once again.
As it doesn't change the semantics of the allocator, there's no
reason to worry about that.
Finally, about 'reserved networks'. The heuristics we use are
now properly documented. It was discovered that we don't check
routes for IPv6 allocations -- this can't be changed because
there's no such thing as on-link routes for IPv6.
(Kudos to Rob Murray for coming up with the linear-search idea.)
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
Commit 8921897e3b introduced the uses of `clear()`,
which requires go1.21, but Go is downgrading this file to go1.16 when used in
other projects (due to us not yet being a go module);
0.175 + xx-go build '-gcflags=' -ldflags '-X github.com/moby/buildkit/version.Version=b53a13e -X github.com/moby/buildkit/version.Revision=b53a13e4f5c8d7e82716615e0f23656893df89af -X github.com/moby/buildkit/version.Package=github.com/moby/buildkit -extldflags '"'"'-static'"'" -tags 'osusergo netgo static_build seccomp ' -o /usr/bin/buildkitd ./cmd/buildkitd
181.8 # github.com/docker/docker/libnetwork/internal/resolvconf
181.8 vendor/github.com/docker/docker/libnetwork/internal/resolvconf/resolvconf.go:509:2: clear requires go1.21 or later (-lang was set to go1.16; check go.mod)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Rather than error out if the host's resolv.conf has a bad ndots option,
just ignore it. Still validate ndots supplied via '--dns-option' and
treat failure as an error.
Signed-off-by: Rob Murray <rob.murray@docker.com>
When configuring the internal DNS resolver - rather than keep IPv6
nameservers read from the host's resolv.conf in the container's
resolv.conf, treat them like IPv4 addresses and use them as upstream
resolvers.
For IPv6 nameservers, if there's a zone identifier in the address or
the container itself doesn't have IPv6 support, mark the upstream
addresses for use in the host's network namespace.
Signed-off-by: Rob Murray <rob.murray@docker.com>
A common pattern in libnetwork is to delete an object using
`DeleteAtomic`, ie. to check the optimistic lock, but put in a retry
loop to refresh the data and the version index used by the optimistic
lock.
This commit introduces a new `Delete` method to delete without
checking the optimistic lock. It focuses only on the few places where
it's obvious the calling code doesn't rely on the side-effects of the
retry loop (ie. refreshing the object to be deleted).
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
Replace regex matching/replacement and re-reading of generated files
with a simple parser, and struct to remember and manipulate the file
content.
Annotate the generated file with a header comment saying the file is
generated, but can be modified, and a trailing comment describing how
the file was generated and listing external nameservers.
Always start with the host's resolv.conf file, whether generating config
for host networking, or with/without an internal resolver - rather than
editing a file previously generated for a different use-case.
Resolves an issue where rewrites of the generated file resulted in
default IPv6 nameservers being unnecessarily added to the config.
Signed-off-by: Rob Murray <rob.murray@docker.com>
Previous commit made getDBhandle a one-liner returning a struct
member -- making it useless. Inline it.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
This parameter was used to tell the boltdb kvstore not to open/close
the underlying boltdb db file before/after each get/put operation.
Since d21d0884ae, we've a single datastore instance shared by all
components that need it. That commit set `PersistConnection=true`.
We can now safely remove this param altogether, and remove all the
code that was opening and closing the db file before and after each
operation -- it's dead code!
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
File paths can contain commas, particularly paths returned from
t.TempDir() in subtests which include commas in their names. There is
only one datastore provider and it only supports a single address, so
the only use of parsing the address is to break tests in mysterious
ways.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Calculate the IPv6 addreesses needed on a bridge, then reconcile them
with the addresses on an existing bridge by deleting then adding as
required.
(Previously, required addresses were added one-by-one, then unwanted
addresses were removed. This meant the daemon failed to start if, for
example, an existing bridge had address '2000:db8::/64' and the config
was changed to '2000:db8::/80'.)
IPv6 addresses are now calculated and applied in one go, so there's no
need for setupVerifyAndReconcile() to check the set of IPv6 addresses on
the bridge. And, it was guarded by !config.InhibitIPv4, which can't have
been right. So, removed its IPv6 parts, and added IPv4 to its name.
Link local addresses, the example given in the original ticket, are now
released when containers are stopped. Not releasing them meant that
when using an LL subnet on the default bridge, no container could be
started after a container was stopped (because the calculated address
could not be re-allocated). In non-default bridge networks using an
LL subnet, addresses leaked.
Linux always uses the standard 'fe80::/64' LL network. So, if a bridge
is configured with an LL subnet prefix that overlaps with it, a config
error is reported. Non-overlapping LL subnet prefixes are allowed.
Signed-off-by: Rob Murray <rob.murray@docker.com>
This repository is not yet a module (i.e., does not have a `go.mod`). This
is not problematic when building the code in GOPATH or "vendor" mode, but
when using the code as a module-dependency (in module-mode), different semantics
are applied since Go1.21, which switches Go _language versions_ on a per-module,
per-package, or even per-file base.
A condensed summary of that logic [is as follows][1]:
- For modules that have a go.mod containing a go version directive; that
version is considered a minimum _required_ version (starting with the
go1.19.13 and go1.20.8 patch releases: before those, it was only a
recommendation).
- For dependencies that don't have a go.mod (not a module), go language
version go1.16 is assumed.
- Likewise, for modules that have a go.mod, but the file does not have a
go version directive, go language version go1.16 is assumed.
- If a go.work file is present, but does not have a go version directive,
language version go1.17 is assumed.
When switching language versions, Go _downgrades_ the language version,
which means that language features (such as generics, and `any`) are not
available, and compilation fails. For example:
# github.com/docker/cli/cli/context/store
/go/pkg/mod/github.com/docker/cli@v25.0.0-beta.2+incompatible/cli/context/store/storeconfig.go:6:24: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
/go/pkg/mod/github.com/docker/cli@v25.0.0-beta.2+incompatible/cli/context/store/store.go:74:12: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
Note that these fallbacks are per-module, per-package, and can even be
per-file, so _(indirect) dependencies_ can still use modern language
features, as long as their respective go.mod has a version specified.
Unfortunately, these failures do not occur when building locally (using
vendor / GOPATH mode), but will affect consumers of the module.
Obviously, this situation is not ideal, and the ultimate solution is to
move to go modules (add a go.mod), but this comes with a non-insignificant
risk in other areas (due to our complex dependency tree).
We can revert to using go1.16 language features only, but this may be
limiting, and may still be problematic when (e.g.) matching signatures
of dependencies.
There is an escape hatch: adding a `//go:build` directive to files that
make use of go language features. From the [go toolchain docs][2]:
> The go line for each module sets the language version the compiler enforces
> when compiling packages in that module. The language version can be changed
> on a per-file basis by using a build constraint.
>
> For example, a module containing code that uses the Go 1.21 language version
> should have a `go.mod` file with a go line such as `go 1.21` or `go 1.21.3`.
> If a specific source file should be compiled only when using a newer Go
> toolchain, adding `//go:build go1.22` to that source file both ensures that
> only Go 1.22 and newer toolchains will compile the file and also changes
> the language version in that file to Go 1.22.
This patch adds `//go:build` directives to those files using recent additions
to the language. It's currently using go1.19 as version to match the version
in our "vendor.mod", but we can consider being more permissive ("any" requires
go1.18 or up), or more "optimistic" (force go1.21, which is the version we
currently use to build).
For completeness sake, note that any file _without_ a `//go:build` directive
will continue to use go1.16 language version when used as a module.
[1]: 58c28ba286/src/cmd/go/internal/gover/version.go (L9-L56)
[2]: https://go.dev/doc/toolchain
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Make the code slightly more idiomatic; remove some "var" declarations,
remove some intermediate variables and redundant error-checks, and remove
the filePerm const.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This boolean was not used anywhere, so we can remove it. Also cleaning up
the implementation a bit.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The WriteOptions struct was only used to set the "IsDir" option. This option
was added in d635a8e32b
and was only supported by the etcd libkv store.
The BoltDB store does not support this option, making the WriteOptions
struct fully redundant.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The only remaining kvstore is BoltDB, which doesn't use TLS connections
or authentication, so we can remove these options.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
