Skip to content

Commit

Permalink
libnet: Controller: drop getStore()
Browse files Browse the repository at this point in the history
This method does nothing more than `return c.store`. It has no value and
adds an unecessary level of indirection. Let's ditch it.

Signed-off-by: Albin Kerouanton <[email protected]>
  • Loading branch information
akerouanton committed May 10, 2024
1 parent 294d493 commit 9583539
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 39 deletions.
8 changes: 4 additions & 4 deletions libnetwork/endpoint_cnt.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,15 +105,15 @@ func (ec *endpointCnt) EndpointCnt() uint64 {
}

func (ec *endpointCnt) updateStore() error {
store := ec.n.getController().getStore()
c := ec.n.ctrlr
// 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()
Expand All @@ -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 {
Expand Down
11 changes: 2 additions & 9 deletions libnetwork/sandbox_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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.
Expand Down
31 changes: 9 additions & 22 deletions libnetwork/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,6 @@ import (
"github.com/docker/docker/libnetwork/scope"
)

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 {
Expand All @@ -26,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)
}
Expand All @@ -38,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
Expand All @@ -57,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)
}
Expand All @@ -93,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)
}
Expand All @@ -105,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.ctrlr.store.List(&Endpoint{network: n})
if err != nil {
if err != datastore.ErrKeyNotFound {
return nil, fmt.Errorf("failed to get endpoints for network %s: %w",
Expand All @@ -124,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
}
Expand All @@ -137,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())
Expand Down
4 changes: 2 additions & 2 deletions libnetwork/store_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions libnetwork/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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)
}
Expand Down

0 comments on commit 9583539

Please sign in to comment.