Revert "Share libkv store handles across datastore handles"

libnetwork/datastore/datastore.go

@@ -5,7 +5,6 @@ import (
 	"log"
 	"reflect"
 	"strings"
-	"sync"
 
 	"github.com/docker/libkv"
 	"github.com/docker/libkv/store"
@@ -56,7 +55,6 @@ type datastore struct {
 	scope string
 	store store.Store
 	cache *cache
-	cfg   ScopeCfg
 }
 
 // KVObject is  Key/Value interface used by objects to be part of the DataStore
@@ -105,12 +103,6 @@ type ScopeClientCfg struct {
 	Config   *store.Config
 }
 
-type storeTableData struct {
-	refCnt int
-	store  store.Store
-	once   sync.Once
-}
-
 const (
 	// LocalScope indicates to store the KV object in local datastore such as boltdb
 	LocalScope = "local"
@@ -128,8 +120,6 @@ const (
 
 var (
 	defaultScopes = makeDefaultScopes()
-	storeLock     sync.Mutex
-	storeTable    = make(map[ScopeCfg]*storeTableData)
 )
 
 func makeDefaultScopes() map[string]*ScopeCfg {
@@ -198,7 +188,11 @@ func ParseKey(key string) ([]string, error) {
 }
 
 // newClient used to connect to KV Store
-func newClient(scope string, kv string, addrs string, config *store.Config, cached bool) (*datastore, error) {
+func newClient(scope string, kv string, addrs string, config *store.Config, cached bool) (DataStore, error) {
+	if cached && scope != LocalScope {
+		return nil, fmt.Errorf("caching supported only for scope %s", LocalScope)
+	}
+
 	if config == nil {
 		config = &store.Config{}
 	}
@@ -217,76 +211,24 @@ func newClient(scope string, kv string, addrs string, config *store.Config, cach
 
 // NewDataStore creates a new instance of LibKV data store
 func NewDataStore(scope string, cfg *ScopeCfg) (DataStore, error) {
-	var (
-		err error
-		ds  *datastore
-	)
-
-	if !cfg.IsValid() {
-		return nil, fmt.Errorf("invalid datastore configuration passed for scope %s", scope)
-	}
-
-	storeLock.Lock()
-	sdata, ok := storeTable[*cfg]
-	if ok {
-		sdata.refCnt++
-		// If sdata already has a store nothing to do. Just
-		// create a datastore handle using it and return with
-		// that.
-		if sdata.store != nil {
-			storeLock.Unlock()
-			return &datastore{scope: scope, cfg: *cfg, store: sdata.store}, nil
-		}
-	} else {
-		// If sdata is not present create one and add ito
-		// storeTable while holding the lock.
-		sdata = &storeTableData{refCnt: 1}
-		storeTable[*cfg] = sdata
-	}
-	storeLock.Unlock()
-
-	// We come here either because:
-	//
-	// 1. We just created the store table data OR
-	// 2. We picked up the store table data from table but store was not initialized.
-	//
-	// In both cases the once function will ensure the store
-	// initialization happens exactly once
-	sdata.once.Do(func() {
-		ds, err = newClient(scope, cfg.Client.Provider, cfg.Client.Address, cfg.Client.Config, scope == LocalScope)
-		if err != nil {
-			return
+	if cfg == nil || cfg.Client.Provider == "" || cfg.Client.Address == "" {
+		c, ok := defaultScopes[scope]
+		if !ok || c.Client.Provider == "" || c.Client.Address == "" {
+			return nil, fmt.Errorf("unexpected scope %s without configuration passed", scope)
 		}
 
-		ds.cfg = *cfg
-		sdata.store = ds.store
-	})
+		cfg = c
+	}
 
-	if err != nil {
-		return nil, err
+	var cached bool
+	if scope == LocalScope {
+		cached = true
 	}
 
-	return ds, nil
+	return newClient(scope, cfg.Client.Provider, cfg.Client.Address, cfg.Client.Config, cached)
 }
 
 func (ds *datastore) Close() {
-	storeLock.Lock()
-	sdata := storeTable[ds.cfg]
-
-	if sdata == nil {
-		storeLock.Unlock()
-		return
-	}
-
-	sdata.refCnt--
-	if sdata.refCnt > 0 {
-		storeLock.Unlock()
-		return
-	}
-
-	delete(storeTable, ds.cfg)
-	storeLock.Unlock()
-
 	ds.store.Close()
 }
 

libnetwork/store_test.go

@@ -42,17 +42,9 @@ func TestBoltdbBackend(t *testing.T) {
 
 func testLocalBackend(t *testing.T, provider, url string, storeConfig *store.Config) {
 	cfgOptions := []config.Option{}
-	if provider != "" {
-		cfgOptions = append(cfgOptions, config.OptionLocalKVProvider(provider))
-	}
-
-	if url != "" {
-		cfgOptions = append(cfgOptions, config.OptionLocalKVProviderURL(url))
-	}
-
-	if storeConfig != nil {
-		cfgOptions = append(cfgOptions, config.OptionLocalKVProviderConfig(storeConfig))
-	}
+	cfgOptions = append(cfgOptions, config.OptionLocalKVProvider(provider))
+	cfgOptions = append(cfgOptions, config.OptionLocalKVProviderURL(url))
+	cfgOptions = append(cfgOptions, config.OptionLocalKVProviderConfig(storeConfig))
 
 	driverOptions := options.Generic{}
 	genericOption := make(map[string]interface{})
@@ -78,7 +70,7 @@ func testLocalBackend(t *testing.T, provider, url string, storeConfig *store.Con
 	if exists, err := store.Exists(datastore.Key([]string{datastore.EndpointKeyPrefix, string(nw.ID()), string(ep.ID())}...)); !exists || err != nil {
 		t.Fatalf("Endpoint key should have been created.")
 	}
-	ctrl.(*controller).getStore(datastore.LocalScope).Close()
+	store.Close()
 
 	// test restore of local store
 	ctrl, err = New(cfgOptions...)