From 9f4acceb6ab8337771d252170114a563f1e071a2 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 3 Jul 2023 23:47:40 +0200 Subject: [PATCH 1/3] remove redundant alias for libnetwork/datastore imports These aliases were not needed, and only used in a couple of places, which made it inconsistent, so let's use the import without aliasing. Signed-off-by: Sebastiaan van Stijn --- api/server/router/network/network_routes.go | 4 ++-- daemon/cluster/convert/network.go | 6 +++--- daemon/cluster/executor/container/container.go | 4 ++-- daemon/container_operations.go | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/api/server/router/network/network_routes.go b/api/server/router/network/network_routes.go index 01114c8682..79713d227c 100644 --- a/api/server/router/network/network_routes.go +++ b/api/server/router/network/network_routes.go @@ -13,7 +13,7 @@ import ( "github.com/docker/docker/api/types/versions" "github.com/docker/docker/errdefs" "github.com/docker/docker/libnetwork" - netconst "github.com/docker/docker/libnetwork/datastore" + "github.com/docker/docker/libnetwork/datastore" "github.com/pkg/errors" ) @@ -144,7 +144,7 @@ func (n *networkRouter) getNetwork(ctx context.Context, w http.ResponseWriter, r // or if the get network was passed with a network name and scope as swarm // return the network. Skipped using isMatchingScope because it is true if the scope // is not set which would be case if the client API v1.30 - if strings.HasPrefix(nwk.ID, term) || (netconst.SwarmScope == scope) { + if strings.HasPrefix(nwk.ID, term) || (datastore.SwarmScope == scope) { // If we have a previous match "backend", return it, we need verbose when enabled // ex: overlay/partial_ID or name/swarm_scope if nwv, ok := listByPartialID[nwk.ID]; ok { diff --git a/daemon/cluster/convert/network.go b/daemon/cluster/convert/network.go index c65a76fba5..fba1770451 100644 --- a/daemon/cluster/convert/network.go +++ b/daemon/cluster/convert/network.go @@ -6,7 +6,7 @@ import ( basictypes "github.com/docker/docker/api/types" networktypes "github.com/docker/docker/api/types/network" types "github.com/docker/docker/api/types/swarm" - netconst "github.com/docker/docker/libnetwork/datastore" + "github.com/docker/docker/libnetwork/datastore" gogotypes "github.com/gogo/protobuf/types" swarmapi "github.com/moby/swarmkit/v2/api" ) @@ -31,7 +31,7 @@ func networkFromGRPC(n *swarmapi.Network) types.Network { Attachable: n.Spec.Attachable, Ingress: IsIngressNetwork(n), IPAMOptions: ipamFromGRPC(n.Spec.IPAM), - Scope: netconst.SwarmScope, + Scope: datastore.SwarmScope, }, IPAMOptions: ipamFromGRPC(n.IPAM), } @@ -160,7 +160,7 @@ func BasicNetworkFromGRPC(n swarmapi.Network) basictypes.NetworkResource { nr := basictypes.NetworkResource{ ID: n.ID, Name: n.Spec.Annotations.Name, - Scope: netconst.SwarmScope, + Scope: datastore.SwarmScope, EnableIPv6: spec.Ipv6Enabled, IPAM: ipam, Internal: spec.Internal, diff --git a/daemon/cluster/executor/container/container.go b/daemon/cluster/executor/container/container.go index 945ca3b291..1aec7df3df 100644 --- a/daemon/cluster/executor/container/container.go +++ b/daemon/cluster/executor/container/container.go @@ -20,7 +20,7 @@ import ( "github.com/docker/docker/daemon/cluster/convert" executorpkg "github.com/docker/docker/daemon/cluster/executor" clustertypes "github.com/docker/docker/daemon/cluster/provider" - netconst "github.com/docker/docker/libnetwork/datastore" + "github.com/docker/docker/libnetwork/datastore" "github.com/docker/go-connections/nat" "github.com/docker/go-units" gogotypes "github.com/gogo/protobuf/types" @@ -645,7 +645,7 @@ func (c *containerConfig) networkCreateRequest(name string) (clustertypes.Networ Ingress: convert.IsIngressNetwork(na.Network), EnableIPv6: na.Network.Spec.Ipv6Enabled, CheckDuplicate: true, - Scope: netconst.SwarmScope, + Scope: datastore.SwarmScope, } if na.Network.Spec.GetNetwork() != "" { diff --git a/daemon/container_operations.go b/daemon/container_operations.go index 54aa445d03..774c769452 100644 --- a/daemon/container_operations.go +++ b/daemon/container_operations.go @@ -18,7 +18,7 @@ import ( "github.com/docker/docker/daemon/network" "github.com/docker/docker/errdefs" "github.com/docker/docker/libnetwork" - netconst "github.com/docker/docker/libnetwork/datastore" + "github.com/docker/docker/libnetwork/datastore" "github.com/docker/docker/libnetwork/netlabel" "github.com/docker/docker/libnetwork/options" "github.com/docker/docker/libnetwork/types" @@ -270,7 +270,7 @@ func (daemon *Daemon) updateNetworkSettings(container *container.Container, n li // is an attachable network, which may not // be locally available previously. // So always update. - if n.Info().Scope() == netconst.SwarmScope { + if n.Info().Scope() == datastore.SwarmScope { continue } // Avoid duplicate config From 8ea78b34ab3e3f4ec16ea5e19a928395d8978a4c Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 3 Jul 2023 23:49:57 +0200 Subject: [PATCH 2/3] rename some variables that shadowed imports or package types Signed-off-by: Sebastiaan van Stijn --- api/server/router/network/network_routes.go | 72 ++++++++++----------- daemon/container_operations.go | 14 ++-- 2 files changed, 43 insertions(+), 43 deletions(-) diff --git a/api/server/router/network/network_routes.go b/api/server/router/network/network_routes.go index 79713d227c..a5ce894cb9 100644 --- a/api/server/router/network/network_routes.go +++ b/api/server/router/network/network_routes.go @@ -121,20 +121,20 @@ func (n *networkRouter) getNetwork(ctx context.Context, w http.ResponseWriter, r if scope != "" { filter.Add("scope", scope) } - nw, _ := n.backend.GetNetworks(filter, types.NetworkListConfig{Detailed: true, Verbose: verbose}) - for _, network := range nw { - if network.ID == term { - return httputils.WriteJSON(w, http.StatusOK, network) + networks, _ := n.backend.GetNetworks(filter, types.NetworkListConfig{Detailed: true, Verbose: verbose}) + for _, nw := range networks { + if nw.ID == term { + return httputils.WriteJSON(w, http.StatusOK, nw) } - if network.Name == term { + if nw.Name == term { // No need to check the ID collision here as we are still in // local scope and the network ID is unique in this scope. - listByFullName[network.ID] = network + listByFullName[nw.ID] = nw } - if strings.HasPrefix(network.ID, term) { + if strings.HasPrefix(nw.ID, term) { // No need to check the ID collision here as we are still in // local scope and the network ID is unique in this scope. - listByPartialID[network.ID] = network + listByPartialID[nw.ID] = nw } } @@ -156,25 +156,25 @@ func (n *networkRouter) getNetwork(ctx context.Context, w http.ResponseWriter, r } } - nr, _ := n.cluster.GetNetworks(filter) - for _, network := range nr { - if network.ID == term { - return httputils.WriteJSON(w, http.StatusOK, network) + networks, _ = n.cluster.GetNetworks(filter) + for _, nw := range networks { + if nw.ID == term { + return httputils.WriteJSON(w, http.StatusOK, nw) } - if network.Name == term { + if nw.Name == term { // Check the ID collision as we are in swarm scope here, and // the map (of the listByFullName) may have already had a // network with the same ID (from local scope previously) - if _, ok := listByFullName[network.ID]; !ok { - listByFullName[network.ID] = network + if _, ok := listByFullName[nw.ID]; !ok { + listByFullName[nw.ID] = nw } } - if strings.HasPrefix(network.ID, term) { + if strings.HasPrefix(nw.ID, term) { // Check the ID collision as we are in swarm scope here, and // the map (of the listByPartialID) may have already had a // network with the same ID (from local scope previously) - if _, ok := listByPartialID[network.ID]; !ok { - listByPartialID[network.ID] = network + if _, ok := listByPartialID[nw.ID]; !ok { + listByPartialID[nw.ID] = nw } } } @@ -326,42 +326,42 @@ func (n *networkRouter) findUniqueNetwork(term string) (types.NetworkResource, e listByPartialID := map[string]types.NetworkResource{} filter := filters.NewArgs(filters.Arg("idOrName", term)) - nw, _ := n.backend.GetNetworks(filter, types.NetworkListConfig{Detailed: true}) - for _, network := range nw { - if network.ID == term { - return network, nil + networks, _ := n.backend.GetNetworks(filter, types.NetworkListConfig{Detailed: true}) + for _, nw := range networks { + if nw.ID == term { + return nw, nil } - if network.Name == term && !network.Ingress { + if nw.Name == term && !nw.Ingress { // No need to check the ID collision here as we are still in // local scope and the network ID is unique in this scope. - listByFullName[network.ID] = network + listByFullName[nw.ID] = nw } - if strings.HasPrefix(network.ID, term) { + if strings.HasPrefix(nw.ID, term) { // No need to check the ID collision here as we are still in // local scope and the network ID is unique in this scope. - listByPartialID[network.ID] = network + listByPartialID[nw.ID] = nw } } - nr, _ := n.cluster.GetNetworks(filter) - for _, network := range nr { - if network.ID == term { - return network, nil + networks, _ = n.cluster.GetNetworks(filter) + for _, nw := range networks { + if nw.ID == term { + return nw, nil } - if network.Name == term { + if nw.Name == term { // Check the ID collision as we are in swarm scope here, and // the map (of the listByFullName) may have already had a // network with the same ID (from local scope previously) - if _, ok := listByFullName[network.ID]; !ok { - listByFullName[network.ID] = network + if _, ok := listByFullName[nw.ID]; !ok { + listByFullName[nw.ID] = nw } } - if strings.HasPrefix(network.ID, term) { + if strings.HasPrefix(nw.ID, term) { // Check the ID collision as we are in swarm scope here, and // the map (of the listByPartialID) may have already had a // network with the same ID (from local scope previously) - if _, ok := listByPartialID[network.ID]; !ok { - listByPartialID[network.ID] = network + if _, ok := listByPartialID[nw.ID]; !ok { + listByPartialID[nw.ID] = nw } } } diff --git a/daemon/container_operations.go b/daemon/container_operations.go index 774c769452..2a8a4e4351 100644 --- a/daemon/container_operations.go +++ b/daemon/container_operations.go @@ -392,7 +392,7 @@ func (daemon *Daemon) findAndAttachNetwork(container *container.Container, idOrN } var ( - config *networktypes.NetworkingConfig + nwCfg *networktypes.NetworkingConfig retryCount int ) @@ -406,7 +406,7 @@ func (daemon *Daemon) findAndAttachNetwork(container *container.Container, idOrN // trigger attachment in the swarm cluster manager. if daemon.clusterProvider != nil { var err error - config, err = daemon.clusterProvider.AttachNetwork(id, container.ID, addresses) + nwCfg, err = daemon.clusterProvider.AttachNetwork(id, container.ID, addresses) if err != nil { return nil, nil, err } @@ -427,7 +427,7 @@ func (daemon *Daemon) findAndAttachNetwork(container *container.Container, idOrN // attached to the swarm scope network went down // and removed the network while we were in // the process of attaching. - if config != nil { + if nwCfg != nil { if _, ok := err.(libnetwork.ErrNoSuchNetwork); ok { if retryCount >= 5 { return nil, nil, fmt.Errorf("could not find network %s after successful attachment", idOrName) @@ -446,7 +446,7 @@ func (daemon *Daemon) findAndAttachNetwork(container *container.Container, idOrN // This container has attachment to a swarm scope // network. Update the container network settings accordingly. container.NetworkSettings.HasSwarmEndpoint = true - return n, config, nil + return n, nwCfg, nil } // updateContainerNetworkSettings updates the network settings @@ -737,7 +737,7 @@ func (daemon *Daemon) connectToNetwork(cfg *config.Config, container *container. endpointConfig = &networktypes.EndpointSettings{} } - n, config, err := daemon.findAndAttachNetwork(container, idOrName, endpointConfig) + n, nwCfg, err := daemon.findAndAttachNetwork(container, idOrName, endpointConfig) if err != nil { return err } @@ -746,8 +746,8 @@ func (daemon *Daemon) connectToNetwork(cfg *config.Config, container *container. } var operIPAM bool - if config != nil { - if epConfig, ok := config.EndpointsConfig[n.Name()]; ok { + if nwCfg != nil { + if epConfig, ok := nwCfg.EndpointsConfig[n.Name()]; ok { if endpointConfig.IPAMConfig == nil || (endpointConfig.IPAMConfig.IPv4Address == "" && endpointConfig.IPAMConfig.IPv6Address == "" && From 8846c7e0ae37be385be01951f48e8ae531b3ef16 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 4 Jul 2023 00:04:24 +0200 Subject: [PATCH 3/3] daemon/cluster/executor/container: fix mixed pointer/value receiver Got a linter warning on this one, and I don't think eventFilter() was intentionally using a value (not pointer). > Struct containerConfig has methods on both value and pointer receivers. > Such usage is not recommended by the Go Documentation Signed-off-by: Sebastiaan van Stijn --- daemon/cluster/executor/container/container.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/daemon/cluster/executor/container/container.go b/daemon/cluster/executor/container/container.go index 1aec7df3df..434330df40 100644 --- a/daemon/cluster/executor/container/container.go +++ b/daemon/cluster/executor/container/container.go @@ -720,7 +720,7 @@ func (c *containerConfig) applyPrivileges(hc *enginecontainer.HostConfig) { } } -func (c containerConfig) eventFilter() filters.Args { +func (c *containerConfig) eventFilter() filters.Args { return filters.NewArgs( filters.Arg("type", events.ContainerEventType), filters.Arg("name", c.name()),