Merge pull request #47820 from akerouanton/libnet-store-is-never-nil-…

…followup

libnet: Controller: more c.store clean-ups

libnetwork/controller.go

@@ -106,9 +106,16 @@ 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)
+ if err != nil {
+ return nil, fmt.Errorf("libnet controller initialization: %w", err)
+ }
+
  c := &Controller{
  id: stringid.GenerateRandomID(),
- cfg: config.New(cfgOptions...),
+ cfg: cfg,
+ store: store,
  sandboxes: map[string]*Sandbox{},
  svcRecords: make(map[string]*svcInfo),
  serviceBindings: make(map[serviceKey]*service),
@@ -117,10 +124,6 @@ func New(cfgOptions ...config.Option) (*Controller, error) {
  DiagnosticServer: diagnostic.New(),
  }
 
- if err := c.initStores(); err != nil {
- return nil, err
- }
-
  c.drvRegistry.Notify = c
 
  // External plugins don't need config passed through daemon. They can
@@ -1086,7 +1089,7 @@ func (c *Controller) getIPAMDriver(name string) (ipamapi.Ipam, *ipamapi.Capabili
 
 // Stop stops the network controller.
 func (c *Controller) Stop() {
- c.closeStores()
+ c.store.Close()
  c.stopExternalKeyListener()
  osl.GC()
 }

libnetwork/endpoint_cnt.go

@@ -105,15 +105,15 @@ func (ec *endpointCnt) EndpointCnt() uint64 {
 }
 
 func (ec *endpointCnt) updateStore() error {
- store := ec.n.getController().getStore()
+ c := ec.n.getController()
  // make a copy of count and n to avoid being overwritten by store.GetObject
  count := ec.EndpointCnt()
  n := ec.n
  for {
- if err := ec.n.getController().updateToStore(ec); err == nil || err != datastore.ErrKeyModified {
+ if err := c.updateToStore(ec); err == nil || err != datastore.ErrKeyModified {
  return err
  }
- if err := store.GetObject(ec); err != nil {
+ if err := c.store.GetObject(ec); err != nil {
  return fmt.Errorf("could not update the kvobject to latest on endpoint count update: %v", err)
  }
  ec.Lock()
@@ -131,7 +131,7 @@ func (ec *endpointCnt) setCnt(cnt uint64) error {
 }
 
 func (ec *endpointCnt) atomicIncDecEpCnt(inc bool) error {
- store := ec.n.getController().getStore()
+ store := ec.n.getController().store
 
  tmp := &endpointCnt{n: ec.n}
  if err := store.GetObject(tmp); err != nil {

libnetwork/sandbox_store.go

@@ -159,12 +159,7 @@ retry:
 }
 
 func (sb *Sandbox) storeDelete() error {
- cs := sb.controller.getStore()
- if cs == nil {
- return fmt.Errorf("datastore is not initialized")
- }
-
- return cs.DeleteObject(&sbState{
+ return sb.controller.store.DeleteObject(&sbState{
  c: sb.controller,
  ID: sb.id,
  Cid: sb.containerID,
@@ -173,9 +168,7 @@ func (sb *Sandbox) storeDelete() error {
 }
 
 func (c *Controller) sandboxCleanup(activeSandboxes map[string]interface{}) error {
- store := c.getStore()
-
- sandboxStates, err := store.List(&sbState{c: c})
+ sandboxStates, err := c.store.List(&sbState{c: c})
  if err != nil {
  if err == datastore.ErrKeyNotFound {
  // It's normal for no sandboxes to be found. Just bail out.

libnetwork/store.go

@@ -10,29 +10,6 @@ import (
  "github.com/docker/docker/libnetwork/scope"
 )
 
-func (c *Controller) initStores() error {
- if c.cfg == nil {
- return nil
- }
- var err error
- c.store, err = datastore.New(c.cfg.Scope)
- if err != nil {
- return err
- }
-
- return nil
-}
-
-func (c *Controller) closeStores() {
- if store := c.store; store != nil {
- store.Close()
- }
-}
-
-func (c *Controller) getStore() *datastore.Store {
- return c.store
-}
-
 func (c *Controller) getNetworkFromStore(nid string) (*Network, error) {
  for _, n := range c.getNetworksFromStore(context.TODO()) {
  if n.id == nid {
@@ -45,9 +22,7 @@ func (c *Controller) getNetworkFromStore(nid string) (*Network, error) {
 func (c *Controller) getNetworks() ([]*Network, error) {
  var nl []*Network
 
- store := c.getStore()
-
- kvol, err := store.List(&Network{ctrlr: c})
+ kvol, err := c.store.List(&Network{ctrlr: c})
  if err != nil && err != datastore.ErrKeyNotFound {
  return nil, fmt.Errorf("failed to get networks: %w", err)
  }
@@ -57,7 +32,7 @@ func (c *Controller) getNetworks() ([]*Network, error) {
  n.ctrlr = c
 
  ec := &endpointCnt{n: n}
- err = store.GetObject(ec)
+ err = c.store.GetObject(ec)
  if err != nil && !n.inDelete {
  log.G(context.TODO()).Warnf("Could not find endpoint count key %s for network %s while listing: %v", datastore.Key(ec.Key()...), n.Name(), err)
  continue
@@ -76,16 +51,15 @@ func (c *Controller) getNetworks() ([]*Network, error) {
 func (c *Controller) getNetworksFromStore(ctx context.Context) []*Network { // FIXME: unify with c.getNetworks()
  var nl []*Network
 
- store := c.getStore()
- kvol, err := store.List(&Network{ctrlr: c})
+ kvol, err := c.store.List(&Network{ctrlr: c})
  if err != nil {
  if err != datastore.ErrKeyNotFound {
  log.G(ctx).Debugf("failed to get networks from store: %v", err)
  }
  return nil
  }
 
- kvep, err := store.Map(datastore.Key(epCntKeyPrefix), &endpointCnt{})
+ kvep, err := c.store.Map(datastore.Key(epCntKeyPrefix), &endpointCnt{})
  if err != nil && err != datastore.ErrKeyNotFound {
  log.G(ctx).Warnf("failed to get endpoint_count map from store: %v", err)
  }
@@ -112,9 +86,8 @@ func (c *Controller) getNetworksFromStore(ctx context.Context) []*Network { // F
 }
 
 func (n *Network) getEndpointFromStore(eid string) (*Endpoint, error) {
- store := n.ctrlr.getStore()
  ep := &Endpoint{id: eid, network: n}
- err := store.GetObject(ep)
+ err := n.ctrlr.store.GetObject(ep)
  if err != nil {
  return nil, fmt.Errorf("could not find endpoint %s: %w", eid, err)
  }
@@ -124,8 +97,7 @@ func (n *Network) getEndpointFromStore(eid string) (*Endpoint, error) {
 func (n *Network) getEndpointsFromStore() ([]*Endpoint, error) {
  var epl []*Endpoint
 
- store := n.getController().getStore()
- kvol, err := store.List(&Endpoint{network: n})
+ kvol, err := n.getController().store.List(&Endpoint{network: n})
  if err != nil {
  if err != datastore.ErrKeyNotFound {
  return nil, fmt.Errorf("failed to get endpoints for network %s: %w",
@@ -143,9 +115,7 @@ func (n *Network) getEndpointsFromStore() ([]*Endpoint, error) {
 }
 
 func (c *Controller) updateToStore(kvObject datastore.KVObject) error {
- cs := c.getStore()
-
- if err := cs.PutObjectAtomic(kvObject); err != nil {
+ if err := c.store.PutObjectAtomic(kvObject); err != nil {
  if err == datastore.ErrKeyModified {
  return err
  }
@@ -156,12 +126,10 @@ func (c *Controller) updateToStore(kvObject datastore.KVObject) error {
 }
 
 func (c *Controller) deleteFromStore(kvObject datastore.KVObject) error {
- cs := c.getStore()
-
 retry:
- if err := cs.DeleteObjectAtomic(kvObject); err != nil {
+ if err := c.store.DeleteObjectAtomic(kvObject); err != nil {
  if err == datastore.ErrKeyModified {
- if err := cs.GetObject(kvObject); err != nil {
+ if err := c.store.GetObject(kvObject); err != nil {
  return fmt.Errorf("could not update the kvobject to latest when trying to delete: %v", err)
  }
  log.G(context.TODO()).Warnf("Error (%v) deleting object %v, retrying....", err, kvObject.Key())

libnetwork/store_linux_test.go

@@ -41,7 +41,7 @@ func TestNoPersist(t *testing.T) {
  defer testController.Stop()
 
  nwKVObject := &Network{id: nw.ID()}
- err = testController.getStore().GetObject(nwKVObject)
+ err = testController.store.GetObject(nwKVObject)
  if !errors.Is(err, store.ErrKeyNotFound) {
  t.Errorf("Expected %q error when retrieving network from store, got: %q", store.ErrKeyNotFound, err)
  }
@@ -50,7 +50,7 @@ func TestNoPersist(t *testing.T) {
  }
 
  epKVObject := &Endpoint{network: nw, id: ep.ID()}
- err = testController.getStore().GetObject(epKVObject)
+ err = testController.store.GetObject(epKVObject)
  if !errors.Is(err, store.ErrKeyNotFound) {
  t.Errorf("Expected %v error when retrieving endpoint from store, got: %v", store.ErrKeyNotFound, err)
  }

libnetwork/store_test.go

@@ -37,7 +37,7 @@ func testLocalBackend(t *testing.T, provider, url string, storeConfig *store.Con
  }
 
  nwKVObject := &Network{id: nw.ID()}
- err = testController.getStore().GetObject(nwKVObject)
+ err = testController.store.GetObject(nwKVObject)
  if err != nil {
  t.Errorf("Error when retrieving network key from store: %v", err)
  }
@@ -46,7 +46,7 @@ func testLocalBackend(t *testing.T, provider, url string, storeConfig *store.Con
  }
 
  epKVObject := &Endpoint{network: nw, id: ep.ID()}
- err = testController.getStore().GetObject(epKVObject)
+ err = testController.store.GetObject(epKVObject)
  if err != nil {
  t.Errorf("Error when retrieving Endpoint key from store: %v", err)
  }