bmclib interface comparison
joelrebel opened this issue · 2 comments
About
This ticket is mainly a discussion around the newer and older bmclib interfaces
The newer interface methods are referred to as v2, while the older as v1.
For reference the #196 PR implements both the v1, v2 interfaces for Firmware
Outcome
The idea here is to figure if the following points make sense to work through ,
- Reduce the amount of boilerplate required for a v2 interface method to be added under the
bmc
directory - Could we have device probing - to identify the hardware model, type be part of the v2 interface
- Is theres a possibility to have some of the v1 methods available as part of the v2 interface (of if this is not a good idea at all)
What follows is a comparison of the two interfaces...
v2 vs v1
v2 - the newer bmclib interface
Pros
- The interfaces are smaller, and don't have to be implemented by all providers
- No need to type cast the bmc connection/client returned by the New() method
- Protocol fallthrough
- Feature declaration
- Driver registry
- Context propagation
Cons
- Driver registry and setup could be simpler
- Quite a bit of scafolding/boilerplate code along with test code needs to to be implemented - for example,
BMCVersion interface declaration, implementation
- Requires a Probing method to identify vendor/model
- The Compatibility method based on the
registrar.Verfier
interface could be hacked up to do this although it would be ideal to have one method to probe and identify hardware.
v1 - the older bmclib interface
Pros
- A single interface to implement
- Low cognitive overhead to add support for new devices/updating
- Capable of probing the device to identify vendor/model and returns the appropriate Bmc interface
Cons
- All providers need to be updated for a newer interface method or interface method signature change
- Lack of context propagation
- A new provider needs to implement all
Bmc
interface methods - even if its going to support just a single method
1. Reduce the amount of boilerplate required for a v2 interface method to be added under the bmc directory
I agree there is added boilerplate for each interface we add. For me, this is one-time work and just copy/paste too. The advantages we gain from these helper methods (single client interaction via client.go) far outway the one time need to add this code. Once all the interfaces have been added this won't need to be touched. Maybe when generics come out we revisit. We definitely need more documentation around implementing a provider with the new interfaces. That is sorely lacking.
2. Could we have device probing - to identify the hardware model, type be part of the v2 interface.
Device probing is acheived though client.Registry.FilterForCompatible
. Each provider implements a Compatible
function.
3. Is theres a possibility to have some of the v1 methods available as part of the v2 interface (of if this is not a good idea at all)
I don't see any reason by v1 methods couldn't be used in the implementation of v2 methods.
I'm not quite sure I follow the v1/v2 comparison you have. The new interfaces have been discussed for some time here. I definitely think we need more documentation around implementing a provider using the new interfaces. I, unfortunately, haven't had time to accomplish this yet. There is an issue open for that here.
Yep, I admit I did not entirely realize the effort that would be required for adding in the boilerplate bits,
For the probing I see we need to have a few providers
added that will provide ssh/redfish/generic https
drivers,
which can then fall through similar to the existing ScanAndConnect
code - do I have that right?
Now having implemented the newer interface methods and going through the proposal again,
I understand the approach better, the comparision was a result of stepping through implementing the v1/v2 interfaces, your proposal covers it better.
As for documenting some of this, I'll take a stab at it :)