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.

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: Vm#ems_created_on field is empty for vm (VMware provider) #19185

Merged
merged 6 commits into from
Aug 27, 2019

Conversation

yrudman
Copy link
Contributor

@yrudman yrudman commented Aug 21, 2019

ISSUE: ems_created_on field is empty for WMvare provider

FIX:

  • use reject instead of reject! on ActiveRecord::Relation
  • fixed query for vm.ems_events
  • assigning Vm#created_on should be executed on server which has ems_operations role
  • capture VmDeployedEvent when looking for newly provisioned VMs

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1743579

@miq-bot add-label changelog/yes, bug, core, hammer/yes, ivanchuk/yes

yrudman and others added 3 commits August 22, 2019 12:45
When cloning a VM from a template a VmDeployedEvent is raised which
wasn't being picked up by assign_ems_created_on
The assign_ems_created_on method will query the provider so it has to be
queued for the ems_operations role
@agrare agrare force-pushed the fixed-created_date-assignment-for-vm branch from bfba1b5 to 057fb9c Compare August 22, 2019 16:50
The query for vm.ems_events wasn't returning any events where the vm was
the dest_vm_or_template_id because an extra AND
ems_event.vm_or_template_id = id was being added to the where query.
@agrare agrare force-pushed the fixed-created_date-assignment-for-vm branch from 057fb9c to 219bc92 Compare August 22, 2019 17:48
@@ -120,7 +120,7 @@ class VmOrTemplate < ApplicationRecord
has_one :openscap_result, :as => :resource, :dependent => :destroy

# EMS Events
has_many :ems_events, ->(vmt) { where(["vm_or_template_id = ? OR dest_vm_or_template_id = ?", vmt.id, vmt.id]).order(:timestamp) },
has_many :ems_events, ->(vmt) { unscope(where: :vm_or_template_id).where(["vm_or_template_id = ? OR dest_vm_or_template_id = ?", vmt.id, vmt.id]).order(:timestamp) },
Copy link
Member

Choose a reason for hiding this comment

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

Props to @djberg96 for this ^

@yrudman yrudman force-pushed the fixed-created_date-assignment-for-vm branch 2 times, most recently from fd67e1d to b16f469 Compare August 27, 2019 13:08
@yrudman yrudman changed the title [WIP] Fix: 'Created on Time' field is empty for vm Fix: Vm#ems_created_on field is empty for vm (WMvare provider) Aug 27, 2019
@yrudman
Copy link
Contributor Author

yrudman commented Aug 27, 2019

\cc @agrare

@miq-bot miq-bot removed the wip label Aug 27, 2019
Copy link
Member

@agrare agrare left a comment

Choose a reason for hiding this comment

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

LGTM but I have some commits here so I'll let someone else review&merge

@agrare
Copy link
Member

agrare commented Aug 27, 2019

@bdunne can you take a look?

@yrudman yrudman force-pushed the fixed-created_date-assignment-for-vm branch from b16f469 to 4c6670e Compare August 27, 2019 14:22
@yrudman yrudman force-pushed the fixed-created_date-assignment-for-vm branch from 4c6670e to 7e7b0c7 Compare August 27, 2019 14:30
@miq-bot
Copy link
Member

miq-bot commented Aug 27, 2019

Checked commits yrudman/manageiq@fb27840~...7e7b0c7 with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🍪

@bdunne bdunne merged commit dc78cc0 into ManageIQ:master Aug 27, 2019
@bdunne bdunne self-assigned this Aug 27, 2019
@bdunne bdunne added this to the Sprint 119 Ending Sep 2, 2019 milestone Aug 27, 2019
@yrudman yrudman deleted the fixed-created_date-assignment-for-vm branch August 27, 2019 15:09
@agrare agrare changed the title Fix: Vm#ems_created_on field is empty for vm (WMvare provider) Fix: Vm#ems_created_on field is empty for vm (VMware provider) Sep 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants