Browse code

libnetwork: share a single datastore with drivers

The bbolt library wants exclusive access to the boltdb file and uses
file locking to assure that is the case. The controller and each network
driver that needs persistent storage instantiates its own unique
datastore instance, backed by the same boltdb file. The boltdb kvstore
implementation works around multiple access to the same boltdb file by
aggressively closing the boltdb file between each transaction. This is
very inefficient. Have the controller pass its datastore instance into
the drivers and enable the PersistConnection option to disable closing
the boltdb between transactions.

Set data-dir in unit tests which instantiate libnetwork controllers so
they don't hang trying to lock the default boltdb database file.

Signed-off-by: Cory Snider <csnider@mirantis.com>

Cory Snider authored on 2024/02/01 07:24:47
Showing 16 changed files
... ...
@@ -11,6 +11,7 @@ import (
11 11
 	"github.com/docker/docker/daemon/config"
12 12
 	"github.com/docker/docker/daemon/network"
13 13
 	"github.com/docker/docker/libnetwork"
14
+	nwconfig "github.com/docker/docker/libnetwork/config"
14 15
 	"github.com/google/go-cmp/cmp/cmpopts"
15 16
 	"github.com/opencontainers/runtime-spec/specs-go"
16 17
 	"golang.org/x/sys/unix"
... ...
@@ -27,7 +28,7 @@ func setupFakeDaemon(t *testing.T, c *container.Container) *Daemon {
27 27
 	err := os.MkdirAll(rootfs, 0o755)
28 28
 	assert.NilError(t, err)
29 29
 
30
-	netController, err := libnetwork.New()
30
+	netController, err := libnetwork.New(nwconfig.OptionDataDir(t.TempDir()))
31 31
 	assert.NilError(t, err)
32 32
 
33 33
 	d := &Daemon{
... ...
@@ -360,7 +360,7 @@ func TestDaemonReloadNetworkDiagnosticPort(t *testing.T) {
360 360
 		},
361 361
 	}
362 362
 
363
-	netOptions, err := daemon.networkOptions(&config.Config{}, nil, nil)
363
+	netOptions, err := daemon.networkOptions(&config.Config{CommonConfig: config.CommonConfig{Root: t.TempDir()}}, nil, nil)
364 364
 	if err != nil {
365 365
 		t.Fatal(err)
366 366
 	}
... ...
@@ -344,14 +344,7 @@ func (c *Controller) makeDriverConfig(ntype string) map[string]interface{} {
344 344
 	}
345 345
 
346 346
 	if c.cfg.Scope.IsValid() {
347
-		// FIXME: every driver instance constructs a new DataStore
348
-		// instance against the same database. Yikes!
349
-		cfg[netlabel.LocalKVClient] = discoverapi.DatastoreConfigData{
350
-			Scope:    scope.Local,
351
-			Provider: c.cfg.Scope.Client.Provider,
352
-			Address:  c.cfg.Scope.Client.Address,
353
-			Config:   c.cfg.Scope.Client.Config,
354
-		}
347
+		cfg[netlabel.LocalKVClient] = c.store
355 348
 	}
356 349
 
357 350
 	return cfg
... ...
@@ -6,7 +6,6 @@ import (
6 6
 	"sync"
7 7
 	"time"
8 8
 
9
-	"github.com/docker/docker/libnetwork/discoverapi"
10 9
 	store "github.com/docker/docker/libnetwork/internal/kvstore"
11 10
 	"github.com/docker/docker/libnetwork/internal/kvstore/boltdb"
12 11
 	"github.com/docker/docker/libnetwork/types"
... ...
@@ -93,6 +92,7 @@ func DefaultScope(dataDir string) ScopeCfg {
93 93
 			Config: &store.Config{
94 94
 				Bucket:            "libnetwork",
95 95
 				ConnectionTimeout: time.Minute,
96
+				PersistConnection: true,
96 97
 			},
97 98
 		},
98 99
 	}
... ...
@@ -147,32 +147,6 @@ func New(cfg ScopeCfg) (*Store, error) {
147 147
 	return newClient(cfg.Client.Provider, cfg.Client.Address, cfg.Client.Config)
148 148
 }
149 149
 
150
-// FromConfig creates a new instance of LibKV data store starting from the datastore config data.
151
-func FromConfig(dsc discoverapi.DatastoreConfigData) (*Store, error) {
152
-	var (
153
-		ok    bool
154
-		sCfgP *store.Config
155
-	)
156
-
157
-	sCfgP, ok = dsc.Config.(*store.Config)
158
-	if !ok && dsc.Config != nil {
159
-		return nil, fmt.Errorf("cannot parse store configuration: %v", dsc.Config)
160
-	}
161
-
162
-	ds, err := New(ScopeCfg{
163
-		Client: ScopeClientCfg{
164
-			Address:  dsc.Address,
165
-			Provider: dsc.Provider,
166
-			Config:   sCfgP,
167
-		},
168
-	})
169
-	if err != nil {
170
-		return nil, fmt.Errorf("failed to construct datastore client from datastore configuration %v: %v", dsc, err)
171
-	}
172
-
173
-	return ds, err
174
-}
175
-
176 150
 // Close closes the data store.
177 151
 func (ds *Store) Close() {
178 152
 	ds.store.Close()
... ...
@@ -30,6 +30,8 @@ type NodeDiscoveryData struct {
30 30
 }
31 31
 
32 32
 // DatastoreConfigData is the data for the datastore update event message
33
+//
34
+// Deprecated: no longer used.
33 35
 type DatastoreConfigData struct {
34 36
 	Scope    string
35 37
 	Provider string
... ...
@@ -10,7 +10,6 @@ import (
10 10
 
11 11
 	"github.com/containerd/log"
12 12
 	"github.com/docker/docker/libnetwork/datastore"
13
-	"github.com/docker/docker/libnetwork/discoverapi"
14 13
 	"github.com/docker/docker/libnetwork/netlabel"
15 14
 	"github.com/docker/docker/libnetwork/types"
16 15
 )
... ...
@@ -25,17 +24,13 @@ const (
25 25
 
26 26
 func (d *driver) initStore(option map[string]interface{}) error {
27 27
 	if data, ok := option[netlabel.LocalKVClient]; ok {
28
-		var err error
29
-		dsc, ok := data.(discoverapi.DatastoreConfigData)
28
+		var ok bool
29
+		d.store, ok = data.(*datastore.Store)
30 30
 		if !ok {
31 31
 			return types.InternalErrorf("incorrect data in datastore configuration: %v", data)
32 32
 		}
33
-		d.store, err = datastore.FromConfig(dsc)
34
-		if err != nil {
35
-			return types.InternalErrorf("bridge driver failed to initialize data store: %v", err)
36
-		}
37 33
 
38
-		err = d.populateNetworks()
34
+		err := d.populateNetworks()
39 35
 		if err != nil {
40 36
 			return err
41 37
 		}
... ...
@@ -10,7 +10,6 @@ import (
10 10
 
11 11
 	"github.com/containerd/log"
12 12
 	"github.com/docker/docker/libnetwork/datastore"
13
-	"github.com/docker/docker/libnetwork/discoverapi"
14 13
 	"github.com/docker/docker/libnetwork/netlabel"
15 14
 	"github.com/docker/docker/libnetwork/types"
16 15
 )
... ...
@@ -44,17 +43,13 @@ type ipSubnet struct {
44 44
 // initStore drivers are responsible for caching their own persistent state
45 45
 func (d *driver) initStore(option map[string]interface{}) error {
46 46
 	if data, ok := option[netlabel.LocalKVClient]; ok {
47
-		var err error
48
-		dsc, ok := data.(discoverapi.DatastoreConfigData)
47
+		var ok bool
48
+		d.store, ok = data.(*datastore.Store)
49 49
 		if !ok {
50 50
 			return types.InternalErrorf("incorrect data in datastore configuration: %v", data)
51 51
 		}
52
-		d.store, err = datastore.FromConfig(dsc)
53
-		if err != nil {
54
-			return types.InternalErrorf("ipvlan driver failed to initialize data store: %v", err)
55
-		}
56 52
 
57
-		err = d.populateNetworks()
53
+		err := d.populateNetworks()
58 54
 		if err != nil {
59 55
 			return err
60 56
 		}
... ...
@@ -10,7 +10,6 @@ import (
10 10
 
11 11
 	"github.com/containerd/log"
12 12
 	"github.com/docker/docker/libnetwork/datastore"
13
-	"github.com/docker/docker/libnetwork/discoverapi"
14 13
 	"github.com/docker/docker/libnetwork/netlabel"
15 14
 	"github.com/docker/docker/libnetwork/types"
16 15
 )
... ...
@@ -43,17 +42,13 @@ type ipSubnet struct {
43 43
 // initStore drivers are responsible for caching their own persistent state
44 44
 func (d *driver) initStore(option map[string]interface{}) error {
45 45
 	if data, ok := option[netlabel.LocalKVClient]; ok {
46
-		var err error
47
-		dsc, ok := data.(discoverapi.DatastoreConfigData)
46
+		var ok bool
47
+		d.store, ok = data.(*datastore.Store)
48 48
 		if !ok {
49 49
 			return types.InternalErrorf("incorrect data in datastore configuration: %v", data)
50 50
 		}
51
-		d.store, err = datastore.FromConfig(dsc)
52
-		if err != nil {
53
-			return types.InternalErrorf("macvlan driver failed to initialize data store: %v", err)
54
-		}
55 51
 
56
-		err = d.populateNetworks()
52
+		err := d.populateNetworks()
57 53
 		if err != nil {
58 54
 			return err
59 55
 		}
... ...
@@ -10,7 +10,6 @@ import (
10 10
 
11 11
 	"github.com/containerd/log"
12 12
 	"github.com/docker/docker/libnetwork/datastore"
13
-	"github.com/docker/docker/libnetwork/discoverapi"
14 13
 	"github.com/docker/docker/libnetwork/netlabel"
15 14
 	"github.com/docker/docker/libnetwork/types"
16 15
 )
... ...
@@ -22,17 +21,13 @@ const (
22 22
 
23 23
 func (d *driver) initStore(option map[string]interface{}) error {
24 24
 	if data, ok := option[netlabel.LocalKVClient]; ok {
25
-		var err error
26
-		dsc, ok := data.(discoverapi.DatastoreConfigData)
25
+		var ok bool
26
+		d.store, ok = data.(*datastore.Store)
27 27
 		if !ok {
28 28
 			return types.InternalErrorf("incorrect data in datastore configuration: %v", data)
29 29
 		}
30
-		d.store, err = datastore.FromConfig(dsc)
31
-		if err != nil {
32
-			return types.InternalErrorf("windows driver failed to initialize data store: %v", err)
33
-		}
34 30
 
35
-		err = d.populateNetworks()
31
+		err := d.populateNetworks()
36 32
 		if err != nil {
37 33
 			return err
38 34
 		}
... ...
@@ -54,12 +54,14 @@ func TestUserChain(t *testing.T) {
54 54
 			defer netnsutils.SetupTestOSContext(t)()
55 55
 			defer resetIptables(t)
56 56
 
57
-			c, err := New(config.OptionDriverConfig("bridge", map[string]any{
58
-				netlabel.GenericData: options.Generic{
59
-					"EnableIPTables":  tc.iptables,
60
-					"EnableIP6Tables": tc.iptables,
61
-				},
62
-			}))
57
+			c, err := New(
58
+				OptionBoltdbWithRandomDBFile(t),
59
+				config.OptionDriverConfig("bridge", map[string]any{
60
+					netlabel.GenericData: options.Generic{
61
+						"EnableIPTables":  tc.iptables,
62
+						"EnableIP6Tables": tc.iptables,
63
+					},
64
+				}))
63 65
 			assert.NilError(t, err)
64 66
 			defer c.Stop()
65 67
 
... ...
@@ -314,7 +314,7 @@ func compareNwLists(a, b []*net.IPNet) bool {
314 314
 func TestAuxAddresses(t *testing.T) {
315 315
 	defer netnsutils.SetupTestOSContext(t)()
316 316
 
317
-	c, err := New()
317
+	c, err := New(OptionBoltdbWithRandomDBFile(t))
318 318
 	if err != nil {
319 319
 		t.Fatal(err)
320 320
 	}
... ...
@@ -353,7 +353,7 @@ func TestSRVServiceQuery(t *testing.T) {
353 353
 
354 354
 	defer netnsutils.SetupTestOSContext(t)()
355 355
 
356
-	c, err := New()
356
+	c, err := New(OptionBoltdbWithRandomDBFile(t))
357 357
 	if err != nil {
358 358
 		t.Fatal(err)
359 359
 	}
... ...
@@ -451,7 +451,7 @@ func TestServiceVIPReuse(t *testing.T) {
451 451
 
452 452
 	defer netnsutils.SetupTestOSContext(t)()
453 453
 
454
-	c, err := New()
454
+	c, err := New(OptionBoltdbWithRandomDBFile(t))
455 455
 	if err != nil {
456 456
 		t.Fatal(err)
457 457
 	}
... ...
@@ -1197,7 +1197,7 @@ func TestInvalidRemoteDriver(t *testing.T) {
1197 1197
 		t.Fatal(err)
1198 1198
 	}
1199 1199
 
1200
-	ctrlr, err := libnetwork.New()
1200
+	ctrlr, err := libnetwork.New(libnetwork.OptionBoltdbWithRandomDBFile(t))
1201 1201
 	if err != nil {
1202 1202
 		t.Fatal(err)
1203 1203
 	}
... ...
@@ -13,7 +13,7 @@ import (
13 13
 // test only works on linux
14 14
 func TestDNSIPQuery(t *testing.T) {
15 15
 	defer netnsutils.SetupTestOSContext(t)()
16
-	c, err := New()
16
+	c, err := New(OptionBoltdbWithRandomDBFile(t))
17 17
 	if err != nil {
18 18
 		t.Fatal(err)
19 19
 	}
... ...
@@ -110,7 +110,7 @@ func TestDNSProxyServFail(t *testing.T) {
110 110
 	osctx := netnsutils.SetupTestOSContextEx(t)
111 111
 	defer osctx.Cleanup(t)
112 112
 
113
-	c, err := New()
113
+	c, err := New(OptionBoltdbWithRandomDBFile(t))
114 114
 	if err != nil {
115 115
 		t.Fatal(err)
116 116
 	}
... ...
@@ -15,7 +15,7 @@ import (
15 15
 func TestDNSOptions(t *testing.T) {
16 16
 	skip.If(t, runtime.GOOS == "windows", "test only works on linux")
17 17
 
18
-	c, err := New()
18
+	c, err := New(OptionBoltdbWithRandomDBFile(t))
19 19
 	assert.NilError(t, err)
20 20
 
21 21
 	sb, err := c.NewSandbox("cnt1", nil)
... ...
@@ -12,7 +12,7 @@ import (
12 12
 
13 13
 func TestCleanupServiceDiscovery(t *testing.T) {
14 14
 	defer netnsutils.SetupTestOSContext(t)()
15
-	c, err := New()
15
+	c, err := New(OptionBoltdbWithRandomDBFile(t))
16 16
 	assert.NilError(t, err)
17 17
 	defer c.Stop()
18 18
 
... ...
@@ -2,18 +2,13 @@ package libnetwork
2 2
 
3 3
 import (
4 4
 	"errors"
5
-	"os"
6 5
 	"path/filepath"
7 6
 	"testing"
8 7
 
9
-	"github.com/docker/docker/libnetwork/config"
10
-	"github.com/docker/docker/libnetwork/datastore"
11 8
 	store "github.com/docker/docker/libnetwork/internal/kvstore"
12 9
 )
13 10
 
14 11
 func TestBoltdbBackend(t *testing.T) {
15
-	defer os.Remove(datastore.DefaultScope("").Client.Address)
16
-	testLocalBackend(t, "", "", nil)
17 12
 	tmpPath := filepath.Join(t.TempDir(), "boltdb.db")
18 13
 	testLocalBackend(t, "boltdb", tmpPath, &store.Config{
19 14
 		Bucket: "testBackend",
... ...
@@ -21,12 +16,7 @@ func TestBoltdbBackend(t *testing.T) {
21 21
 }
22 22
 
23 23
 func TestNoPersist(t *testing.T) {
24
-	dbFile := filepath.Join(t.TempDir(), "bolt.db")
25
-	configOption := func(c *config.Config) {
26
-		c.Scope.Client.Provider = "boltdb"
27
-		c.Scope.Client.Address = dbFile
28
-		c.Scope.Client.Config = &store.Config{Bucket: "testBackend"}
29
-	}
24
+	configOption := OptionBoltdbWithRandomDBFile(t)
30 25
 	testController, err := New(configOption)
31 26
 	if err != nil {
32 27
 		t.Fatalf("Error creating new controller: %v", err)