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_ip_address: add a "tags" parameter to ensure idempotency #39016

Merged
merged 2 commits into from Apr 22, 2018

Conversation

dpassante
Copy link
Contributor

SUMMARY

Due to Cloudstack API limitations, the calls related to IP addresses aren't idempotent.
This implies implementing some complex mechanisms in playbooks that manage public access to Cloudstack deployments.

However, resources tagging can be used to uniquely identify IP addresses.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

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

ANSIBLE VERSION
2.6
ADDITIONAL INFORMATION

@ansibot
Copy link
Contributor

ansibot commented Apr 19, 2018

@ansibot ansibot added cloud cloudstack community_review In order to be merged, this PR must follow the community review workflow. feature This issue/PR relates to a feature request. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. support:community This issue/PR relates to code supported by the Ansible community. test This PR relates to tests. labels Apr 19, 2018
@resmo
Copy link
Contributor

resmo commented Apr 19, 2018

Thanks for this PR: This is a very interesting approach.

I like the general idea to use a unique tag. However, this would be a "special case" in terms of we handle tags in this module differently than anywhere else. That is why I think carefully about it.

Questions I made for myself:

  1. Can this break existing setups using tags, if their tags are unique? At some point (first tag) is always unique right?

  2. Would it make sense to pre-define a tag key e.g. unique_id just for identify the resource? Just looking for the tag with key unique_id if it exists and value matches?

  3. Would it make sense to use this key across all resources supporting tags to avoid a "special case"?

Any thoughts?

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

@ansibot ansibot removed the needs_triage Needs a first human triage before being processed. label Apr 19, 2018
@dpassante
Copy link
Contributor Author

Whatever the implementation, I think this shouldn’t break anything as long as the tags are “really unique” across the domain e.g random uuid as value.
If they don't, they could match an existing deployment, but only if it uses an identical key/value pair.

Defining a special tag was my first approach but it seemed an “even more special case” to me.
Anyway, I think it would makes sense if it can be used across all resources supporting tags.
As there is always “special cases”, I guess that could be useful for some setups.

@resmo
Copy link
Contributor

resmo commented Apr 22, 2018

shipit

Thanks again for this helpful contribution!

@ansibot ansibot added 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 Apr 22, 2018
@resmo resmo merged commit 7437d6f into ansible:devel Apr 22, 2018
oolongbrothers pushed a commit to oolongbrothers/ansible that referenced this pull request May 14, 2018
…#39016)

* cs_ip_address: add a "tags" parameter to manage idempotency

* cs_ip_address: add integration tests
oolongbrothers pushed a commit to oolongbrothers/ansible that referenced this pull request May 15, 2018
…#39016)

* cs_ip_address: add a "tags" parameter to manage idempotency

* cs_ip_address: add integration tests
ilicmilan pushed a commit to ilicmilan/ansible that referenced this pull request Nov 7, 2018
…#39016)

* cs_ip_address: add a "tags" parameter to manage idempotency

* cs_ip_address: add integration tests
@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
cloud cloudstack feature This issue/PR relates to a feature request. module This issue/PR relates to a module. shipit This PR is ready to be merged by Core support:community This issue/PR relates to code supported by the Ansible community. test This PR relates to tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants