manageiq icon indicating copy to clipboard operation
manageiq copied to clipboard

Chopping Block ✂️ 🔥

Open Fryguy opened this issue 2 years ago • 10 comments

This issue is a list of candidates for removal or serious refactoring due to limited demand of the feature crossed with level of effort in maintenance / footprint

Complete removal:

  • [x] #21378
  • [x] #21379 @jrafanie
  • [x] #21380
  • [x] https://github.com/ManageIQ/manageiq-ui-classic/pull/8093
  • [ ] Log Collection / FileDepot
    • [x] https://github.com/ManageIQ/manageiq/pull/21808
    • [ ] https://github.com/ManageIQ/manageiq-ui-classic/pull/8235
    • [ ] FileDepot (depends on removal of log collection in the UI)
  • [ ] #21381

Possible archiving:

  • [ ] manageiq-consumption plugin
    • Need to discuss how close this was to full implementation vs what is current in the chargeback code

Refactoring / Fixes:

  • [ ] oVirt smartstate
    • Does not work in pods, and will likely break in root-less appliances.
    • Better to refactor as having a separate appliance that a user runs in their ovirt system that has access to the disk data, with a VDDK-like API.
  • [ ] PXE
    • Might not work in podified due to mount of NFS server. Possibly switch to a FUSE mount.

Needs more discussion

  • [ ] Service UI
    • Can we add the missing features to the classic UI? (shopping cart)
  • [ ] Monitoring Manager (OpenShift alerts)
  • [ ] Policy actions
    • limit the action to just notifications and running automate/playbook
  • [ ] External Logging on EmsContainer
    • Remove entirely or link out to the external kibana?
  • [ ] "Menu" widgets including the built-in links and quick links
  • [ ] custom branding

Other things discussed for removal and NAK'd

  • SCVMM provider

Fryguy avatar Aug 13 '21 17:08 Fryguy

cc @chessbyte @jrafanie @kbrock @bdunne @agrare @NickLaMuro

Fryguy avatar Aug 13 '21 18:08 Fryguy

I'll open up individual issue for the ones we agreed on

Fryguy avatar Aug 13 '21 18:08 Fryguy

I added FileDepot to the OP. I believe the only thing it is still used for is log collection, but I'm wondering if we can do log collections completely differently and then just completely delete this class. Let's discuss.

Fryguy avatar Mar 04 '22 17:03 Fryguy

Added discussion for the removal of "Menu" widgets. These seem completely pointless to me, but maybe I'm misunderstanding some fundamental thing. cc @GilbertCherrie

Fryguy avatar Mar 07 '22 16:03 Fryguy

can chop custom.css -- Since the file does not have a timestamp, we have a short timeout, and the client checks if the file has changed on every page load.

Also, in the pod world, I do not think that the customer is even able to modify the custom.css.

But this is a business feature and it may be used.

kbrock avatar Mar 10 '22 23:03 kbrock

@Fryguy Removed live metrics in https://github.com/ManageIQ/manageiq-ui-classic/pull/8093

kavyanekkalapu avatar Apr 04 '22 19:04 kavyanekkalapu

Log Collection code deletion is being handled here https://github.com/ManageIQ/manageiq-ui-classic/pull/8235

MelsHyrule avatar Apr 28 '22 14:04 MelsHyrule

I added FileDepot to the OP. I believe the only thing it is still used for is log collection, but I'm wondering if we can do log collections completely differently and then just completely delete this class. Let's discuss.

I think we still have code that depends on the FileDepot via the FileDepotMixin.


/Users/joerafaniello/.gem/ruby/2.7.5/bundler/gems/manageiq-ui-classic-bda2273e49af/app/controllers/pxe_controller/pxe_servers.rb
181:      PxeServer.verify_depot_settings(params[:pxe])

/Users/joerafaniello/.gem/ruby/2.7.5/bundler/gems/manageiq-ui-classic-bda2273e49af/app/controllers/application_controller.rb
462:        PxeServer.verify_depot_settings(settings)
joerafaniello@Joes-MacBook-Pro-2 manageiq % git grep FileDepotMixin
app/models/mixins/file_depot_mixin.rb:module FileDepotMixin
app/models/pxe_server.rb:  include FileDepotMixin

I think we can delete the LogFile first and then tackle removing the FileDepotMixin. I'm not familiar with the PxeServer code to know if that depot code can be removed at this time.

jrafanie avatar Apr 28 '22 20:04 jrafanie

Note, I would love to just delete the FileDepot but I think the PxeServer code is still using parts of the FileDepot via the following calls to with_depot:

sync_pxe_images
sync_windows_images
read_file
write_file
delete_file
delete_directory
create_provisioning_files
delete_provisioning_files

jrafanie avatar Apr 28 '22 20:04 jrafanie

Note, as I look at this more, FileDepot and subclasses seem to be unrelated to the FileDepotMixin module other than the name being similar.

FileDepotMixin is using 'mount/miq_generic_mount_session' to mount a NFS/SMB/FTP/etc. session and then using that to upload/download/delete files. This is what PxeServer is using. The LogFile class is using FileDepot and subclasses with explicit rows in the database.

FileDepot and subclasses seem to be used for LogFiles, schedules, and MiqServers. We might be able to detangle that and remove the FileDepot and subclasses if PxeServer and FileDepotMixin don't require FileDepot.

But for now, I think LogFile is completely safe to remove.

jrafanie avatar Apr 29 '22 20:04 jrafanie

@jrafanie @kbrock @agrare @MelsHyrule As discussed, I added https://github.com/ManageIQ/manageiq/issues/22365 to the Chopping Block

Fryguy avatar Feb 16 '23 22:02 Fryguy

per stale, I added https://github.com/ManageIQ/manageiq/pull/22642 (based upon a NickL PR that is simple but requires migrations that possibly hit a number of tables)

kbrock avatar Aug 01 '23 19:08 kbrock

Some old Transform VM cleanup: https://github.com/ManageIQ/manageiq/issues/21874

Fryguy avatar Nov 08 '23 18:11 Fryguy