Skip to content

Commit

Permalink
Merge pull request moby#46853 from akerouanton/libnet-ep-dns-names
Browse files Browse the repository at this point in the history
libnet: Endpoint: remove isAnonymous & myAliases
  • Loading branch information
thaJeztah committed Dec 20, 2023
2 parents 388216f + 13915f6 commit 7bc56c5
Show file tree
Hide file tree
Showing 18 changed files with 295 additions and 173 deletions.
15 changes: 15 additions & 0 deletions api/swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2530,6 +2530,21 @@ definitions:
example:
com.example.some-label: "some-value"
com.example.some-other-label: "some-other-value"
DNSNames:
description: |
List of all DNS names an endpoint has on a specific network. This
list is based on the container name, network aliases, container short
ID, and hostname.
These DNS names are non-fully qualified but can contain several dots.
You can get fully qualified DNS names by appending `.<network-name>`.
For instance, if container name is `my.ctr` and the network is named
`testnet`, `DNSNames` will contain `my.ctr` and the FQDN will be
`my.ctr.testnet`.
type: array
items:
type: string
example: ["foobar", "server_x", "server_y", "my.ctr"]

EndpointIPAMConfig:
description: |
Expand Down
11 changes: 10 additions & 1 deletion api/types/network/endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ type EndpointSettings struct {
// Configurations
IPAMConfig *EndpointIPAMConfig
Links []string
Aliases []string
Aliases []string // Aliases holds the list of extra, user-specified DNS names for this endpoint.
MacAddress string
// Operational data
NetworkID string
Expand All @@ -25,6 +25,9 @@ type EndpointSettings struct {
GlobalIPv6Address string
GlobalIPv6PrefixLen int
DriverOpts map[string]string
// DNSNames holds all the (non fully qualified) DNS names associated to this endpoint. First entry is used to
// generate PTR records.
DNSNames []string
}

// Copy makes a deep copy of `EndpointSettings`
Expand All @@ -43,6 +46,12 @@ func (es *EndpointSettings) Copy() *EndpointSettings {
aliases := make([]string, 0, len(es.Aliases))
epCopy.Aliases = append(aliases, es.Aliases...)
}

if len(es.DNSNames) > 0 {
epCopy.DNSNames = make([]string, len(es.DNSNames))
copy(epCopy.DNSNames, es.DNSNames)
}

return &epCopy
}

Expand Down
7 changes: 3 additions & 4 deletions daemon/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,8 @@ func (daemon *Daemon) Register(c *container.Container) error {

func (daemon *Daemon) newContainer(name string, operatingSystem string, config *containertypes.Config, hostConfig *containertypes.HostConfig, imgID image.ID, managed bool) (*container.Container, error) {
var (
id string
err error
noExplicitName = name == ""
id string
err error
)
id, name, err = daemon.generateIDAndName(name)
if err != nil {
Expand All @@ -151,7 +150,7 @@ func (daemon *Daemon) newContainer(name string, operatingSystem string, config *
base.Config = config
base.HostConfig = &containertypes.HostConfig{}
base.ImageID = imgID
base.NetworkSettings = &network.Settings{IsAnonymousEndpoint: noExplicitName}
base.NetworkSettings = &network.Settings{}
base.Name = name
base.Driver = daemon.imageService.StorageDriver()
base.OS = operatingSystem
Expand Down
48 changes: 25 additions & 23 deletions daemon/container_operations.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/docker/docker/daemon/network"
"github.com/docker/docker/errdefs"
"github.com/docker/docker/internal/multierror"
"github.com/docker/docker/internal/sliceutil"
"github.com/docker/docker/libnetwork"
"github.com/docker/docker/libnetwork/netlabel"
"github.com/docker/docker/libnetwork/options"
Expand Down Expand Up @@ -650,29 +651,7 @@ func cleanOperationalData(es *network.EndpointSettings) {

func (daemon *Daemon) updateNetworkConfig(container *container.Container, n *libnetwork.Network, endpointConfig *networktypes.EndpointSettings, updateSettings bool) error {
if containertypes.NetworkMode(n.Name()).IsUserDefined() {
addShortID := true
shortID := stringid.TruncateID(container.ID)
for _, alias := range endpointConfig.Aliases {
if alias == shortID {
addShortID = false
break
}
}
if addShortID {
endpointConfig.Aliases = append(endpointConfig.Aliases, shortID)
}
if container.Name != container.Config.Hostname {
addHostname := true
for _, alias := range endpointConfig.Aliases {
if alias == container.Config.Hostname {
addHostname = false
break
}
}
if addHostname {
endpointConfig.Aliases = append(endpointConfig.Aliases, container.Config.Hostname)
}
}
endpointConfig.DNSNames = buildEndpointDNSNames(container, endpointConfig.Aliases)
}

if err := validateEndpointSettings(n, n.Name(), endpointConfig); err != nil {
Expand All @@ -687,6 +666,29 @@ func (daemon *Daemon) updateNetworkConfig(container *container.Container, n *lib
return nil
}

// buildEndpointDNSNames constructs the list of DNSNames that should be assigned to a given endpoint. The order within
// the returned slice is important as the first entry will be used to generate the PTR records (for IPv4 and v6)
// associated to this endpoint.
func buildEndpointDNSNames(ctr *container.Container, aliases []string) []string {
var dnsNames []string

if ctr.Name != "" {
dnsNames = append(dnsNames, strings.TrimPrefix(ctr.Name, "/"))
}

dnsNames = append(dnsNames, aliases...)

if ctr.ID != "" {
dnsNames = append(dnsNames, stringid.TruncateID(ctr.ID))
}

if ctr.Config.Hostname != "" {
dnsNames = append(dnsNames, ctr.Config.Hostname)
}

return sliceutil.Dedup(dnsNames)
}

func (daemon *Daemon) connectToNetwork(cfg *config.Config, container *container.Container, idOrName string, endpointConfig *networktypes.EndpointSettings, updateSettings bool) (err error) {
start := time.Now()
if container.HostConfig.NetworkMode.IsContainer() {
Expand Down
55 changes: 55 additions & 0 deletions daemon/container_operations_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package daemon

import (
"encoding/json"
"testing"

containertypes "github.com/docker/docker/api/types/container"
networktypes "github.com/docker/docker/api/types/network"
"github.com/docker/docker/container"
"github.com/docker/docker/libnetwork"
"gotest.tools/v3/assert"
is "gotest.tools/v3/assert/cmp"
)

func TestDNSNamesOrder(t *testing.T) {
d := &Daemon{}
ctr := &container.Container{
ID: "35de8003b19e27f636fc6ecbf4d7072558b872a8544f287fd69ad8182ad59023",
Name: "foobar",
Config: &containertypes.Config{
Hostname: "baz",
},
}
nw := buildNetwork(t, map[string]any{
"id": "1234567890",
"name": "testnet",
"networkType": "bridge",
"enableIPv6": false,
})
epSettings := &networktypes.EndpointSettings{
Aliases: []string{"myctr"},
}

if err := d.updateNetworkConfig(ctr, nw, epSettings, false); err != nil {
t.Fatal(err)
}

assert.Check(t, is.DeepEqual(epSettings.DNSNames, []string{"foobar", "myctr", "35de8003b19e", "baz"}))
}

func buildNetwork(t *testing.T, config map[string]any) *libnetwork.Network {
t.Helper()

b, err := json.Marshal(config)
if err != nil {
t.Fatal(err)
}

nw := &libnetwork.Network{}
if err := nw.UnmarshalJSON(b); err != nil {
t.Fatal(err)
}

return nw
}
14 changes: 14 additions & 0 deletions daemon/inspect.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import (
"github.com/docker/docker/daemon/config"
"github.com/docker/docker/daemon/network"
"github.com/docker/docker/errdefs"
"github.com/docker/docker/internal/sliceutil"
"github.com/docker/docker/pkg/stringid"
"github.com/docker/go-connections/nat"
)

Expand All @@ -27,6 +29,18 @@ func (daemon *Daemon) ContainerInspect(ctx context.Context, name string, size bo
return daemon.containerInspectPre120(ctx, name)
case versions.Equal(version, "1.20"):
return daemon.containerInspect120(name)
case versions.LessThan(version, "1.45"):
ctr, err := daemon.ContainerInspectCurrent(ctx, name, size)
if err != nil {
return nil, err
}

shortCID := stringid.TruncateID(ctr.ID)
for _, ep := range ctr.NetworkSettings.Networks {
ep.Aliases = sliceutil.Dedup(append(ep.Aliases, shortCID, ctr.Config.Hostname))
}

return ctr, nil
default:
return daemon.ContainerInspectCurrent(ctx, name, size)
}
Expand Down
9 changes: 2 additions & 7 deletions daemon/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -793,10 +793,6 @@ func buildCreateEndpointOptions(c *container.Container, n *libnetwork.Network, e
var genericOptions = make(options.Generic)

nwName := n.Name()
defaultNetName := runconfig.DefaultDaemonNetworkMode().NetworkName()
if c.NetworkSettings.IsAnonymousEndpoint || (nwName == defaultNetName && !serviceDiscoveryOnDefaultNetwork()) {
createOptions = append(createOptions, libnetwork.CreateOptionAnonymous())
}

if epConfig != nil {
if ipam := epConfig.IPAMConfig; ipam != nil {
Expand All @@ -822,9 +818,8 @@ func buildCreateEndpointOptions(c *container.Container, n *libnetwork.Network, e
createOptions = append(createOptions, libnetwork.CreateOptionIpam(ip, ip6, ipList, nil))
}

for _, alias := range epConfig.Aliases {
createOptions = append(createOptions, libnetwork.CreateOptionMyAlias(alias))
}
createOptions = append(createOptions, libnetwork.CreateOptionDNSNames(epConfig.DNSNames))

for k, v := range epConfig.DriverOpts {
createOptions = append(createOptions, libnetwork.EndpointOptionGeneric(options.Generic{k: v}))
}
Expand Down
1 change: 0 additions & 1 deletion daemon/network/settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ type Settings struct {
Ports nat.PortMap
SecondaryIPAddresses []networktypes.Address
SecondaryIPv6Addresses []networktypes.Address
IsAnonymousEndpoint bool
HasSwarmEndpoint bool
}

Expand Down
56 changes: 50 additions & 6 deletions daemon/rename.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/containerd/log"
"github.com/docker/docker/api/types/events"
dockercontainer "github.com/docker/docker/container"
"github.com/docker/docker/daemon/network"
"github.com/docker/docker/errdefs"
"github.com/docker/docker/libnetwork"
"github.com/pkg/errors"
Expand Down Expand Up @@ -38,7 +39,6 @@ func (daemon *Daemon) ContainerRename(oldName, newName string) (retErr error) {
defer container.Unlock()

oldName = container.Name
oldIsAnonymousEndpoint := container.NetworkSettings.IsAnonymousEndpoint

if oldName == newName {
return errdefs.InvalidParameter(errors.New("Renaming a container with the same name as its current name"))
Expand All @@ -62,12 +62,10 @@ func (daemon *Daemon) ContainerRename(oldName, newName string) (retErr error) {
}

container.Name = newName
container.NetworkSettings.IsAnonymousEndpoint = false

defer func() {
if retErr != nil {
container.Name = oldName
container.NetworkSettings.IsAnonymousEndpoint = oldIsAnonymousEndpoint
daemon.reserveName(container.ID, oldName)
for k, v := range links {
daemon.containersReplica.ReserveName(oldName+k, v.ID)
Expand Down Expand Up @@ -101,7 +99,6 @@ func (daemon *Daemon) ContainerRename(oldName, newName string) (retErr error) {
defer func() {
if retErr != nil {
container.Name = oldName
container.NetworkSettings.IsAnonymousEndpoint = oldIsAnonymousEndpoint
if err := container.CheckpointTo(daemon.containersReplica); err != nil {
log.G(context.TODO()).WithFields(log.Fields{
"containerID": container.ID,
Expand All @@ -118,10 +115,57 @@ func (daemon *Daemon) ContainerRename(oldName, newName string) (retErr error) {
return err
}

err = sb.Rename(strings.TrimPrefix(container.Name, "/"))
if err != nil {
if err := sb.Rename(newName[1:]); err != nil {
return err
}
defer func() {
if retErr != nil {
if err := sb.Rename(oldName); err != nil {
log.G(context.TODO()).WithFields(log.Fields{
"sandboxID": sid,
"oldName": oldName,
"newName": newName,
"error": err,
}).Errorf("failed to revert sandbox rename")
}
}
}()

for nwName, epConfig := range container.NetworkSettings.Networks {
nw, err := daemon.FindNetwork(nwName)
if err != nil {
return err
}

ep, err := nw.EndpointByID(epConfig.EndpointID)
if err != nil {
return err
}

oldDNSNames := make([]string, len(epConfig.DNSNames))
copy(oldDNSNames, epConfig.DNSNames)

epConfig.DNSNames = buildEndpointDNSNames(container, epConfig.Aliases)
if err := ep.UpdateDNSNames(epConfig.DNSNames); err != nil {
return err
}

defer func(ep *libnetwork.Endpoint, epConfig *network.EndpointSettings, oldDNSNames []string) {
if retErr == nil {
return
}

epConfig.DNSNames = oldDNSNames
if err := ep.UpdateDNSNames(epConfig.DNSNames); err != nil {
log.G(context.TODO()).WithFields(log.Fields{
"sandboxID": sid,
"oldName": oldName,
"newName": newName,
"error": err,
}).Errorf("failed to revert DNSNames update")
}
}(ep, epConfig, oldDNSNames)
}
}

daemon.LogContainerEventWithAttributes(container, events.ActionRename, attributes)
Expand Down
7 changes: 7 additions & 0 deletions docs/api/version-history.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,13 @@ keywords: "API, Docker, rcli, REST, documentation"
* The `Container` and `ContainerConfig` fields in the `GET /images/{name}/json`
response are deprecated and will no longer be included in API v1.45.
* `GET /info` now includes `status` properties in `Runtimes`.
* A new field named `DNSNames` and containing all non-fully qualified DNS names
a container takes on a specific network has been added to `GET /containers/{name:.*}/json`.
* The `Aliases` field returned in calls to `GET /containers/{name:.*}/json` in v1.44 and older
versions contains the short container ID. This will change in the next API version, v1.45.
Starting with that API version, this specific value will be removed from the `Aliases` field
such that this field will reflect exactly the values originally submitted to the
`POST /containers/create` endpoint. The newly introduced `DNSNames` should now be used instead.

## v1.43 API changes

Expand Down
3 changes: 2 additions & 1 deletion hack/make/test-docker-py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ source hack/make/.integration-test-helpers
# --deselect=tests/integration/api_container_test.py::AttachContainerTest::test_attach_no_stream
# TODO re-enable test_attach_no_stream after https://github.com/docker/docker-py/issues/2513 is resolved
# TODO re-enable test_run_container_reading_socket_ws. It's reported in https://github.com/docker/docker-py/issues/1478, and we're getting that error in our tests.
: "${PY_TEST_OPTIONS:=--junitxml=${DEST}/junit-report.xml --deselect=tests/integration/api_container_test.py::AttachContainerTest::test_attach_no_stream --deselect=tests/integration/api_container_test.py::AttachContainerTest::test_run_container_reading_socket_ws}"
# TODO re-enable test_run_with_networking_config once this issue is fixed: https://github.com/moby/moby/pull/46853#issuecomment-1864679942.
: "${PY_TEST_OPTIONS:=--junitxml=${DEST}/junit-report.xml --deselect=tests/integration/api_container_test.py::AttachContainerTest::test_attach_no_stream --deselect=tests/integration/api_container_test.py::AttachContainerTest::test_run_container_reading_socket_ws --deselect=tests/integration/models_containers_test.py::ContainerCollectionTest::test_run_with_networking_config}"

# build --squash is not supported with containerd integration.
if [ -n "$TEST_INTEGRATION_USE_SNAPSHOTTER" ]; then
Expand Down
Loading

0 comments on commit 7bc56c5

Please sign in to comment.