Read Api cleanup

[mapuri]

This commit brings in following broad changes:
- Remove ReadRecursive() API in core, since it assumes a structure to the state organization,
which might not be true for all key-value data stores
- Added ReadAll() api instead to EtcdStateDriver for now, that returns the values
(bytes arrays) contained under a base-key
- Added specific ReadAll<>() functions to the various states that need us to read this today.
Again, haven't added it to core.State since it might not be supported for all kinds of states.
But we can revisit this, if more use cases arise.
- Modified gstate.Cfg and gstate.Oper to adhere to the core.State API
- With the above changes there are some possible enhancements/optimizations
that can be made to prevent extra read since we already have read the state.
But that can be addressed as separate commits.

[jainvipin]

Instead of returning the values, could we return the keys? Returning values may be a lot of data.

[mapuri]

@jainvipin

Did you mean return keys of the form /contiv//id? The issue with returning keys is that it makes higher level interfaces like Driver, NetMaster etc having to know about the key structure, when that information should actually be hidden in core.State and they should not be making direct calls to core.StateDriver.

Or did you mean return just the 'Ids' (i.e. endpoint-id, network-id etc? That should be possible. But there are two considerations, one that even to get Ids we need to read all the data once. And then usually we end up doing another read to fetch the data again from Id. That was my last bullet, that we can actually avoid the extra read by reusing the read data. The length of array returned by the ReadAll<>() APIs should not be much as we pass around array of pointers.

[jainvipin]

@mapuri - I meant just the ids. Not the hierarchy, because that violates the abstraction.
I am not sure if you necessarily have to read the data to get to the keys. Then on other hand, if all you need is the keys, then reading all data may be un-necessary. Fetching keys from data may be an extra step in those cases.

[mapuri]

I am not sure if you necessarily have to read the data to get to the keys.

I understand but what I meant earlier was that etcd doesn't provides a key-only read (but will check again), it does a complete read of values and keys. So we have to extract keys from the response and drop values. So, even if we have to just return Ids we will be getting the data either ways. Also our usecases today for these API seem to require a subsequent read for the data from the Id, so I just kept one API that returns values.

[mapuri]

@jainvipin, Just a heads up. I merged this PR. You might see conflicts because of this with your outstanding PR, let me know if you see any issues.

[jainvipin]

@mapuri: As I was merging these changes, I think there is some gotcha here. With the changes, now I can't call:
ReadAllOvsCfgNetworks() to any stateDriver, because it must be qualified with 'drivers' because it is not part of the interface drivers.ReadAllOvsCfgNetworks()

Subsequently unit tests are failing because it is attempting to do state conversion from fakeStateDriver to etcdStateDriver, which crashes the system.

I do believe that keeping a ReadAll API in stateDriver interface is the best way to go, and then let rest of the code loop through the keys. This will keep one API in the state driver but not have functions outside the interface being exposed, like it is now.

Your thoughts on how to resolve this?

[mapuri]

I see. I didn't make ReadAll() API to be part of core.StateDriver and core.State interface as I wasn't sure if it made sense for all StateDrivers and State, because this API still needs a baseKey to be passed which assumes that all StateDrivers will have some sort of tree hierarchy associated with their keys and all the state will belong to just that (sub-)tree. Example, objectstore in iNXOS allows objects of same type to be contained under different parent-types thereby allowing them to be in different sub-trees. In that case a ReadAll() needs to happen on 'class' of the object, instead of a fixed sub-tree.

But I don't have a good solution. For now may be we can add ReadAll() to core.StateDriver until we hit a driver with different requirement.

[jainvipin]

I think it makes sense to keep ReadAll to be part of the interface. Because there will be function that requires walk on the table/object, therefore instead of assuming any hierarchy, all you are exposing is that you are interested in reading all instances of an object. How are they organized is up to the state i.e. hierarchical, DB, tables, or in a flat maps (like fakeStateDriver, today).

I will make the change and send for the review in my other PR.