Skip to content
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.

Sign up for GitHub

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

Reload the ems object in the event catcher if we fail to start #14736

Merged
merged 1 commit into from
Apr 12, 2017
Merged

Reload the ems object in the event catcher if we fail to start #14736

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions app/models/manageiq/providers/embedded_ansible/automation_manager/event_catcher/runner.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
class ManageIQ::Providers::EmbeddedAnsible::AutomationManager::EventCatcher::Runner < ManageIQ::Providers::BaseManager::EventCatcher::Runner
include ManageIQ::Providers::AnsibleTower::Shared::AutomationManager::EventCatcher::Runner

def start_event_monitor
tid = super
return tid unless tid.nil?

# Get a new copy of the ems record in case the embedded ansible role changed servers
@ems.reload
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so I am not very familiar with the life cycle, 3 (maybe obvious to others) questions,

  • Do we need to reload before the call to super so that whatever being done in super will reflect the new state in ems?
  • The url is stored in endpoint which will be reloaded as well, right?
  • Is this behavior applicable to other managers?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Do we need to reload before the call to super so that whatever being done in super will reflect the new state in ems?

I didn't do it this way because I wanted to stay as close to the original implementation as possible which meant keeping the @ems "cache" around and only reloading when it might be necessary. This prevents us from having to go to the database every call (although I'm not sure this method is called very frequently so maybe that doesn't matter).

  • The url is stored in endpoint which will be reloaded as well, right?

It is and it definitely seems that way (I tested this out on live machines).

  • Is this behavior applicable to other managers?

I don't think so because other managers don't expect the endpoint url to change, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering if we need this for the refresh worker also ... we didn't see the same problem, but maybe that's just because refreshes happen less frequently?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spoke with @Fryguy and @agrare, we think this shouldn't be an issue for the refresher.

nil
end
end