-
Notifications
You must be signed in to change notification settings - Fork 69
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
Use the new VM's ems_ref instead of an annotation #408
Use the new VM's ems_ref instead of an annotation #408
Conversation
Instead of setting a UUID to the annotations field check for the new vm's ems_ref. Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1724751
It is possible that EmsRefresh can have a really early view of the VM without all final data populated by the time the CloneVM_Task completes which can lead to post-provision tasks failing if they depend on some data in the vm record. Waiting until the Ems' inventory date is at least as recent as the CloneVM_Task completeTime we guarantee that the VM record represents the final VM state.
98e837b
to
3c45a85
Compare
@gmcculloug @Fryguy updated to include waiting for the ems last_inventory_date to be at least as new as the CloneVM_Task completeTime |
@Fryguy @gmcculloug thoughts? |
task_props = ["info.state", "info.error", "info.result", "info.progress", "info.completeTime"] | ||
task = vim.getMoProp(clone_task_mor, task_props) | ||
|
||
case task&.info&.state |
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.
Overall this looks good. Since we are referencing task&.info
in 5 places we should set that to a variable to avoid the lookup each time: task_info = task&.info
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.
👍 will do
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.
@gmcculloug done
end | ||
# Check that the EMS inventory is as up to date as the CloneVM_Task completeTime | ||
# to prevent issues with post-provision depending on data that isn't in VMDB yet | ||
return if source.ext_management_system.last_inventory_date < phase_context[:clone_vm_task_completion_time] |
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.
👍
Checked commits agrare/manageiq-providers-vmware@e9604ba~...e91a334 with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0 |
Instead of setting a UUID to the annotations field check for the new
vm's ems_ref.
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1724751