[WIP] Delete DatabaseBackup/FileDepot/LogFile #21360

NickLaMuro · 2021-08-02T23:43:18Z

Part of the #21100 effort.

This will most likely be split up into more digestible commits, but just getting some test runs going to see what breaks.

Checklist

Links

Remove Database Backups from core (move to appliance console) #21100

Fryguy · 2021-08-09T21:49:44Z

cc @jrafanie ✂️ 🔥 🗑️

Well... mostly. A single file is needed as the appliance_console uses this method to check the current database connection. This code could be moved into the console itself, but since it was released recently, this allows that code to still function until a new version not requiring it can be released.

miq-bot · 2021-08-10T19:16:40Z

Checked commits NickLaMuro/manageiq@b493938~...8535e53 with ruby 2.6.3, rubocop 1.13.0, haml-lint 0.35.0, and yamllint
15 files checked, 3 offenses detected

app/models/miq_schedule.rb

❗ - Line 43, Col 34 - Style/PercentLiteralDelimiters - %w-literals should be delimited by [ and ].

lib/tasks/evm_dba.rake

❗ - Line 138, Col 43 - Style/ZeroLengthPredicate - Use !empty? instead of length > 0.
❗ - Line 146, Col 45 - Style/ZeroLengthPredicate - Use !empty? instead of length > 0.

kbrock · 2021-08-12T20:59:13Z

um +30 / −3,214 ✂️ ✂️ ✂️ 🔥
and it passes tests 🍏

Anything in particular for us to do to help you un-WIP this?

NickLaMuro · 2021-08-12T21:11:38Z

I probably should double check that the stuff in lib/tasks/evm_dba.rake still works as I ripped it out of PostgresAdmin:

https://github.com/ManageIQ/manageiq-gems-pending/blob/11c6c05cbb83077f8d325ae0a4f584c19546f81e/lib/gems/pending/util/postgres_admin.rb#L226-L263

And I doubt it will #justWork™ since I did have to combine it a bit.

Aside from that, though, I don't know that I left in much that needed to be converted.

kbrock

It could be argued that gc is a user script or is out of scope for our project all together.
So even if you introduced bugs with gc, I think we can live with it.

This looks nice. Very big delete.

LGTM

Fryguy · 2021-08-13T14:11:33Z

lib/tasks/evm_dba.rake

-      EvmDatabaseOps.gc(opts)
+      args    = {}
+      opts    = opts.delete_if { |_, v| v == false }
+      options = (options[:aggressive] ? GC_AGGRESSIVE_DEFAULTS : GC_DEFAULTS).merge(opts)


Are these constants present or from a require? I can't see them but maybe I just haven't looked hard enough

Nope! Probably forgot them, good catch!

Fryguy · 2021-08-13T14:34:17Z

This is awesome!

I do realize this is WIP, but wanted to get a checklist started on the stuff left to do from what I can see (you might have more)

One question - has log management been removed from the UI as well? This PR drops log management backend, so I wasn't sure.

schema
- Drop DatabaseBackup, FileDepot, LogFile tables
- FileDepot - Remove associated has_many schedules, auths
- MiqServer - Remove associated log_files, log_file_depot_id column
- MiqSchedule - file_depot_id column
- Zone - log_file_depot_id column
- Remove auths attached to classes with FileDepotMixin (e.g. PxeServer)
log management
- Drop Settings.log.collection from settings.yml
- Data migration to drop /log/collection settings_changes
- Can Settings.task.active_task_time be dropped?
import/export
- what if an old import has a FileDepotContent section?
- Data migration to drop import/export schedules? (are those even in the database?)
Can we drop the net/ftp gem?
Does this also end up dropping the mount sessions?
Drop Vmdb::Util.get_evm_log_for_date, get_log_start_end_times, zip_logs, and compressed_log_patterns
Drop MiqException::MiqDatabaseBackupInsufficientSpace
Drop the miq_event for evm_server_db_backup_low_space from the yaml
- Data migration to drop record and possibly detach from any policies (or drop the policy entirely if it's the only one left)

NickLaMuro · 2021-08-13T14:49:06Z

@Fryguy A few of these are things I might have considered at one point, but definitely forgot most of them, and the rest I didn't even consider. Nice list!

Does this also end up dropping the mount sessions?

I have a PR for this over in manageiq-gems-pending if that is what you are asking:

ManageIQ/manageiq-gems-pending#523

Can we drop the net/ftp gem?

I would have to look, but that is a good point! Most likely we can.

Will look at the other questions and answer as well, but figured I would ones out of the way that I knew off the top of my head.

NickLaMuro · 2021-08-16T21:00:38Z

Can we drop the net/ftp gem?

Uh... just kidding... this isn't a gem... it is stdlib... 😆

Fryguy · 2021-08-17T13:59:42Z

Can we just drop stdlib? 😂

jrafanie · 2021-08-17T16:33:39Z

😍

I know file depot was one of them that we needed the descendant loader / sti loader for. I wonder what 3+ deep STI ancestry trees we'd have left if this is gone. ❤️

As much as I'd love this, didn't we use file depots for pxe and possibly fleecing?

NickLaMuro · 2021-08-17T19:48:22Z

As much as I'd love this, didn't we use file depots for pxe...

DANG IT LJ! Why do you have to be right about this!

...and possibly fleecing?

Hmm.... don't seem to see this being the case however. Maybe you were just thinking about the mounting libraries in general?

NickLaMuro · 2021-08-17T20:08:11Z

Welp, closing this in favor of #21384 as that at least gets us some progress...

We can always re-open this with the EvmDatabaseOps commit already merged when we are more ready to removing everything else.

jrafanie · 2021-08-18T14:00:07Z

As much as I'd love this, didn't we use file depots for pxe...

DANG IT LJ! Why do you have to be right about this!

😭

...and possibly fleecing?

Hmm.... don't seem to see this being the case however. Maybe you were just thinking about the mounting libraries in general?

Yeah, that's it.

NickLaMuro · 2021-08-18T14:07:41Z

Hmm.... don't seem to see this being the case however. Maybe you were just thinking about the mounting libraries in general?

I should flesh this out for those potentially looking back at this in the future (or if we re-open this PR in the future):

manageiq-smartstate ("fleecing") used the mounting libraries, but just in one spot, and I already had a PR to import the relevant ones into that library:

ManageIQ/manageiq-smartstate#160

So that was actually taken care of. This would still have allowed this entire PR to move forward, since the FileDepot code was assumed to have most of the usage of the mounting code.

However, as LJ mentioned, the PXE functionality needs the entirety of the FileDepot code to function, which in turn is all of the mounting code which makes the aforementioned PR, as well as this and a few other related ones invalid. So closing them is the option chosen.

kbrock · 2021-08-18T17:39:19Z

of note, #21377 lists that we may have some big changes in PXE.
Wonder if this would give you the option to 🔥 this code.

Fryguy · 2021-08-19T15:29:27Z

That's an interesting point on PXE. If it's the IPMI portion of PXE, that's on the chopping block. @NickLaMuro do you have more details about what in PXE is using FileDepot?

NickLaMuro · 2021-08-19T19:41:27Z

@Fryguy Pretty much all of PxeServer:

https://github.com/ManageIQ/manageiq/blob/d6294e54dc46c8e364de96d6de6357411e2487eb/app/models/pxe_server.rb

And then things like PxeImage call some of the read_file/write_file/delete_file methods via the pxe_server relationship.

So unless you are axing a large part of the PxeServer , I don't think it will go away anytime soon.

jrafanie · 2021-08-20T17:55:36Z

So unless you are axing a large part of the PxeServer , I don't think it will go away anytime soon.

😢

One day.

NickLaMuro requested review from agrare, Fryguy, jrafanie and kbrock as code owners August 2, 2021 23:43

This was referenced Aug 2, 2021

Remove Database Backups from core (move to appliance console) #21100

Closed

[WIP] Remove DatabaseBackup/FileDepot views ManageIQ/manageiq-ui-classic#7816

Closed

NickLaMuro force-pushed the remove-file-depot branch from ef83ee7 to b77bc62 Compare August 4, 2021 21:50

NickLaMuro mentioned this pull request Aug 10, 2021

Remove PostgresAdmin ManageIQ/manageiq-gems-pending#516

Merged

2 tasks

miq-bot added wip dependencies labels Aug 10, 2021

NickLaMuro added 2 commits August 10, 2021 14:12

Delete FileDepot/LogFile

b493938

NickLaMuro force-pushed the remove-file-depot branch from b77bc62 to 8535e53 Compare August 10, 2021 19:12

NickLaMuro mentioned this pull request Aug 12, 2021

[WIP] Remove/Extract File/Object storage classes ManageIQ/manageiq-gems-pending#523

Closed

1 task

kbrock approved these changes Aug 13, 2021

View reviewed changes

Fryguy reviewed Aug 13, 2021

View reviewed changes

NickLaMuro changed the title ~~[WIP] Delete FileDepot/LogFile~~ [WIP] Delete DatabaseBackup/FileDepot/LogFile Aug 16, 2021

jrafanie mentioned this pull request Aug 17, 2021

Allow vddk to be dynamically added without rebuilding container images ManageIQ/manageiq-pods#734

Open

NickLaMuro closed this Aug 17, 2021

NickLaMuro mentioned this pull request Aug 17, 2021

[WIP] Importing NFS/GlusterFS libs ManageIQ/manageiq-smartstate#160

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Delete DatabaseBackup/FileDepot/LogFile #21360

[WIP] Delete DatabaseBackup/FileDepot/LogFile #21360

NickLaMuro commented Aug 2, 2021 •

edited

Loading

Fryguy commented Aug 9, 2021

miq-bot commented Aug 10, 2021

kbrock commented Aug 12, 2021

NickLaMuro commented Aug 12, 2021

kbrock left a comment •

edited

Loading

Fryguy Aug 13, 2021

NickLaMuro Aug 13, 2021

Fryguy commented Aug 13, 2021

NickLaMuro commented Aug 13, 2021

NickLaMuro commented Aug 16, 2021

Fryguy commented Aug 17, 2021

jrafanie commented Aug 17, 2021

NickLaMuro commented Aug 17, 2021

NickLaMuro commented Aug 17, 2021

jrafanie commented Aug 18, 2021

NickLaMuro commented Aug 18, 2021

kbrock commented Aug 18, 2021

Fryguy commented Aug 19, 2021 •

edited

Loading

NickLaMuro commented Aug 19, 2021 •

edited

Loading

jrafanie commented Aug 20, 2021

[WIP] Delete DatabaseBackup/FileDepot/LogFile #21360

[WIP] Delete DatabaseBackup/FileDepot/LogFile #21360

Conversation

NickLaMuro commented Aug 2, 2021 • edited Loading

Checklist

Links

Fryguy commented Aug 9, 2021

miq-bot commented Aug 10, 2021

kbrock commented Aug 12, 2021

NickLaMuro commented Aug 12, 2021

kbrock left a comment • edited Loading

Choose a reason for hiding this comment

Fryguy Aug 13, 2021

Choose a reason for hiding this comment

NickLaMuro Aug 13, 2021

Choose a reason for hiding this comment

Fryguy commented Aug 13, 2021

NickLaMuro commented Aug 13, 2021

NickLaMuro commented Aug 16, 2021

Fryguy commented Aug 17, 2021

jrafanie commented Aug 17, 2021

NickLaMuro commented Aug 17, 2021

NickLaMuro commented Aug 17, 2021

jrafanie commented Aug 18, 2021

NickLaMuro commented Aug 18, 2021

kbrock commented Aug 18, 2021

Fryguy commented Aug 19, 2021 • edited Loading

NickLaMuro commented Aug 19, 2021 • edited Loading

jrafanie commented Aug 20, 2021

NickLaMuro commented Aug 2, 2021 •

edited

Loading

kbrock left a comment •

edited

Loading

Fryguy commented Aug 19, 2021 •

edited

Loading

NickLaMuro commented Aug 19, 2021 •

edited

Loading