-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Expiration notifications are possibly broken in 3.0.0 #9
Comments
@wheeskers : Thanks for the report. I'll have to look into that in more detail, but as a hotfix you can change def compute_provides?(attr)
compute? && compute_resource && compute_resource.provided_attributes.keys.include?(attr)
end |
@timogoebel, thank you for the fast reply! I've applied this hotfix and it partially worked because initial exception changed. It seems that Log:
Note: I substituted some sensitive data in log above (hostnames, emails etc) but everything else is intact. |
@wheeskers: I'm sorry, this wasn't a quick fix. But it revealed the actually error happening here. I have looked at the error and have a pretty good understanding, what's causing this. I will get back to you with a fix asap. |
@timogoebel, thanks! |
This is really weird and although I have an idea what is happening, I have no clue as to why this happens. I have a couple of questions. Maybe this helps us find the issue.
diff --git a/lib/expire_hosts_notifications.rb b/lib/expire_hosts_notifications.rb
index defd467..ea033b5 100644
--- a/lib/expire_hosts_notifications.rb
+++ b/lib/expire_hosts_notifications.rb
@@ -24,11 +24,25 @@ module ExpireHostsNotifications
deletable_hosts.each do |deletable_host|
Rails.logger.info "Deleting expired host #{deletable_host}"
deletable_host.audit_comment = _('Destroyed since it got expired on %s') % deletable_host.expired_on
+ puts "Before destroy"
+ puts "Current User: #{User.current}"
+ puts "User Tax: Loc: #{User.current.try(:locations)} - Orgs: #{User.current.try(:organizations)}"
+ puts "Host: #{deletable_host.inspect}"
+ puts "Tax: Location: #{deletable_host.location} Organization: #{deletable_host.organization}"
+ puts "Compute Resource: ID: #{deletable_host.compute_resource_id} Record: #{deletable_host.compute_resource}"
+ puts "Owner: ID: #{deletable_host.owner_id} Type: #{deletable_host.owner_type} Record: #{deletable_host.owner}"
if deletable_host.destroy
deleted_hosts << deletable_host
else
failed_delete_hosts << deletable_host
end
+ puts "After destroy"
+ puts "Current User: #{User.current}"
+ puts "User Tax: Loc: #{User.current.try(:locations)} - Orgs: #{User.current.try(:organizations)}"
+ puts "Host: #{deletable_host.inspect}"
+ puts "Tax: Location: #{deletable_host.location} Organization: #{deletable_host.organization}"
+ puts "Compute Resource: ID: #{deletable_host.compute_resource_id} Record: #{deletable_host.compute_resource}"
+ puts "Owner: ID: #{deletable_host.owner_id} Type: #{deletable_host.owner_type} Record: #{deletable_host.owner}"
end
unless deleted_hosts.empty?
ExpireHostsNotifications.hosts_by_user(deleted_hosts).each do |user_id, hosts_hash|
diff --git a/lib/tasks/expired_hosts.rake b/lib/tasks/expired_hosts.rake
index 7af8092..f822f24 100644
--- a/lib/tasks/expired_hosts.rake
+++ b/lib/tasks/expired_hosts.rake
@@ -2,10 +2,12 @@
namespace :expired_hosts do
desc 'Delete all expired hosts, send notification email about expiring hosts'
task :deliver_notifications => :environment do
- ExpireHostsNotifications.delete_expired_hosts
- ExpireHostsNotifications.stop_expired_hosts
- ExpireHostsNotifications.deliver_expiry_warning_notification(1)
- ExpireHostsNotifications.deliver_expiry_warning_notification(2)
+ User.as_anonymous_admin do
+ ExpireHostsNotifications.delete_expired_hosts
+ ExpireHostsNotifications.stop_expired_hosts
+ ExpireHostsNotifications.deliver_expiry_warning_notification(1)
+ ExpireHostsNotifications.deliver_expiry_warning_notification(2)
+ end
end
end |
Currently there are multiple Foreman Locations in use with "relations" (i.e. Names are I couldn't find any mentions about "Organizations" in admin interface or searching in our task history so I guess this feature is not used at all.
Yes, sure. Can't do this right now because we are crating a large group of VMs today so I'll have to wait some time until most of the operations complete. Thanks again for the help, I'll try this as soon as possible and post new log output. |
@timogoebel, it seems that the problem is more complex. I have added debug code to As far as I know this is already happened before I were tasked to review this problem in our environment. I suppose this may be related to some specific hosts (i.e. "bare metal" provisioned) that are periodically deleted manually by VM users and then this exception goes away on its own. This is just an assumption though because there were no code modifications on TFM host (except the debug code from this issue). Launch command is still the same: Log lines are below. I've substituted some sensitive data (person names, locations, host names, etc) by simple placeholder values but they are still consistent (same hostname = same substitute through log file). Null or empty values are not modified and left 'as is'.
|
@wheeskers : Thanks for providing the data. This helps a lot. In the data you can see, that even before deletion the associations cannot be resolved properly. I seriously suspect, that this is related to a permissions problem. Could you try next with this patch applied? diff --git a/lib/tasks/expired_hosts.rake b/lib/tasks/expired_hosts.rake
index 7af8092..f822f24 100644
--- a/lib/tasks/expired_hosts.rake
+++ b/lib/tasks/expired_hosts.rake
@@ -2,10 +2,12 @@
namespace :expired_hosts do
desc 'Delete all expired hosts, send notification email about expiring hosts'
task :deliver_notifications => :environment do
- ExpireHostsNotifications.delete_expired_hosts
- ExpireHostsNotifications.stop_expired_hosts
- ExpireHostsNotifications.deliver_expiry_warning_notification(1)
- ExpireHostsNotifications.deliver_expiry_warning_notification(2)
+ User.as_anonymous_admin do
+ ExpireHostsNotifications.delete_expired_hosts
+ ExpireHostsNotifications.stop_expired_hosts
+ ExpireHostsNotifications.deliver_expiry_warning_notification(1)
+ ExpireHostsNotifications.deliver_expiry_warning_notification(2)
+ end
end
end |
@timogoebel, I've applied this patch to Log output:
|
@wheeskers: We're getting somewhere. The associations are now resolved properly. I'll open a PR with a fix for the permissions soon. |
No problem, I'll try to add it ASAP when I'm at the office.
I did some basic review of existing logs. Do you have any ideas what to look for? May be there is something specific that I am missing out. |
@wheeskers: Can you delete the host manually via the UI? This error should only occur when the host cannot be deleted properly. One reason for this could be that for example a DNS record could not be deleted on the smart-proxy. You should check the logs for that. However, this plugin should still continue to delete other hosts and not stop at the first failure. This should prevent this from happening: diff --git a/lib/expire_hosts_notifications.rb b/lib/expire_hosts_notifications.rb
index 233294e..91d8745 100644
--- a/lib/expire_hosts_notifications.rb
+++ b/lib/expire_hosts_notifications.rb
@@ -23,7 +23,7 @@ module ExpireHostsNotifications
deletable_hosts.each do |deletable_host|
Rails.logger.info "Deleting expired host #{deletable_host}"
deletable_host.audit_comment = _('Destroyed since it got expired on %s') % deletable_host.expired_on
- if deletable_host.destroy
+ if safe_destroy(deletable_host)
deleted_hosts << deletable_host
else
failed_delete_hosts << deletable_host
@@ -115,5 +115,17 @@ module ExpireHostsNotifications
end
hosts_hash
end
+
+ def safe_destroy(host)
+ host.destroy!
+ rescue ActiveRecord::RecordNotDestroyed => invalid
+ message = _("Failed to delete Host %{host}: %{message} - Errors: %{errors}") % {
+ :host => host,
+ :message => invalid.message,
+ :errors => invalid.record.errors.full_messages.to_sentence
+ }
+ Foreman::Logging.exception(message, invalid)
+ false
+ end
end
end |
Errors regarding one of our smart-proxies are not very common because majority of hosts are located in "normal" environment. This specific proxy is located in "nonstandard" environment and may be problematic. We are planning to remove it or migrate it to different platform/environment.
Thank you! I'll add this code soon and report back results. |
@timogoebel, sorry for the delayed reply. I've tested this on small group of expired hosts today and it worked. Thanks! Will this hotfix skip all/any other hosts with "bad" permissions (i.e. insufficient vSphere account permissions)? |
Yes. That's the aim. I'll close this for now. Thanks for your reply. The next plugin release will contain all the fixes and UI notifications. |
Thanks @timogoebel, I can confirm that at least notifications are working with your fix. We are waiting for grace period ending for test hosts to ensure that hosts will be really deleted. This plugin was the real relief and allowed us to start managing lifecycle of VMs in our environment. Hovewer, I'm going to open one more bug and one feature request regarding notifications. I'll mention this ticket there. |
@vladimirsafronov1251: Sure, go ahead and let me know if you need anything. Happy to help. |
Hi @timogoebel, Can you please clarify:
Thanks! |
@wheeskers: Updates to Foreman core in the 1.15 branch should have no impact to the hotfix. We are currently preparing Foreman 1.16 for release. I will release a new version of this plugin (probably 4.0.0) that will require 1.16 with all the fixes and features we discussed. Sorry for not backporting this to 1.15, I didn't have the time. |
@timogoebel, thanks again for the detailed reply and your help. |
Hi TFM team,
We have an issue with
foreman_expire_hosts
plugin: hosts' owners no longer receive notifications about host expiration and entering grace period.I'll try to provide as many details as I can but I must say that I am not very familiar with Rails framework so please excuse me if my understanding of the situation isn't complete. I was not involved in this TFM configuration but I will try to contact any person who may have any useful information if needed.
Versions:
1.15.2
;foreman_expire_hosts
plugin –3.0.0
.Running
rake
command:Produces the same error as in system logs (see below).
Please share any ideas if there's a way to fix this without major downtime or TFM instance reinstalling.
If this is not a bug but configuration/installation flaw -- comments on what is wrong would be great.
Thanks!
Log lines from
/var/log/foreman/expired_hosts.log
file:The text was updated successfully, but these errors were encountered: