purge archived only brings back archived records

[kbrock]

Based upon feedback from http://talk.manageiq.org/t/vm-destroy-is-slow/2141/4

It was pointed out that we were bringing back every vm to determine if it was archived. This is slow.
Since that script was written, archived has become a virtual attribute and a scope. So we have faster means of accomplishing this request.

• use a scope and only bring back the archived vms.
• remove REPORT_ONLY which before turned the script into a no-op by default. Now it actually purges them by default.

Not sure if the updated_on was put into the query for performance or business reasons. I left it in there. Suggestions for us to remove this clause? It seems arbitrary. UPDATE: no, it is intentional

	before	after
non-delete	30 seconds	3 seconds

[chessbyte]

@kbrock I thought the REPORT_ONLY was acting like a Linux standard --dry_run option. Maybe it makes sense to have that option available. /cc @Fryguy

[Fryguy]

@chessbyte You are correct. REPORT_ONLY is more or less "--dry-run" which I would like to keep for purging scripts as it allows a customer to run it and see what might change before deleting anything. In purge_metrics (which is the most fleshed out of the scripts), we call this parameter --mode, and the default there is count. The user has to opt-in to purging by changing that to purge

We should probably update all of the purge scripts at some point to a) have a common trollop based interface and b) use --dry-run instead of --mode as it's more intuitive

[Fryguy]

Not sure if the updated_on was put into the query for performance or business reasons

If you remove the clause then all archived VMs will go away which is not the intent. A user wants to know about recently archived VMs, but may want to clear out ones older than some time period (the ARCHIVE_CUTOFF)

[Fryguy]

We should probably update all of the purge scripts at some point to a) have a common trollop based interface and b) use --dry-run instead of --mode as it's more intuitive

In hindsight, I change my mind. --dry-run as an option would imply that the default action is to delete everything, which I don't want. These scripts are highly destructive and so their default mode of operation should be non-destructive and just informative. The user should be forced to opt-in via a parameter. I'm open to better parameter names.

[kbrock]

@Fryguy Well, since the script is named purge_archived_vms.rb I had thought it would be destructive.

ok, anyway. sorry for deleting the stuff I thought was not useful. I guess I brought upon the scope creep upon myself

[kbrock]

@Fryguy ok, rolled back all the code.

I just made the change to not bring back every vm and instead only bring back vms to purge.

[miq-bot]

Checked commit kbrock@f2a05b1 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks good. 👍

[kbrock]

NOTE: if this gets backported, ensure that Vm.all_archived no longer has the to_a - if it does, then make fix accordingly.