From bcca214e362cf2a777c755072477e764a3d9adaf Mon Sep 17 00:00:00 2001 From: Cory Snider Date: Tue, 17 Oct 2023 19:43:12 -0400 Subject: [PATCH] libnetwork: open-code updating svc records Inline the tortured logic for deciding when to skip updating the svc records to give us a fighting chance at deciphering the logic behind the logic and spotting logic bugs. Update the service records synchronously. The only potential for issues is if this change introduces deadlocks, which should be fixed by restrucuting the mutexes rather than papering over the issue with sketchy hacks like deferring the operation to a goroutine. Signed-off-by: Cory Snider --- libnetwork/endpoint.go | 11 +++++++---- libnetwork/network.go | 15 ++++++++------- libnetwork/sandbox_store.go | 6 ++++-- libnetwork/store.go | 27 --------------------------- 4 files changed, 19 insertions(+), 40 deletions(-) diff --git a/libnetwork/endpoint.go b/libnetwork/endpoint.go index 4eaad732cd..0de9e9198c 100644 --- a/libnetwork/endpoint.go +++ b/libnetwork/endpoint.go @@ -12,6 +12,7 @@ import ( "github.com/docker/docker/libnetwork/ipamapi" "github.com/docker/docker/libnetwork/netlabel" "github.com/docker/docker/libnetwork/options" + "github.com/docker/docker/libnetwork/scope" "github.com/docker/docker/libnetwork/types" ) @@ -456,9 +457,10 @@ func (ep *Endpoint) sbJoin(sb *Sandbox, options ...EndpointOption) (err error) { } }() - // Watch for service records if !n.getController().isAgent() { - n.getController().watchSvcRecord(ep) + if !n.getController().isSwarmNode() || n.Scope() != scope.Swarm || !n.driverIsMultihost() { + n.updateSvcRecord(ep, true) + } } // Do not update hosts file with internal networks endpoint IP @@ -803,8 +805,9 @@ func (ep *Endpoint) Delete(force bool) error { } }() - // unwatch for service records - n.getController().unWatchSvcRecord(ep) + if !n.getController().isSwarmNode() || n.Scope() != scope.Swarm || !n.driverIsMultihost() { + n.updateSvcRecord(ep, false) + } if err = ep.deleteEndpoint(force); err != nil && !force { return err diff --git a/libnetwork/network.go b/libnetwork/network.go index f8799ea567..8e3b8ac0cd 100644 --- a/libnetwork/network.go +++ b/libnetwork/network.go @@ -1225,13 +1225,14 @@ func (n *Network) createEndpoint(name string, options ...EndpointOption) (*Endpo return nil, err } - // Watch for service records - n.getController().watchSvcRecord(ep) - defer func() { - if err != nil { - n.getController().unWatchSvcRecord(ep) - } - }() + if !n.getController().isSwarmNode() || n.Scope() != scope.Swarm || !n.driverIsMultihost() { + n.updateSvcRecord(ep, true) + defer func() { + if err != nil { + n.updateSvcRecord(ep, false) + } + }() + } // Increment endpoint count to indicate completion of endpoint addition if err = n.getEpCnt().IncEndpointCnt(); err != nil { diff --git a/libnetwork/sandbox_store.go b/libnetwork/sandbox_store.go index a37f5635df..9d2afd47bb 100644 --- a/libnetwork/sandbox_store.go +++ b/libnetwork/sandbox_store.go @@ -278,9 +278,11 @@ func (c *Controller) sandboxCleanup(activeSandboxes map[string]interface{}) erro } for _, ep := range sb.endpoints { - // Watch for service records if !c.isAgent() { - c.watchSvcRecord(ep) + n := ep.getNetwork() + if !c.isSwarmNode() || n.Scope() != scope.Swarm || !n.driverIsMultihost() { + n.updateSvcRecord(ep, true) + } } } } diff --git a/libnetwork/store.go b/libnetwork/store.go index f0a599fcbc..8546f9a476 100644 --- a/libnetwork/store.go +++ b/libnetwork/store.go @@ -7,7 +7,6 @@ import ( "github.com/containerd/log" "github.com/docker/docker/libnetwork/datastore" - "github.com/docker/docker/libnetwork/scope" ) func (c *Controller) initStores() error { @@ -184,32 +183,6 @@ retry: return nil } -func (c *Controller) watchSvcRecord(ep *Endpoint) { - go c.processEndpointCreate(ep) -} - -func (c *Controller) unWatchSvcRecord(ep *Endpoint) { - go c.processEndpointDelete(ep) -} - -func (c *Controller) processEndpointCreate(ep *Endpoint) { - n := ep.getNetwork() - if c.isSwarmNode() && n.Scope() == scope.Swarm && n.driverIsMultihost() { - return - } - - n.updateSvcRecord(ep, true) -} - -func (c *Controller) processEndpointDelete(ep *Endpoint) { - n := ep.getNetwork() - if c.isSwarmNode() && n.Scope() == scope.Swarm && n.driverIsMultihost() { - return - } - - n.updateSvcRecord(ep, false) -} - func (c *Controller) networkCleanup() { for _, n := range c.getNetworksFromStore() { if n.inDelete {