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

Add OSP attributes to VM ServiceResource options #18045

Conversation

AparnaKarve
Copy link
Contributor

Adds OSP attributes - osp_security_group_id and osp_flavor_id to VM ServiceResource options hash.

@AparnaKarve
Copy link
Contributor Author

AparnaKarve commented Oct 1, 2018

@miq-bot add_label hammer/yes

@miq-bot add_reviewer @bzwei

@miq-bot assign @roliveri

@miq-bot
Copy link
Member

miq-bot commented Oct 1, 2018

@AparnaKarve Cannot apply the following label because they are not recognized: hammer

@miq-bot
Copy link
Member

miq-bot commented Oct 1, 2018

@AparnaKarve unrecognized command 'add_assignee', ignoring...

Accepted commands are: add_label, add_reviewer, assign, close_issue, move_issue, remove_label, rm_label, set_milestone

@miq-bot miq-bot requested a review from bzwei October 1, 2018 21:07
@AparnaKarve
Copy link
Contributor Author

@miq-bot add_label hammer/yes

@miq-bot
Copy link
Member

miq-bot commented Oct 1, 2018

@AparnaKarve unrecognized command 'add_assignee', ignoring...

Accepted commands are: add_label, add_reviewer, assign, close_issue, move_issue, remove_label, rm_label, set_milestone

@@ -30,6 +30,8 @@ def validate_config_info(options)
vm_options = {}
vm_options[:pre_ansible_playbook_service_template_id] = pre_service_id if vm_hash[:pre_service]
vm_options[:post_ansible_playbook_service_template_id] = post_service_id if vm_hash[:post_service]
vm_options[:osp_security_group_id] = vm_hash[:osp_security_group] if vm_hash[:osp_security_group].present?
Copy link
Contributor

Choose a reason for hiding this comment

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

The backend accepts either vm_hash[:osp_security_group] as an object or vm_hash[:osp_security_group_id] as an id. You can convert them all to ids and save in vm_options[:osp_security_group_id].
Please look at line 25 and 27 as an example. Either vm_hash[:vm_id] or vm_hash[:vm] is accepted.

Same for osp_flavor_id.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The actions hash coming from the frontend looks like this -

"actions": [
  {
    "vm_id": "20000000000057",
    "pre_service": false,
    "post_service": false,
    "osp_security_group": "20000000000007",
    "osp_flavor": "20000000000001"
  }
]

So the security_group and flavor info is available here in terms of id and not object.

On the frontend, we should perhaps change the osp_ keys in the actions hash to be osp_security_group_id and osp_flavor_id, just to be consistent with the naming convention.

Copy link
Contributor

Choose a reason for hiding this comment

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

In your example, yes, please change the frontend to use group and flavor ids.

Although v2v frontend never sends actual object, our backend for all service templates always accept both object and ids.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks -- I made the necessary naming adjustment in ManageIQ/manageiq-v2v#680, and also changed to vm_hash[:osp_security_group_id] and vm_hash[:osp_flavor_id] in this PR

@miq-bot miq-bot requested a review from bzwei October 2, 2018 13:38
@AparnaKarve AparnaKarve force-pushed the add_osp_attributes_to_service_resource_options branch from 70d0045 to 856479d Compare October 2, 2018 18:29
@miq-bot
Copy link
Member

miq-bot commented Oct 2, 2018

Checked commits AparnaKarve/manageiq@2d2e350~...856479d with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🍰

@AparnaKarve
Copy link
Contributor Author

@bzwei Anything else to address here?

@blomquisg blomquisg merged commit 532970f into ManageIQ:master Oct 4, 2018
@AparnaKarve AparnaKarve deleted the add_osp_attributes_to_service_resource_options branch October 4, 2018 14:48
simaishi pushed a commit that referenced this pull request Oct 4, 2018
…ice_resource_options

Add OSP attributes to VM ServiceResource options

(cherry picked from commit 532970f)
@simaishi
Copy link
Contributor

simaishi commented Oct 4, 2018

Hammer backport details:

$ git log -1
commit 28a765dd38b8f303d0ddb9c97390da7dee4973ad
Author: Greg Blomquist <[email protected]>
Date:   Thu Oct 4 10:46:42 2018 -0400

    Merge pull request #18045 from AparnaKarve/add_osp_attributes_to_service_resource_options
    
    Add OSP attributes to VM ServiceResource options
    
    (cherry picked from commit 532970f2a340422f0a98f5578aa7b87c6e4561b1)

@mfeifer mfeifer added this to the Sprint 96 Ending Oct 8, 2018 milestone Jan 27, 2020
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.

7 participants