Skip to content

Commit ed08486

Browse files
akerouantonakerouanton
committed
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 <[email protected]>
1 parent 3ca91a6 commit ed08486

9 files changed

Lines changed: 34 additions & 121 deletions

File tree

‎libnetwork/config/config.go‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,15 +39,16 @@ type Config struct {
3939
ClusterProvider cluster.Provider
4040
NetworkControlPlaneMTU int
4141
DefaultAddressPool []*ipamutils.NetworkToSplit
42-
Scope datastore.ScopeCfg
42+
DatastoreBucket string
4343
ActiveSandboxes map[string]interface{}
4444
PluginGetter plugingetter.PluginGetter
4545
}
4646

4747
// New creates a new Config and initializes it with the given Options.
4848
func New(opts ...Option) *Config {
4949
cfg := &Config{
50-
driverCfg: make(map[string]map[string]any),
50+
driverCfg: make(map[string]map[string]any),
51+
DatastoreBucket: datastore.DefaultBucket,
5152
}
5253

5354
for _, opt := range opts {
@@ -56,10 +57,6 @@ func New(opts ...Option) *Config {
5657
}
5758
}
5859

59-
// load default scope configs which don't have explicit user specified configs.
60-
if cfg.Scope == (datastore.ScopeCfg{}) {
61-
cfg.Scope = datastore.DefaultScope(cfg.DataDir)
62-
}
6360
return cfg
6461
}
6562

‎libnetwork/controller.go‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ type Controller struct {
108108
// New creates a new instance of network controller.
109109
func New(cfgOptions ...config.Option) (*Controller, error) {
110110
cfg := config.New(cfgOptions...)
111-
store, err := datastore.New(cfg.Scope)
111+
store, err := datastore.New(cfg.DataDir, cfg.DatastoreBucket)
112112
if err != nil {
113113
return nil, fmt.Errorf("libnet controller initialization: %w", err)
114114
}
@@ -333,7 +333,9 @@ func (c *Controller) makeDriverConfig(ntype string) map[string]interface{} {
333333
return nil
334334
}
335335

336-
cfg := map[string]interface{}{}
336+
cfg := map[string]interface{}{
337+
netlabel.LocalKVClient: c.store,
338+
}
337339
for _, label := range c.cfg.Labels {
338340
key, val, _ := strings.Cut(label, "=")
339341
if !strings.HasPrefix(key, netlabel.DriverPrefix+"."+ntype) {
@@ -348,10 +350,6 @@ func (c *Controller) makeDriverConfig(ntype string) map[string]interface{} {
348350
cfg[k] = v
349351
}
350352

351-
if c.cfg.Scope.IsValid() {
352-
cfg[netlabel.LocalKVClient] = c.store
353-
}
354-
355353
return cfg
356354
}
357355

‎libnetwork/datastore/datastore.go‎

Lines changed: 10 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
package datastore
22

33
import (
4-
"fmt"
4+
"errors"
5+
"path"
56
"strings"
67
"sync"
7-
"time"
88

99
store "github.com/docker/docker/libnetwork/internal/kvstore"
1010
"github.com/docker/docker/libnetwork/internal/kvstore/boltdb"
@@ -50,18 +50,6 @@ type KVObject interface {
5050
CopyTo(KVObject) error
5151
}
5252

53-
// ScopeCfg represents Datastore configuration.
54-
type ScopeCfg struct {
55-
Client ScopeClientCfg
56-
}
57-
58-
// ScopeClientCfg represents Datastore Client-only mode configuration
59-
type ScopeClientCfg struct {
60-
Provider string
61-
Address string
62-
Config *store.Config
63-
}
64-
6553
const (
6654
// NetworkKeyPrefix is the prefix for network key in the kv store
6755
NetworkKeyPrefix = "network"
@@ -74,37 +62,7 @@ var (
7462
rootChain = defaultRootChain
7563
)
7664

77-
const defaultPrefix = "/var/lib/docker/network/files"
78-
79-
// DefaultScope returns a default scope config for clients to use.
80-
func DefaultScope(dataDir string) ScopeCfg {
81-
var dbpath string
82-
if dataDir == "" {
83-
dbpath = defaultPrefix + "/local-kv.db"
84-
} else {
85-
dbpath = dataDir + "/network/files/local-kv.db"
86-
}
87-
88-
return ScopeCfg{
89-
Client: ScopeClientCfg{
90-
Provider: string(store.BOLTDB),
91-
Address: dbpath,
92-
Config: &store.Config{
93-
Bucket: "libnetwork",
94-
ConnectionTimeout: time.Minute,
95-
},
96-
},
97-
}
98-
}
99-
100-
// IsValid checks if the scope config has valid configuration.
101-
func (cfg *ScopeCfg) IsValid() bool {
102-
if cfg == nil || strings.TrimSpace(cfg.Client.Provider) == "" || strings.TrimSpace(cfg.Client.Address) == "" {
103-
return false
104-
}
105-
106-
return true
107-
}
65+
const DefaultBucket = "libnetwork"
10866

10967
// Key provides convenient method to create a Key
11068
func Key(key ...string) string {
@@ -118,33 +76,23 @@ func Key(key ...string) string {
11876
return b.String()
11977
}
12078

121-
// newClient used to connect to KV Store
122-
func newClient(kv string, addr string, config *store.Config) (*Store, error) {
123-
if kv != string(store.BOLTDB) {
124-
return nil, fmt.Errorf("unsupported KV store")
79+
// New creates a new Store instance.
80+
func New(dir, bucket string) (*Store, error) {
81+
if dir == "" {
82+
return nil, errors.New("empty dir")
12583
}
126-
127-
if config == nil {
128-
config = &store.Config{}
84+
if bucket == "" {
85+
return nil, errors.New("empty bucket")
12986
}
13087

131-
s, err := boltdb.New(addr, config)
88+
s, err := boltdb.New(path.Join(dir, "local-kv.db"), bucket)
13289
if err != nil {
13390
return nil, err
13491
}
13592

13693
return &Store{store: s, cache: newCache(s)}, nil
13794
}
13895

139-
// New creates a new Store instance.
140-
func New(cfg ScopeCfg) (*Store, error) {
141-
if cfg.Client.Provider == "" || cfg.Client.Address == "" {
142-
cfg = DefaultScope("")
143-
}
144-
145-
return newClient(cfg.Client.Provider, cfg.Client.Address, cfg.Client.Config)
146-
}
147-
14896
// Close closes the data store.
14997
func (ds *Store) Close() {
15098
ds.store.Close()

‎libnetwork/datastore/datastore_test.go‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,16 +23,6 @@ func TestKey(t *testing.T) {
2323
assert.Check(t, is.Equal(sKey, expected))
2424
}
2525

26-
func TestInvalidDataStore(t *testing.T) {
27-
_, err := New(ScopeCfg{
28-
Client: ScopeClientCfg{
29-
Provider: "invalid",
30-
Address: "localhost:8500",
31-
},
32-
})
33-
assert.Check(t, is.Error(err, "unsupported KV store"))
34-
}
35-
3626
func TestKVObjectFlatKey(t *testing.T) {
3727
store := NewTestDataStore()
3828
expected := dummyKVObject("1000", true)

‎libnetwork/internal/kvstore/boltdb/boltdb.go‎

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,16 @@ package boltdb
33
import (
44
"bytes"
55
"encoding/binary"
6-
"errors"
76
"os"
87
"path/filepath"
98
"sync"
109
"sync/atomic"
10+
"time"
1111

1212
store "github.com/docker/docker/libnetwork/internal/kvstore"
1313
bolt "go.etcd.io/bbolt"
1414
)
1515

16-
// ErrBoltBucketOptionMissing is thrown when boltBucket config option is missing
17-
var ErrBoltBucketOptionMissing = errors.New("boltBucket config option missing")
18-
1916
const filePerm = 0o644
2017

2118
// BoltDB type implements the Store interface
@@ -30,27 +27,23 @@ type BoltDB struct {
3027
const libkvmetadatalen = 8
3128

3229
// New opens a new BoltDB connection to the specified path and bucket
33-
func New(endpoint string, options *store.Config) (store.Store, error) {
34-
if (options == nil) || (len(options.Bucket) == 0) {
35-
return nil, ErrBoltBucketOptionMissing
36-
}
37-
38-
dir, _ := filepath.Split(endpoint)
30+
func New(path, bucket string) (store.Store, error) {
31+
dir, _ := filepath.Split(path)
3932
if err := os.MkdirAll(dir, 0o750); err != nil {
4033
return nil, err
4134
}
4235

43-
db, err := bolt.Open(endpoint, filePerm, &bolt.Options{
44-
Timeout: options.ConnectionTimeout,
36+
db, err := bolt.Open(path, filePerm, &bolt.Options{
37+
Timeout: time.Minute,
4538
})
4639
if err != nil {
4740
return nil, err
4841
}
4942

5043
b := &BoltDB{
5144
client: db,
52-
path: endpoint,
53-
boltBucket: []byte(options.Bucket),
45+
path: path,
46+
boltBucket: []byte(bucket),
5447
}
5548

5649
return b, nil

‎libnetwork/internal/kvstore/kvstore.go‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package kvstore
22

33
import (
44
"errors"
5-
"time"
65
)
76

87
// Backend represents a KV Store Backend
@@ -24,12 +23,6 @@ var (
2423
ErrKeyExists = errors.New("Previous K/V pair exists, cannot complete Atomic operation")
2524
)
2625

27-
// Config contains the options for a storage client
28-
type Config struct {
29-
ConnectionTimeout time.Duration
30-
Bucket string
31-
}
32-
3326
// Store represents the backend K/V storage
3427
// Each store should support every call listed
3528
// here. Or it couldn't be implemented as a K/V

‎libnetwork/libnetwork_linux_test.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import (
2121
"github.com/docker/docker/internal/testutils/netnsutils"
2222
"github.com/docker/docker/libnetwork"
2323
"github.com/docker/docker/libnetwork/config"
24-
"github.com/docker/docker/libnetwork/datastore"
2524
"github.com/docker/docker/libnetwork/driverapi"
2625
"github.com/docker/docker/libnetwork/ipams/defaultipam"
2726
"github.com/docker/docker/libnetwork/ipams/null"
@@ -45,7 +44,7 @@ const (
4544

4645
func TestMain(m *testing.M) {
4746
// Cleanup local datastore file
48-
_ = os.Remove(datastore.DefaultScope("").Client.Address)
47+
_ = os.Remove("/var/lib/docker/network/files/local-kv.db")
4948

5049
os.Exit(m.Run())
5150
}

‎libnetwork/store_linux_test.go‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,7 @@ import (
1212

1313
func TestBoltdbBackend(t *testing.T) {
1414
tmpPath := filepath.Join(t.TempDir(), "boltdb.db")
15-
testLocalBackend(t, "boltdb", tmpPath, &store.Config{
16-
Bucket: "testBackend",
17-
})
15+
testLocalBackend(t, tmpPath, "testBackend")
1816
}
1917

2018
func TestNoPersist(t *testing.T) {

‎libnetwork/store_test.go‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,21 +5,18 @@ import (
55
"testing"
66

77
"github.com/docker/docker/libnetwork/config"
8-
store "github.com/docker/docker/libnetwork/internal/kvstore"
98
"github.com/docker/docker/libnetwork/netlabel"
109
"github.com/docker/docker/libnetwork/options"
1110
)
1211

13-
func testLocalBackend(t *testing.T, provider, url string, storeConfig *store.Config) {
14-
cfgOptions := []config.Option{func(c *config.Config) {
15-
c.Scope.Client.Provider = provider
16-
c.Scope.Client.Address = url
17-
c.Scope.Client.Config = storeConfig
18-
}}
19-
20-
cfgOptions = append(cfgOptions, config.OptionDriverConfig("host", map[string]interface{}{
21-
netlabel.GenericData: options.Generic{},
22-
}))
12+
func testLocalBackend(t *testing.T, path, bucket string) {
13+
cfgOptions := []config.Option{
14+
config.OptionDataDir(path),
15+
func(c *config.Config) { c.DatastoreBucket = bucket },
16+
config.OptionDriverConfig("host", map[string]interface{}{
17+
netlabel.GenericData: options.Generic{},
18+
}),
19+
}
2320

2421
testController, err := New(cfgOptions...)
2522
if err != nil {

0 commit comments

Comments
 (0)