Open Stack

On Jul 9, 2014, at 3:15 PM, Zane Bitter <zbitter at 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 at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev