v2v: Handle IMPORTEXPORT_STARTING_IMPORT_VM event #149

smelamud · 2017-07-24T11:32:47Z

Added VmImportWaitForVm state machine that is created when
IMPORTEXPORT_IMPORT_VM event arrives. VmImportWaitForVm state machine
waits for the converted VM to appear in the database and then sets its
import status. After that VmImport state machine can continue its
operation.

https://bugzilla.redhat.com/show_bug.cgi?id=1469498
https://bugzilla.redhat.com/show_bug.cgi?id=1469492

smelamud · 2017-07-24T11:48:54Z

CC @tinaafitz

smelamud · 2017-07-24T11:49:46Z

CC @pkliczewski

pkliczewski · 2017-07-24T11:57:40Z

...cture/VM/Transform/StateMachines/VmImportWaitForVm.class/__methods__/wait_for_imported_vm.rb

-      module Event
-        module EmsEvent
-          module RHEVM
-            class UpdateVmImportStatus


What happened to updateVmImportStatus? I do not see any information why are you replacing it.

@pkliczewski UpdateVmImportStatus cannot work as it is because when IMPORTEXPORT_IMPORT_VM event arrives nobody guarantees that the VM object is accessible. That's why I'm replacing the UpdateVmImportStatus method with a state machine that will wait until the VM object is accessible and only after that will update its import status.

@smelamud It was not clear to me from your description. Can you please update it?

@pkliczewski Do you know if the VM will show up in our inventory before the import is complete? The reason I ask is, if it's not included in our inventory until the import is complete, we don't need the additional state machine because the import state machine will catch the VM when it's complete.

@tinaafitz I am not sure about the sequence of the events during the import but as far as I know other parts of the code we always create an entity in the db (call refresh) when the action is done.
@smelamud please respond to the question

@tinaafitz oVirt always creates a VM at the beginning of V2V process. But V2V that converts the VM and refresh that puts VM into CFME's inventory are two independent processes. And my and Martin Betak's testing shows that sometimes the VM appears before and sometimes after the V2V process completes.

@smelamud What is the event triggered by ovirt when a vm is created during v2v? For example during vm creating process we have two events (we have more but for sake of discussion I simplify ) USER_ADD_VM and USER_ADD_VM_FINISHED_SUCCESS. The last event triggers refresh to update the inventory. I wonder why do we need it at the beginning?

BTW do we know what is the difference between two v2v runs when a vm is inventory before and after the process completes? It sounds like a bug to me.

@pkliczewski @smelamud - @mkanoor, @agrare and I discussed various aspects of the issue.
I have a few suggestions:

Change the event refresh call.
The RHEVM event IMPORTEXPORT_IMPORT_VM instance currently calls for a refresh with the following:
/System/event_handlers/event_action_refresh?target=src_vm

It should call this instead:
/System/event_handlers/event_action_refresh_new_target?target=src_vm

This new state machine is not necessary. The original import state machine will be pick up the VM on a retry and the failure will be caught by the state machine when it exceeds the specified number of retries.

Remove the custom_get logic from the original state machine wait_for_vm_import method.
status = vm.custom_get(:import_status)
The method is a success if there is a VM, otherwise setup the retry.

pkliczewski · 2017-07-24T12:45:08Z

@miq-bot assign @gmcculloug

gmcculloug · 2017-07-24T12:49:30Z

@smelamud Is there a Bugzilla issue related to this? If so can you add a link in the commit/PR description. See PR #138 for an example.

smelamud · 2017-07-25T17:57:37Z

Added more detailed description and bug URL.

agrare · 2017-07-25T18:14:36Z

Okay so couple of questions @smelamud:

Is IMPORTEXPORT_IMPORT_VM emitted when the process starts, or when it completes successfully?
If it is when it starts, is there an event emitted when it completes?
If it is when it is finished successfully, is there an event emitted when the VM is initially created but not finished importing?

tinaafitz

@pkliczewski @smelamud - @mkanoor, @agrare and I discussed various aspects of the issue.
I have a few suggestions:

Change the event refresh call.
The RHEVM event IMPORTEXPORT_IMPORT_VM instance currently calls for a refresh with the following:
/System/event_handlers/event_action_refresh?target=src_vm

It should call this instead:
/System/event_handlers/event_action_refresh_new_target?target=src_vm

This new state machine is not necessary. The original import state machine will be pick up the VM on a retry and the failure will be caught by the state machine when it exceeds the specified number of retries.
Remove the custom_get logic from the original state machine wait_for_vm_import method.
status = vm.custom_get(:import_status)
The method is a success if there is a VM, otherwise setup the retry.

smelamud · 2017-07-25T18:26:15Z

@agrare IMPORTEXPORT_IMPORT_VM is emitted when the process completes successfully.

There is IMPORTEXPORT_STARTING_IMPORT_VM event that is emitted when the import is started.

agrare · 2017-07-25T18:28:10Z

@smelamud okay great, if we can get the ems_ref from IMPORTEXPORT_STARTING_IMPORT_VM can we trigger a refresh_new_target from there?

smelamud · 2017-07-25T18:28:40Z

@pkliczewski

The original import state machine will be pick up the VM on a retry and the failure will be caught by the state machine when it exceeds the specified number of retries.

The original import state machine (VmImport) has interval 30 minutes and maximum number of retries - 100. That means that failure will be detected in 2 days.

smelamud · 2017-07-25T18:38:28Z

@agrare Sorry, I don't clearly understand how ems_ref is related to this. You mean that I need to write

- rel4:
    value: /System/event_handlers/event_action_refresh_new_target?target=src_vm

in importexport_starting_import_vm.yaml?

But if this requires ems_ref to be set, I'm not sure it will be. For IMPORTEXPORT_IMPORT_VM event I've checked together with @tinaafitz that ems_ref is nil when this event is emitted.

agrare · 2017-07-25T18:42:17Z

@smelamud looking at your parser it would be if :vm were present in the event payload. We need the ems_ref to be able to create the basic vm record.

tinaafitz · 2017-07-25T18:49:13Z

@agrare, @smelamud and I were looking at the event_stream object the other day. The vm information is available here:
full_data = {:id=>"1097", :href=>"/ovirt-engine/api/events/1097", :cluster=>{:id=>"58cfb470-03b9-01d0-03b9-0000000001e7", :href=>"/ovirt-engine/api/clusters/58cfb470-03b9-01d0-03b9-0000000001e7"}, :data_center=>{:id=>"58cfb470-02f3-03d7-0386-0000000003bc", :href=>"/ovirt-engine/api/datacenters/58cfb470-02f3-03d7-0386-0000000003bc"}, :host=>{:id=>"1a42c3b2-60eb-44d0-845c-1bf57c3d8b1d", :href=>"/ovirt-engine/api/hosts/1a42c3b2-60eb-44d0-845c-1bf57c3d8b1d"},:storage_domain=>{:id=>"4ab713ed-964e-4ce7-b970-183d4651fa41", :href=>"/ovirt-engine/api/storagedomains/4ab713ed-964e-4ce7-b970-183d4651fa41"}, :vm=>{:id=>"d839535c-2466-4dd9-98ca-71452dd2f2c1", :href=>"/ovirt-engine/api/vms/d839535c-2466-4dd9-98ca-71452dd2f2c1"}, :description=>"Vm mb-win10 was imported successfully to Data Center Default, Cluster Default", :severity=>"normal", :code=>1152, :time=>2017-07-21 00:34:17 +0300, :name=>"IMPORTEXPORT_IMPORT_VM"}
[----] I, [2017-07-21T00:39:59.286084 #30760:2b167a10fdf4] INFO -- : <AEMethod

smelamud · 2017-07-25T18:49:54Z

@agrare No, I'm using event_stream.full_data[:vm][:id] to extract VM UUID and construct ems_ref from it. There is no ems_ref in the payload.

[----] I, [2017-07-21T02:32:56.355943 #15873:2b00e4687030]  INFO -- : <AEMethod wait_for_imported_vm> Listing Root Object Attributes:
[----] I, [2017-07-21T02:32:56.377704 #15873:2b00e50be6e8]  INFO -- : <AEMethod wait_for_imported_vm> 	ae_next_state: 
[----] I, [2017-07-21T02:32:56.378612 #15873:2b00e50be6e8]  INFO -- : <AEMethod wait_for_imported_vm> 	ae_result: ok
[----] I, [2017-07-21T02:32:56.379429 #15873:2b00e50be6e8]  INFO -- : <AEMethod wait_for_imported_vm> 	ae_retry_server_affinity: false
[----] I, [2017-07-21T02:32:56.380166 #15873:2b00e50be6e8]  INFO -- : <AEMethod wait_for_imported_vm> 	ae_state: Launch
[----] I, [2017-07-21T02:32:56.380919 #15873:2b00e50be6e8]  INFO -- : <AEMethod wait_for_imported_vm> 	ae_state_max_retries: 100
[----] I, [2017-07-21T02:32:56.381685 #15873:2b00e50be6e8]  INFO -- : <AEMethod wait_for_imported_vm> 	ae_state_retries: 6
[----] I, [2017-07-21T02:32:56.382461 #15873:2b00e50be6e8]  INFO -- : <AEMethod wait_for_imported_vm> 	ae_state_started: 2017-07-20 23:26:22 UTC
[----] I, [2017-07-21T02:32:56.383190 #15873:2b00e50be6e8]  INFO -- : <AEMethod wait_for_imported_vm> 	ae_state_step: main
[----] I, [2017-07-21T02:32:56.383943 #15873:2b00e50be6e8]  INFO -- : <AEMethod wait_for_imported_vm> 	event_id: 2471
[----] I, [2017-07-21T02:32:56.385147 #15873:2b00e50be6e8]  INFO -- : <AEMethod wait_for_imported_vm> 	event_stream: #<MiqAeMethodService::MiqAeServiceEmsEvent:0x005601d451a398>
[----] I, [2017-07-21T02:32:56.385862 #15873:2b00e50be6e8]  INFO -- : <AEMethod wait_for_imported_vm> 	event_stream_id: 2471
[----] I, [2017-07-21T02:32:56.386686 #15873:2b00e50be6e8]  INFO -- : <AEMethod wait_for_imported_vm> 	event_type: IMPORTEXPORT_IMPORT_VM
[----] I, [2017-07-21T02:32:56.388973 #15873:2b00e50be6e8]  INFO -- : <AEMethod wait_for_imported_vm> 	host: deimos
[----] I, [2017-07-21T02:32:56.389638 #15873:2b00e50be6e8]  INFO -- : <AEMethod wait_for_imported_vm> 	host_id: 1
[----] I, [2017-07-21T02:32:56.390601 #15873:2b00e50be6e8]  INFO -- : <AEMethod wait_for_imported_vm> 	miq_group: #<MiqAeMethodService::MiqAeServiceMiqGroup:0x005601d5655928>
[----] I, [2017-07-21T02:32:56.391570 #15873:2b00e50be6e8]  INFO -- : <AEMethod wait_for_imported_vm> 	miq_server: #<MiqAeMethodService::MiqAeServiceMiqServer:0x005601d4620bc0>
[----] I, [2017-07-21T02:32:56.392168 #15873:2b00e50be6e8]  INFO -- : <AEMethod wait_for_imported_vm> 	miq_server_id: 1
[----] I, [2017-07-21T02:32:56.393261 #15873:2b00e50be6e8]  INFO -- : <AEMethod wait_for_imported_vm> 	object_name: Event
[----] I, [2017-07-21T02:32:56.394370 #15873:2b00e50be6e8]  INFO -- : <AEMethod wait_for_imported_vm> 	tenant: #<MiqAeMethodService::MiqAeServiceTenant:0x005601d4c53290>
[----] I, [2017-07-21T02:32:56.395519 #15873:2b00e50be6e8]  INFO -- : <AEMethod wait_for_imported_vm> 	user: #<MiqAeMethodService::MiqAeServiceUser:0x005601d57fe1f8>
[----] I, [2017-07-21T02:32:56.396249 #15873:2b00e50be6e8]  INFO -- : <AEMethod wait_for_imported_vm> 	user_id: 1
[----] I, [2017-07-21T02:32:56.396953 #15873:2b00e50be6e8]  INFO -- : <AEMethod wait_for_imported_vm> 	vmdb_object_type: event_stream

And event_stream:

[----] I, [2017-07-21T00:39:59.269332 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  Begin Attributes [object.attributes]
[----] I, [2017-07-21T00:39:59.273397 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  availability_zone_id = nil
[----] I, [2017-07-21T00:39:59.274161 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  chain_id = nil
[----] I, [2017-07-21T00:39:59.274867 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  container_group_id = nil
[----] I, [2017-07-21T00:39:59.275440 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  container_group_name = nil
[----] I, [2017-07-21T00:39:59.275915 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  container_id = nil
[----] I, [2017-07-21T00:39:59.276411 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  container_name = nil
[----] I, [2017-07-21T00:39:59.276846 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  container_namespace = nil
[----] I, [2017-07-21T00:39:59.277306 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  container_node_id = nil
[----] I, [2017-07-21T00:39:59.277757 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  container_node_name = nil
[----] I, [2017-07-21T00:39:59.278171 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  container_replicator_id = nil
[----] I, [2017-07-21T00:39:59.278633 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  container_replicator_name = nil
[----] I, [2017-07-21T00:39:59.279111 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  created_on = Thu, 20 Jul 2017 21:34:30 UTC +00:00
[----] I, [2017-07-21T00:39:59.279566 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  dest_ems_cluster_id = nil
[----] I, [2017-07-21T00:39:59.280009 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  dest_ems_cluster_name = nil
[----] I, [2017-07-21T00:39:59.280506 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  dest_ems_cluster_uid = nil
[----] I, [2017-07-21T00:39:59.280924 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  dest_host_id = nil
[----] I, [2017-07-21T00:39:59.281450 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  dest_host_name = nil
[----] I, [2017-07-21T00:39:59.281911 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  dest_vm_location = nil
[----] I, [2017-07-21T00:39:59.282355 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  dest_vm_name = nil
[----] I, [2017-07-21T00:39:59.282753 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  dest_vm_or_template_id = nil
[----] I, [2017-07-21T00:39:59.283173 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  ems_cluster_id = 1
[----] I, [2017-07-21T00:39:59.283668 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  ems_cluster_name = "Default"
[----] I, [2017-07-21T00:39:59.284094 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  ems_cluster_uid = nil
[----] I, [2017-07-21T00:39:59.284540 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  ems_id = 1
[----] I, [2017-07-21T00:39:59.285020 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  event_type = "IMPORTEXPORT_IMPORT_VM"
[----] I, [2017-07-21T00:39:59.285663 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  full_data = {:id=>"1097", :href=>"/ovirt-engine/api/events/1097", :cluster=>{:id=>"58cfb470-03b9-01d0-03b9-0000000001e7", :href=>"/ovirt-engine/api/clusters/58cfb470-03b9-01d0-03b9-0000000001e7"}, :data_center=>{:id=>"58cfb470-02f3-03d7-0386-0000000003bc", :href=>"/ovirt-engine/api/datacenters/58cfb470-02f3-03d7-0386-0000000003bc"}, :host=>{:id=>"1a42c3b2-60eb-44d0-845c-1bf57c3d8b1d", :href=>"/ovirt-engine/api/hosts/1a42c3b2-60eb-44d0-845c-1bf57c3d8b1d"},:storage_domain=>{:id=>"4ab713ed-964e-4ce7-b970-183d4651fa41", :href=>"/ovirt-engine/api/storagedomains/4ab713ed-964e-4ce7-b970-183d4651fa41"}, :vm=>{:id=>"d839535c-2466-4dd9-98ca-71452dd2f2c1", :href=>"/ovirt-engine/api/vms/d839535c-2466-4dd9-98ca-71452dd2f2c1"}, :description=>"Vm mb-win10 was imported successfully to Data Center Default, Cluster Default", :severity=>"normal", :code=>1152, :time=>2017-07-21 00:34:17 +0300, :name=>"IMPORTEXPORT_IMPORT_VM"}
[----] I, [2017-07-21T00:39:59.286084 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  generating_ems_id = nil
[----] I, [2017-07-21T00:39:59.286565 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  host_id = 1
[----] I, [2017-07-21T00:39:59.287017 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  host_name = "deimos"
[----] I, [2017-07-21T00:39:59.287471 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  id = 2277
[----] I, [2017-07-21T00:39:59.287874 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  is_task = nil
[----] I, [2017-07-21T00:39:59.288308 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  message = "Vm mb-win10 was imported successfully to Data Center Default, Cluster Default"
[----] I, [2017-07-21T00:39:59.288718 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  middleware_deployment_id = nil
[----] I, [2017-07-21T00:39:59.289158 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  middleware_deployment_name = nil
[----] I, [2017-07-21T00:39:59.289669 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  middleware_server_id = nil
[----] I, [2017-07-21T00:39:59.290071 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  middleware_server_name = nil
[----] I, [2017-07-21T00:39:59.290464 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  physical_server_id = nil
[----] I, [2017-07-21T00:39:59.290871 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  source = "RHEVM"
[----] I, [2017-07-21T00:39:59.291310 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  target_id = nil
[----] I, [2017-07-21T00:39:59.291691 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  target_type = nil
[----] I, [2017-07-21T00:39:59.292125 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  timestamp = Thu, 20 Jul 2017 21:34:17 UTC +00:00
[----] I, [2017-07-21T00:39:59.292528 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  type = "EmsEvent"
[----] I, [2017-07-21T00:39:59.292910 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  username = nil
[----] I, [2017-07-21T00:39:59.293345 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  vm_location = nil
[----] I, [2017-07-21T00:39:59.293716 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  vm_name = nil
[----] I, [2017-07-21T00:39:59.294094 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm>  vm_or_template_id = nil
[----] I, [2017-07-21T00:39:59.295926 #30760:2b167a10fdf4]  INFO -- : <AEMethod wait_for_imported_vm> event_stream.vm = nil

agrare · 2017-07-25T18:53:41Z

@smelamud @tinaafitz so it sounds like we're all agreeing that we can get the new VM's unique reference from the IMPORTEXPORT_STARTING_IMPORT_VM event? 😄
@smelamud when I say :ems_ref I mean what gets translated by the parser here https://github.com/ManageIQ/manageiq-providers-ovirt/blob/master/app/models/manageiq/providers/redhat/infra_manager/event_parsing/parser.rb#L52 which we use for the :ems_ref column in the :vms table on the refresh side, not that there is an :ems_ref in the event payload

smelamud · 2017-07-25T18:56:49Z

it sounds like we're all agreeing that we can get the new VM's unique reference from the IMPORTEXPORT_STARTING_IMPORT_VM event?

@agrare No, I was talking about IMPORTEXPORT_IMPORT_VM event. I didn't look into IMPORTEXPORT_STARTING_IMPORT_VM so I'm not sure the VM reference is present there.

smelamud · 2017-07-25T19:01:55Z

@agrare

when I say :ems_ref I mean what gets translated by the parser here ... not that there is an :ems_ref in the event payload

If so, yes, we have a way to construct ems_ref. What should I do to trigger a refresh_new_target with this information?

agrare · 2017-07-25T19:04:38Z

@smelamud check out user_add_vm_finished_success.yaml,

"/System/event_handlers/event_action_refresh_new_target?target=src_vm"

smelamud · 2017-07-25T19:11:02Z

@agrare OK, this I understand. But how ems_ref is related to this? Why you asked about it?

agrare · 2017-07-25T19:17:30Z

@smelamud because I wasn't sure if the IMPORTEXPORT_STARTING_IMPORT_VM event container enough information to create a basic vm record, the ems_ref being the most important one there.

smelamud · 2017-07-25T20:52:48Z

@agrare @pkliczewski OK, let's summarize how it should be working.

On IMPORTEXPORT_STARTING_IMPORT_VM we trigger event_action_refresh_new_target.
VmImport state machine in wait_for_vm_import waits for VM to appear. But it is not enough. VM appears when V2V process starts, but we need to know when it finishes. The only way to know this is to react to IMPORTEXPORT_IMPORT_VM event.
On IMPORTEXPORT_IMPORT_VM event we get the VM object and set :import_status on it. Are we absolutely sure that VM object exists at this point? Because if it doesn't, we loose the event and wait_for_vm_import will wait for 2 days till the timeout. It has no other way to know that V2V was finished. That's why I added this new state machine - to wait until VM object is available and set :import_status on it - to be sure we don't loose the event.
VmImport in wait_for_vm_import method sees the :import_status and goes to the next state.

mkanoor · 2017-07-25T21:22:49Z

@smelamud
Why would we have a half baked vm be visible in our database. If the Import is going to take say 30 minutes to finish and we have this vm record in our database, a user could interact with it (power off/retire/reconfigure). So does it make sense for this VM to be present in our database while the import is running?

agrare · 2017-07-25T21:27:00Z

@mkanoor if the VM is present in the provider it can be in our DB, it will get picked up by other refreshes anyway. If we want to prevent users from interacting with the VM while it is importing we should use another method to do that.

smelamud · 2017-07-25T22:27:39Z

We need to decide this ASAP, because this bug blocks V2V feature from being included into the nearest release.

tinaafitz · 2017-07-27T14:19:41Z

@smelamud Can you confirm that the refresh_new_target works properly on import?

smelamud · 2017-07-27T15:18:00Z

@tinaafitz @agrare I just retested it again on the latest master. Yes, after refresh_new_target the VM appears in the inventory, so the solution is correct. But:

The VM appears, but other VMs under RHV provider disappear. I don't understand whether it is a bug in refresh_new_target or I need to fix something on my side.
Seems that IMPORTEXPORT_STARTING_IMPORT_VM event passes a wrong VM UUID (the VM UUID of a wrong VM). Looks like a bug in RHV. IMPORTEXPORT_IMPORT_VM event passes a correct VM UUID.

agrare · 2017-07-27T15:24:01Z

@smelamud EmsRefresh.refresh_new_target shouldn't be disconnecting anything (https://github.com/ManageIQ/manageiq/blob/master/app/models/ems_refresh.rb#L105) but there definitely could be a bug there.

Check the logs for that refresh and see if you see Disconnecting Vms in the evm.log.

smelamud · 2017-07-27T15:30:17Z

@agrare Yes, I see Disconnecting Vms in the log. I can email you the log.

agrare · 2017-07-27T15:31:07Z

@smelamud yes can you send me the log

agrare · 2017-07-27T16:10:06Z

@smelamud okay the problem is definitely with the duplicate UUIDs:

[----] I, [2017-07-27T17:38:20.869847 #9458:2af9057bf114]  INFO -- : MIQ(EmsRefresh.save_ems_infra_inventory) EMS: [Jupiter], id: [1] Saving EMS Inventory...
[----] I, [2017-07-27T17:38:20.883602 #9458:2af9057bf114]  INFO -- : MIQ(EmsRefresh.save_vms_inventory) EMS: [Jupiter], id: [1] Duplicate unique values found: ["423578a6-7acf-b5a6-8d6a-94b5d15f98cb"]
[----] I, [2017-07-27T17:38:20.884129 #9458:2af9057bf114]  INFO -- : MIQ(EmsRefresh.save_vms_inventory) EMS: [Jupiter], id: [1] Updating Vm [WIN] id: [2] location: [423578a6-7acf-b5a6-8d6a-94b5d15f98cb.ovf] storage id: [1] uid_ems: [423578a6-7acf-b5a6-8d6a-94b5d15f98cb] ems_ref: [/api/vms/423578a6-7acf-b5a6-8d6a-94b5d15f98cb]
[----] I, [2017-07-27T17:38:20.906144 #9458:2af9057bf114]  INFO -- : MIQ(EmsRefresh.save_vms_inventory) EMS: [Jupiter], id: [1] Disconnecting Vms id: [77] name: [cfme30], id: [78] name: [cfme31], id: [72] name: [cfme28], id: [73] name: [cfme29], id: [54] name: [mb-win10], id: [1] name: [VM001]

tinaafitz · 2017-07-27T17:13:23Z

@smelamud The wait_for_vm_import method uses the custom_get(:import_status) to determine success or failure:
status = vm.custom_get(:import_status)
Based on the conversations in this PR, is that still valid?

smelamud · 2017-07-27T17:43:45Z

@agrare

the problem is definitely with the duplicate UUIDs

What does it mean? If refresh_new_target is called twice with the same VM, is it OK?

smelamud · 2017-07-27T17:46:02Z

@tinaafitz Yes, it is still valid. We set :import_status when IMPORTEXPORT_IMPORT_VM (or IMPORTEXPORT_IMPORT_VM_FAILED) event is received, so we know that v2v process was finished.

tinaafitz

@smelamud Looks good.

agrare · 2017-07-28T17:40:17Z

@smelamud you can call refresh_new_target twice on an existing VM without any issue. The duplicate UUID issue looks like it was returning the UUID of some other (random?) VM in the payload for the first event.

agrare · 2017-07-28T17:49:28Z

@smelamud can you try to reproduce that disconnect issue with refresh_new_target in isolation of this feature?

ManageIQ#149

jelkosz · 2017-08-07T14:00:34Z

@miq-bot add_label fine/yes

simaishi · 2017-08-08T15:17:09Z

@smelamud For fine backport, what target should be in .yaml files?

#119 isn't backported to Fine, so importexport_import_vm.yaml has target=ems. What value should be used in importexport_import_vm_failed.yaml and importexport_starting_import_vm.yaml?

jelkosz · 2017-08-09T10:16:58Z

@simaishi you need to use the src_vm

simaishi · 2017-08-09T11:53:28Z

@jelkosz use src_vm in all 3 yaml files, or just 2 files (importexport_import_vm_failed.yaml and importexport_starting_import_vm.yaml)?

Using src_vm in all 3 files means I will partially backport #119. But using src_vm in 2 files only will mean the values will be inconsistent across yaml files, so I just want to make sure I do this right...

jelkosz · 2017-08-09T12:20:11Z

@simaishi well, the #119 is targeted to 5.9 so maybe I would not backport the whole thing.

I have tested on my setup only to backport this particular patch and use the "src_vm" only for importexport_import_vm.yaml, importexport_import_vm_failed.yaml and importexport_starting_import_vm.yaml and all worked well.

simaishi · 2017-08-09T12:22:08Z

@jelkosz src_vm in all 3 yaml files then - thanks!

v2v: Handle IMPORTEXPORT_STARTING_IMPORT_VM event (cherry picked from commit 2e4892c) https://bugzilla.redhat.com/show_bug.cgi?id=1479454 https://bugzilla.redhat.com/show_bug.cgi?id=1479453

simaishi · 2017-08-09T12:37:55Z

Fine backport details:

$ git log -1
commit 8984f281d368d1d7766add67a0538edb5f94e66a
Author: Greg McCullough <[email protected]>
Date:   Fri Jul 28 13:42:53 2017 -0400

    Merge pull request #149 from smelamud/vm-import-wait-for-vm
    
    v2v: Handle IMPORTEXPORT_STARTING_IMPORT_VM event
    (cherry picked from commit 2e4892ca43df34433677e91ba27282315b37e2e1)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1479454
    https://bugzilla.redhat.com/show_bug.cgi?id=1479453

smelamud force-pushed the vm-import-wait-for-vm branch from 6aea139 to 61bb9c5 Compare July 24, 2017 11:47

pkliczewski reviewed Jul 24, 2017

View reviewed changes

miq-bot assigned gmcculloug Jul 24, 2017

gmcculloug requested a review from tinaafitz July 24, 2017 12:46

gmcculloug added the bug label Jul 24, 2017

smelamud force-pushed the vm-import-wait-for-vm branch from 61bb9c5 to 3998957 Compare July 25, 2017 17:45

tinaafitz suggested changes Jul 25, 2017

View reviewed changes

pkliczewski approved these changes Jul 27, 2017

View reviewed changes

tinaafitz approved these changes Jul 28, 2017

View reviewed changes

gmcculloug merged commit 2e4892c into ManageIQ:master Jul 28, 2017

gmcculloug added this to the Sprint 66 Ending Aug 7, 2017 milestone Jul 28, 2017

tinaafitz added a commit to tinaafitz/manageiq-content that referenced this pull request Aug 1, 2017

Fix test failure caused by:

8c63383

ManageIQ#149

tinaafitz mentioned this pull request Aug 1, 2017

Fix build failure #157

Merged

miq-bot added the fine/yes label Aug 7, 2017

simaishi added the fine/conflict label Aug 8, 2017

simaishi added fine/backported and removed fine/conflict labels Aug 9, 2017

v2v: Handle IMPORTEXPORT_STARTING_IMPORT_VM event #149

v2v: Handle IMPORTEXPORT_STARTING_IMPORT_VM event #149

Conversation

smelamud commented Jul 24, 2017 • edited by simaishi Loading

smelamud commented Jul 24, 2017

smelamud commented Jul 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pkliczewski commented Jul 24, 2017

gmcculloug commented Jul 24, 2017

smelamud commented Jul 25, 2017

agrare commented Jul 25, 2017

tinaafitz left a comment

Choose a reason for hiding this comment

smelamud commented Jul 25, 2017

agrare commented Jul 25, 2017

smelamud commented Jul 25, 2017

smelamud commented Jul 25, 2017

agrare commented Jul 25, 2017

tinaafitz commented Jul 25, 2017

smelamud commented Jul 25, 2017

agrare commented Jul 25, 2017

smelamud commented Jul 25, 2017 • edited Loading

smelamud commented Jul 25, 2017

agrare commented Jul 25, 2017

smelamud commented Jul 25, 2017

agrare commented Jul 25, 2017

smelamud commented Jul 25, 2017

mkanoor commented Jul 25, 2017

agrare commented Jul 25, 2017

smelamud commented Jul 25, 2017

tinaafitz commented Jul 27, 2017

smelamud commented Jul 27, 2017 • edited Loading

agrare commented Jul 27, 2017

smelamud commented Jul 27, 2017

agrare commented Jul 27, 2017

agrare commented Jul 27, 2017

tinaafitz commented Jul 27, 2017

smelamud commented Jul 27, 2017

smelamud commented Jul 27, 2017

tinaafitz left a comment

Choose a reason for hiding this comment

agrare commented Jul 28, 2017

agrare commented Jul 28, 2017

jelkosz commented Aug 7, 2017

simaishi commented Aug 8, 2017 • edited Loading

jelkosz commented Aug 9, 2017

simaishi commented Aug 9, 2017

jelkosz commented Aug 9, 2017

simaishi commented Aug 9, 2017

simaishi commented Aug 9, 2017

smelamud commented Jul 24, 2017 •

edited by simaishi

Loading

smelamud commented Jul 25, 2017 •

edited

Loading

smelamud commented Jul 27, 2017 •

edited

Loading

simaishi commented Aug 8, 2017 •

edited

Loading