The vm_id coming in from API is a string.

[lfu]

Followup of #17627.

Fixes ManageIQ/manageiq-v2v#471.

@miq-bot assign @gmcculloug
@miq-bot add_label enhancement, transformation

cc @AparnaKarve @bzwei

[AparnaKarve]

LGTM - thanks @lfu !!

[miq-bot]

Checked commit lfu@bb86d41 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
3 files checked, 0 offenses detected
Everything looks fine. 👍

[AparnaKarve]

@bzwei @lfu Just so you are aware, this change with vm_id under config_info.actions is going to impact our frontend code in a big way.

Before -

"options": {
  "config_info": {
  "transformation_mapping_id": "3",
  "vm_ids": [
    "1",
    "2"
    ]
  }
}

After -

"options": {
  "config_info": {
  "transformation_mapping_id": "3",
  "actions": [
  {
      "vm_id": "4",
      "pre_service": false,
      "post_service": false
   }
  ]
 }
}

We have a couple of places in the code where we reference vm_ids directly from config_info -- all those places would now have to be rectified.
Not to mention, none of our existing Plans are going to work because of the above change.

Just wanted to check if it would be possible to re-introduce the vm_ids array under config_info?
Or the expectation going forward is that the old plans are not supposed to work?

Also, would this change be backported to Gaprindashvili?

/cc @priley86

[bzwei]

v2v feature is not officially released. I don't think we have to maintain backward compatibility. Please ping PM if it is a concern.

Sorry it requires more front end code changes, but it is not reasonable to keep vm ids and playbooks separate.

[bzwei]

BTW, you can omit

      "pre_service": false,
      "post_service": false

if the value is false.

[gmcculloug]

@AparnaKarve Are you ok with @bzwei response here or do we need to discuss?

[AparnaKarve]

@gmcculloug I think we can merge this.

Would this be backported to Gaprindashvili?

[AparnaKarve]

Just got the confirmation that we are planning on supporting pre and post playbook in 5.9.4, which means, this PR and #17627 should be gaprindashvili/yes.
Thanks.

[JPrause]

@miq-bot add_label blocker

[JPrause]

If this can be backported, please add the gaprindashvili/yes label

[AparnaKarve]

@miq-bot add_label gaprindashvili/yes

[simaishi]

Gaprindashvili backport details:

$ git log -1
commit c217a7b63a89deadc6abddad450d2bd380200dab
Author: Greg McCullough <gmccullo@redhat.com>
Date:   Mon Jul 9 16:10:19 2018 -0400

    Merge pull request #17674 from lfu/v2v_pre_post_ansibile_service
    
    The vm_id coming in from API is a string.
    (cherry picked from commit 1bb13c5e8d627d50b9cede9505c43392f11f3bec)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1608420