Repository: moby/moby
File: daemon/libnetwork/endpoint.go (Go)

Code before:
	sb.mu.Lock()
	osSbox := sb.osSbox
	delete(sb.populatedEndpoints, ep.id)
	delete(sb.epPriority, ep.id)
	sb.endpoints = slices.DeleteFunc(sb.endpoints, func(other *Endpoint) bool { return other.id == ep.id })
	sb.mu.Unlock()

	// Delete interfaces, routes etc. from the OS.
	if osSbox != nil {
		releaseOSSboxResources(osSbox, ep)

		// Even if the interface was initially created in the container's namespace, it's
		// now been moved out. When a legacy link is deleted, the Endpoint is removed and
		// then re-added to the Sandbox. So, to make sure the re-add works, note that the
		// interface is now outside the container's netns.
		ep.iface.createdInContainer = false
	}

	// Update gateway / static routes if the ep was the gateway.
	var gwepAfter4, gwepAfter6 *Endpoint
	if needNewGwEp {
		gwepAfter4, gwepAfter6 = sb.getGatewayEndpoint()
		if err := sb.updateGateway(gwepAfter4, gwepAfter6); err != nil {
			log.G(ctx).WithFields(log.Fields{
				"gw4":   epShortId(gwepAfter4),
				"gw6":   epShortId(gwepAfter6),
				"error": err,
			}).Warn("Configuring gateway after network disconnect")
		}
	}

	// Update the store about the sandbox detach only after we
	// have completed sb.clearNetworkResources above to avoid
	// spurious logs when cleaning up the sandbox when the daemon
	// ungracefully exits and restarts before completing sandbox
	// detach but after store has been updated.
	if err := n.getController().storeEndpoint(ctx, ep); err != nil {
		return err
	}

	if e := ep.deleteDriverInfoFromCluster(); e != nil {
		log.G(ctx).WithError(e).Error("Failed to delete endpoint state for endpoint from cluster")
	}

	// When a container is connected to a network, it gets /etc/hosts
	// entries for its addresses on that network. So, when it's connected
	// to two networks, it has a hosts entry for each. For example, if
	// the hostname is the default short-id, and it's connected to two
	// networks (172.19.0.0/16 and 172.20.0.0/17, plus IPv6 address for
	// each), the hosts file might include:
	//

Review comment (suggestion):
It's probably worth adding a comment explaining why we're logging instead of returning an error. There's still a bunch of code paths in libnetwork that adopt this error handling strategy, so readers may just think this is one of these injustified instances and not even git blame.

Code after:
	sb.mu.Lock()
	osSbox := sb.osSbox
	delete(sb.populatedEndpoints, ep.id)
	delete(sb.epPriority, ep.id)
	sb.endpoints = slices.DeleteFunc(sb.endpoints, func(other *Endpoint) bool { return other.id == ep.id })
	sb.mu.Unlock()

	// Delete interfaces, routes etc. from the OS.
	if osSbox != nil {
		releaseOSSboxResources(osSbox, ep)

		// Even if the interface was initially created in the container's namespace, it's
		// now been moved out. When a legacy link is deleted, the Endpoint is removed and
		// then re-added to the Sandbox. So, to make sure the re-add works, note that the
		// interface is now outside the container's netns.
		ep.iface.createdInContainer = false
	}

	// Update gateway / static routes if the ep was the gateway.
	var gwepAfter4, gwepAfter6 *Endpoint
	if needNewGwEp {
		gwepAfter4, gwepAfter6 = sb.getGatewayEndpoint()
		if err := sb.updateGateway(gwepAfter4, gwepAfter6); err != nil {
			// Don't return an error here without adding proper rollback of the work done above.
			// See https://github.com/moby/moby/issues/51578
			log.G(ctx).WithFields(log.Fields{
				"gw4":   epShortId(gwepAfter4),
				"gw6":   epShortId(gwepAfter6),
				"error": err,
			}).Warn("Configuring gateway after network disconnect")
		}
	}

	// Update the store about the sandbox detach only after we
	// have completed sb.clearNetworkResources above to avoid
	// spurious logs when cleaning up the sandbox when the daemon
	// ungracefully exits and restarts before completing sandbox
	// detach but after store has been updated.
	if err := n.getController().storeEndpoint(ctx, ep); err != nil {
		return err
	}

	if e := ep.deleteDriverInfoFromCluster(); e != nil {
		log.G(ctx).WithError(e).Error("Failed to delete endpoint state for endpoint from cluster")
	}

	// When a container is connected to a network, it gets /etc/hosts
	// entries for its addresses on that network. So, when it's connected
	// to two networks, it has a hosts entry for each. For example, if
	// the hostname is the default short-id, and it's connected to two
	// networks (172.19.0.0/16 and 172.20.0.0/17, plus IPv6 address for