Merge pull request #47285 from corhere/libn/one-datastore-to-rule-them-all

libnetwork: share a single datastore with drivers

daemon/oci_linux_test.go

@@ -11,6 +11,7 @@ import (
 	"github.com/docker/docker/daemon/config"
 	"github.com/docker/docker/daemon/network"
 	"github.com/docker/docker/libnetwork"
+	nwconfig "github.com/docker/docker/libnetwork/config"
 	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/opencontainers/runtime-spec/specs-go"
 	"golang.org/x/sys/unix"
@@ -27,7 +28,7 @@ func setupFakeDaemon(t *testing.T, c *container.Container) *Daemon {
 	err := os.MkdirAll(rootfs, 0o755)
 	assert.NilError(t, err)
 
-	netController, err := libnetwork.New()
+	netController, err := libnetwork.New(nwconfig.OptionDataDir(t.TempDir()))
 	assert.NilError(t, err)
 
 	d := &Daemon{

daemon/reload_test.go

@@ -360,7 +360,7 @@ func TestDaemonReloadNetworkDiagnosticPort(t *testing.T) {
 		},
 	}
 
-	netOptions, err := daemon.networkOptions(&config.Config{}, nil, nil)
+	netOptions, err := daemon.networkOptions(&config.Config{CommonConfig: config.CommonConfig{Root: t.TempDir()}}, nil, nil)
 	if err != nil {
 		t.Fatal(err)
 	}

libnetwork/controller.go

@@ -344,14 +344,7 @@ func (c *Controller) makeDriverConfig(ntype string) map[string]interface{} {
 	}
 
 	if c.cfg.Scope.IsValid() {
-		// FIXME: every driver instance constructs a new DataStore
-		// instance against the same database. Yikes!
-		cfg[netlabel.LocalKVClient] = discoverapi.DatastoreConfigData{
-			Scope:    scope.Local,
-			Provider: c.cfg.Scope.Client.Provider,
-			Address:  c.cfg.Scope.Client.Address,
-			Config:   c.cfg.Scope.Client.Config,
-		}
+		cfg[netlabel.LocalKVClient] = c.store
 	}
 
 	return cfg

libnetwork/datastore/datastore.go

@@ -6,7 +6,6 @@ import (
 	"sync"
 	"time"
 
-	"github.com/docker/docker/libnetwork/discoverapi"
 	store "github.com/docker/docker/libnetwork/internal/kvstore"
 	"github.com/docker/docker/libnetwork/internal/kvstore/boltdb"
 	"github.com/docker/docker/libnetwork/types"
@@ -93,6 +92,7 @@ func DefaultScope(dataDir string) ScopeCfg {
 			Config: &store.Config{
 				Bucket:            "libnetwork",
 				ConnectionTimeout: time.Minute,
+				PersistConnection: true,
 			},
 		},
 	}
@@ -129,8 +129,7 @@ func newClient(kv string, addr string, config *store.Config) (*Store, error) {
 		config = &store.Config{}
 	}
 
-	// Parse file path
-	s, err := boltdb.New(strings.Split(addr, ","), config)
+	s, err := boltdb.New(addr, config)
 	if err != nil {
 		return nil, err
 	}
@@ -147,32 +146,6 @@ func New(cfg ScopeCfg) (*Store, error) {
 	return newClient(cfg.Client.Provider, cfg.Client.Address, cfg.Client.Config)
 }
 
-// FromConfig creates a new instance of LibKV data store starting from the datastore config data.
-func FromConfig(dsc discoverapi.DatastoreConfigData) (*Store, error) {
-	var (
-		ok    bool
-		sCfgP *store.Config
-	)
-
-	sCfgP, ok = dsc.Config.(*store.Config)
-	if !ok && dsc.Config != nil {
-		return nil, fmt.Errorf("cannot parse store configuration: %v", dsc.Config)
-	}
-
-	ds, err := New(ScopeCfg{
-		Client: ScopeClientCfg{
-			Address:  dsc.Address,
-			Provider: dsc.Provider,
-			Config:   sCfgP,
-		},
-	})
-	if err != nil {
-		return nil, fmt.Errorf("failed to construct datastore client from datastore configuration %v: %v", dsc, err)
-	}
-
-	return ds, err
-}
-
 // Close closes the data store.
 func (ds *Store) Close() {
 	ds.store.Close()

libnetwork/discoverapi/discoverapi.go

@@ -30,6 +30,8 @@ type NodeDiscoveryData struct {
 }
 
 // DatastoreConfigData is the data for the datastore update event message
+//
+// Deprecated: no longer used.
 type DatastoreConfigData struct {
 	Scope    string
 	Provider string

libnetwork/drivers/bridge/bridge_store.go

@@ -10,7 +10,6 @@ import (
 
 	"github.com/containerd/log"
 	"github.com/docker/docker/libnetwork/datastore"
-	"github.com/docker/docker/libnetwork/discoverapi"
 	"github.com/docker/docker/libnetwork/netlabel"
 	"github.com/docker/docker/libnetwork/types"
 )
@@ -25,17 +24,13 @@ const (
 
 func (d *driver) initStore(option map[string]interface{}) error {
 	if data, ok := option[netlabel.LocalKVClient]; ok {
-		var err error
-		dsc, ok := data.(discoverapi.DatastoreConfigData)
+		var ok bool
+		d.store, ok = data.(*datastore.Store)
 		if !ok {
 			return types.InternalErrorf("incorrect data in datastore configuration: %v", data)
 		}
-		d.store, err = datastore.FromConfig(dsc)
-		if err != nil {
-			return types.InternalErrorf("bridge driver failed to initialize data store: %v", err)
-		}
 
-		err = d.populateNetworks()
+		err := d.populateNetworks()
 		if err != nil {
 			return err
 		}

libnetwork/drivers/ipvlan/ipvlan_store.go

@@ -10,7 +10,6 @@ import (
 
 	"github.com/containerd/log"
 	"github.com/docker/docker/libnetwork/datastore"
-	"github.com/docker/docker/libnetwork/discoverapi"
 	"github.com/docker/docker/libnetwork/netlabel"
 	"github.com/docker/docker/libnetwork/types"
 )
@@ -44,17 +43,13 @@ type ipSubnet struct {
 // initStore drivers are responsible for caching their own persistent state
 func (d *driver) initStore(option map[string]interface{}) error {
 	if data, ok := option[netlabel.LocalKVClient]; ok {
-		var err error
-		dsc, ok := data.(discoverapi.DatastoreConfigData)
+		var ok bool
+		d.store, ok = data.(*datastore.Store)
 		if !ok {
 			return types.InternalErrorf("incorrect data in datastore configuration: %v", data)
 		}
-		d.store, err = datastore.FromConfig(dsc)
-		if err != nil {
-			return types.InternalErrorf("ipvlan driver failed to initialize data store: %v", err)
-		}
 
-		err = d.populateNetworks()
+		err := d.populateNetworks()
 		if err != nil {
 			return err
 		}

libnetwork/drivers/macvlan/macvlan_store.go

@@ -10,7 +10,6 @@ import (
 
 	"github.com/containerd/log"
 	"github.com/docker/docker/libnetwork/datastore"
-	"github.com/docker/docker/libnetwork/discoverapi"
 	"github.com/docker/docker/libnetwork/netlabel"
 	"github.com/docker/docker/libnetwork/types"
 )
@@ -43,17 +42,13 @@ type ipSubnet struct {
 // initStore drivers are responsible for caching their own persistent state
 func (d *driver) initStore(option map[string]interface{}) error {
 	if data, ok := option[netlabel.LocalKVClient]; ok {
-		var err error
-		dsc, ok := data.(discoverapi.DatastoreConfigData)
+		var ok bool
+		d.store, ok = data.(*datastore.Store)
 		if !ok {
 			return types.InternalErrorf("incorrect data in datastore configuration: %v", data)
 		}
-		d.store, err = datastore.FromConfig(dsc)
-		if err != nil {
-			return types.InternalErrorf("macvlan driver failed to initialize data store: %v", err)
-		}
 
-		err = d.populateNetworks()
+		err := d.populateNetworks()
 		if err != nil {
 			return err
 		}

libnetwork/drivers/windows/windows_store.go

@@ -10,7 +10,6 @@ import (
 
 	"github.com/containerd/log"
 	"github.com/docker/docker/libnetwork/datastore"
-	"github.com/docker/docker/libnetwork/discoverapi"
 	"github.com/docker/docker/libnetwork/netlabel"
 	"github.com/docker/docker/libnetwork/types"
 )
@@ -22,17 +21,13 @@ const (
 
 func (d *driver) initStore(option map[string]interface{}) error {
 	if data, ok := option[netlabel.LocalKVClient]; ok {
-		var err error
-		dsc, ok := data.(discoverapi.DatastoreConfigData)
+		var ok bool
+		d.store, ok = data.(*datastore.Store)
 		if !ok {
 			return types.InternalErrorf("incorrect data in datastore configuration: %v", data)
 		}
-		d.store, err = datastore.FromConfig(dsc)
-		if err != nil {
-			return types.InternalErrorf("windows driver failed to initialize data store: %v", err)
-		}
 
-		err = d.populateNetworks()
+		err := d.populateNetworks()
 		if err != nil {
 			return err
 		}

libnetwork/firewall_linux_test.go

@@ -54,12 +54,14 @@ func TestUserChain(t *testing.T) {
 			defer netnsutils.SetupTestOSContext(t)()
 			defer resetIptables(t)
 
-			c, err := New(config.OptionDriverConfig("bridge", map[string]any{
-				netlabel.GenericData: options.Generic{
-					"EnableIPTables":  tc.iptables,
-					"EnableIP6Tables": tc.iptables,
-				},
-			}))
+			c, err := New(
+				OptionBoltdbWithRandomDBFile(t),
+				config.OptionDriverConfig("bridge", map[string]any{
+					netlabel.GenericData: options.Generic{
+						"EnableIPTables":  tc.iptables,
+						"EnableIP6Tables": tc.iptables,
+					},
+				}))
 			assert.NilError(t, err)
 			defer c.Stop()
 

libnetwork/internal/kvstore/boltdb/boltdb.go

@@ -15,9 +15,6 @@ import (
 )
 
 var (
-	// ErrMultipleEndpointsUnsupported is thrown when multiple endpoints specified for
-	// BoltDB. Endpoint has to be a local file path
-	ErrMultipleEndpointsUnsupported = errors.New("boltdb supports one endpoint and should be a file path")
 	// ErrBoltBucketOptionMissing is thrown when boltBcuket config option is missing
 	ErrBoltBucketOptionMissing = errors.New("boltBucket config option missing")
 )
@@ -46,16 +43,12 @@ const (
 )
 
 // New opens a new BoltDB connection to the specified path and bucket
-func New(endpoints []string, options *store.Config) (store.Store, error) {
-	if len(endpoints) > 1 {
-		return nil, ErrMultipleEndpointsUnsupported
-	}
-
+func New(endpoint string, options *store.Config) (store.Store, error) {
 	if (options == nil) || (len(options.Bucket) == 0) {
 		return nil, ErrBoltBucketOptionMissing
 	}
 
-	dir, _ := filepath.Split(endpoints[0])
+	dir, _ := filepath.Split(endpoint)
 	if err := os.MkdirAll(dir, 0o750); err != nil {
 		return nil, err
 	}
@@ -63,7 +56,7 @@ func New(endpoints []string, options *store.Config) (store.Store, error) {
 	var db *bolt.DB
 	if options.PersistConnection {
 		var err error
-		db, err = bolt.Open(endpoints[0], filePerm, &bolt.Options{
+		db, err = bolt.Open(endpoint, filePerm, &bolt.Options{
 			Timeout: options.ConnectionTimeout,
 		})
 		if err != nil {
@@ -78,7 +71,7 @@ func New(endpoints []string, options *store.Config) (store.Store, error) {
 
 	b := &BoltDB{
 		client:            db,
-		path:              endpoints[0],
+		path:              endpoint,
 		boltBucket:        []byte(options.Bucket),
 		timeout:           timeout,
 		PersistConnection: options.PersistConnection,

libnetwork/libnetwork_internal_test.go

@@ -314,7 +314,7 @@ func compareNwLists(a, b []*net.IPNet) bool {
 func TestAuxAddresses(t *testing.T) {
 	defer netnsutils.SetupTestOSContext(t)()
 
-	c, err := New()
+	c, err := New(OptionBoltdbWithRandomDBFile(t))
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -353,7 +353,7 @@ func TestSRVServiceQuery(t *testing.T) {
 
 	defer netnsutils.SetupTestOSContext(t)()
 
-	c, err := New()
+	c, err := New(OptionBoltdbWithRandomDBFile(t))
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -451,7 +451,7 @@ func TestServiceVIPReuse(t *testing.T) {
 
 	defer netnsutils.SetupTestOSContext(t)()
 
-	c, err := New()
+	c, err := New(OptionBoltdbWithRandomDBFile(t))
 	if err != nil {
 		t.Fatal(err)
 	}

libnetwork/libnetwork_linux_test.go

@@ -1197,7 +1197,7 @@ func TestInvalidRemoteDriver(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	ctrlr, err := libnetwork.New()
+	ctrlr, err := libnetwork.New(libnetwork.OptionBoltdbWithRandomDBFile(t))
 	if err != nil {
 		t.Fatal(err)
 	}

libnetwork/resolver_unix_test.go

@@ -13,7 +13,7 @@ import (
 // test only works on linux
 func TestDNSIPQuery(t *testing.T) {
 	defer netnsutils.SetupTestOSContext(t)()
-	c, err := New()
+	c, err := New(OptionBoltdbWithRandomDBFile(t))
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -110,7 +110,7 @@ func TestDNSProxyServFail(t *testing.T) {
 	osctx := netnsutils.SetupTestOSContextEx(t)
 	defer osctx.Cleanup(t)
 
-	c, err := New()
+	c, err := New(OptionBoltdbWithRandomDBFile(t))
 	if err != nil {
 		t.Fatal(err)
 	}

libnetwork/sandbox_dns_unix_test.go

@@ -15,7 +15,7 @@ import (
 func TestDNSOptions(t *testing.T) {
 	skip.If(t, runtime.GOOS == "windows", "test only works on linux")
 
-	c, err := New()
+	c, err := New(OptionBoltdbWithRandomDBFile(t))
 	assert.NilError(t, err)
 
 	sb, err := c.NewSandbox("cnt1", nil)

libnetwork/service_common_unix_test.go

@@ -12,7 +12,7 @@ import (
 
 func TestCleanupServiceDiscovery(t *testing.T) {
 	defer netnsutils.SetupTestOSContext(t)()
-	c, err := New()
+	c, err := New(OptionBoltdbWithRandomDBFile(t))
 	assert.NilError(t, err)
 	defer c.Stop()
 

libnetwork/store_linux_test.go

@@ -2,18 +2,13 @@ package libnetwork
 
 import (
 	"errors"
-	"os"
 	"path/filepath"
 	"testing"
 
-	"github.com/docker/docker/libnetwork/config"
-	"github.com/docker/docker/libnetwork/datastore"
 	store "github.com/docker/docker/libnetwork/internal/kvstore"
 )
 
 func TestBoltdbBackend(t *testing.T) {
-	defer os.Remove(datastore.DefaultScope("").Client.Address)
-	testLocalBackend(t, "", "", nil)
 	tmpPath := filepath.Join(t.TempDir(), "boltdb.db")
 	testLocalBackend(t, "boltdb", tmpPath, &store.Config{
 		Bucket: "testBackend",
@@ -21,12 +16,7 @@ func TestBoltdbBackend(t *testing.T) {
 }
 
 func TestNoPersist(t *testing.T) {
-	dbFile := filepath.Join(t.TempDir(), "bolt.db")
-	configOption := func(c *config.Config) {
-		c.Scope.Client.Provider = "boltdb"
-		c.Scope.Client.Address = dbFile
-		c.Scope.Client.Config = &store.Config{Bucket: "testBackend"}
-	}
+	configOption := OptionBoltdbWithRandomDBFile(t)
 	testController, err := New(configOption)
 	if err != nil {
 		t.Fatalf("Error creating new controller: %v", err)