On Jul 9, 2014, at 3:15 PM, Zane Bitter <zbit...@redhat.com> wrote: > On 08/07/14 17:13, Angus Salkeld wrote: >> -----BEGIN PGP SIGNED MESSAGE----- >> Hash: SHA1 >> >> On 08/07/14 09:14, Zane Bitter wrote: >>> I see that the new client plugins are loaded using stevedore, which is >>> great and IMO absolutely the right tool for that job. Thanks to Angus & >>> Steve B for implementing it. >>> >>> Now that we have done that work, I think there are more places we can >>> take advantage of it too - for example, we currently have competing >>> native wait condition resource types being implemented by Jason[1] and >>> Steve H[2] respectively, and IMHO that is a mistake. We should have >>> *one* native wait condition resource type, one AWS-compatible one, >>> software deployments and any custom plugin that require signalling; and >>> they should all use a common SignalResponder implementation that would >>> call an API that is pluggable using stevedore. (In summary, what we're >> >> what's wrong with using the environment for that? Just have two resources >> and you do something like this: >> https://github.com/openstack/heat/blob/master/etc/heat/environment.d/default.yaml#L7 > > It doesn't cover other things that need signals, like software deployments > (third-party plugin authors are also on their own). We only want n > implementations not n*(number of resources that use signals) implementations. > >>> trying to make configurable is an implementation that should be >>> invisible to the user, not an interface that is visible to the user, and >>> therefore the correct unit of abstraction is an API, not a resource.) >>> >> >> Totally depends if we want this to be operator configurable (config file or >> plugin) >> or end user configurable (use their environment to choose the >> implementation). >> >>> >>> I just noticed, however, that there is an already-partially-implemented >>> blueprint[3] and further pending patches[4] to use stevedore for *all* >>> types of plugins - particularly resource plugins[5] - in Heat. I feel >>> very strongly that stevedore is _not_ a good fit for all of those use >>> cases. (Disclaimer: obviously I _would_ think that, since I implemented >>> the current system instead of using stevedore for precisely that reason.) >> >> haha. >> >>> >>> The stated benefit of switching to stevedore is that it solves issues >>> like https://launchpad.net/bugs/1292655 that are caused by the current >>> convoluted layout of /contrib. I think the layout stems at least in part >> >> I think another great reason is consistency with how all other plugins are >> openstack >> are written (stevedore). > > Sure, consistency is nice, sometimes even at the expense of being not quite > the right tool for the job. But there are limits to that trade-off. > >> Also I *really* don't think we should optimize for our contrib plugins >> but for: >> 1) our built in plugins >> 2) out of tree plugins > > I completely agree, which is why I was surprised by this change. It seems to > be deprecating a system that is working well for built-in and out-of-tree > plugins in order to make minor improvements to how we handle contrib.
FWIW, when it comes to deploying Heat with non-built-in, there's no substantive difference in the experience between contrib and out-of-tree plugins, so neither system is more or less optimized for either. However, with the current system, there's no "easy" way to get rid of the "built-in" ones you don't want. > >>> from a misunderstanding of how the current plugin_manager works. The >>> point of the plugin_manager is that each plugin directory does *not* >>> have to be a Python package - it can be any directory. Modules in the >>> directory then appear in the package heat.engine.plugins once imported. >>> So there is no need to do what we are currently doing, creating a >>> resources package, and then a parent package that contains the tests >>> package as well, and then in the tests doing: >>> >>> from ..resources import docker_container ## noqa >>> >>> All we really need to do is throw the resources in any old directory, >>> add that directory to the plugin_dirs list, stick the tests in any old >>> package, and from the tests do >>> >>> from heat.engine.plugins import docker_container >>> >>> The main reason we haven't done this seems to be to avoid having to list >>> the various contrib plugin dirs separately. Stevedore "solves" this by >>> forcing us to list not only each directory but each class in each module >>> in each directory separately. The tricky part of fixing the current >>> layout is ensuring the contrib plugin directories get added to the >>> plugin_dirs list during the unit tests and only during the unit tests. >>> However, I'm confident that could be fixed with no more difficulty than >>> the stevedore changes and with far less disruption to existing operators >>> using custom plugins. >>> >>> Stevedore is ideal for configuring an implementation for a small number >>> of well known plug points. It does not appear to be ideal for managing >>> an application like Heat that comprises a vast collection of >>> implementations of the same interface, each bound to its own plug point. >>> >> I wouldn't call our resources "vast". > > I count 73 in your patch, not including contrib and assuming you didn't miss > any ;). It's seems clear to me that we're well past the point of what the > Extensions API was designed for. When everything is an extension you need > different tools to manage it. Quantity has a quality all it's own ;) > >> Really, I think it works great. > > As discussed on IRC yesterday, we could potentially make the "plugin" the > existing resource_mapping() functions (or equivalent - stevedore seems to > require it to be a class, not a module, for some mysterious reason?), and > load them using the Hooks rather than the Extensions API in stevedore. That > could give us the best of both worlds - getting rid of our custom import code > in favour of a library, while keeping the definitions of resource names close > to the code. > > It's still not entirely clear to me what the benefit is to users to offset > the cost of deprecating the current system and forcing them to eventually > migrate, but if there was a consensus around this approach I would support > it. I maintain that the Extensions API doesn't seem like a great fit. > >>> For example, there's a subtle difference in how plugin_manager loads >>> external modules - by searching a list of plugin directories for Python >>> modules - and how stevedore does it, by loading a specified module >>> already in the Python path. The latter is great for selecting one of a >>> number of implementations that already exist in the code, but not so >>> great for dropping in an additional external module, which now needs to >>> be wrapped in a package that has to be installed in the path *and* >>> there's still a configuration file to edit. This is way harder for a >>> packager and/or operator to set up. >> >> I think you have this the wrong way around. >> With stevedore you don't need to edit a config file and with pluginmanager >> you do if that dir isn't in the list already. >> >> stevedore relies on namespaces, so you add your plugins into the >> "heat.resources" >> namespace and then heat will load them (no editing of config files). >> You do *not* need to edit heat/setup.cfg to add an out of tree resource, >> you edit your own project's setup.cfg. >> >> I really don't think adding setup.cfg/py is a big issue for someone that >> is exposing new functionality for their endusers. It's not like I am >> going to knock up a random python file toss it into a dir and forget >> about it. More likely someone will make a little git repo with their >> plugin and add some unit tests and have a way of updating the final >> deployment (at a minimum a script that downloads the current version >> and installs it). > > OK, I see the disconnect here. I was thinking only about OS-level packaging, > but you are talking about Python packaging. Probably my bias is showing here > because I am really not a fan of Python packaging, but that isn't a good > reason to pick one approach over the other ;) May be. Seems to me, one wouldn't necessarily prefer one over the other in terms of actual deployment. FWIW, we use python packaging over distro-specific packages when we deploy Heat. Its a little more complex than that, of course, but in general its the case. > >>> This approach actually precludes a number of things we know we want to >>> do in the future - for example it would be great if the native and AWS >>> resource plugins were distributed as separate subpackages so that "yum >>> install heat-engine" installed only the native resources, and a separate >>> "yum install heat-cfn-plugins" added the AWS-compatibility resources. >>> You can't (safely) package things that way if the installation would >>> involve editing a config file. >> >> >> I'd suggest we go further than this: >> seperate the "engine" code from the resources and make seperate repos >> - - heat >> - - heat-resources-native >> - - heat-resources-aws >> - - heat-resources-<contrib> >> >> Then the repo matches the packaging more naturally, and stevedore handles >> this well too. >> >> If you *really* don't like this ^ > > I am very -2 on this, as I discussed with Steve Baker last time it came up. > It would make my work an order of magnitude harder. But it would make my work as a deployer of Heat with custom plugins and other built-ins turned off an order of magnitude easier. > If we could make them separate Python packages within a single Git repo, I > would be +2 on that. I don't know if that's feasible with our current tooling > (though I guess it's not dissimilar to what you're doing with the contrib > stuff in this patch series?). > >> We can default to "resources are loaded but not enabled by default" >> the each distro package can drop a file into /etc/environment.d/ that >> enables it's resources. > > Sounds like a recipe for distro bugs. As someone who deploys Heat on a regular basis and turning different plug-ins on and off, this has a lot of appeal to me. > >>> One of the main design goals of the current resource plugins was to move >>> the mapping from resource names to classes away from one central point >>> (where all of the modules were imported) and place the configuration >>> alongside the code it applies to. I am definitely not looking forward to >>> having to go look up a config file to find out what each resource is >>> every time I open the autoscaling module (and I do need to remind myself >>> _every_ time I open it), to say nothing of the constant merge conflicts >>> that we used to have to deal with when there was a central registry. >> >> They are grouped by name, so you will only run into an issue when someone >> else is working on the same area as you. >> >>> >>> A central registry is also problematic for operators that modify it, who >>> will have a difficult, manual and potentially error-prone merge task to >>> perform on the config file every time they upgrade. >> >> I don't see why an operator will be editing this, they should be using >> the environment to disable plugins/rename things. You don't have to >> touch this if you are adding your own plugin. >> >>> >>> Constraints, I feel, are very similar to resources in this respect. I am >>> less concerned about template formats, since there are so few of them... >>> although it would be really nice to be able to install these as >>> subpackages too, and using stevedore appears to eliminate that as an >>> option :( >>> >>> Intrinsic functions are a different story. I'm equally opposed to >>> defining them in a config file instead of near the code that implements >>> them, but I now think I probably made a mistake in making them pluggable >>> at all. (The idea to make functions pluggable pre-dated the idea of >>> making template formats pluggable.) The ability to plug in functions to >>> existing template formats is an invitation for operators to do so, and >>> that is a recipe for a lot of incompatible templates being released into >>> the world with the same version string. We should probably have each >>> template format return a hard-coded map of intrinsic functions, and >>> allow operators to create their own subclass to return a different set, >>> and encourage them to register said subclass with a different version >>> string (although we couldn't stop them from overriding the built-in >>> implementation if they really wanted). >>> >>> >>> Summarising, my view of the right tool for the job at each of the >>> various plugin interfaces: >>> >>> Client plugins stevedore >> >> +1 >> >>> Signals stevedore >> >> environment ^ >> >>> Resources plugin_manager >>> Constraints plugin_manager >>> Template formats plugin_manager or maybe stevedore >> >> stevedore ^ :-) >> >>> Intrinsic functions neither (should be bound to template format) >> >> I agree the intrinsic's partly define the template version, so should be >> more tightly bound. > > OK, I think there's consensus on the intrinsics at least :) Let's go ahead > and make that change. Agreed. Intrinsics are part of the template format and should "come with". > > cheers, > Zane. > > _______________________________________________ > OpenStack-dev mailing list > OpenStack-dev@lists.openstack.org > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev _______________________________________________ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
