On 07/23/2014 03:04 PM, Dan Smith wrote:
FWIW, I do actually agree with not exposing plugin points to things
that are not stable APIs and if they didn't already exist, I'd not
approve adding them. I'd actually go further and say not even the
virt driver API should be a plugin point, since we arbitrarily change
it during development any time we need to. The latter is not a serious
or practical view right now though given our out of tree Docker/Ironic
drivers. I'm just concerned that we've had these various extension
points exposed for a long time and we've not clearly articulated
that they are liable to be killed off (besides marking vif_driver
as deprecated)
Yep, I think we agree. I think that as a project we've identified
exposing plug points that aren't stable (or intended to be replaceable)
as a bad thing, and thus we should be iterating on removing them.
Especially if we're generous with our deprecate-before-remove rules,
then I think that we're not likely to bite anyone suddenly with
something they're shipping while working it upstream in parallel. I
*really* thought we had called this one out on the ReleaseNotes, but
apparently that didn't happen (probably because we decide to throw in
those helper classes to avoid breaking configs). Going forward, marking
it deprecated in the code for a cycle, noting it on the release notes,
and then removing it the next cycle seems like plenty of warning.
The following are "plugin" points that I feel should be scrapped (sorry,
I mean deprecated over a release cycle), as they really are not things
that anyone actually provides extensions for and, IMO, they just add
needless code abstraction, noise and indirection:
All of these are pointless:
* metadata_manager=nova.api.manager.MetadataManager
* compute_manager=nova.compute.manager.ComputeManager
* console_manager=nova.console.manager.ConsoleProxyManager
* consoleauth_manager=nova.consoleauth.manager.ConsoleAuthManager
* cert_manager=nova.cert.manager.CertManager
* scheduler_manager=nova.scheduler.manager.SchedulerManager
* db_driver=nova.db (pretty sure that ship has long since sailed)
* network_api_class=nova.network.api.API
* volume_api_class=nova.volume.cinder.API
* manager=nova.cells.manager.CellsManager
* manager=nova.conductor.manager.ConductorManager
Then there are the funnies:
This should not be a manager class at all, but rather a selector that
switches the behaviour of the underlying network implementation -- i.e.
it should not be swapped out by custom code but instead just have a
switch option to indicate the type of network model in use):
* network_manager=nova.network.manager.VlanManager
Same goes for this one, which should just be selected based on the
network model:
* l3_lib=nova.network.l3.LinuxNetL3
These ones should similarly be selected based on the binding_type, not
provided as a plugin point (as Ian Wells alluded to):
* vif_driver=nova.virt.libvirt.vif.LibvirtGenericVIFDriver
* vif_driver=nova.virt.xenapi.vif.XenAPIBridgeDriver
These config options should be renamed to use driver, not manager:
* floating_ip_dns_manager=nova.network.noop_dns_driver.NoopDNSDriver
* instance_dns_manager=nova.network.noop_dns_driver.NoopDNSDriver
* scheduler_host_manager=nova.scheduler.host_manager.HostManager
* power_manager=nova.virt.baremetal.ipmi.IPMI
This config option should be renamed to use driver, not api_class:
* api_class=nova.keymgr.conf_key_mgr.ConfKeyManager
This one should be renamed to use driver, not handler:
* image_upload_handler=nova.virt.xenapi.image.glance.GlanceStore
This one... who knows? There are no other schedulers for the cells
module other than this one, and it doesn't follow the same manager ->
driver pattern as most of Nova, so, should it be called scheduler_driver
or just scrapped?:
* scheduler=nova.cells.scheduler.CellsScheduler
This one isn't properly set up as a driver-based system but actually
implements an API, which you'd have to then subclass identically and
there would be zero point in doing that since you would need to return
the same data as is set in the Stats class' methods:
* compute_stats_class=nova.compute.stats.Stats
I think it's pretty clear there's lots of room for consistency and
improvements.
All the best,
-jay
_______________________________________________
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
