libnet/ds: simplify datastore.New()

That function was needlessly complex. Instead of relying on a struct and
a sub-struct, it now just takes two string params: a path and a bucket
name.

Libnetwork config is now initialized with default values. A new struct
is introduced in libnetwork/config to let tests customize the path and
bucket name.

Signed-off-by: Albin Kerouanton <albinker@gmail.com>

libnetwork/config/config.go

@@ -39,7 +39,7 @@ type Config struct {
 	ClusterProvider        cluster.Provider
 	NetworkControlPlaneMTU int
 	DefaultAddressPool     []*ipamutils.NetworkToSplit
-	Scope                  datastore.ScopeCfg
+	DatastoreBucket        string
 	ActiveSandboxes        map[string]interface{}
 	PluginGetter           plugingetter.PluginGetter
 }
@@ -47,7 +47,8 @@ type Config struct {
 // New creates a new Config and initializes it with the given Options.
 func New(opts ...Option) *Config {
 	cfg := &Config{
-		driverCfg: make(map[string]map[string]any),
+		driverCfg:       make(map[string]map[string]any),
+		DatastoreBucket: datastore.DefaultBucket,
 	}
 
 	for _, opt := range opts {
@@ -56,10 +57,6 @@ func New(opts ...Option) *Config {
 		}
 	}
 
-	// load default scope configs which don't have explicit user specified configs.
-	if cfg.Scope == (datastore.ScopeCfg{}) {
-		cfg.Scope = datastore.DefaultScope(cfg.DataDir)
-	}
 	return cfg
 }
 

libnetwork/controller.go

@@ -108,7 +108,7 @@ type Controller struct {
 // New creates a new instance of network controller.
 func New(cfgOptions ...config.Option) (*Controller, error) {
 	cfg := config.New(cfgOptions...)
-	store, err := datastore.New(cfg.Scope)
+	store, err := datastore.New(cfg.DataDir, cfg.DatastoreBucket)
 	if err != nil {
 		return nil, fmt.Errorf("libnet controller initialization: %w", err)
 	}
@@ -333,7 +333,9 @@ func (c *Controller) makeDriverConfig(ntype string) map[string]interface{} {
 		return nil
 	}
 
-	cfg := map[string]interface{}{}
+	cfg := map[string]interface{}{
+		netlabel.LocalKVClient: c.store,
+	}
 	for _, label := range c.cfg.Labels {
 		key, val, _ := strings.Cut(label, "=")
 		if !strings.HasPrefix(key, netlabel.DriverPrefix+"."+ntype) {
@@ -348,10 +350,6 @@ func (c *Controller) makeDriverConfig(ntype string) map[string]interface{} {
 		cfg[k] = v
 	}
 
-	if c.cfg.Scope.IsValid() {
-		cfg[netlabel.LocalKVClient] = c.store
-	}
-
 	return cfg
 }
 

libnetwork/datastore/datastore.go

@@ -1,10 +1,10 @@
 package datastore
 
 import (
-	"fmt"
+	"errors"
+	"path"
 	"strings"
 	"sync"
-	"time"
 
 	store "github.com/docker/docker/libnetwork/internal/kvstore"
 	"github.com/docker/docker/libnetwork/internal/kvstore/boltdb"
@@ -50,18 +50,6 @@ type KVObject interface {
 	CopyTo(KVObject) error
 }
 
-// ScopeCfg represents Datastore configuration.
-type ScopeCfg struct {
-	Client ScopeClientCfg
-}
-
-// ScopeClientCfg represents Datastore Client-only mode configuration
-type ScopeClientCfg struct {
-	Provider string
-	Address  string
-	Config   *store.Config
-}
-
 const (
 	// NetworkKeyPrefix is the prefix for network key in the kv store
 	NetworkKeyPrefix = "network"
@@ -74,37 +62,7 @@ var (
 	rootChain        = defaultRootChain
 )
 
-const defaultPrefix = "/var/lib/docker/network/files"
-
-// DefaultScope returns a default scope config for clients to use.
-func DefaultScope(dataDir string) ScopeCfg {
-	var dbpath string
-	if dataDir == "" {
-		dbpath = defaultPrefix + "/local-kv.db"
-	} else {
-		dbpath = dataDir + "/network/files/local-kv.db"
-	}
-
-	return ScopeCfg{
-		Client: ScopeClientCfg{
-			Provider: string(store.BOLTDB),
-			Address:  dbpath,
-			Config: &store.Config{
-				Bucket:            "libnetwork",
-				ConnectionTimeout: time.Minute,
-			},
-		},
-	}
-}
-
-// IsValid checks if the scope config has valid configuration.
-func (cfg *ScopeCfg) IsValid() bool {
-	if cfg == nil || strings.TrimSpace(cfg.Client.Provider) == "" || strings.TrimSpace(cfg.Client.Address) == "" {
-		return false
-	}
-
-	return true
-}
+const DefaultBucket = "libnetwork"
 
 // Key provides convenient method to create a Key
 func Key(key ...string) string {
@@ -118,17 +76,16 @@ func Key(key ...string) string {
 	return b.String()
 }
 
-// newClient used to connect to KV Store
-func newClient(kv string, addr string, config *store.Config) (*Store, error) {
-	if kv != string(store.BOLTDB) {
-		return nil, fmt.Errorf("unsupported KV store")
+// New creates a new Store instance.
+func New(dir, bucket string) (*Store, error) {
+	if dir == "" {
+		return nil, errors.New("empty dir")
+	}
+	if bucket == "" {
+		return nil, errors.New("empty bucket")
 	}
 
-	if config == nil {
-		config = &store.Config{}
-	}
-
-	s, err := boltdb.New(addr, config)
+	s, err := boltdb.New(path.Join(dir, "local-kv.db"), bucket)
 	if err != nil {
 		return nil, err
 	}
@@ -136,15 +93,6 @@ func newClient(kv string, addr string, config *store.Config) (*Store, error) {
 	return &Store{store: s, cache: newCache(s)}, nil
 }
 
-// New creates a new Store instance.
-func New(cfg ScopeCfg) (*Store, error) {
-	if cfg.Client.Provider == "" || cfg.Client.Address == "" {
-		cfg = DefaultScope("")
-	}
-
-	return newClient(cfg.Client.Provider, cfg.Client.Address, cfg.Client.Config)
-}
-
 // Close closes the data store.
 func (ds *Store) Close() {
 	ds.store.Close()

libnetwork/datastore/datastore_test.go

@@ -23,16 +23,6 @@ func TestKey(t *testing.T) {
 	assert.Check(t, is.Equal(sKey, expected))
 }
 
-func TestInvalidDataStore(t *testing.T) {
-	_, err := New(ScopeCfg{
-		Client: ScopeClientCfg{
-			Provider: "invalid",
-			Address:  "localhost:8500",
-		},
-	})
-	assert.Check(t, is.Error(err, "unsupported KV store"))
-}
-
 func TestKVObjectFlatKey(t *testing.T) {
 	store := NewTestDataStore()
 	expected := dummyKVObject("1000", true)

libnetwork/internal/kvstore/boltdb/boltdb.go

@@ -3,19 +3,16 @@ package boltdb
 import (
 	"bytes"
 	"encoding/binary"
-	"errors"
 	"os"
 	"path/filepath"
 	"sync"
 	"sync/atomic"
+	"time"
 
 	store "github.com/docker/docker/libnetwork/internal/kvstore"
 	bolt "go.etcd.io/bbolt"
 )
 
-// ErrBoltBucketOptionMissing is thrown when boltBucket config option is missing
-var ErrBoltBucketOptionMissing = errors.New("boltBucket config option missing")
-
 const filePerm = 0o644
 
 // BoltDB type implements the Store interface
@@ -30,18 +27,14 @@ type BoltDB struct {
 const libkvmetadatalen = 8
 
 // New opens a new BoltDB connection to the specified path and bucket
-func New(endpoint string, options *store.Config) (store.Store, error) {
-	if (options == nil) || (len(options.Bucket) == 0) {
-		return nil, ErrBoltBucketOptionMissing
-	}
-
-	dir, _ := filepath.Split(endpoint)
+func New(path, bucket string) (store.Store, error) {
+	dir, _ := filepath.Split(path)
 	if err := os.MkdirAll(dir, 0o750); err != nil {
 		return nil, err
 	}
 
-	db, err := bolt.Open(endpoint, filePerm, &bolt.Options{
-		Timeout: options.ConnectionTimeout,
+	db, err := bolt.Open(path, filePerm, &bolt.Options{
+		Timeout: time.Minute,
 	})
 	if err != nil {
 		return nil, err
@@ -49,8 +42,8 @@ func New(endpoint string, options *store.Config) (store.Store, error) {
 
 	b := &BoltDB{
 		client:     db,
-		path:       endpoint,
-		boltBucket: []byte(options.Bucket),
+		path:       path,
+		boltBucket: []byte(bucket),
 	}
 
 	return b, nil

libnetwork/internal/kvstore/kvstore.go

@@ -2,7 +2,6 @@ package kvstore
 
 import (
 	"errors"
-	"time"
 )
 
 // Backend represents a KV Store Backend
@@ -24,12 +23,6 @@ var (
 	ErrKeyExists = errors.New("Previous K/V pair exists, cannot complete Atomic operation")
 )
 
-// Config contains the options for a storage client
-type Config struct {
-	ConnectionTimeout time.Duration
-	Bucket            string
-}
-
 // Store represents the backend K/V storage
 // Each store should support every call listed
 // here. Or it couldn't be implemented as a K/V

libnetwork/libnetwork_linux_test.go

@@ -21,7 +21,6 @@ import (
 	"github.com/docker/docker/internal/testutils/netnsutils"
 	"github.com/docker/docker/libnetwork"
 	"github.com/docker/docker/libnetwork/config"
-	"github.com/docker/docker/libnetwork/datastore"
 	"github.com/docker/docker/libnetwork/driverapi"
 	"github.com/docker/docker/libnetwork/ipams/defaultipam"
 	"github.com/docker/docker/libnetwork/ipams/null"
@@ -45,7 +44,7 @@ const (
 
 func TestMain(m *testing.M) {
 	// Cleanup local datastore file
-	_ = os.Remove(datastore.DefaultScope("").Client.Address)
+	_ = os.Remove("/var/lib/docker/network/files/local-kv.db")
 
 	os.Exit(m.Run())
 }

libnetwork/store_linux_test.go

@@ -12,9 +12,7 @@ import (
 
 func TestBoltdbBackend(t *testing.T) {
 	tmpPath := filepath.Join(t.TempDir(), "boltdb.db")
-	testLocalBackend(t, "boltdb", tmpPath, &store.Config{
-		Bucket: "testBackend",
-	})
+	testLocalBackend(t, tmpPath, "testBackend")
 }
 
 func TestNoPersist(t *testing.T) {

libnetwork/store_test.go

@@ -5,21 +5,18 @@ import (
 	"testing"
 
 	"github.com/docker/docker/libnetwork/config"
-	store "github.com/docker/docker/libnetwork/internal/kvstore"
 	"github.com/docker/docker/libnetwork/netlabel"
 	"github.com/docker/docker/libnetwork/options"
 )
 
-func testLocalBackend(t *testing.T, provider, url string, storeConfig *store.Config) {
-	cfgOptions := []config.Option{func(c *config.Config) {
-		c.Scope.Client.Provider = provider
-		c.Scope.Client.Address = url
-		c.Scope.Client.Config = storeConfig
-	}}
-
-	cfgOptions = append(cfgOptions, config.OptionDriverConfig("host", map[string]interface{}{
-		netlabel.GenericData: options.Generic{},
-	}))
+func testLocalBackend(t *testing.T, path, bucket string) {
+	cfgOptions := []config.Option{
+		config.OptionDataDir(path),
+		func(c *config.Config) { c.DatastoreBucket = bucket },
+		config.OptionDriverConfig("host", map[string]interface{}{
+			netlabel.GenericData: options.Generic{},
+		}),
+	}
 
 	testController, err := New(cfgOptions...)
 	if err != nil {