Add tags to ecs cluster#2347
Conversation
Docs Build 📝Thank you for contribution!✨ The docsite for this PR is available for download as an artifact from this run: You can compare to the docs for the File changes:
Click to see the diff comparison.NOTE: only file modifications are shown here. New and deleted files are excluded. diff --git a/home/runner/work/community.aws/community.aws/docsbuild/base/collections/community/aws/ecs_cluster_module.html b/home/runner/work/community.aws/community.aws/docsbuild/head/collections/community/aws/ecs_cluster_module.html
index 6f3471d..06cb4f0 100644
--- a/home/runner/work/community.aws/community.aws/docsbuild/base/collections/community/aws/ecs_cluster_module.html
+++ b/home/runner/work/community.aws/community.aws/docsbuild/head/collections/community/aws/ecs_cluster_module.html
@@ -310,6 +310,20 @@ see <a class="reference internal" href="#ansible-collections-community-aws-ecs-c
</div></td>
</tr>
<tr class="row-even"><td><div class="ansible-option-cell">
+<div class="ansibleOptionAnchor" id="parameter-purge_tags"></div><p class="ansible-option-title" id="ansible-collections-community-aws-ecs-cluster-module-parameter-purge-tags"><strong>purge_tags</strong></p>
+<a class="ansibleOptionLink" href="#parameter-purge_tags" title="Permalink to this option"></a><p class="ansible-option-type-line"><span class="ansible-option-type">boolean</span></p>
+</div></td>
+<td><div class="ansible-option-cell"><p>If <code class="ansible-option-value docutils literal notranslate"><a class="reference internal" href="#ansible-collections-community-aws-ecs-cluster-module-parameter-purge-tags"><span class="std std-ref"><span class="pre">purge_tags=true</span></span></a></code> and <code class="ansible-option docutils literal notranslate"><strong><a class="reference internal" href="#ansible-collections-community-aws-ecs-cluster-module-parameter-tags"><span class="std std-ref"><span class="pre">tags</span></span></a></strong></code> is set, existing tags will be purged from the resource to match exactly what is defined by <code class="ansible-option docutils literal notranslate"><strong><a class="reference internal" href="#ansible-collections-community-aws-ecs-cluster-module-parameter-tags"><span class="std std-ref"><span class="pre">tags</span></span></a></strong></code> parameter.</p>
+<p>If the <code class="ansible-option docutils literal notranslate"><strong><a class="reference internal" href="#ansible-collections-community-aws-ecs-cluster-module-parameter-tags"><span class="std std-ref"><span class="pre">tags</span></span></a></strong></code> parameter is not set then tags will not be modified, even if <code class="ansible-option-value docutils literal notranslate"><a class="reference internal" href="#ansible-collections-community-aws-ecs-cluster-module-parameter-purge-tags"><span class="std std-ref"><span class="pre">purge_tags=True</span></span></a></code>.</p>
+<p>Tag keys beginning with <code class="ansible-value docutils literal notranslate"><span class="pre">aws:</span></code> are reserved by Amazon and can not be modified. As such they will be ignored for the purposes of the <code class="ansible-option docutils literal notranslate"><strong><a class="reference internal" href="#ansible-collections-community-aws-ecs-cluster-module-parameter-purge-tags"><span class="std std-ref"><span class="pre">purge_tags</span></span></a></strong></code> parameter. See the Amazon documentation for more information <a class="reference external" href="https://docs.aws.amazon.com/general/latest/gr/aws_tagging.html#tag-conventions">https://docs.aws.amazon.com/general/latest/gr/aws_tagging.html#tag-conventions</a>.</p>
+<p class="ansible-option-line"><strong class="ansible-option-choices">Choices:</strong></p>
+<ul class="simple">
+<li><p><code class="ansible-option-choices-entry docutils literal notranslate"><span class="pre">false</span></code></p></li>
+<li><p><code class="ansible-option-default-bold docutils literal notranslate"><strong><span class="pre">true</span></strong></code> <span class="ansible-option-choices-default-mark">← (default)</span></p></li>
+</ul>
+</div></td>
+</tr>
+<tr class="row-odd"><td><div class="ansible-option-cell">
<div class="ansibleOptionAnchor" id="parameter-region"></div>
<div class="ansibleOptionAnchor" id="parameter-aws_region"></div><p class="ansible-option-title" id="ansible-collections-community-aws-ecs-cluster-module-parameter-region"><span id="ansible-collections-community-aws-ecs-cluster-module-parameter-aws-region"></span><strong>region</strong></p>
<a class="ansibleOptionLink" href="#parameter-region" title="Permalink to this option"></a><p class="ansible-option-type-line"><span class="ansible-option-aliases">aliases: aws_region</span></p>
@@ -321,7 +335,7 @@ see <a class="reference internal" href="#ansible-collections-community-aws-ecs-c
<p>See the Amazon AWS documentation for more information <a class="reference external" href="http://docs.aws.amazon.com/general/latest/gr/rande.html#ec2_region">http://docs.aws.amazon.com/general/latest/gr/rande.html#ec2_region</a>.</p>
</div></td>
</tr>
-<tr class="row-odd"><td><div class="ansible-option-cell">
+<tr class="row-even"><td><div class="ansible-option-cell">
<div class="ansibleOptionAnchor" id="parameter-repeat"></div><p class="ansible-option-title" id="ansible-collections-community-aws-ecs-cluster-module-parameter-repeat"><strong>repeat</strong></p>
<a class="ansibleOptionLink" href="#parameter-repeat" title="Permalink to this option"></a><p class="ansible-option-type-line"><span class="ansible-option-type">integer</span></p>
</div></td>
@@ -329,7 +343,7 @@ see <a class="reference internal" href="#ansible-collections-community-aws-ecs-c
<p class="ansible-option-line"><strong class="ansible-option-default-bold">Default:</strong> <code class="ansible-option-default docutils literal notranslate"><span class="pre">10</span></code></p>
</div></td>
</tr>
-<tr class="row-even"><td><div class="ansible-option-cell">
+<tr class="row-odd"><td><div class="ansible-option-cell">
<div class="ansibleOptionAnchor" id="parameter-secret_key"></div>
<div class="ansibleOptionAnchor" id="parameter-aws_secret_access_key"></div>
<div class="ansibleOptionAnchor" id="parameter-aws_secret_key"></div><p class="ansible-option-title" id="ansible-collections-community-aws-ecs-cluster-module-parameter-secret-key"><span id="ansible-collections-community-aws-ecs-cluster-module-parameter-aws-secret-key"></span><span id="ansible-collections-community-aws-ecs-cluster-module-parameter-aws-secret-access-key"></span><strong>secret_key</strong></p>
@@ -343,7 +357,7 @@ see <a class="reference internal" href="#ansible-collections-community-aws-ecs-c
<p>The <em>aws_secret_access_key</em> alias was added in release 5.1.0 for consistency with the AWS botocore SDK.</p>
</div></td>
</tr>
-<tr class="row-odd"><td><div class="ansible-option-cell">
+<tr class="row-even"><td><div class="ansible-option-cell">
<div class="ansibleOptionAnchor" id="parameter-session_token"></div>
<div class="ansibleOptionAnchor" id="parameter-aws_session_token"></div><p class="ansible-option-title" id="ansible-collections-community-aws-ecs-cluster-module-parameter-session-token"><span id="ansible-collections-community-aws-ecs-cluster-module-parameter-aws-session-token"></span><strong>session_token</strong></p>
<a class="ansibleOptionLink" href="#parameter-session_token" title="Permalink to this option"></a><p class="ansible-option-type-line"><span class="ansible-option-aliases">aliases: aws_session_token</span></p>
@@ -355,7 +369,7 @@ see <a class="reference internal" href="#ansible-collections-community-aws-ecs-c
<p>The <em>session_token</em> and <em>profile</em> options are mutually exclusive.</p>
</div></td>
</tr>
-<tr class="row-even"><td><div class="ansible-option-cell">
+<tr class="row-odd"><td><div class="ansible-option-cell">
<div class="ansibleOptionAnchor" id="parameter-state"></div><p class="ansible-option-title" id="ansible-collections-community-aws-ecs-cluster-module-parameter-state"><strong>state</strong></p>
<a class="ansibleOptionLink" href="#parameter-state" title="Permalink to this option"></a><p class="ansible-option-type-line"><span class="ansible-option-type">string</span> / <span class="ansible-option-required">required</span></p>
</div></td>
@@ -368,6 +382,16 @@ see <a class="reference internal" href="#ansible-collections-community-aws-ecs-c
</ul>
</div></td>
</tr>
+<tr class="row-even"><td><div class="ansible-option-cell">
+<div class="ansibleOptionAnchor" id="parameter-tags"></div>
+<div class="ansibleOptionAnchor" id="parameter-resource_tags"></div><p class="ansible-option-title" id="ansible-collections-community-aws-ecs-cluster-module-parameter-tags"><span id="ansible-collections-community-aws-ecs-cluster-module-parameter-resource-tags"></span><strong>tags</strong></p>
+<a class="ansibleOptionLink" href="#parameter-tags" title="Permalink to this option"></a><p class="ansible-option-type-line"><span class="ansible-option-aliases">aliases: resource_tags</span></p>
+<p class="ansible-option-type-line"><span class="ansible-option-type">dictionary</span></p>
+</div></td>
+<td><div class="ansible-option-cell"><p>A dictionary representing the tags to be applied to the resource.</p>
+<p>If the <code class="ansible-option docutils literal notranslate"><strong><a class="reference internal" href="#ansible-collections-community-aws-ecs-cluster-module-parameter-tags"><span class="std std-ref"><span class="pre">tags</span></span></a></strong></code> parameter is not set then tags will not be modified.</p>
+</div></td>
+</tr>
<tr class="row-odd"><td><div class="ansible-option-cell">
<div class="ansibleOptionAnchor" id="parameter-validate_certs"></div><p class="ansible-option-title" id="ansible-collections-community-aws-ecs-cluster-module-parameter-validate-certs"><strong>validate_certs</strong></p>
<a class="ansibleOptionLink" href="#parameter-validate_certs" title="Permalink to this option"></a><p class="ansible-option-type-line"><span class="ansible-option-type">boolean</span></p>
@@ -522,6 +546,15 @@ see <a class="reference internal" href="#ansible-collections-community-aws-ecs-c
<p class="ansible-option-line ansible-option-sample"><strong class="ansible-option-sample-bold">Sample:</strong> <code class="ansible-option-sample docutils literal notranslate"><span class="pre">"ACTIVE"</span></code></p>
</div></td>
</tr>
+<tr class="row-odd"><td><div class="ansible-option-cell">
+<div class="ansibleOptionAnchor" id="return-tags"></div><p class="ansible-option-title" id="ansible-collections-community-aws-ecs-cluster-module-return-tags"><strong>tags</strong></p>
+<a class="ansibleOptionLink" href="#return-tags" title="Permalink to this return value"></a><p class="ansible-option-type-line"><span class="ansible-option-type">dictionary</span></p>
+</div></td>
+<td><div class="ansible-option-cell"><p>Cluster tags.</p>
+<p class="ansible-option-line"><strong class="ansible-option-returned-bold">Returned:</strong> always</p>
+<p class="ansible-option-line ansible-option-sample"><strong class="ansible-option-sample-bold">Sample:</strong> <code class="ansible-option-sample docutils literal notranslate"><span class="pre">{"key":</span> <span class="pre">"value"}</span></code></p>
+</div></td>
+</tr>
</tbody>
</table>
<section id="authors">
|
|
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 4m 16s (non-voting) |
| type: bool | ||
| default: true | ||
| purge_tags: | ||
| version_added: 10.1.0 |
There was a problem hiding this comment.
There is no need to manually document tags and purge_tags here, as they are automatically included when the amazon.aws.tags doc fragment is imported. However, I suggest adding a note for the module to indicate when support for these parameters was introduced (likely version 11.0.0).
There was a problem hiding this comment.
thanks. I will update the version once we're ok merging this.
Shaps
force-pushed
the
add-tags-to-ecs-cluster
branch
2 times, most recently
from
410e3d4 to
2a3c54d
Compare
|
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 4m 03s (non-voting) |
Shaps
force-pushed
the
add-tags-to-ecs-cluster
branch
from
bd51193 to
60a36f6
Compare
|
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 8m 02s (non-voting) |
|
@alinabuzachis looks like the docs expect |
the `tags` option allows the creation and update of tags on a cluster. Since tags are not replaced by default, the `purge_tags` option allows for all the tags to be re-set to the values passed
Shaps
force-pushed
the
add-tags-to-ecs-cluster
branch
from
60a36f6 to
aac6452
Compare
|
Build succeeded. ❌ ansible-galaxy-importer FAILURE in 3m 59s (non-voting) |
s-hertel
left a comment
s-hertel
left a comment
There was a problem hiding this comment.
I have not tested this, just gave a cursory read-through.
| ), | ||
| ), | ||
| tags=dict(type="dict", required=False, aliases=["resource_tags"]), | ||
| purge_tags=dict(required=False, type="bool", default=False), |
There was a problem hiding this comment.
This should default to True. It should only purge existing tags if tags is explicitly set to a dictionary.
| requested_cp = existing_cp | ||
| requested_cps = existing_cps | ||
|
|
||
| if module.params["tags"]: |
There was a problem hiding this comment.
If it's an empty dictionary I'd expect it to remove all tags, not skip purging.
| module.params["tags"], | ||
| ) | ||
|
|
||
| tags_changed = True if recursive_diff(existing_tags, module.params["tags"]) is not None else False |
There was a problem hiding this comment.
Why did you use recursive_diff instead the compare_aws_tags helper? There are some general guidelines here https://docs.ansible.com/projects/ansible/latest/collections/amazon/aws/docsite/dev_guidelines.html#dealing-with-tags.
|
|
||
| if module.params["tags"]: | ||
| if module.params["purge_tags"]: | ||
| tags_to_remove = existing_tags |
There was a problem hiding this comment.
I'd expect tags_to_remove only contains existing tags that are not specified for the task.
| target_swap_mb: 0 | ||
| target_swappiness: 80 | ||
| ecs_task_containers: | ||
| - name: "{{ ecs_task_name }}" |
There was a problem hiding this comment.
I recommend removing all the stylistic changes to existing tests (indentation, booleans, quotes), unless maintainers ask for such a change. This will make your pull request easier to review and avoids polluting the git blame with unenforced personal preferences that can easily be changed again by the next contributor.
| else: | ||
| results["cluster"] = existing | ||
|
|
||
| results["cluster"] = cluster_mgr.refresh_cluster_state(cluster_name=module.params["name"]) |
There was a problem hiding this comment.
I think you could just set the cluster tags to the calculated value. Calling describe_clusters again seems unnecessary.
| cluster_mgr.update_tags( | ||
| existing["clusterArn"], | ||
| tags_to_remove, | ||
| module.params["tags"], | ||
| ) |
There was a problem hiding this comment.
One more thought - this modules supports check mode, and this breaks it. You should add a test to make sure check mode detects tag changes but doesn't actually modify tags.
3 participants
SUMMARY
Adds 2 new options to the
ecs_clustermodule:tagsAllows adding or updating tags on an ECS Cluster. Tags are not replaced by defaultpurge_tagsallows re-setting the tags to the passed values. Overrides the default behaviour of not re-setting the tags.ISSUE TYPE
COMPONENT NAME
ecs_cluster
ADDITIONAL INFORMATION
Not much additional info to be added, I required the tags to be added on cluster creation.
I have also added some lint fixes in the integration test playbook/tasks