From 0fa873c0fef0c117d7561b08b083292cc3305b79 Mon Sep 17 00:00:00 2001 From: Albin Kerouanton Date: Fri, 10 Feb 2023 16:00:42 +0100 Subject: [PATCH] libnetwork: remove global store from overlay driver The overlay driver was creating a global store whenever netlabel.GlobalKVClient was specified in its config argument. This specific label is not used anymore since 142b522 (moby/moby#44875). golangci-lint now detects dead code. This will be fixed in subsequent commits. Signed-off-by: Albin Kerouanton --- libnetwork/drivers/overlay/ov_network.go | 94 +--------------------- libnetwork/drivers/overlay/overlay.go | 76 ----------------- libnetwork/drivers/overlay/overlay_test.go | 21 ----- 3 files changed, 3 insertions(+), 188 deletions(-) diff --git a/libnetwork/drivers/overlay/ov_network.go b/libnetwork/drivers/overlay/ov_network.go index 1eaeb41301..d98263c63c 100644 --- a/libnetwork/drivers/overlay/ov_network.go +++ b/libnetwork/drivers/overlay/ov_network.go @@ -166,10 +166,6 @@ func (d *driver) CreateNetwork(id string, option map[string]interface{}, nInfo d return fmt.Errorf("attempt to create overlay network %v that already exists", n.id) } - if err := n.writeToStore(); err != nil { - return fmt.Errorf("failed to update data store for network %v: %v", n.id, err) - } - // Make sure no rule is on the way from any stale secure network if !n.secure { for _, vni := range vnis { @@ -213,10 +209,7 @@ func (d *driver) DeleteNetwork(nid string) error { // This is similar to d.network(), but we need to keep holding the lock // until we are done removing this network. - n, ok := d.networks[nid] - if !ok { - n = d.restoreNetworkFromStore(nid) - } + n := d.networks[nid] if n == nil { return fmt.Errorf("could not find network with id %s", nid) } @@ -878,42 +871,14 @@ func (n *network) watchMiss(nlSock *nl.NetlinkSocket, nsPath string) { } } -// Restore a network from the store to the driver if it is present. -// Must be called with the driver locked! -func (d *driver) restoreNetworkFromStore(nid string) *network { - n := d.getNetworkFromStore(nid) - if n != nil { - n.driver = d - n.endpoints = endpointTable{} - d.networks[nid] = n - } - return n -} - func (d *driver) network(nid string) *network { d.Lock() - n, ok := d.networks[nid] - if !ok { - n = d.restoreNetworkFromStore(nid) - } + n := d.networks[nid] d.Unlock() return n } -func (d *driver) getNetworkFromStore(nid string) *network { - if d.store == nil { - return nil - } - - n := &network{id: nid} - if err := d.store.GetObject(datastore.Key(n.Key()...), n); err != nil { - return nil - } - - return n -} - func (n *network) sandbox() osl.Sandbox { n.Lock() defer n.Unlock() @@ -926,12 +891,6 @@ func (n *network) vxlanID(s *subnet) uint32 { return s.vni } -func (n *network) setVxlanID(s *subnet, vni uint32) { - n.Lock() - s.vni = vni - n.Unlock() -} - func (n *network) Key() []string { return []string{"overlay", "network", n.id} } @@ -1047,14 +1006,6 @@ func (n *network) DataScope() string { return datastore.GlobalScope } -func (n *network) writeToStore() error { - if n.driver.store == nil { - return nil - } - - return n.driver.store.PutObjectAtomic(n) -} - func (n *network) releaseVxlanID() ([]uint32, error) { n.Lock() nSubnets := len(n.subnets) @@ -1063,17 +1014,6 @@ func (n *network) releaseVxlanID() ([]uint32, error) { return nil, nil } - if n.driver.store != nil { - if err := n.driver.store.DeleteObjectAtomic(n); err != nil { - if err == datastore.ErrKeyModified || err == datastore.ErrKeyNotFound { - // In both the above cases we can safely assume that the key has been removed by some other - // instance and so simply get out of here - return nil, nil - } - - return nil, fmt.Errorf("failed to delete network to vxlan id map: %v", err) - } - } var vnis []uint32 n.Lock() for _, s := range n.subnets { @@ -1096,35 +1036,7 @@ func (n *network) obtainVxlanID(s *subnet) error { if n.vxlanID(s) != 0 { return nil } - - if n.driver.store == nil { - return fmt.Errorf("no valid vxlan id and no datastore configured, cannot obtain vxlan id") - } - - for { - if err := n.driver.store.GetObject(datastore.Key(n.Key()...), n); err != nil { - return fmt.Errorf("getting network %q from datastore failed %v", n.id, err) - } - - if n.vxlanID(s) == 0 { - vxlanID, err := n.driver.vxlanIdm.GetID(true) - if err != nil { - return fmt.Errorf("failed to allocate vxlan id: %v", err) - } - - n.setVxlanID(s, uint32(vxlanID)) - if err := n.writeToStore(); err != nil { - n.driver.vxlanIdm.Release(uint64(n.vxlanID(s))) - n.setVxlanID(s, 0) - if err == datastore.ErrKeyModified { - continue - } - return fmt.Errorf("network %q failed to update data store: %v", n.id, err) - } - return nil - } - return nil - } + return fmt.Errorf("no valid vxlan id and no datastore configured, cannot obtain vxlan id") } // contains return true if the passed ip belongs to one the network's diff --git a/libnetwork/drivers/overlay/overlay.go b/libnetwork/drivers/overlay/overlay.go index e5c364c307..a97578a1c6 100644 --- a/libnetwork/drivers/overlay/overlay.go +++ b/libnetwork/drivers/overlay/overlay.go @@ -25,8 +25,6 @@ const ( networkType = "overlay" vethPrefix = "veth" vethLen = len(vethPrefix) + 7 - vxlanIDStart = 256 - vxlanIDEnd = (1 << 24) - 1 vxlanEncap = 50 secureOption = "encrypted" ) @@ -45,7 +43,6 @@ type driver struct { secMap *encrMap serfInstance *serf.Serf networks networkTable - store datastore.DataStore localStore datastore.DataStore vxlanIdm *idm.Idm initOS sync.Once @@ -71,18 +68,6 @@ func Register(r driverapi.Registerer, config map[string]interface{}) error { config: config, } - if data, ok := config[netlabel.GlobalKVClient]; ok { - var err error - dsc, ok := data.(discoverapi.DatastoreConfigData) - if !ok { - return types.InternalErrorf("incorrect data in datastore configuration: %v", data) - } - d.store, err = datastore.NewDataStoreFromConfig(dsc) - if err != nil { - return types.InternalErrorf("failed to initialize data store: %v", err) - } - } - if data, ok := config[netlabel.LocalKVClient]; ok { var err error dsc, ok := data.(discoverapi.DatastoreConfigData) @@ -171,32 +156,6 @@ func (d *driver) configure() error { // Apply OS specific kernel configs if needed d.initOS.Do(applyOStweaks) - if d.store == nil { - return nil - } - - if d.vxlanIdm == nil { - return d.initializeVxlanIdm() - } - - return nil -} - -func (d *driver) initializeVxlanIdm() error { - var err error - - initVxlanIdm <- true - defer func() { <-initVxlanIdm }() - - if d.vxlanIdm != nil { - return nil - } - - d.vxlanIdm, err = idm.New(d.store, "vxlan-id", vxlanIDStart, vxlanIDEnd) - if err != nil { - return fmt.Errorf("failed to initialize vxlan id manager: %v", err) - } - return nil } @@ -208,25 +167,6 @@ func (d *driver) IsBuiltIn() bool { return true } -func validateSelf(node string) error { - advIP := net.ParseIP(node) - if advIP == nil { - return fmt.Errorf("invalid self address (%s)", node) - } - - addrs, err := net.InterfaceAddrs() - if err != nil { - return fmt.Errorf("Unable to get interface addresses %v", err) - } - for _, addr := range addrs { - ip, _, err := net.ParseCIDR(addr.String()) - if err == nil && ip.Equal(advIP) { - return nil - } - } - return fmt.Errorf("Multi-Host overlay networking requires cluster-advertise(%s) to be configured with a local ip-address that is reachable within the cluster", advIP.String()) -} - func (d *driver) nodeJoin(advertiseAddress, bindAddress string, self bool) { if self && !d.isSerfAlive() { d.Lock() @@ -239,22 +179,6 @@ func (d *driver) nodeJoin(advertiseAddress, bindAddress string, self bool) { d.localJoinOnce.Do(func() { d.peerDBUpdateSelf() }) - - // If there is no cluster store there is no need to start serf. - if d.store != nil { - if err := validateSelf(advertiseAddress); err != nil { - logrus.Warn(err.Error()) - } - err := d.serfInit() - if err != nil { - logrus.Errorf("initializing serf instance failed: %v", err) - d.Lock() - d.advertiseAddress = "" - d.bindAddress = "" - d.Unlock() - return - } - } } d.Lock() diff --git a/libnetwork/drivers/overlay/overlay_test.go b/libnetwork/drivers/overlay/overlay_test.go index 27c0475fbb..87d1c8d8a3 100644 --- a/libnetwork/drivers/overlay/overlay_test.go +++ b/libnetwork/drivers/overlay/overlay_test.go @@ -128,27 +128,6 @@ func TestOverlayFiniWithoutConfig(t *testing.T) { cleanupDriver(t, dt) } -func TestOverlayConfig(t *testing.T) { - dt := setupDriver(t) - - time.Sleep(1 * time.Second) - - d := dt.d - if d.notifyCh == nil { - t.Fatal("Driver notify channel wasn't initialized after Config method") - } - - if d.exitCh == nil { - t.Fatal("Driver serfloop exit channel wasn't initialized after Config method") - } - - if d.serfInstance == nil { - t.Fatal("Driver serfinstance hasn't been initialized after Config method") - } - - cleanupDriver(t, dt) -} - func TestOverlayType(t *testing.T) { dt := &driverTester{t: t} if err := Register(dt, nil); err != nil {