Manageiq: [Providers] Pluggability Checklist

[agrare]

This is an ongoing list of items which need to be extracted out of core repos (manageiq, manageiq-api, manageiq-ui) and into plugins.

A continuation of https://github.com/ManageIQ/manageiq/issues/14840

Specs

• [ ] Provider subclasses in factories - e.g. vm_amazon, vm_vmware in https://github.com/ManageIQ/manageiq/blob/master/spec/factories/vm_or_template.rb
  These factories should be moved to their respective plugins and specs in core should be updated to not reference provider factories
  
  
  
  • [ ] [vm_or_template](https://github.com/ManageIQ/manageiq/blob/master/spec/factories/vm_or_template.rb)
  
  
  • [ ] [host](https://github.com/ManageIQ/manageiq/blob/master/spec/factories/host.rb)
  
  
• [ ] Don't use provider classes in core specs (create a dummy provider to workaround ExtManagementSystem leaf class issue)

Providers

• [x] Remove the VMwareWebService gem from core (https://github.com/ManageIQ/manageiq/issues/18428)
  
  
  
  • [x] https://github.com/ManageIQ/manageiq/pull/19430 - Remove ems_ref_obj which was serialized YAML of VMwareWebService/VimStrings
  
  
  • [x] https://github.com/ManageIQ/manageiq/pull/19551 - Remove VMwareWebService/MiqVimVm from Snapshot and MiqAction
  
  
  • [x] Remove VixDiskLib logic from ServerSmartProxy
  
  
• [x] https://github.com/ManageIQ/manageiq/pull/19534 - EmsFolder subclasses - folders should be subclassed by their provider (do any providers besides vmware even need folders?)
• [x] https://github.com/ManageIQ/manageiq-schema/pull/443 - Storage subclasses (RHV has IsoDatastore, can that just be a subclass of Storage?)
• [x] https://github.com/ManageIQ/manageiq/pull/19529 - ResourcePool subclasses
• [ ] Get any VMware specific logic around folder structure (e.g. Datacenters, hidden folders, vms and templates vs hosts and clusters view) out of the UI
• [x] https://github.com/ManageIQ/manageiq/pull/19452 - VmOrTemplate run_command_via_parent - the raw_* methods for most of the VM Operations call run_command_via_parent and execute a method on the ExtManagementSystem. This pattern was initially for VMware where the connection logic was in the InfraManager and was copied to a lot of other providers. The raw_* methods in core should raise NotImplementedError and each provider should define how the methods are run.
• [ ] Extract host operations to vmware repo and raise NotImplementedError in core methods (E.g. https://github.com/ManageIQ/manageiq/blob/master/app/models/host.rb#L424-L429)
• [x] https://github.com/ManageIQ/manageiq/issues/19469 - VmScan needs to be split up (E.g. https://github.com/ManageIQ/manageiq/blob/master/app/models/vm_scan.rb#L103-L130 and https://github.com/ManageIQ/manageiq/blob/master/app/models/vm_scan.rb#L246-L253)
• [ ] SmartState
• [ ] Gems Pending
• [ ] Move provider specific state machines into provider plugins (e.g. https://github.com/ManageIQ/manageiq-content/blob/master/content/automate/ManageIQ/Infrastructure/VM/Provisioning/StateMachines/Methods.class/__methods__/vmware_preprovision_clone_to_vm.rb)
• [ ] Move provider specific event handlers into provider plugins (e.g. https://github.com/ManageIQ/manageiq-content/tree/master/content/automate/ManageIQ/System/Event/EmsEvent/VC.class)
• [ ] Move provider specific settings out of core
  
  
  
  • [ ] [events](https://github.com/ManageIQ/manageiq/blob/8ba1f61/config/settings.yml#L175-L180)
  
  
  • [ ] [worker settings](https://github.com/ManageIQ/manageiq/blob/8ba1f61/config/settings.yml#L1189-L1197) - will be handled by https://github.com/ManageIQ/manageiq/pull/20975
  
  
• [ ] Vendor provider icon SVGs (https://github.com/ManageIQ/manageiq-providers-dummy_provider/issues/3)
• [ ] VENDOR_TYPES in core VmOrTemplate (https://github.com/ManageIQ/manageiq-providers-dummy_provider/issues/2) https://github.com/ManageIQ/manageiq/pull/20372
• [ ] Move provider specifics from MiqFileStorage into their respective provider repos e.g. miq_s3_storage.rb/miq_swift_storage.rb (https://github.com/ManageIQ/manageiq-gems-pending/issues/403) @NickLaMuro
• [x] CATALOG_ITEM_TYPES in core ServiceTemplate https://github.com/ManageIQ/manageiq/pull/20039
• [ ] Discovery support and names (https://github.com/ManageIQ/manageiq-ui-classic/issues/2529)
• [x] Assign Ems Created On is looking for VMware specific events [ref] - https://github.com/ManageIQ/manageiq-providers-vmware/pull/655 https://github.com/ManageIQ/manageiq/pull/20709
• [x] MiqTemplate.eligible_for_provisioning has a hard-coded list of provider subclasses [ref] (https://github.com/ManageIQ/manageiq/pull/20486)

• [x] ExtManagementSystem spec has a test that expects to know every single plugin [ref]
  https://github.com/ManageIQ/manageiq/pull/20595

  Core Pluggability

• [x] Worker definitions https://github.com/ManageIQ/manageiq/pull/19536 + https://github.com/ManageIQ/manageiq-schema/pull/437

• [x] Bring your own logger https://github.com/ManageIQ/manageiq/pull/20950 + https://github.com/ManageIQ/manageiq/pull/20960
  
  
  
  • [x] move over api and ae loggers still - https://github.com/ManageIQ/manageiq/pull/20965 & https://github.com/ManageIQ/manageiq/pull/20968
  
  
• [x] SupportsFeatureMixin - Right now all features for every model are in one big hash
• [x] Tag Mapper tag prefixes https://github.com/ManageIQ/manageiq/blob/master/app/models/provider_tag_mapping.rb#L25 - https://github.com/ManageIQ/manageiq/pull/20776
• [ ] Move IsoDatastore advertised images collection out of the core model (https://github.com/ManageIQ/manageiq/blob/master/app/models/iso_datastore.rb#L25-L52)
• [ ] Replace AvailabilityMixin users with SupportsFeatureMixin (https://github.com/ManageIQ/manageiq/issues/21179)

UI

• [x] Pluggable provider creation and credential checking (https://github.com/ManageIQ/manageiq/issues/18818)
• [ ] Replace/Remove provider specific class checks from UI (if/else blocks that check and do stuff differently depending upon provider type)
• [ ] Remove provider specifics out of UI spec tests (cc @himdel @martinpovolny)
• [ ] Vendor icons
• [ ] Decorators
• [ ] Remove hard-coded provider workflow classes in provision dialogs - https://github.com/ManageIQ/manageiq-ui-classic/issues/7353
• [ ] Tag mapper categories (https://github.com/ManageIQ/manageiq-ui-classic/blob/master/app/controllers/ops_controller/settings/label_tag_mapping.rb#L21-L28)

[agrare]

cc @Fryguy @chessbyte

[juliancheal]

One thing @agrare is removing the need to check node_types (in core, I guess) i.e. "nodes" (openstack) or "hosts" (non-openstack) for instance here https://github.com/ManageIQ/manageiq/blob/master/app/models/host.rb#L1722-L1744

[agrare]

That's a good one @juliancheal, we can probably just drop the title_for_host and title_for_clusters methods in the UI then those methods aren't needed at all

[Fryguy]

I had spoken to @chessbyte about that with respect to his EmsCluster changes (see here), and we came up with 2 possibilities

• Drop the "feature" entirely
• If we really need the feature, then one way to do it pluggably is to make the display_name method on the class deal with it. That is, the base display_name method would query all of the known types, then ask each of the known types what their preferred_display_name is, and then the base method just combines them together with .join(" / "). Then the plugin owns the name as well as the i18n for it. Then, in the ui code, this, this, and this effectively go away.

[chessbyte]

Created https://github.com/ManageIQ/manageiq/issues/19469 for refactoring VmScan code.

[carbonin]

@agrare Does https://github.com/ManageIQ/manageiq/pull/19536 solve the worker definitions bullet or is that more complicated?

[agrare]

@carbonin that absolutely covers that case

[NickLaMuro]

@agrare This exists, which I am currently working on:

https://github.com/ManageIQ/manageiq-gems-pending/issues/403

Not sure where it fits into what you have at the moment. Sorry it took me so long to figure the git-stuff out for that...

[agrare]

Awesome thanks @NickLaMuro, added to the list and linked to your issue.

[NickLaMuro]

For what it is worth, for the end goal of that ticket item:

Move provider specifics from MiqFileStorage into their respective provider repos e.g. miq_s3_storage.rb/miq_swift_storage.rb (ManageIQ/manageiq-gems-pending#403) @NickLaMuro

That PR doesn't complete that, but just moves it back into the main repo via:

https://github.com/ManageIQ/manageiq/pull/19547

Moving the swift and s3 parts to their respective provide repos is a different step all together, and figuring out how we make those "pluggable" is something I haven't quite figured out yet, and might require a bit of discussion. I might try tackling that once the two above are merged though.

[Fryguy]

Moving the swift and s3 parts to their respective provide repos is a different step all together, and figuring out how we make those "pluggable" is something I haven't quite figured out yet, and might require a bit of discussion. I might try tackling that once the two above are merged though.

This is one of those weird cases where even though it's S3, it's been argued that it doesn't belong in the amazon provider. The reasoning is that someone could use S3 as a File Depot, without requiring an Amazon provider and vice versa. This is sort of an independent pluggable thing. I didn't think this way previously, but now I'm leaning more towards that way.

[skateman]

I got a green light from @Fryguy to move decorators, we might introduce external tests from the decorator library repo for providers using a similar approach to https://github.com/ManageIQ/manageiq/blob/master/lib/tasks/test_providers_common.rake
https://github.com/ManageIQ/manageiq/blob/master/spec/models/event_stream_spec.rb

[Fryguy]

I added a "Decorators" bullet to the checklist in the OP. @skateman Be sure that if there are any patterned changes that all providers need to implement, that they are first implemented in the provider generator and then blasted out to all existing providers.

[Fryguy]

Merged #20595, but wasn't sure which checklist item above to check off

[agrare]

@Fryguy linked the pr and checked it off