-
Notifications
You must be signed in to change notification settings - Fork 92
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
Fix Provisioning of disconnected VolumeTemplate #173
Conversation
d5bcba0
to
741f9bf
Compare
rescue NoMethodError => ex | ||
_log.debug("Unable to find Provison Source ExtmanagementSystem: #{ex}") | ||
_log.debug("Trying use attribute src_ems_id=#{options[:src_ems_id].try(:first)} instead.") | ||
vm_model_class.find_by(:ems_id => options[:src_ems_id].try(:first), :ems_ref => ems_ref) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we can just use this call right away? Or is there a case when options[:src_ems_id] is nil?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not aware of such case, but I'd prefer keep original method using source to feel safe..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right, it bugs me a bit that this will basically fail everytime for volume_template, so maybe we should not use it?
@@ -1,4 +1,12 @@ | |||
module ManageIQ::Providers::Openstack::CloudManager::Provision::Cloning | |||
def find_destination_in_vmdb(ems_ref) | |||
vm_model_class.find_by(:ems_id => source.ext_management_system.id, :ems_ref => ems_ref) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we will keep this ,maybe we can just call super
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sense, thanks, will update it.
@aufi also, can you add todo, that this will not be needed when we do soft delete of VmOrTemplate without removing the vm_or_template foreign_key? |
Since provisioning from OpenStack Volumes was enabled, situation when original Volume template can be disconnected from EMS by refresher can happen and it doesn't look to be possible fix it on OpenStack side. This PR allows EMS lookup fallback to options[:src_ems_id] when getting EMS from source object fails. Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1518381
741f9bf
to
1a8b0aa
Compare
Checked commit aufi@1a8b0aa with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great. 👍
Tested locally and should be working (kind of proof: checked timestamps - VolumeTemplate was disconnected from EMS approx 20 seconds before last update of provisioned Vm and provision succeed). @miq-bot rm_label wip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Fix Provisioning of disconnected VolumeTemplate (cherry picked from commit e14333a) Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1526062
Gaprindashvili backport details:
|
Since provisioning from OpenStack Volumes was enabled, situation when original
Volume template can be disconnected from EMS by refresher can happen and it
doesn't look to be possible fix it on OpenStack side.
This PR allows EMS lookup fallback to options[:src_ems_id] when getting EMS
from source object fails.
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1518381
Links
Steps for Testing/QA
Vm provision process from OpenStack Volume should pass even refresh of given provider runs faster than provisioning process.