Open Stack

Daniel P. Berrange wrote:
> On Mon, Feb 29, 2016 at 11:59:06AM -0500, Sean Dague wrote:
>> The nova/hooks.py infrastructure has been with us since early Nova. It's
>> currently only annotated on a few locations - 'build_instance',
>> 'create_instance', 'delete_instance', and 'instance_network_info'. It's
>> got a couple of unit tests on it, but nothing that actually tests real
>> behavior of the hooks we have specified.
>>
>> It does get used in the wild, and we do break it with changes we didn't
>> ever anticipate would impact it -
>> https://bugs.launchpad.net/nova/+bug/1518321
>>
>> However, when you look into how that is used, it's really really odd and
>> fragile -
>> https://github.com/richm/rdo-vm-factory/blob/master/rdo-ipa-nova/novahooks.py#L248
>>
>>
>>     def pre(self, *args, **kwargs):
>>         # args[7] is the injected_files parameter array
>>         # the value is ('filename', 'base64 encoded contents')
>>         ipaotp = str(uuid.uuid4())
>>         ipainject = ('/tmp/ipaotp', base64.b64encode(ipaotp))
>>         args[7].extend(self.inject_files)
>>         args[7].append(ipainject)
>>
>> In our continued quest on being more explicit about plug points it feels
>> like we should other document the interface (which means creating
>> stability on the hook parameters) or we should deprecate this construct
>> as part of a bygone era.
>>
>> I lean on deprecation because it feels like a thing we don't really want
>> to support going forward, but I can go either way.
> 
> As it is designed, I think the hooks mechanism is really unsupportable
> long term. It is exposing users to arbitrary internal Nova data structures
> which we have changed at will and we cannot possibly ever consider them
> to be a stable consumable API. I'm rather surprised we've not seen more
> bugs like the one you've shown above - most likely thats a reflection
> on not many people actually using this facility.
> 
> I'd be strongly in favour of deprecation & removal of this hooking
> mechanism, as its unsupportable in any sane manner when it exposes
> code to our internal unstable API & object model.
> 
> If there's stuff people are doing in hooks that's impossible any other
> way, we should really be looking at what we need to provide in our
> public API to achieve the same goal, if it is use case we wish to be
> able to support.

I'll just add that lots and lots of software has hooks. Just because the
initial implementation decided to expose internal structures (which lots
of other APIs do in order to be interesting) doesn't make it inherently
bad. It just requires a stable API rather than passing *args, **kwargs
around. It seems like the original design was to throw in the kitchen
sink when a more targeted set of arguments would probably do just fine.

rob