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

VMware: Find unique tag using category id #61082

Open
wants to merge 1 commit into
base: devel
from

Conversation

@Akasurde
Copy link
Member

commented Aug 22, 2019

SUMMARY

If two tags with same name and different category exists, vmware_tag_manager
used to take first found tag.

This commit use combination of tag and category to identify the category.

Fixes: #59379

Signed-off-by: Abhijeet Kasurde akasurde@redhat.com

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

changelogs/fragments/59379-vmware_tag_manager-use_category.yml
lib/ansible/module_utils/vmware_rest_client.py
lib/ansible/modules/cloud/vmware/vmware_tag.py
lib/ansible/modules/cloud/vmware/vmware_tag_manager.py
test/integration/targets/vmware_tag/tasks/main.yml
test/integration/targets/vmware_tag/tasks/tag_manager_duplicate_tag_cat.yml

@ansibot

This comment has been minimized.

@ansibot ansibot removed the needs_triage label Aug 23, 2019

@goneri

This comment has been minimized.

Copy link
Contributor

commented Aug 26, 2019

Could you extend a bit the commit message (and the PR desc) to explain the problem that you fix?

@ansibot ansibot added the stale_ci label Sep 3, 2019

VMware: Find unique tag using category id
If two tags with same name and different category exists, vmware_tag_manager
used to take first found tag.

This commit use combination of tag and category to identify the category.

Fixes: #59379

Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>

@Akasurde Akasurde force-pushed the Akasurde:i59379 branch from 644dea6 to ce14e49 Sep 12, 2019

@ansibot ansibot removed the stale_ci label Sep 12, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Sep 12, 2019

The test ansible-test sanity --test pep8 [explain] failed with 2 errors:

lib/ansible/modules/cloud/vmware/vmware_tag_manager.py:213:161: E501: line too long (170 > 160 characters)
lib/ansible/modules/cloud/vmware/vmware_tag_manager.py:267:161: E501: line too long (169 > 160 characters)

click here for bot help

- name: Check if we assigned correct tags
assert:
that:
- vm_tag_info.changed

This comment has been minimized.

Copy link
@jillr

jillr Sep 12, 2019

Contributor

This doesn't check the value of the tag anymore, checking vm_tag_info.changed is good but I think we should also assert:
cat_one in vm_tag_info.tag_status.current_tags[0].

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.