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

cs_vpc_offering: listVPCOffering API returns any matching names #37783

Merged
merged 4 commits into from Mar 23, 2018

Conversation

dpassante
Copy link
Contributor

SUMMARY

Make an explicit comparison between args['name'] and the result of listVPCOfferings API.

In some cases, get_vpc_offering() returns a bad VPC Offering since the Cloudstack listVPCOffering API returns a list of all VPC offerings that are matching "%name%" argument.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

lib/ansible/modules/cloud/cloudstack/cs_vpc_offering.py

ANSIBLE VERSION
 $ ansible --version
ansible 2.6.0 (listVPCOffering_multiple_match 9f5ac8367e) last updated 2018/03/22 15:13:46 (GMT +200)
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/home/dpassante/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /mnt/filer/Github/ansible/lib/ansible
  executable location = /mnt/filer/Github/ansible/bin/ansible
  python version = 2.7.6 (default, Nov 23 2017, 15:49:48) [GCC 4.8.4]
ADDITIONAL INFORMATION

As get_vpc_offering() returns the first offering that match the name '%test%', running this playbook twice is failing:

---
- name: listVPCOffering_multiple_match
  hosts: localhost
  tasks:
  - name: test
    cs_vpc_offering:
      name: "test"
      display_text: "test"
      supported_services: [ Dns, Dhcp ]

  - name: test1
    cs_vpc_offering:
      name: "test1"
      display_text: "test1"
      supported_services: [ Dns, Dhcp ]

@ansibot
Copy link
Contributor

ansibot commented Mar 22, 2018

cc @resmo
click here for bot help

@ansibot ansibot added bug This issue/PR relates to a bug. cloud cloudstack community_review In order to be merged, this PR must follow the community review workflow. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. owner_pr This PR is made by the module's maintainer. support:community This issue/PR relates to code supported by the Ansible community. labels Mar 22, 2018
@samdoran samdoran removed the needs_triage Needs a first human triage before being processed. label Mar 22, 2018
@@ -149,7 +149,9 @@ def get_vpc_offering(self):
vo = self.query_api('listVPCOfferings', **args)

if vo:
self.vpc_offering = vo['vpcoffering'][0]
for index, vpc_offer in enumerate(vo['vpcoffering']):
Copy link
Contributor

Choose a reason for hiding this comment

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

any reason why we should not use a simplified loop?

for vpc_offer in vo['vpcoffering']:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, a simplified loop do the same.

@resmo
Copy link
Contributor

resmo commented Mar 23, 2018

Thanks for this fix, are you aware of other queries (is this a general thing?) which have the same behaviour?

Copy link
Contributor

@resmo resmo left a comment

Choose a reason for hiding this comment

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

needs_info

@dpassante
Copy link
Contributor Author

From what I understand from the Cloudstack code, the following queries have the same behaviour:

@dpassante
Copy link
Contributor Author

cs_configuration always returns 'Changed' when name=workers for example.

@resmo
Copy link
Contributor

resmo commented Mar 23, 2018

I created a separate issue #37824

@resmo
Copy link
Contributor

resmo commented Mar 23, 2018

shipit

@ansibot ansibot added automerge This PR was automatically merged by ansibot. shipit This PR is ready to be merged by Core and removed community_review In order to be merged, this PR must follow the community review workflow. labels Mar 23, 2018
@ansibot ansibot merged commit 270e799 into ansible:devel Mar 23, 2018
@resmo
Copy link
Contributor

resmo commented Mar 23, 2018

cherry-picked for 2.5.1 by #37832

@dpassante dpassante deleted the listVPCOffering_multiple_match branch March 23, 2018 14:50
resmo added a commit that referenced this pull request Mar 31, 2018
@ansible ansible locked and limited conversation to collaborators Apr 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
automerge This PR was automatically merged by ansibot. bug This issue/PR relates to a bug. cloud cloudstack module This issue/PR relates to a module. owner_pr This PR is made by the module's maintainer. shipit This PR is ready to be merged by Core support:community This issue/PR relates to code supported by the Ansible community.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants