[openstack-dev] [nova] nova hooks - document & test or deprecate?

Sean Dague sean at dague.net
Tue Mar 1 20:02:26 UTC 2016


On 03/01/2016 02:04 PM, Rob Crittenden wrote:
> 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.

The original implementation comes from a time when OpenStack was a kit
to build cloud software out of. And nearly every bit was extendable in
various ways.

We are now transitioning into a phase where interoperability is more
important than differentiation. That means deprecating out some
generalized hook points and requiring that features be baked in, so that
all clouds will have them, and user experience on different things that
are OpenStack (tm) are more similar.

At the end of the day this is python, so you can monkey patch anything
you want. But by documenting a hook point we endorse that. And the
sentiment in this thread is pretty much that we can't endorse this one
any more.

	-Sean

-- 
Sean Dague
http://dague.net



More information about the OpenStack-dev mailing list