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. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| __________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
