theforeman / foreman_expire_hosts

Foreman plugin for limiting host lifetime
GNU General Public License v3.0
5 stars 12 forks source link

Add a setting that enables/disables the deletion of hosts once they expire #51

Open timogoebel opened 4 years ago

timogoebel commented 4 years ago

There has been feedback in the Foreman community forums to add a setting so admins can choose whether hosts should be deleted automatically on expiration or not.

nodermatt commented 4 years ago

@timogoebel I can see where the new settings should be added: here

But I wonder where to add the safety check for the automated delete in the service. At first I thought about the base.rb

but now I think inside the action would be better -since I do have access to the host variable as well:

I will give it a shot to submit a PR for the changes this weekend.

timogoebel commented 4 years ago

Thanks for taking the effort to fix this. I‘d definitely do this in the action. A simple return unless Setting... on the first line of the method should be enough. The host will be skipped then because the method will return nil. You might want to check if you can skip the whole delete code earlier, e.g. here: https://github.com/theforeman/foreman_expire_hosts/blob/75efbebd26bff458006e9ed0586846edb70bb468/lib/expire_hosts_notifications.rb#L6 This gets called from the rake task (in lib/tasks/...) that is called in the daily cronjob.