0ct0pu5/moby
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Merge pull request #47208 from akerouanton/libnet-ds-remove-unused-key-params

Browse source 
libnet/ds: remove unused param `key` from `GetObject` and `List`
This commit is contained in:
Sebastiaan van Stijn 2024-01-25 11:46:20 +01:00 • committed by GitHub
commit d864df5a1d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 32 additions and 36 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
libnetwork/datastore/datastore.go
View file

@ -216,7 +216,7 @@ func (ds *Store) PutObjectAtomic(kvObject KVObject) error {
}
// GetObject gets data from the store and unmarshals to the specified object.
func (ds *Store) GetObject(key string, o KVObject) error {
func (ds *Store) GetObject(o KVObject) error {
ds.mu.Lock()
defer ds.mu.Unlock()
@ -236,7 +236,7 @@ func (ds *Store) ensureParent(parent string) error {
// List returns of a list of KVObjects belonging to the parent key. The caller
// must pass a KVObject of the same type as the objects that need to be listed.
func (ds *Store) List(key string, kvObject KVObject) ([]KVObject, error) {
func (ds *Store) List(kvObject KVObject) ([]KVObject, error) {
ds.mu.Lock()
defer ds.mu.Unlock()

6
libnetwork/datastore/datastore_test.go
View file

@ -40,7 +40,7 @@ func TestKVObjectFlatKey(t *testing.T) {
assert.Check(t, err)
n := dummyObject{ID: "1000"} // GetObject uses KVObject.Key() for cache lookup.
err = store.GetObject(Key(dummyKey, "1000"), &n)
err = store.GetObject(&n)
assert.Check(t, err)
assert.Check(t, is.Equal(n.Name, expected.Name))
}
@ -61,7 +61,7 @@ func TestAtomicKVObjectFlatKey(t *testing.T) {
// Get the latest index and try PutObjectAtomic again for the same Key
// This must succeed as well
n := dummyObject{ID: "1111"} // GetObject uses KVObject.Key() for cache lookup.
err = store.GetObject(Key(expected.Key()...), &n)
err = store.GetObject(&n)
assert.Check(t, err)
n.ReturnValue = true
err = store.PutObjectAtomic(&n)
@ -69,7 +69,7 @@ func TestAtomicKVObjectFlatKey(t *testing.T) {
// Get the Object using GetObject, then set again.
newObj := dummyObject{ID: "1111"} // GetObject uses KVObject.Key() for cache lookup.
err = store.GetObject(Key(expected.Key()...), &newObj)
err = store.GetObject(&newObj)
assert.Check(t, err)
assert.Check(t, newObj.Exists())
err = store.PutObjectAtomic(&n)

6
libnetwork/drivers/bridge/bridge_store.go
View file

@ -50,7 +50,7 @@ func (d *driver) initStore(option map[string]interface{}) error {
}
func (d *driver) populateNetworks() error {
kvol, err := d.store.List(datastore.Key(bridgePrefix), &networkConfiguration{})
kvol, err := d.store.List(&networkConfiguration{})
if err != nil && err != datastore.ErrKeyNotFound {
return fmt.Errorf("failed to get bridge network configurations from store: %v", err)
}
@ -72,7 +72,7 @@ func (d *driver) populateNetworks() error {
}
func (d *driver) populateEndpoints() error {
kvol, err := d.store.List(datastore.Key(bridgeEndpointPrefix), &bridgeEndpoint{})
kvol, err := d.store.List(&bridgeEndpoint{})
if err != nil && err != datastore.ErrKeyNotFound {
return fmt.Errorf("failed to get bridge endpoints from store: %v", err)
}
@ -122,7 +122,7 @@ func (d *driver) storeDelete(kvObject datastore.KVObject) error {
retry:
if err := d.store.DeleteObjectAtomic(kvObject); err != nil {
if err == datastore.ErrKeyModified {
if err := d.store.GetObject(datastore.Key(kvObject.Key()...), kvObject); err != nil {
if err := d.store.GetObject(kvObject); err != nil {
return fmt.Errorf("could not update the kvobject to latest when trying to delete: %v", err)
}
goto retry

6
libnetwork/drivers/ipvlan/ipvlan_store.go
View file

@ -69,7 +69,7 @@ func (d *driver) initStore(option map[string]interface{}) error {
// populateNetworks is invoked at driver init to recreate persistently stored networks
func (d *driver) populateNetworks() error {
kvol, err := d.store.List(datastore.Key(ipvlanNetworkPrefix), &configuration{})
kvol, err := d.store.List(&configuration{})
if err != nil && err != datastore.ErrKeyNotFound {
return fmt.Errorf("failed to get ipvlan network configurations from store: %v", err)
}
@ -88,7 +88,7 @@ func (d *driver) populateNetworks() error {
}
func (d *driver) populateEndpoints() error {
kvol, err := d.store.List(datastore.Key(ipvlanEndpointPrefix), &endpoint{})
kvol, err := d.store.List(&endpoint{})
if err != nil && err != datastore.ErrKeyNotFound {
return fmt.Errorf("failed to get ipvlan endpoints from store: %v", err)
}
@ -137,7 +137,7 @@ func (d *driver) storeDelete(kvObject datastore.KVObject) error {
retry:
if err := d.store.DeleteObjectAtomic(kvObject); err != nil {
if err == datastore.ErrKeyModified {
if err := d.store.GetObject(datastore.Key(kvObject.Key()...), kvObject); err != nil {
if err := d.store.GetObject(kvObject); err != nil {
return fmt.Errorf("could not update the kvobject to latest when trying to delete: %v", err)
}
goto retry

6
libnetwork/drivers/macvlan/macvlan_store.go
View file

@ -68,7 +68,7 @@ func (d *driver) initStore(option map[string]interface{}) error {
// populateNetworks is invoked at driver init to recreate persistently stored networks
func (d *driver) populateNetworks() error {
kvol, err := d.store.List(datastore.Key(macvlanPrefix), &configuration{})
kvol, err := d.store.List(&configuration{})
if err != nil && err != datastore.ErrKeyNotFound {
return fmt.Errorf("failed to get macvlan network configurations from store: %v", err)
}
@ -87,7 +87,7 @@ func (d *driver) populateNetworks() error {
}
func (d *driver) populateEndpoints() error {
kvol, err := d.store.List(datastore.Key(macvlanEndpointPrefix), &endpoint{})
kvol, err := d.store.List(&endpoint{})
if err != nil && err != datastore.ErrKeyNotFound {
return fmt.Errorf("failed to get macvlan endpoints from store: %v", err)
}
@ -136,7 +136,7 @@ func (d *driver) storeDelete(kvObject datastore.KVObject) error {
retry:
if err := d.store.DeleteObjectAtomic(kvObject); err != nil {
if err == datastore.ErrKeyModified {
if err := d.store.GetObject(datastore.Key(kvObject.Key()...), kvObject); err != nil {
if err := d.store.GetObject(kvObject); err != nil {
return fmt.Errorf("could not update the kvobject to latest when trying to delete: %v", err)
}
goto retry

6
libnetwork/drivers/windows/windows_store.go
View file

@ -47,7 +47,7 @@ func (d *driver) initStore(option map[string]interface{}) error {
}
func (d *driver) populateNetworks() error {
kvol, err := d.store.List(datastore.Key(windowsPrefix), &networkConfiguration{Type: d.name})
kvol, err := d.store.List(&networkConfiguration{Type: d.name})
if err != nil && err != datastore.ErrKeyNotFound {
return fmt.Errorf("failed to get windows network configurations from store: %v", err)
}
@ -70,7 +70,7 @@ func (d *driver) populateNetworks() error {
}
func (d *driver) populateEndpoints() error {
kvol, err := d.store.List(datastore.Key(windowsEndpointPrefix), &hnsEndpoint{Type: d.name})
kvol, err := d.store.List(&hnsEndpoint{Type: d.name})
if err != nil && err != datastore.ErrKeyNotFound {
return fmt.Errorf("failed to get endpoints from store: %v", err)
}
@ -122,7 +122,7 @@ func (d *driver) storeDelete(kvObject datastore.KVObject) error {
retry:
if err := d.store.DeleteObjectAtomic(kvObject); err != nil {
if err == datastore.ErrKeyModified {
if err := d.store.GetObject(datastore.Key(kvObject.Key()...), kvObject); err != nil {
if err := d.store.GetObject(kvObject); err != nil {
return fmt.Errorf("could not update the kvobject to latest when trying to delete: %v", err)
}
goto retry

6
libnetwork/endpoint_cnt.go
View file

@ -116,7 +116,7 @@ func (ec *endpointCnt) updateStore() error {
if err := ec.n.getController().updateToStore(ec); err == nil || err != datastore.ErrKeyModified {
return err
}
if err := store.GetObject(datastore.Key(ec.Key()...), ec); err != nil {
if err := store.GetObject(ec); err != nil {
return fmt.Errorf("could not update the kvobject to latest on endpoint count update: %v", err)
}
ec.Lock()
@ -140,7 +140,7 @@ func (ec *endpointCnt) atomicIncDecEpCnt(inc bool) error {
}
tmp := &endpointCnt{n: ec.n}
if err := store.GetObject(datastore.Key(ec.Key()...), tmp); err != nil {
if err := store.GetObject(tmp); err != nil {
return err
}
retry:
@ -156,7 +156,7 @@ retry:
if err := ec.n.getController().updateToStore(ec); err != nil {
if err == datastore.ErrKeyModified {
if err := store.GetObject(datastore.Key(ec.Key()...), ec); err != nil {
if err := store.GetObject(ec); err != nil {
return fmt.Errorf("could not update the kvobject to latest when trying to atomic add endpoint count: %v", err)
}

2
libnetwork/sandbox_store.go
View file

@ -174,7 +174,7 @@ func (c *Controller) sandboxCleanup(activeSandboxes map[string]interface{}) erro
return fmt.Errorf("could not find local scope store")
}
sandboxStates, err := store.List(datastore.Key(sandboxPrefix), &sbState{c: c})
sandboxStates, err := store.List(&sbState{c: c})
if err != nil {
if err == datastore.ErrKeyNotFound {
// It's normal for no sandboxes to be found. Just bail out.

14
libnetwork/store.go
View file

@ -50,8 +50,7 @@ func (c *Controller) getNetworks() ([]*Network, error) {
return nil, nil
}
kvol, err := store.List(datastore.Key(datastore.NetworkKeyPrefix),
&Network{ctrlr: c})
kvol, err := store.List(&Network{ctrlr: c})
if err != nil && err != datastore.ErrKeyNotFound {
return nil, fmt.Errorf("failed to get networks: %w", err)
}
@ -61,7 +60,7 @@ func (c *Controller) getNetworks() ([]*Network, error) {
n.ctrlr = c
ec := &endpointCnt{n: n}
err = store.GetObject(datastore.Key(ec.Key()...), ec)
err = 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
@ -81,7 +80,7 @@ func (c *Controller) getNetworksFromStore(ctx context.Context) []*Network { // F
var nl []*Network
store := c.getStore()
kvol, err := store.List(datastore.Key(datastore.NetworkKeyPrefix), &Network{ctrlr: c})
kvol, err := store.List(&Network{ctrlr: c})
if err != nil {
if err != datastore.ErrKeyNotFound {
log.G(ctx).Debugf("failed to get networks from store: %v", err)
@ -118,7 +117,7 @@ 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(datastore.Key(ep.Key()...), ep)
err := store.GetObject(ep)
if err != nil {
return nil, fmt.Errorf("could not find endpoint %s: %w", eid, err)
}
@ -128,9 +127,8 @@ func (n *Network) getEndpointFromStore(eid string) (*Endpoint, error) {
func (n *Network) getEndpointsFromStore() ([]*Endpoint, error) {
var epl []*Endpoint
tmp := Endpoint{network: n}
store := n.getController().getStore()
kvol, err := store.List(datastore.Key(tmp.KeyPrefix()...), &Endpoint{network: n})
kvol, err := store.List(&Endpoint{network: n})
if err != nil {
if err != datastore.ErrKeyNotFound {
return nil, fmt.Errorf("failed to get endpoints for network %s: %w",
@ -172,7 +170,7 @@ func (c *Controller) deleteFromStore(kvObject datastore.KVObject) error {
retry:
if err := cs.DeleteObjectAtomic(kvObject); err != nil {
if err == datastore.ErrKeyModified {
if err := cs.GetObject(datastore.Key(kvObject.Key()...), kvObject); err != nil {
if err := cs.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())

5
libnetwork/store_linux_test.go
View file

@ -50,9 +50,8 @@ func TestNoPersist(t *testing.T) {
}
defer testController.Stop()
// FIXME(thaJeztah): GetObject uses the given key for lookups if no cache-store is present, but the KvObject's Key() to look up in cache....
nwKVObject := &Network{id: nw.ID()}
err = testController.getStore().GetObject(datastore.Key(datastore.NetworkKeyPrefix, nw.ID()), nwKVObject)
err = testController.getStore().GetObject(nwKVObject)
if !errors.Is(err, store.ErrKeyNotFound) {
t.Errorf("Expected %q error when retrieving network from store, got: %q", store.ErrKeyNotFound, err)
}
@ -61,7 +60,7 @@ func TestNoPersist(t *testing.T) {
}
epKVObject := &Endpoint{network: nw, id: ep.ID()}
err = testController.getStore().GetObject(datastore.Key(datastore.EndpointKeyPrefix, nw.ID(), ep.ID()), epKVObject)
err = testController.getStore().GetObject(epKVObject)
if !errors.Is(err, store.ErrKeyNotFound) {
t.Errorf("Expected %v error when retrieving endpoint from store, got: %v", store.ErrKeyNotFound, err)
}

7
libnetwork/store_test.go
View file

@ -6,7 +6,6 @@ import (
"testing"
"github.com/docker/docker/libnetwork/config"
"github.com/docker/docker/libnetwork/datastore"
store "github.com/docker/docker/libnetwork/internal/kvstore"
"github.com/docker/docker/libnetwork/netlabel"
"github.com/docker/docker/libnetwork/options"
@ -36,9 +35,9 @@ func testLocalBackend(t *testing.T, provider, url string, storeConfig *store.Con
if err != nil {
t.Fatalf("Error creating endpoint: %v", err)
}
// FIXME(thaJeztah): GetObject uses the given key for lookups if no cache-store is present, but the KvObject's Key() to look up in cache....
nwKVObject := &Network{id: nw.ID()}
err = testController.getStore().GetObject(datastore.Key(datastore.NetworkKeyPrefix, nw.ID()), nwKVObject)
err = testController.getStore().GetObject(nwKVObject)
if err != nil {
t.Errorf("Error when retrieving network key from store: %v", err)
}
@ -47,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(datastore.Key(datastore.EndpointKeyPrefix, nw.ID(), ep.ID()), epKVObject)
err = testController.getStore().GetObject(epKVObject)
if err != nil {
t.Errorf("Error when retrieving Endpoint key from store: %v", err)
}