Skip to content

Commit d21d088

Browse files
corherecorhere
committed
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 <[email protected]>
1 parent 8a81b9d commit d21d088

16 files changed

Lines changed: 36 additions & 94 deletions

‎daemon/oci_linux_test.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"github.com/docker/docker/daemon/config"
1212
"github.com/docker/docker/daemon/network"
1313
"github.com/docker/docker/libnetwork"
14+
nwconfig "github.com/docker/docker/libnetwork/config"
1415
"github.com/google/go-cmp/cmp/cmpopts"
1516
"github.com/opencontainers/runtime-spec/specs-go"
1617
"golang.org/x/sys/unix"
@@ -27,7 +28,7 @@ func setupFakeDaemon(t *testing.T, c *container.Container) *Daemon {
2728
err := os.MkdirAll(rootfs, 0o755)
2829
assert.NilError(t, err)
2930

30-
netController, err := libnetwork.New()
31+
netController, err := libnetwork.New(nwconfig.OptionDataDir(t.TempDir()))
3132
assert.NilError(t, err)
3233

3334
d := &Daemon{

‎daemon/reload_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,7 @@ func TestDaemonReloadNetworkDiagnosticPort(t *testing.T) {
360360
},
361361
}
362362

363-
netOptions, err := daemon.networkOptions(&config.Config{}, nil, nil)
363+
netOptions, err := daemon.networkOptions(&config.Config{CommonConfig: config.CommonConfig{Root: t.TempDir()}}, nil, nil)
364364
if err != nil {
365365
t.Fatal(err)
366366
}

‎libnetwork/controller.go‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -344,14 +344,7 @@ func (c *Controller) makeDriverConfig(ntype string) map[string]interface{} {
344344
}
345345

346346
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
355348
}
356349

357350
return cfg

‎libnetwork/datastore/datastore.go‎

Lines changed: 1 addition & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66
"sync"
77
"time"
88

9-
"github.com/docker/docker/libnetwork/discoverapi"
109
store "github.com/docker/docker/libnetwork/internal/kvstore"
1110
"github.com/docker/docker/libnetwork/internal/kvstore/boltdb"
1211
"github.com/docker/docker/libnetwork/types"
@@ -93,6 +92,7 @@ func DefaultScope(dataDir string) ScopeCfg {
9392
Config: &store.Config{
9493
Bucket: "libnetwork",
9594
ConnectionTimeout: time.Minute,
95+
PersistConnection: true,
9696
},
9797
},
9898
}
@@ -147,32 +147,6 @@ func New(cfg ScopeCfg) (*Store, error) {
147147
return newClient(cfg.Client.Provider, cfg.Client.Address, cfg.Client.Config)
148148
}
149149

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-
176150
// Close closes the data store.
177151
func (ds *Store) Close() {
178152
ds.store.Close()

‎libnetwork/discoverapi/discoverapi.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ type NodeDiscoveryData struct {
3030
}
3131

3232
// DatastoreConfigData is the data for the datastore update event message
33+
//
34+
// Deprecated: no longer used.
3335
type DatastoreConfigData struct {
3436
Scope string
3537
Provider string

‎libnetwork/drivers/bridge/bridge_store.go‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010

1111
"github.com/containerd/log"
1212
"github.com/docker/docker/libnetwork/datastore"
13-
"github.com/docker/docker/libnetwork/discoverapi"
1413
"github.com/docker/docker/libnetwork/netlabel"
1514
"github.com/docker/docker/libnetwork/types"
1615
)
@@ -25,17 +24,13 @@ const (
2524

2625
func (d *driver) initStore(option map[string]interface{}) error {
2726
if data, ok := option[netlabel.LocalKVClient]; ok {
28-
var err error
29-
dsc, ok := data.(discoverapi.DatastoreConfigData)
27+
var ok bool
28+
d.store, ok = data.(*datastore.Store)
3029
if !ok {
3130
return types.InternalErrorf("incorrect data in datastore configuration: %v", data)
3231
}
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-
}
3732

38-
err = d.populateNetworks()
33+
err := d.populateNetworks()
3934
if err != nil {
4035
return err
4136
}

‎libnetwork/drivers/ipvlan/ipvlan_store.go‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010

1111
"github.com/containerd/log"
1212
"github.com/docker/docker/libnetwork/datastore"
13-
"github.com/docker/docker/libnetwork/discoverapi"
1413
"github.com/docker/docker/libnetwork/netlabel"
1514
"github.com/docker/docker/libnetwork/types"
1615
)
@@ -44,17 +43,13 @@ type ipSubnet struct {
4443
// initStore drivers are responsible for caching their own persistent state
4544
func (d *driver) initStore(option map[string]interface{}) error {
4645
if data, ok := option[netlabel.LocalKVClient]; ok {
47-
var err error
48-
dsc, ok := data.(discoverapi.DatastoreConfigData)
46+
var ok bool
47+
d.store, ok = data.(*datastore.Store)
4948
if !ok {
5049
return types.InternalErrorf("incorrect data in datastore configuration: %v", data)
5150
}
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-
}
5651

57-
err = d.populateNetworks()
52+
err := d.populateNetworks()
5853
if err != nil {
5954
return err
6055
}

‎libnetwork/drivers/macvlan/macvlan_store.go‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010

1111
"github.com/containerd/log"
1212
"github.com/docker/docker/libnetwork/datastore"
13-
"github.com/docker/docker/libnetwork/discoverapi"
1413
"github.com/docker/docker/libnetwork/netlabel"
1514
"github.com/docker/docker/libnetwork/types"
1615
)
@@ -43,17 +42,13 @@ type ipSubnet struct {
4342
// initStore drivers are responsible for caching their own persistent state
4443
func (d *driver) initStore(option map[string]interface{}) error {
4544
if data, ok := option[netlabel.LocalKVClient]; ok {
46-
var err error
47-
dsc, ok := data.(discoverapi.DatastoreConfigData)
45+
var ok bool
46+
d.store, ok = data.(*datastore.Store)
4847
if !ok {
4948
return types.InternalErrorf("incorrect data in datastore configuration: %v", data)
5049
}
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-
}
5550

56-
err = d.populateNetworks()
51+
err := d.populateNetworks()
5752
if err != nil {
5853
return err
5954
}

‎libnetwork/drivers/windows/windows_store.go‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010

1111
"github.com/containerd/log"
1212
"github.com/docker/docker/libnetwork/datastore"
13-
"github.com/docker/docker/libnetwork/discoverapi"
1413
"github.com/docker/docker/libnetwork/netlabel"
1514
"github.com/docker/docker/libnetwork/types"
1615
)
@@ -22,17 +21,13 @@ const (
2221

2322
func (d *driver) initStore(option map[string]interface{}) error {
2423
if data, ok := option[netlabel.LocalKVClient]; ok {
25-
var err error
26-
dsc, ok := data.(discoverapi.DatastoreConfigData)
24+
var ok bool
25+
d.store, ok = data.(*datastore.Store)
2726
if !ok {
2827
return types.InternalErrorf("incorrect data in datastore configuration: %v", data)
2928
}
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-
}
3429

35-
err = d.populateNetworks()
30+
err := d.populateNetworks()
3631
if err != nil {
3732
return err
3833
}

‎libnetwork/firewall_linux_test.go‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -54,12 +54,14 @@ func TestUserChain(t *testing.T) {
5454
defer netnsutils.SetupTestOSContext(t)()
5555
defer resetIptables(t)
5656

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+
}))
6365
assert.NilError(t, err)
6466
defer c.Stop()
6567

0 commit comments

Comments
 (0)