diff --git a/daemon/cluster/executor/container/adapter.go b/daemon/cluster/executor/container/adapter.go index 0208ab9975..b4ac57bede 100644 --- a/daemon/cluster/executor/container/adapter.go +++ b/daemon/cluster/executor/container/adapter.go @@ -180,13 +180,13 @@ func (c *containerAdapter) waitNodeAttachments(ctx context.Context) error { // set a flag ready to true. if we try to get a network IP that doesn't // exist yet, we will set this flag to "false" ready := true - for _, attachment := range c.container.networksAttachments { + for _, nw := range c.container.networks { // we only need node attachments (IP address) for overlay networks // TODO(dperny): unsure if this will work with other network // drivers, but i also don't think other network drivers use the // node attachment IP address. - if attachment.Network.DriverState.Name == "overlay" { - if _, exists := attachmentStore.GetIPForNetwork(attachment.Network.ID); !exists { + if nw.DriverState.Name == "overlay" { + if _, exists := attachmentStore.GetIPForNetwork(nw.ID); !exists { ready = false } } @@ -207,12 +207,8 @@ func (c *containerAdapter) waitNodeAttachments(ctx context.Context) error { } func (c *containerAdapter) createNetworks(ctx context.Context) error { - for name := range c.container.networksAttachments { - ncr, err := c.container.networkCreateRequest(name) - if err != nil { - return err - } - + for name, nw := range c.container.networks { + ncr := networkCreateRequest(name, nw) if err := c.backend.CreateManagedNetwork(ncr); err != nil { // todo name missing if _, ok := err.(libnetwork.NetworkNameError); ok { continue @@ -235,8 +231,8 @@ func (c *containerAdapter) removeNetworks(ctx context.Context) error { errNoSuchNetwork libnetwork.ErrNoSuchNetwork ) - for name, v := range c.container.networksAttachments { - if err := c.backend.DeleteManagedNetwork(v.Network.ID); err != nil { + for name, nw := range c.container.networks { + if err := c.backend.DeleteManagedNetwork(nw.ID); err != nil { switch { case errors.As(err, &activeEndpointsError): continue diff --git a/daemon/cluster/executor/container/adapter_test.go b/daemon/cluster/executor/container/adapter_test.go index 03b3b9ab45..8db2b0949d 100644 --- a/daemon/cluster/executor/container/adapter_test.go +++ b/daemon/cluster/executor/container/adapter_test.go @@ -34,35 +34,29 @@ func TestWaitNodeAttachment(t *testing.T) { // actually; only the networkAttachments are needed. container := &containerConfig{ task: nil, - networksAttachments: map[string]*api.NetworkAttachment{ + networks: map[string]*api.Network{ // network1 is already present in the attachment store. "network1": { - Network: &api.Network{ - ID: "network1", - DriverState: &api.Driver{ - Name: "overlay", - }, + ID: "network1", + DriverState: &api.Driver{ + Name: "overlay", }, }, // network2 is not yet present in the attachment store, and we // should block while waiting for it. "network2": { - Network: &api.Network{ - ID: "network2", - DriverState: &api.Driver{ - Name: "overlay", - }, + ID: "network2", + DriverState: &api.Driver{ + Name: "overlay", }, }, // localnetwork is not and will never be in the attachment store, // but we should not block on it, because it is not an overlay // network "localnetwork": { - Network: &api.Network{ - ID: "localnetwork", - DriverState: &api.Driver{ - Name: "bridge", - }, + ID: "localnetwork", + DriverState: &api.Driver{ + Name: "bridge", }, }, }, diff --git a/daemon/cluster/executor/container/container.go b/daemon/cluster/executor/container/container.go index 4e698d2d22..6bbdec4ecc 100644 --- a/daemon/cluster/executor/container/container.go +++ b/daemon/cluster/executor/container/container.go @@ -3,7 +3,6 @@ package container // import "github.com/docker/docker/daemon/cluster/executor/co import ( "context" "encoding/json" - "errors" "fmt" "net" "strconv" @@ -37,8 +36,8 @@ const ( // containerConfig converts task properties into docker container compatible // components. type containerConfig struct { - task *api.Task - networksAttachments map[string]*api.NetworkAttachment + task *api.Task + networks map[string]*api.Network } // newContainerConfig returns a validated container config. No methods should @@ -65,9 +64,16 @@ func (c *containerConfig) setTask(t *api.Task, node *api.NodeDescription) error } // index the networks by name - c.networksAttachments = make(map[string]*api.NetworkAttachment, len(t.Networks)) + c.networks = make(map[string]*api.Network, len(t.Networks)) for _, attachment := range t.Networks { - c.networksAttachments[attachment.Network.Spec.Annotations.Name] = attachment + // It looks like using a map is only for convenience, but not used + // for validation, nor for looking up the network by name. The name + // is part of the Network's properties (Network.Spec.Annotations.Name), + // and effectively only used for debugging; we should consider to + // change it to a slice. + // + // TODO(thaJeztah): should this check for empty and duplicate names? + c.networks[attachment.Network.Spec.Annotations.Name] = attachment.Network } c.task = t @@ -620,57 +626,51 @@ func (c *containerConfig) serviceConfig() *clustertypes.ServiceConfig { return svcCfg } -func (c *containerConfig) networkCreateRequest(name string) (clustertypes.NetworkCreateRequest, error) { - na, ok := c.networksAttachments[name] - if !ok { - return clustertypes.NetworkCreateRequest{}, errors.New("container: unknown network referenced") - } - +func networkCreateRequest(name string, nw *api.Network) clustertypes.NetworkCreateRequest { ipv4Enabled := true - ipv6Enabled := na.Network.Spec.Ipv6Enabled + ipv6Enabled := nw.Spec.Ipv6Enabled options := network.CreateOptions{ - // ID: na.Network.ID, - Labels: na.Network.Spec.Annotations.Labels, - Internal: na.Network.Spec.Internal, - Attachable: na.Network.Spec.Attachable, - Ingress: convert.IsIngressNetwork(na.Network), + // ID: nw.ID, + Labels: nw.Spec.Annotations.Labels, + Internal: nw.Spec.Internal, + Attachable: nw.Spec.Attachable, + Ingress: convert.IsIngressNetwork(nw), EnableIPv4: &ipv4Enabled, EnableIPv6: &ipv6Enabled, Scope: scope.Swarm, } - if na.Network.Spec.GetNetwork() != "" { + if nw.Spec.GetNetwork() != "" { options.ConfigFrom = &network.ConfigReference{ - Network: na.Network.Spec.GetNetwork(), + Network: nw.Spec.GetNetwork(), } } - if na.Network.DriverState != nil { - options.Driver = na.Network.DriverState.Name - options.Options = na.Network.DriverState.Options + if nw.DriverState != nil { + options.Driver = nw.DriverState.Name + options.Options = nw.DriverState.Options } - if na.Network.IPAM != nil { + if nw.IPAM != nil { options.IPAM = &network.IPAM{ - Driver: na.Network.IPAM.Driver.Name, - Options: na.Network.IPAM.Driver.Options, + Driver: nw.IPAM.Driver.Name, + Options: nw.IPAM.Driver.Options, } - for _, ic := range na.Network.IPAM.Configs { - c := network.IPAMConfig{ + for _, ic := range nw.IPAM.Configs { + options.IPAM.Config = append(options.IPAM.Config, network.IPAMConfig{ Subnet: ic.Subnet, IPRange: ic.Range, Gateway: ic.Gateway, - } - options.IPAM.Config = append(options.IPAM.Config, c) + }) } } return clustertypes.NetworkCreateRequest{ - ID: na.Network.ID, + ID: nw.ID, CreateRequest: network.CreateRequest{ - Name: name, + Name: name, // TODO(thaJeztah): this is the same as [nw.Spec.Annotations.Name]; consider using that instead CreateOptions: options, }, - }, nil + } } func (c *containerConfig) applyPrivileges(hc *containertypes.HostConfig) {